fix: also include missing deeply linked workspace packages at headles…

…s installation

.changeset/new-vans-guess.md

@@ -0,0 +1,7 @@
+---
+"@pnpm/core": patch
+"@pnpm/filter-lockfile": patch
+"@pnpm/headless": patch
+---
+
+fix: also include missing deeply linked workspace packages at headless installation

fixtures/workspace-external-depends-deep/.npmrc

@@ -0,0 +1,4 @@
+link-workspace-packages = deep
+prefer-workspace-packages = true
+shared-workspace-lockfile = true
+save-workspace-protocol = rolling

fixtures/workspace-external-depends-deep/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "root",
+  "version": "1.0.0",
+  "dependencies": {
+    "is-positive": "1.0.0"
+  }
+}

fixtures/workspace-external-depends-deep/packages/f/package.json

@@ -0,0 +1,9 @@
+{
+  "name": "@kenrick95/internal-f",
+  "version": "1.0.0",
+  "dependencies": {
+    "is-positive": "1.0.0",
+    "is-negative": "1.0.0"
+  }
+}
+

fixtures/workspace-external-depends-deep/packages/g/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "@kenrick95/internal-g",
+  "version": "1.0.0",
+  "dependencies": {
+    "@kenrick95/external-depend-on-internal-dep": "1.0.0",
+    "is-positive": "1.0.0"
+  }
+}

fixtures/workspace-external-depends-deep/pnpm-workspace.yaml

@@ -0,0 +1,2 @@
+packages:
+  - 'packages/**'

packages/core/package.json

@@ -45,6 +45,7 @@
     "@pnpm/read-modules-dir": "workspace:*",
     "@pnpm/read-package-json": "workspace:*",
     "@pnpm/read-project-manifest": "workspace:*",
+    "@pnpm/read-projects-context": "workspace:*",
     "@pnpm/remove-bins": "workspace:*",
     "@pnpm/resolve-dependencies": "workspace:*",
     "@pnpm/resolver-base": "workspace:*",

packages/core/tsconfig.json

@@ -120,6 +120,9 @@
     {
       "path": "../read-project-manifest"
     },
+    {
+      "path": "../read-projects-context"
+    },
     {
       "path": "../remove-bins"
     },

packages/filter-lockfile/src/filterLockfileByImportersAndEngine.ts

@@ -14,6 +14,31 @@ import filterImporter from './filterImporter'
 
 const logger = pnpmLogger('lockfile')
 
+function toImporterDepPaths (
+  lockfile: Lockfile,
+  importerIds: string[],
+  opts: {
+    include: { [dependenciesField in DependenciesField]: boolean }
+  }
+) {
+  const importerDeps = importerIds
+    .map(importerId => lockfile.importers[importerId])
+    .map(importer => ({
+      ...(opts.include.dependencies ? importer.dependencies : {}),
+      ...(opts.include.devDependencies ? importer.devDependencies : {}),
+      ...(opts.include.optionalDependencies
+        ? importer.optionalDependencies
+        : {}),
+    }))
+    .map(Object.entries)
+
+  const importerDepsPaths = unnest(importerDeps)
+    .map(([pkgName, ref]) => dp.refToRelative(ref, pkgName))
+    .filter(nodeId => nodeId !== null) as string[]
+
+  return importerDepsPaths
+}
+
 export default function filterByImportersAndEngine (
   lockfile: Lockfile,
   importerIds: string[],
@@ -29,30 +54,26 @@ export default function filterByImportersAndEngine (
     lockfileDir: string
     skipped: Set<string>
   }
-): Lockfile {
-  const importerDeps = importerIds
-    .map((importerId) => lockfile.importers[importerId])
-    .map((importer) => ({
-      ...(opts.include.dependencies ? importer.dependencies : {}),
-      ...(opts.include.devDependencies ? importer.devDependencies : {}),
-      ...(opts.include.optionalDependencies ? importer.optionalDependencies : {}),
-    }))
-    .map(Object.entries)
-  const directDepPaths = unnest(importerDeps)
-    .map(([pkgName, ref]) => dp.refToRelative(ref, pkgName))
-    .filter((nodeId) => nodeId !== null) as string[]
+): { lockfile: Lockfile, importerIds: string[] } {
+  const importerIdSet = new Set(importerIds) as Set<string>
 
-  const packages = (lockfile.packages != null)
-    ? pickPkgsWithAllDeps(lockfile.packages, directDepPaths, {
-      currentEngine: opts.currentEngine,
-      engineStrict: opts.engineStrict,
-      failOnMissingDependencies: opts.failOnMissingDependencies,
-      include: opts.include,
-      includeIncompatiblePackages: opts.includeIncompatiblePackages === true,
-      lockfileDir: opts.lockfileDir,
-      skipped: opts.skipped,
-    })
-    : {}
+  const directDepPaths = toImporterDepPaths(lockfile, importerIds, {
+    include: opts.include,
+  })
+
+  const packages =
+    lockfile.packages != null
+      ? pickPkgsWithAllDeps(lockfile, directDepPaths, importerIdSet, {
+        currentEngine: opts.currentEngine,
+        engineStrict: opts.engineStrict,
+        failOnMissingDependencies: opts.failOnMissingDependencies,
+        include: opts.include,
+        includeIncompatiblePackages:
+            opts.includeIncompatiblePackages === true,
+        lockfileDir: opts.lockfileDir,
+        skipped: opts.skipped,
+      })
+      : {}
 
   const importers = importerIds.reduce((acc, importerId) => {
     acc[importerId] = filterImporter(lockfile.importers[importerId], opts.include)
@@ -68,15 +89,19 @@ export default function filterByImportersAndEngine (
   }, { ...lockfile.importers })
 
   return {
-    ...lockfile,
-    importers,
-    packages,
+    lockfile: {
+      ...lockfile,
+      importers,
+      packages,
+    },
+    importerIds: Array.from(importerIdSet),
   }
 }
 
 function pickPkgsWithAllDeps (
-  pkgSnapshots: PackageSnapshots,
+  lockfile: Lockfile,
   depPaths: string[],
+  importerIdSet: Set<string>,
   opts: {
     currentEngine: {
       nodeVersion: string
@@ -91,14 +116,15 @@ function pickPkgsWithAllDeps (
   }
 ) {
   const pickedPackages = {} as PackageSnapshots
-  pkgAllDeps({ pkgSnapshots, pickedPackages }, depPaths, true, opts)
+  pkgAllDeps({ lockfile, pickedPackages, importerIdSet }, depPaths, true, opts)
   return pickedPackages
 }
 
 function pkgAllDeps (
   ctx: {
-    pkgSnapshots: PackageSnapshots
+    lockfile: Lockfile
     pickedPackages: PackageSnapshots
+    importerIdSet: Set<string>
   },
   depPaths: string[],
   parentIsInstallable: boolean,
@@ -117,7 +143,7 @@ function pkgAllDeps (
 ) {
   for (const depPath of depPaths) {
     if (ctx.pickedPackages[depPath]) continue
-    const pkgSnapshot = ctx.pkgSnapshots[depPath]
+    const pkgSnapshot = ctx.lockfile.packages![depPath]
     if (!pkgSnapshot && !depPath.startsWith('link:')) {
       if (opts.failOnMissingDependencies) {
         throw new LockfileMissingDependencyError(depPath)
@@ -140,13 +166,15 @@ function pkgAllDeps (
         libc: pkgSnapshot.libc,
       }
       // TODO: depPath is not the package ID. Should be fixed
-      installable = opts.includeIncompatiblePackages || packageIsInstallable(pkgSnapshot.id ?? depPath, pkg, {
-        engineStrict: opts.engineStrict,
-        lockfileDir: opts.lockfileDir,
-        nodeVersion: opts.currentEngine.nodeVersion,
-        optional: pkgSnapshot.optional === true,
-        pnpmVersion: opts.currentEngine.pnpmVersion,
-      }) !== false
+      installable =
+        opts.includeIncompatiblePackages ||
+        packageIsInstallable(pkgSnapshot.id ?? depPath, pkg, {
+          engineStrict: opts.engineStrict,
+          lockfileDir: opts.lockfileDir,
+          nodeVersion: opts.currentEngine.nodeVersion,
+          optional: pkgSnapshot.optional === true,
+          pnpmVersion: opts.currentEngine.pnpmVersion,
+        }) !== false
       if (!installable) {
         if (!ctx.pickedPackages[depPath] && pkgSnapshot.optional === true) {
           opts.skipped.add(depPath)
@@ -156,14 +184,39 @@ function pkgAllDeps (
       }
     }
     ctx.pickedPackages[depPath] = pkgSnapshot
-    const nextRelDepPaths = Object.entries(
-      {
-        ...pkgSnapshot.dependencies,
-        ...(opts.include.optionalDependencies ? pkgSnapshot.optionalDependencies : {}),
+    const nextRelDepPaths = Object.entries({
+      ...pkgSnapshot.dependencies,
+      ...(opts.include.optionalDependencies
+        ? pkgSnapshot.optionalDependencies
+        : {}),
+    })
+      .map(([pkgName, ref]) => {
+        if (ref.startsWith('link:')) {
+          return ref
+        }
+        return dp.refToRelative(ref, pkgName)
+      })
+      .filter(nodeId => nodeId !== null) as string[]
+
+    // Also include missing deeply linked workspace project
+    const actualNextRelDepPaths = []
+    const additionalImporterIds = []
+    for (const nextDepPath of nextRelDepPaths) {
+      if (nextDepPath.startsWith('link:')) {
+        const ref = nextDepPath.slice(5)
+        additionalImporterIds.push(ref)
+        ctx.importerIdSet.add(ref)
+      } else {
+        actualNextRelDepPaths.push(nextDepPath)
+      }
+    }
+
+    actualNextRelDepPaths.push(
+      ...toImporterDepPaths(ctx.lockfile, additionalImporterIds, {
+        include: opts.include,
       })
-      .map(([pkgName, ref]) => dp.refToRelative(ref, pkgName))
-      .filter((nodeId) => nodeId !== null) as string[]
+    )
 
-    pkgAllDeps(ctx, nextRelDepPaths, installable, opts)
+    pkgAllDeps(ctx, actualNextRelDepPaths, installable, opts)
   }
 }

packages/filter-lockfile/test/filterByImportersAndEngine.ts

@@ -119,7 +119,7 @@ test('filterByImportersAndEngine(): skip packages that are not installable', ()
     }
   )
 
-  expect(filteredLockfile).toStrictEqual({
+  expect(filteredLockfile.lockfile).toStrictEqual({
     importers: {
       'project-1': {
         dependencies: {
@@ -298,7 +298,7 @@ test('filterByImportersAndEngine(): filter the packages that set os and cpu', ()
     }
   )
 
-  expect(filteredLockfile).toStrictEqual({
+  expect(filteredLockfile.lockfile).toStrictEqual({
     importers: {
       'project-1': {
         dependencies: {
@@ -466,7 +466,7 @@ test('filterByImportersAndEngine(): filter the packages that set libc', () => {
     }
   )
 
-  expect(filteredLockfile).toStrictEqual({
+  expect(filteredLockfile.lockfile).toStrictEqual({
     importers: {
       'project-1': {
         dependencies: {

packages/headless/src/index.ts

@@ -240,17 +240,33 @@ export async function headlessInstall (opts: HeadlessOptions) {
     registries: opts.registries,
     skipped,
   }
-  const importerIds = (opts.ignorePackageManifest === true || opts.nodeLinker === 'hoisted')
+  const initialImporterIds = (opts.ignorePackageManifest === true || opts.nodeLinker === 'hoisted')
     ? Object.keys(wantedLockfile.importers)
     : selectedProjects.map(({ id }) => id)
-  const filteredLockfile = filterLockfileByImportersAndEngine(wantedLockfile, importerIds, {
+  const { lockfile: filteredLockfile, importerIds } = filterLockfileByImportersAndEngine(wantedLockfile, initialImporterIds, {
     ...filterOpts,
     currentEngine: opts.currentEngine,
     engineStrict: opts.engineStrict,
     failOnMissingDependencies: true,
     includeIncompatiblePackages: opts.force,
     lockfileDir,
   })
+
+  // Update selectedProjects to add missing projects. importerIds will have the updated ids, found from deeply linked workspace projects
+  const missingIds = [] as string[]
+  const initialImporterIdSet = new Set(initialImporterIds)
+  for (const id of importerIds) {
+    if (!initialImporterIdSet.has(id)) {
+      missingIds.push(id)
+    }
+  }
+  for (const id of missingIds) {
+    const additionalProject = Object.values(opts.allProjects).find((project) => project.id === id)
+    if (additionalProject) {
+      selectedProjects.push(additionalProject)
+    }
+  }
+
   const lockfileToDepGraphOpts = {
     ...opts,
     importerIds,