fix: proper hoisting when doing some actions on a subset of workspace projects

PR #2403
close #2340

packages/filter-lockfile/src/filterLockfileByImporters.ts

@@ -9,7 +9,6 @@ import pnpmLogger from '@pnpm/logger'
 import { DependenciesField, Registries } from '@pnpm/types'
 import R = require('ramda')
 import filterImporter from './filterImporter'
-import filterLockfile from './filterLockfile'
 
 const logger = pnpmLogger('lockfile')
 
@@ -23,9 +22,6 @@ export default function filterByImporters (
     failOnMissingDependencies: boolean,
   },
 ): Lockfile {
-  if (R.equals(importerIds.sort(), R.keys(lockfile.importers).sort())) {
-    return filterLockfile(lockfile, opts)
-  }
   const packages = {} as PackageSnapshots
   if (lockfile.packages) {
     pkgAllDeps(

packages/filter-lockfile/test/filterByImporters.ts

@@ -361,3 +361,100 @@ test('filterByImporters(): do not include skipped packages', (t) => {
   })
   t.end()
 })
+
+test('filterByImporters(): exclude orphan packages', (t) => {
+  const filteredLockfile = filterLockfileByImporters(
+    {
+      importers: {
+        'project-1': {
+          dependencies: {
+            'prod-dep': '1.0.0',
+          },
+          specifiers: {
+            'prod-dep': '^1.0.0',
+          },
+        },
+        'project-2': {
+          dependencies: {
+            'project-2-prod-dep': '1.0.0',
+          },
+          specifiers: {
+            'project-2-prod-dep': '^1.0.0',
+          },
+        },
+      },
+      lockfileVersion: 5.1,
+      packages: {
+        '/orphan/1.0.0': {
+          resolution: { integrity: '' },
+        },
+        '/prod-dep-dep/1.0.0': {
+          resolution: { integrity: '' },
+        },
+        '/prod-dep/1.0.0': {
+          dependencies: {
+            'prod-dep-dep': '1.0.0',
+          },
+          resolution: { integrity: '' },
+        },
+        '/project-2-prod-dep/1.0.0': {
+          resolution: { integrity: '' },
+        },
+      },
+    },
+    ['project-1', 'project-2'],
+    {
+      failOnMissingDependencies: true,
+      include: {
+        dependencies: true,
+        devDependencies: true,
+        optionalDependencies: true,
+      },
+      registries: {
+        default: 'https://registry.npmjs.org/',
+      },
+      skipped: new Set<string>(),
+    },
+  )
+
+  t.deepEqual(filteredLockfile, {
+    importers: {
+      'project-1': {
+        dependencies: {
+          'prod-dep': '1.0.0',
+        },
+        devDependencies: {},
+        optionalDependencies: {},
+        specifiers: {
+          'prod-dep': '^1.0.0',
+        },
+      },
+      'project-2': {
+        dependencies: {
+          'project-2-prod-dep': '1.0.0',
+        },
+        devDependencies: {},
+        optionalDependencies: {},
+        specifiers: {
+          'project-2-prod-dep': '^1.0.0',
+        },
+      },
+    },
+    lockfileVersion: 5.1,
+    packages: {
+      '/prod-dep-dep/1.0.0': {
+        resolution: { integrity: '' },
+      },
+      '/prod-dep/1.0.0': {
+        dependencies: {
+          'prod-dep-dep': '1.0.0',
+        },
+        resolution: { integrity: '' },
+      },
+      '/project-2-prod-dep/1.0.0': {
+        resolution: { integrity: '' },
+      },
+    },
+  })
+  t.end()
+})

packages/hoist/src/index.ts

@@ -55,7 +55,7 @@ export default async function hoistByLockfile (
     ),
   ]
 
-  const aliasesByDependencyPath = await hoistGraph(deps, opts.lockfile.importers['.'].specifiers, {
+  const aliasesByDependencyPath = await hoistGraph(deps, opts.lockfile.importers['.']?.specifiers ?? {}, {
     dryRun: false,
     match,
     modulesDir: opts.modulesDir,

packages/supi/src/install/link.ts

@@ -270,18 +270,7 @@ export default async function linkPackages (
     opts.makePartialCurrentLockfile ||
     !allImportersIncluded
   ) {
-    const filteredCurrentLockfile = allImportersIncluded
-      ? opts.currentLockfile
-      : filterLockfileByImporters(
-        opts.currentLockfile,
-        Object.keys(newWantedLockfile.importers)
-          .filter((projectId) => !projectIds.includes(projectId) && opts.currentLockfile.importers[projectId]),
-        {
-          ...filterOpts,
-          failOnMissingDependencies: false,
-        },
-      )
-    const packages = filteredCurrentLockfile.packages || {}
+    const packages = opts.currentLockfile.packages || {}
     if (newWantedLockfile.packages) {
       for (const relDepPath in newWantedLockfile.packages) { // tslint:disable-line:forin
         const depPath = dp.resolve(opts.registries, relDepPath)
@@ -294,7 +283,17 @@ export default async function linkPackages (
       acc[projectId] = newWantedLockfile.importers[projectId]
       return acc
     }, opts.currentLockfile.importers)
-    currentLockfile = { ...newWantedLockfile, packages, importers: projects }
+    currentLockfile = filterLockfileByImporters(
+      {
+        ...newWantedLockfile,
+        importers: projects,
+        packages,
+      },
+      Object.keys(projects), {
+        ...filterOpts,
+        failOnMissingDependencies: false,
+      },
+    )
   } else if (
     opts.include.dependencies &&
     opts.include.devDependencies &&
@@ -306,27 +305,24 @@ export default async function linkPackages (
     currentLockfile = newCurrentLockfile
   }
 
-  let newHoistedAliases: {[depPath: string]: string[]} = {}
-  if (newDepPaths.length > 0 || removedDepPaths.size > 0) {
-    const rootImporterWithFlatModules = opts.hoistPattern && projects.find(({ id }) => id === '.')
-    if (rootImporterWithFlatModules) {
-      newHoistedAliases = await hoist(matcher(opts.hoistPattern!), {
-        getIndependentPackageLocation: opts.independentLeaves
-          ? async (packageId: string, packageName: string) => {
-            const { dir } = await opts.storeController.getPackageLocation(packageId, packageName, {
-              lockfileDir: opts.lockfileDir,
-              targetEngine: opts.sideEffectsCacheRead && ENGINE_NAME || undefined,
-            })
-            return dir
-          }
-          : undefined,
-        lockfile: currentLockfile,
-        lockfileDir: opts.lockfileDir,
-        modulesDir: opts.hoistedModulesDir,
-        registries: opts.registries,
-        virtualStoreDir: opts.virtualStoreDir,
-      })
-    }
+  let newHoistedAliases: Record<string, string[]> = {}
+  if (opts.hoistPattern && (newDepPaths.length > 0 || removedDepPaths.size > 0)) {
+    newHoistedAliases = await hoist(matcher(opts.hoistPattern!), {
+      getIndependentPackageLocation: opts.independentLeaves
+        ? async (packageId: string, packageName: string) => {
+          const { dir } = await opts.storeController.getPackageLocation(packageId, packageName, {
+            lockfileDir: opts.lockfileDir,
+            targetEngine: opts.sideEffectsCacheRead && ENGINE_NAME || undefined,
+          })
+          return dir
+        }
+        : undefined,
+      lockfile: currentLockfile,
+      lockfileDir: opts.lockfileDir,
+      modulesDir: opts.hoistedModulesDir,
+      registries: opts.registries,
+      virtualStoreDir: opts.virtualStoreDir,
+    })
   }
 
   if (!opts.dryRun) {

packages/supi/test/install/hoist.ts

@@ -1,3 +1,4 @@
+import assertProject from '@pnpm/assert-project'
 import { prepareEmpty, preparePackages } from '@pnpm/prepare'
 import { REGISTRY_MOCK_PORT } from '@pnpm/registry-mock'
 import rimraf = require('@zkochan/rimraf')
@@ -384,3 +385,84 @@ test('hoist-pattern: hoist all dependencies to the virtual store node_modules',
   await projects['package'].hasNot('foo')
   await projects['package'].hasNot('bar')
 })
+
+test('hoist when updating in one of the workspace projects', async (t) => {
+  await addDistTag('dep-of-pkg-with-1-dep', '100.0.0', 'latest')
+
+  const workspaceRootManifest = {
+    name: 'root',
+
+    dependencies: {
+      'pkg-with-1-dep': '100.0.0',
+    },
+  }
+  const workspacePackageManifest = {
+    name: 'package',
+
+    dependencies: {
+      'foo': '100.0.0',
+    },
+  }
+  const projects = preparePackages(t, [
+    {
+      location: '.',
+      package: workspaceRootManifest,
+    },
+    {
+      location: 'package',
+      package: workspacePackageManifest,
+    },
+  ])
+
+  const mutatedProjects: MutatedProject[] = [
+    {
+      buildIndex: 0,
+      manifest: workspaceRootManifest,
+      mutation: 'install',
+      rootDir: process.cwd(),
+    },
+    {
+      buildIndex: 0,
+      manifest: workspacePackageManifest,
+      mutation: 'install',
+      rootDir: path.resolve('package'),
+    },
+  ]
+  await mutateModules(mutatedProjects, await testDefaults({ hoistPattern: '*' }))
+
+  const rootNodeModules = assertProject(t, process.cwd())
+  {
+    const modulesManifest = await rootNodeModules.readModulesManifest()
+    t.deepEqual(modulesManifest?.hoistedAliases, {
+      'localhost+4873/dep-of-pkg-with-1-dep/100.0.0': ['dep-of-pkg-with-1-dep'],
+      'localhost+4873/foo/100.0.0': ['foo'],
+    })
+  }
+
+  await mutateModules([
+    {
+      ...mutatedProjects[0],
+      dependencySelectors: ['foo@100.1.0'],
+      mutation: 'installSome',
+    },
+  ], await testDefaults({ hoistPattern: '*', pruneLockfileImporters: false }))
+
+  const lockfile = await rootNodeModules.readCurrentLockfile()
+
+  t.deepEqual(
+    Object.keys(lockfile.packages),
+    [
+      '/dep-of-pkg-with-1-dep/100.0.0',
+      '/foo/100.0.0',
+      '/foo/100.1.0',
+      '/pkg-with-1-dep/100.0.0',
+    ],
+  )
+
+  {
+    const modulesManifest = await rootNodeModules.readModulesManifest()
+    t.deepEqual(modulesManifest?.hoistedAliases, {
+      'localhost+4873/dep-of-pkg-with-1-dep/100.0.0': ['dep-of-pkg-with-1-dep'],
+    })
+  }
+})

packages/supi/test/install/multipleImporters.ts

@@ -70,6 +70,80 @@ test('install only the dependencies of the specified importer', async (t) => {
   await rootNodeModules.hasNot(`.pnpm/localhost+${REGISTRY_MOCK_PORT}/is-negative/1.0.0`)
 })
 
+test('install only the dependencies of the specified importer. The current lockfile has importers that do not exist anymore', async (t) => {
+  preparePackages(t, [
+    {
+      location: 'project-1',
+      package: { name: 'project-1' },
+    },
+    {
+      location: 'project-2',
+      package: { name: 'project-2' },
+    },
+    {
+      location: 'project-3',
+      package: { name: 'project-3' },
+    },
+  ])
+
+  const importers: MutatedProject[] = [
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project-1',
+        version: '1.0.0',
+
+        dependencies: {
+          'is-positive': '1.0.0',
+        },
+      },
+      mutation: 'install',
+      rootDir: path.resolve('project-1'),
+    },
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project-2',
+        version: '1.0.0',
+
+        dependencies: {
+          'is-negative': '1.0.0',
+        },
+      },
+      mutation: 'install',
+      rootDir: path.resolve('project-2'),
+    },
+    {
+      buildIndex: 0,
+      manifest: {
+        name: 'project-3',
+        version: '1.0.0',
+
+        dependencies: {
+          'foobar': '100.0.0',
+        },
+      },
+      mutation: 'install',
+      rootDir: path.resolve('project-3'),
+    },
+  ]
+  await mutateModules(importers, await testDefaults({ hoistPattern: '*' }))
+  await mutateModules(importers.slice(0, 2), await testDefaults({ lockfileOnly: true, pruneLockfileImporters: true }))
+
+  await mutateModules([
+    {
+      ...importers[0],
+      dependencySelectors: ['pkg-with-1-dep'],
+      mutation: 'installSome',
+    },
+  ], await testDefaults({ hoistPattern: '*' }))
+
+  const rootNodeModules = assertProject(t, process.cwd())
+  const currentLockfile = await rootNodeModules.readCurrentLockfile()
+  t.ok(currentLockfile.importers['project-3'])
+  t.ok(currentLockfile.packages['/foobar/100.0.0'])
+})
+
 test('dependencies of other importers are not pruned when installing for a subset of importers', async (t) => {
   const projects = preparePackages(t, [
     {