fix: stale catalog protocol reference on filtered install (#9126)

#8638

.changeset/smooth-geckos-jog.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/core": patch
+"pnpm": patch
+---
+
+Fix a bug causing catalog protocol dependencies to not re-resolve on a filtered install [#8638](https://github.com/pnpm/pnpm/issues/8638).

pkg-manager/core/src/install/index.ts

@@ -2,6 +2,7 @@ import path from 'path'
 import { buildModules, type DepsStateCache, linkBinsOfDependencies } from '@pnpm/build-modules'
 import { createAllowBuildFunction } from '@pnpm/builder.policy'
 import { parseCatalogProtocol } from '@pnpm/catalogs.protocol-parser'
+import { type Catalogs } from '@pnpm/catalogs.types'
 import {
   LAYOUT_VERSION,
   LOCKFILE_VERSION,
@@ -36,6 +37,7 @@ import {
   writeWantedLockfile,
   cleanGitBranchLockfiles,
   type PatchFile,
+  type CatalogSnapshots,
 } from '@pnpm/lockfile.fs'
 import { writePnpFile } from '@pnpm/lockfile-to-pnp'
 import { extendProjectsWithTargetDirs } from '@pnpm/lockfile.utils'
@@ -379,6 +381,7 @@ export async function mutateModules (
         throw new LockfileConfigMismatchError(outdatedLockfileSettingName!)
       }
     }
+    const _isWantedDepPrefSame = isWantedDepPrefSame.bind(null, ctx.wantedLockfile.catalogs, opts.catalogs)
     const upToDateLockfileMajorVersion = ctx.wantedLockfile.lockfileVersion.toString().startsWith(`${LOCKFILE_MAJOR_VERSION}.`)
     let needsFullResolution = outdatedLockfileSettings ||
       opts.fixLockfile ||
@@ -591,28 +594,6 @@ Note that in CI environments, this setting is enabled by default.`,
     }
     /* eslint-enable no-await-in-loop */
 
-    function isWantedDepPrefSame (alias: string, prevPref: string | undefined, nextPref: string): boolean {
-      if (prevPref !== nextPref) {
-        return false
-      }
-
-      // When pnpm catalogs are used, the specifiers can be the same (e.g.
-      // "catalog:default"), but the wanted versions for the dependency can be
-      // different after resolution if the catalog config was just edited.
-      const catalogName = parseCatalogProtocol(prevPref)
-
-      // If there's no catalog name, the catalog protocol was not used and we
-      // can assume the pref is the same since prevPref and nextPref match.
-      if (catalogName === null) {
-        return true
-      }
-
-      const prevCatalogEntrySpec = ctx.wantedLockfile.catalogs?.[catalogName]?.[alias]?.specifier
-      const nextCatalogEntrySpec = opts.catalogs[catalogName]?.[alias]
-
-      return prevCatalogEntrySpec === nextCatalogEntrySpec
-    }
-
     async function installCase (project: any) { // eslint-disable-line
       const wantedDependencies = getWantedDependencies(project.manifest, {
         autoInstallPeers: opts.autoInstallPeers,
@@ -623,7 +604,7 @@ Note that in CI environments, this setting is enabled by default.`,
         .map((wantedDependency) => ({ ...wantedDependency, updateSpec: true, preserveNonSemverVersionSpec: true }))
 
       if (ctx.wantedLockfile?.importers) {
-        forgetResolutionsOfPrevWantedDeps(ctx.wantedLockfile.importers[project.id], wantedDependencies, isWantedDepPrefSame)
+        forgetResolutionsOfPrevWantedDeps(ctx.wantedLockfile.importers[project.id], wantedDependencies, _isWantedDepPrefSame)
       }
       if (opts.ignoreScripts && project.manifest?.scripts &&
         (project.manifest.scripts.preinstall ||
@@ -755,6 +736,42 @@ function forgetResolutionsOfAllPrevWantedDeps (wantedLockfile: LockfileObject):
   }
 }
 
+/**
+ * Check if a wanted pref is the same.
+ *
+ * It would be different if the user modified a dependency in package.json or a
+ * catalog entry in pnpm-workspace.yaml. This is normally a simple check to see
+ * if the specifier strings match, but catalogs make this more involved since we
+ * also have to check if the catalog config in pnpm-workspace.yaml is the same.
+ */
+function isWantedDepPrefSame (
+  prevCatalogs: CatalogSnapshots | undefined,
+  catalogsConfig: Catalogs | undefined,
+  alias: string,
+  prevPref: string | undefined,
+  nextPref: string
+): boolean {
+  if (prevPref !== nextPref) {
+    return false
+  }
+
+  // When pnpm catalogs are used, the specifiers can be the same (e.g.
+  // "catalog:default"), but the wanted versions for the dependency can be
+  // different after resolution if the catalog config was just edited.
+  const catalogName = parseCatalogProtocol(prevPref)
+
+  // If there's no catalog name, the catalog protocol was not used and we
+  // can assume the pref is the same since prevPref and nextPref match.
+  if (catalogName === null) {
+    return true
+  }
+
+  const prevCatalogEntrySpec = prevCatalogs?.[catalogName]?.[alias]?.specifier
+  const nextCatalogEntrySpec = catalogsConfig?.[catalogName]?.[alias]
+
+  return prevCatalogEntrySpec === nextCatalogEntrySpec
+}
+
 export async function addDependenciesToPackage (
   manifest: ProjectManifest,
   dependencySelectors: string[],
@@ -1370,10 +1387,15 @@ const installInContext: InstallFunction = async (projects, ctx, opts) => {
             nodeExecPath: opts.nodeExecPath,
             injectWorkspacePackages: opts.injectWorkspacePackages,
           }
+          const _isWantedDepPrefSame = isWantedDepPrefSame.bind(null, ctx.wantedLockfile.catalogs, opts.catalogs)
           for (const project of allProjectsLocatedInsideWorkspace) {
             if (!newProjects.some(({ rootDir }) => rootDir === project.rootDir)) {
+              // This code block mirrors the installCase() function in
+              // mutateModules(). Consider a refactor that combines this logic
+              // to deduplicate code.
               const wantedDependencies = getWantedDependencies(project.manifest, getWantedDepsOpts)
                 .map((wantedDependency) => ({ ...wantedDependency, updateSpec: true, preserveNonSemverVersionSpec: true }))
+              forgetResolutionsOfPrevWantedDeps(ctx.wantedLockfile.importers[project.id], wantedDependencies, _isWantedDepPrefSame)
               newProjects.push({
                 mutation: 'install',
                 ...project,

pkg-manager/core/test/catalogs.ts

@@ -3,6 +3,7 @@ import { type ProjectRootDir, type ProjectId, type ProjectManifest } from '@pnpm
 import { prepareEmpty } from '@pnpm/prepare'
 import { addDistTag } from '@pnpm/registry-mock'
 import { type MutatedProject, mutateModules, type ProjectOptions, type MutateModulesOptions, addDependenciesToPackage } from '@pnpm/core'
+import { sync as loadJsonFile } from 'load-json-file'
 import path from 'path'
 import { testDefaults } from './utils'
 
@@ -297,6 +298,87 @@ test('lockfile catalog snapshots retain existing entries on --filter', async ()
   })
 })
 
+// Regression test for https://github.com/pnpm/pnpm/issues/8638
+test('lockfile catalog snapshots do not contain stale references on --filter', async () => {
+  const { options, projects, readLockfile } = preparePackagesAndReturnObjects([
+    {
+      name: 'project1',
+      dependencies: {},
+    },
+    {
+      name: 'project2',
+      dependencies: {
+        'is-positive': 'catalog:',
+      },
+    },
+  ])
+
+  await mutateModules(installProjects(projects), {
+    ...options,
+    catalogs: {
+      default: {
+        'is-positive': '^1.0.0',
+      },
+    },
+  })
+
+  expect(readLockfile().catalogs).toStrictEqual({
+    default: {
+      'is-positive': { specifier: '^1.0.0', version: '1.0.0' },
+    },
+  })
+
+  // This test updates the catalog entry in project2, but only performs a
+  // filtered install on project1. The lockfile catalog snapshots for project2
+  // should still be updated despite it not being part of the filtered install.
+  const onlyProject1 = installProjects(projects).slice(0, 1)
+  expect(onlyProject1).toMatchObject([{ id: 'project1' }])
+
+  await mutateModules(onlyProject1, {
+    ...options,
+    catalogs: {
+      default: {
+        'is-positive': '=3.1.0',
+      },
+    },
+  })
+
+  expect(readLockfile()).toEqual(expect.objectContaining({
+    catalogs: {
+      default: {
+        'is-positive': { specifier: '=3.1.0', version: '3.1.0' },
+      },
+    },
+    importers: expect.objectContaining({
+      project1: {},
+      project2: expect.objectContaining({
+        dependencies: {
+          // project 2 should be updated even though it wasn't part of the
+          // filtered install. This is due to a filtered install updating
+          // the lockfile first: https://github.com/pnpm/pnpm/pull/8183
+          'is-positive': { specifier: 'catalog:', version: '3.1.0' },
+        },
+      }),
+    }),
+  }))
+
+  // is-positive was not updated because only dependencies of project1 were.
+  const pathToIsPositivePkgJson = path.join(options.allProjects[1].rootDir!, 'node_modules/is-positive/package.json')
+  expect(loadJsonFile<ProjectManifest>(pathToIsPositivePkgJson)?.version).toBe('1.0.0')
+
+  await mutateModules(installProjects(projects), {
+    ...options,
+    catalogs: {
+      default: {
+        'is-positive': '=3.1.0',
+      },
+    },
+  })
+
+  // is-positive is now updated because a full install took place.
+  expect(loadJsonFile<ProjectManifest>(pathToIsPositivePkgJson)?.version).toBe('3.1.0')
+})
+
 test('external dependency using catalog protocol errors', async () => {
   const { options, projects } = preparePackagesAndReturnObjects([
     {