fix: flat node_modules inside a workspace

Flat node_modules is only allowed in the workspace root.

shamefully-flatten should only hoists dependencies of the
root workspace package.

close #1928
PR #1931

BREAKING CHANGE: @pnpm/shamefully-flatten

packages/default-reporter/src/reportError.ts

@@ -43,6 +43,8 @@ export default function reportError (logObj: Log) {
         return reportLifecycleError(logObj['message'])
       case 'ERR_PNPM_UNSUPPORTED_ENGINE':
         return reportEngineError(err, logObj['message'])
+      case 'ERR_PNPM_SHAMEFULLY_FLATTEN_NOT_IN_LOCKFILE_DIR':
+        return reportShamefullyFlattenNotInLockfileDirError(logObj['message'])
       default:
         // Errors with known error codes are printed w/o stack trace
         if (err.code && err.code.startsWith && err.code.startsWith('ERR_PNPM_')) {
@@ -270,3 +272,27 @@ function reportEngineError (
   }
   return output || formatErrorSummary(err.message)
 }
+
+function reportShamefullyFlattenNotInLockfileDirError (
+  msg: {
+    lockfileDirectory: string,
+    message: string,
+    shamefullyFlattenDirectory: string,
+  },
+) {
+  return stripIndent`
+    ${formatErrorSummary(`Shamefully flatten can be only used in the lockfile directory`)}
+
+    You were trying to install a flat node_modules in ${msg.shamefullyFlattenDirectory}
+    Flat node_modules is only possible in the directory that contains the lockfile (pnpm-lock.yaml).
+    In your case: ${msg.lockfileDirectory}
+
+    If you really need a flat node_modules in ${msg.shamefullyFlattenDirectory},
+    then you should use a dedicated lockfile.
+    Create a .npmrc in the root of your workspace with the following content:
+    shared-workspace-lockfile=false
+
+    If you don't need a flat node_modules, remove shamefully-flatten=true
+    from the .npmrc file in ${msg.shamefullyFlattenDirectory}
+  `
+}

packages/default-reporter/test/reportingErrors.ts

@@ -331,3 +331,39 @@ test('prints unsupported pnpm and Node versions error', async (t) => {
   err['current'] = { pnpm: '3.0.0', node: '10.0.0' }
   logger.error(err, err)
 })
+
+test('prints shamefully-flatten is only allowed in the root workspace package error', async (t) => {
+  const output$ = toOutput$({
+    context: { argv: ['install'] },
+    streamParser: createStreamParser(),
+  })
+
+  t.plan(1)
+
+  output$.take(1).map(normalizeNewline).subscribe({
+    complete: () => t.end(),
+    error: t.end,
+    next: output => {
+      t.equal(output, stripIndent`
+        ${ERROR} ${chalk.red('Shamefully flatten can be only used in the lockfile directory')}
+
+        You were trying to install a flat node_modules in /home/zoltan/repo/package
+        Flat node_modules is only possible in the directory that contains the lockfile (pnpm-lock.yaml).
+        In your case: /home/zoltan/repo
+
+        If you really need a flat node_modules in /home/zoltan/repo/package,
+        then you should use a dedicated lockfile.
+        Create a .npmrc in the root of your workspace with the following content:
+        shared-workspace-lockfile=false
+
+        If you don't need a flat node_modules, remove shamefully-flatten=true
+        from the .npmrc file in /home/zoltan/repo/package
+      `)
+    },
+  })
+
+  const err = new PnpmError('SHAMEFULLY_FLATTEN_NOT_IN_LOCKFILE_DIR', 'Shamefully flatten can be only used in the lockfile directory')
+  err['shamefullyFlattenDirectory'] = '/home/zoltan/repo/package'
+  err['lockfileDirectory'] = '/home/zoltan/repo'
+  logger.error(err, err)
+})

packages/headless/src/index.ts

@@ -45,7 +45,7 @@ import {
 import pkgIdToFilename from '@pnpm/pkgid-to-filename'
 import { readImporterManifestOnly } from '@pnpm/read-importer-manifest'
 import { fromDir as readPackageFromDir } from '@pnpm/read-package-json'
-import { shamefullyFlattenByLockfile } from '@pnpm/shamefully-flatten'
+import shamefullyFlatten from '@pnpm/shamefully-flatten'
 import {
   PackageFilesResponse,
   StoreController,
@@ -228,27 +228,25 @@ export default async (opts: HeadlessOptions) => {
     })
   }
 
-  await Promise.all(opts.importers.map(async (importer) => {
-    if (importer.shamefullyFlatten) {
-      importer.hoistedAliases = await shamefullyFlattenByLockfile(filteredLockfile, importer.id, {
-        getIndependentPackageLocation: opts.independentLeaves
-          ? async (packageId: string, packageName: string) => {
-            const { directory } = await opts.storeController.getPackageLocation(packageId, packageName, {
-              lockfileDirectory: opts.lockfileDirectory,
-              targetEngine: opts.sideEffectsCacheRead && ENGINE_NAME || undefined,
-            })
-            return directory
-          }
-          : undefined,
-        lockfileDirectory: opts.lockfileDirectory,
-        modulesDir: importer.modulesDir,
-        registries: opts.registries,
-        virtualStoreDir,
-      })
-    } else {
-      importer.hoistedAliases = {}
-    }
-  }))
+  const rootImporterWithFlatModules = opts.importers.find((importer) => importer.id === '.' && importer.shamefullyFlatten)
+  if (rootImporterWithFlatModules) {
+    rootImporterWithFlatModules.hoistedAliases = await shamefullyFlatten({
+      getIndependentPackageLocation: opts.independentLeaves
+        ? async (packageId: string, packageName: string) => {
+          const { directory } = await opts.storeController.getPackageLocation(packageId, packageName, {
+            lockfileDirectory: opts.lockfileDirectory,
+            targetEngine: opts.sideEffectsCacheRead && ENGINE_NAME || undefined,
+          })
+          return directory
+        }
+        : undefined,
+      lockfile: filteredLockfile,
+      lockfileDirectory: opts.lockfileDirectory,
+      modulesDir: rootImporterWithFlatModules.modulesDir,
+      registries: opts.registries,
+      virtualStoreDir,
+    })
+  }
 
   await Promise.all(opts.importers.map(async ({ id, manifest, modulesDir, prefix }) => {
     await linkRootPackages(filteredLockfile, {

packages/pnpm/src/cmd/recursive/index.ts

@@ -225,6 +225,9 @@ export async function recursive (
   if (cmdFullName !== 'rebuild') {
     // For a workspace with shared lockfile
     if (opts.lockfileDirectory && ['install', 'uninstall', 'update'].includes(cmdFullName)) {
+      if (opts.shamefullyFlatten) {
+        logger.info({ message: 'Only the root workspace package is going to have a flat node_modules', prefix: opts.lockfileDirectory })
+      }
       let importers = await getImporters()
       const isFromWorkspace = isSubdir.bind(null, opts.lockfileDirectory)
       importers = await pFilter(importers, async ({ prefix }: { prefix: string }) => isFromWorkspace(await fs.realpath(prefix)))
@@ -238,7 +241,7 @@ export async function recursive (
         const { manifest, writeImporterManifest } = manifestsByPath[prefix]
         const shamefullyFlatten = typeof localConfigs.shamefullyFlatten === 'boolean'
           ? localConfigs.shamefullyFlatten
-          : opts.shamefullyFlatten
+          : (prefix === opts.lockfileDirectory && opts.shamefullyFlatten)
         let currentInput = [...input]
         if (updateToLatest) {
           if (!currentInput || !currentInput.length) {

packages/pnpm/test/install/shamefullyFlatten.ts

@@ -1,9 +1,12 @@
-import prepare from '@pnpm/prepare'
+import prepare, { preparePackages } from '@pnpm/prepare'
+import fs = require('mz/fs')
 import tape = require('tape')
 import promisifyTape from 'tape-promise'
+import writeYamlFile = require('write-yaml-file')
 import { execPnpm } from '../utils'
 
 const test = promisifyTape(tape)
+const testOnly = promisifyTape(tape.only)
 
 test('shamefully flatten the dependency tree', async function (t) {
   const project = prepare(t)
@@ -20,3 +23,36 @@ test('shamefully flatten the dependency tree', async function (t) {
   await project.hasNot('debug')
   await project.hasNot('cookie')
 })
+
+test('shamefully-flatten: applied only to the workspace root package when set to true in the root .npmrc file', async (t: tape.Test) => {
+  const projects = preparePackages(t, [
+    {
+      location: '.',
+      package: {
+        name: 'root',
+
+        dependencies: {
+          'pkg-with-1-dep': '100.0.0',
+        },
+      },
+    },
+    {
+      name: 'project',
+      version: '1.0.0',
+
+      dependencies: {
+        'foobar': '100.0.0',
+      },
+    },
+  ])
+
+  await writeYamlFile('pnpm-workspace.yaml', { packages: ['**', '!store/**'] })
+  await fs.writeFile('.npmrc', 'shamefully-flatten', 'utf8')
+
+  await execPnpm('recursive', 'install')
+
+  await projects['root'].has('dep-of-pkg-with-1-dep')
+  await projects['root'].hasNot('foo')
+  await projects['project'].hasNot('foo')
+  await projects['project'].has('foobar')
+})

packages/pnpm/test/monorepo/index.ts

@@ -423,7 +423,7 @@ test('recursive install with link-workspace-packages and shared-workspace-lockfi
   await writeYamlFile('pnpm-workspace.yaml', { packages: ['**', '!store/**'] })
   await fs.writeFile(
     'is-positive/.npmrc',
-    'shamefully-flatten = true\nsave-exact = true',
+    'save-exact = true',
     'utf8',
   )
   await fs.writeFile(
@@ -435,7 +435,6 @@ test('recursive install with link-workspace-packages and shared-workspace-lockfi
   await execPnpm('recursive', 'install', '--link-workspace-packages', '--shared-workspace-lockfile=true', '--store', 'store')
 
   t.ok(projects['is-positive'].requireModule('is-negative'))
-  t.ok(projects['is-positive'].requireModule('concat-stream'), 'dependencies flattened in is-positive')
   t.notOk(projects['project-1'].requireModule('is-positive/package.json').author, 'local package is linked')
 
   const sharedLockfile = await readYamlFile<Lockfile>(WANTED_LOCKFILE)

packages/shamefully-flatten/src/index.ts

@@ -13,20 +13,19 @@ import * as dp from 'dependency-path'
 import path = require('path')
 import R = require('ramda')
 
-export async function shamefullyFlattenByLockfile (
-  lockfile: Lockfile,
-  importerId: string,
+export default async function shamefullyFlattenByLockfile (
   opts: {
     getIndependentPackageLocation?: (packageId: string, packageName: string) => Promise<string>,
+    lockfile: Lockfile,
     lockfileDirectory: string,
     modulesDir: string,
     registries: Registries,
     virtualStoreDir: string,
   },
 ) {
-  if (!lockfile.packages) return {}
+  if (!opts.lockfile.packages) return {}
 
-  const lockfileImporter = lockfile.importers[importerId]
+  const lockfileImporter = opts.lockfile.importers['.']
 
   const entryNodes = R.toPairs({
     ...lockfileImporter.devDependencies,
@@ -36,7 +35,7 @@ export async function shamefullyFlattenByLockfile (
   .map((pair) => dp.refToRelative(pair[1], pair[0]))
   .filter((nodeId) => nodeId !== null) as string[]
 
-  const deps = await getDependencies(lockfile.packages, entryNodes, new Set(), 0, {
+  const deps = await getDependencies(opts.lockfile.packages, entryNodes, new Set(), 0, {
     getIndependentPackageLocation: opts.getIndependentPackageLocation,
     lockfileDirectory: opts.lockfileDirectory,
     registries: opts.registries,
@@ -125,7 +124,7 @@ export interface Dependency {
   absolutePath: string,
 }
 
-export default async function shamefullyFlattenGraph (
+async function shamefullyFlattenGraph (
   depNodes: Dependency[],
   currentSpecifiers: {[alias: string]: string},
   opts: {

packages/supi/src/install/index.ts

@@ -133,6 +133,17 @@ export async function install (
 
 export type MutatedImporter = ImportersOptions & DependenciesMutation
 
+export class ShamefullyFlattenNotInLockfileDirectoryError extends PnpmError {
+  public readonly shamefullyFlattenDirectory: string
+  public readonly lockfileDirectory: string
+
+  constructor (shamefullyFlattenDirectory: string, lockfileDirectory: string) {
+    super('SHAMEFULLY_FLATTEN_NOT_IN_LOCKFILE_DIR', 'Shamefully flatten can be only used in the lockfile directory')
+    this.shamefullyFlattenDirectory = shamefullyFlattenDirectory
+    this.lockfileDirectory = lockfileDirectory
+  }
+}
+
 export async function mutateModules (
   importers: MutatedImporter[],
   maybeOpts: InstallOptions & {
@@ -154,6 +165,11 @@ export async function mutateModules (
   if (!opts.include.dependencies && opts.include.optionalDependencies) {
     throw new PnpmError('OPTIONAL_DEPS_REQUIRE_PROD_DEPS', 'Optional dependencies cannot be installed without production dependencies')
   }
+  for (const { prefix, shamefullyFlatten } of importers) {
+    if (prefix !== opts.lockfileDirectory && shamefullyFlatten) {
+      throw new ShamefullyFlattenNotInLockfileDirectoryError(prefix, opts.lockfileDirectory)
+    }
+  }
 
   const ctx = await getContext(importers, opts)
 
@@ -789,6 +805,7 @@ async function installInContext (
       outdatedDependencies,
       pruneStore: opts.pruneStore,
       registries: ctx.registries,
+      sideEffectsCacheRead: opts.sideEffectsCacheRead,
       skipped: ctx.skipped,
       storeController: opts.storeController,
       strictPeerDependencies: opts.strictPeerDependencies,

packages/supi/src/install/link.ts

@@ -1,4 +1,5 @@
 import {
+  ENGINE_NAME,
   LOCKFILE_VERSION,
   WANTED_LOCKFILE,
 } from '@pnpm/constants'
@@ -15,7 +16,7 @@ import logger from '@pnpm/logger'
 import { prune } from '@pnpm/modules-cleaner'
 import { IncludedDependencies } from '@pnpm/modules-yaml'
 import { DependenciesTree, LinkedDependency } from '@pnpm/resolve-dependencies'
-import shamefullyFlattenGraph from '@pnpm/shamefully-flatten'
+import shamefullyFlatten from '@pnpm/shamefully-flatten'
 import { StoreController } from '@pnpm/store-controller-types'
 import symlinkDependency, { symlinkDirectRootDependency } from '@pnpm/symlink-dependency'
 import { ImporterManifest, Registries } from '@pnpm/types'
@@ -76,6 +77,7 @@ export default async function linkPackages (
     updateLockfileMinorVersion: boolean,
     outdatedDependencies: {[pkgId: string]: string},
     strictPeerDependencies: boolean,
+    sideEffectsCacheRead: boolean,
   },
 ): Promise<{
   currentLockfile: Lockfile,
@@ -299,27 +301,26 @@ export default async function linkPackages (
     currentLockfile = newCurrentLockfile
   }
 
-  // Important: shamefullyFlattenGraph changes depGraph, so keep this at the end, right before linkBins
   if (newDepPaths.length > 0 || removedDepPaths.size > 0) {
-    await Promise.all(
-      importers.filter(({ shamefullyFlatten }) => shamefullyFlatten)
-        .map(async (importer) => {
-          importer.hoistedAliases = await shamefullyFlattenGraph(
-            depNodes.map((depNode) => ({
-              absolutePath: depNode.absolutePath,
-              children: depNode.children,
-              depth: depNode.depth,
-              location: depNode.independent ? depNode.centralLocation : depNode.peripheralLocation,
-              name: depNode.name,
-            })),
-            currentLockfile.importers[importer.id].specifiers,
-            {
-              dryRun: opts.dryRun,
-              modulesDir: importer.modulesDir,
-            },
-          )
-        }),
-    )
+    const rootImporterWithFlatModules = importers.find((importer) => importer.id === '.' && importer.shamefullyFlatten)
+    if (rootImporterWithFlatModules) {
+      rootImporterWithFlatModules.hoistedAliases = await shamefullyFlatten({
+        getIndependentPackageLocation: opts.independentLeaves
+          ? async (packageId: string, packageName: string) => {
+            const { directory } = await opts.storeController.getPackageLocation(packageId, packageName, {
+              lockfileDirectory: opts.lockfileDirectory,
+              targetEngine: opts.sideEffectsCacheRead && ENGINE_NAME || undefined,
+            })
+            return directory
+          }
+          : undefined,
+        lockfile: currentLockfile,
+        lockfileDirectory: opts.lockfileDirectory,
+        modulesDir: rootImporterWithFlatModules.modulesDir,
+        registries: opts.registries,
+        virtualStoreDir: opts.virtualStoreDir,
+      })
+    }
   }
 
   if (!opts.dryRun) {

packages/supi/test/install/shamefullyFlatten.ts

@@ -1,7 +1,15 @@
-import { prepareEmpty } from '@pnpm/prepare'
+import { prepareEmpty, preparePackages } from '@pnpm/prepare'
 import fs = require('fs')
+import path = require('path')
 import resolveLinkTarget = require('resolve-link-target')
-import { addDependenciesToPackage, install, mutateModules } from 'supi'
+import rimraf = require('rimraf-then')
+import {
+  addDependenciesToPackage,
+  install,
+  MutatedImporter,
+  mutateModules,
+  ShamefullyFlattenNotInLockfileDirectoryError,
+} from 'supi'
 import tape = require('tape')
 import promisifyTape from 'tape-promise'
 import { addDistTag, testDefaults } from '../utils'
@@ -252,3 +260,90 @@ test('should uninstall correctly peer dependencies', async (t) => {
 
   t.throws(() => fs.lstatSync('node_modules/ajv-keywords'), Error, 'symlink to peer dependency is deleted')
 })
+
+test('shamefully-flatten: throw exception when executed on a project that uses an external lockfile', async (t: tape.Test) => {
+  prepareEmpty(t)
+  const lockfileDirectory = path.resolve('..')
+
+  let err!: ShamefullyFlattenNotInLockfileDirectoryError
+  try {
+    await addDependenciesToPackage({}, ['is-negative'], await testDefaults({ shamefullyFlatten: true, lockfileDirectory }))
+    t.fail('installation should have failed')
+  } catch (_err) {
+    err = _err
+  }
+
+  t.ok(err, 'error thrown')
+  t.equal(err.code, 'ERR_PNPM_SHAMEFULLY_FLATTEN_NOT_IN_LOCKFILE_DIR')
+  t.equal(err.shamefullyFlattenDirectory, process.cwd())
+  t.equal(err.lockfileDirectory, lockfileDirectory)
+})
+
+test('shamefully-flatten: only hoists the dependencies of the root workspace package', async (t) => {
+  const workspaceRootManifest = {
+    name: 'root',
+
+    dependencies: {
+      'pkg-with-1-dep': '100.0.0',
+    },
+  }
+  const workspacePackageManifest = {
+    name: 'package',
+
+    dependencies: {
+      'foobar': '100.0.0'
+    },
+  }
+  const projects = preparePackages(t, [
+    {
+      location: '.',
+      package: workspaceRootManifest,
+    },
+    {
+      location: 'package',
+      package: workspacePackageManifest,
+    },
+  ])
+
+  const importers: MutatedImporter[] = [
+    {
+      buildIndex: 0,
+      manifest: workspaceRootManifest,
+      mutation: 'install',
+      prefix: process.cwd(),
+      shamefullyFlatten: true,
+    },
+    {
+      buildIndex: 0,
+      manifest: workspacePackageManifest,
+      mutation: 'install',
+      prefix: path.resolve('package'),
+    },
+  ]
+  await mutateModules(importers, await testDefaults())
+
+  await projects['root'].has('pkg-with-1-dep')
+  await projects['root'].has('dep-of-pkg-with-1-dep')
+  await projects['root'].hasNot('foobar')
+  await projects['root'].hasNot('foo')
+  await projects['root'].hasNot('bar')
+
+  await projects['package'].has('foobar')
+  await projects['package'].hasNot('foo')
+  await projects['package'].hasNot('bar')
+
+  await rimraf('node_modules')
+  await rimraf('package/node_modules')
+
+  await mutateModules(importers, await testDefaults({ frozenLockfile: true }))
+
+  await projects['root'].has('pkg-with-1-dep')
+  await projects['root'].has('dep-of-pkg-with-1-dep')
+  await projects['root'].hasNot('foobar')
+  await projects['root'].hasNot('foo')
+  await projects['root'].hasNot('bar')
+
+  await projects['package'].has('foobar')
+  await projects['package'].hasNot('foo')
+  await projects['package'].hasNot('bar')
+})