From 824363b0baac8aede87917e572db331aeb10abed Mon Sep 17 00:00:00 2001 From: Brandon Cheng Date: Sun, 7 Jul 2024 08:27:22 -0400 Subject: [PATCH] fix: preserve catalog protocol when updating project manifest (#8285) * fix: preserve catalog protocol when updating project manifest This makes `pnpm add foo@catalog:` preserve the catalog protocol when updating `package.json`. This also fixes a quirk with `pnpm update --latest ` when is a catalog'ed dep. The `updateSpec` field is set on all deps if `wantedDependencies` filters to 0. https://github.com/pnpm/pnpm/blob/341656f9b36fd08a138fe31969f21f6a9409b0ea/pkg-manager/resolve-dependencies/src/toResolveImporter.ts#L44-L48 * test: ensure pnpm update does not update catalog dependencies (yet) * test: `pnpm add is-positive@catalog:` --- pkg-manager/core/test/catalogs.ts | 122 +++++++++++++++++- .../src/resolveDependencies.ts | 6 +- .../src/resolveDependencyTree.ts | 11 ++ .../src/updateProjectManifest.ts | 5 +- 4 files changed, 141 insertions(+), 3 deletions(-) diff --git a/pkg-manager/core/test/catalogs.ts b/pkg-manager/core/test/catalogs.ts index 5ce8a56256..81cc81dd5b 100644 --- a/pkg-manager/core/test/catalogs.ts +++ b/pkg-manager/core/test/catalogs.ts @@ -1,7 +1,7 @@ import { createPeersDirSuffix } from '@pnpm/dependency-path' import { type ProjectRootDir, type ProjectId, type ProjectManifest } from '@pnpm/types' import { prepareEmpty } from '@pnpm/prepare' -import { type MutatedProject, mutateModules, type ProjectOptions, type MutateModulesOptions } from '@pnpm/core' +import { type MutatedProject, mutateModules, type ProjectOptions, type MutateModulesOptions, addDependenciesToPackage } from '@pnpm/core' import path from 'path' import { testDefaults } from './utils' @@ -511,3 +511,123 @@ test('lockfile catalog snapshots should remove unused entries', async () => { }) } }) + +describe('add', () => { + test('adding is-positive@catalog: works', async () => { + const { options, projects, readLockfile } = preparePackagesAndReturnObjects([{ + name: 'project1', + dependencies: {}, + }]) + + const updatedManifest = await addDependenciesToPackage( + projects['project1' as ProjectId], + ['is-positive@catalog:'], + { + ...options, + lockfileOnly: true, + allowNew: true, + catalogs: { + default: { 'is-positive': '1.0.0' }, + }, + }) + + expect(updatedManifest).toEqual({ + name: 'project1', + dependencies: { + 'is-positive': 'catalog:', + }, + }) + expect(readLockfile()).toEqual(expect.objectContaining({ + catalogs: { default: { 'is-positive': { specifier: '1.0.0', version: '1.0.0' } } }, + packages: { 'is-positive@1.0.0': expect.objectContaining({}) }, + })) + }) +}) + +// The 'pnpm update' command should eventually support updates of dependencies +// in the catalog. This is a more involved feature since pnpm-workspace.yaml +// needs to be edited. Until the catalog update feature is implemented, ensure +// pnpm update does not touch or rewrite dependencies using the catalog +// protocol. +describe('update', () => { + test('update does not modify catalog: protocol', async () => { + const { options, projects } = preparePackagesAndReturnObjects([{ + name: 'project1', + dependencies: { + 'is-positive': 'catalog:', + }, + }]) + + const updatedManifest = await addDependenciesToPackage( + projects['project1' as ProjectId], + ['is-positive'], + { + ...options, + lockfileOnly: true, + allowNew: false, + update: true, + catalogs: { + default: { 'is-positive': '^1.0.0' }, + }, + }) + + // Expecting the manifest to remain unchanged. + expect(updatedManifest).toEqual({ + name: 'project1', + dependencies: { + 'is-positive': 'catalog:', + }, + }) + }) + + test('update does not upgrade cataloged dependency', async () => { + const { options, projects, readLockfile } = preparePackagesAndReturnObjects([{ + name: 'project1', + dependencies: { + 'is-positive': 'catalog:', + }, + }]) + + const catalogs = { + default: { 'is-positive': '3.0.0' }, + } + const mutateOpts = { + ...options, + lockfileOnly: true, + catalogs, + } + + await mutateModules(installProjects(projects), mutateOpts) + + // Updating the catalog from 3.0.0 to ^3.0.0. This should still lock to the + // existing 3.0.0 version despite version 3.1.0 existing. + catalogs.default['is-positive'] = '^3.0.0' + await mutateModules(installProjects(projects), mutateOpts) + + expect(readLockfile().catalogs.default).toEqual({ + 'is-positive': { specifier: '^3.0.0', version: '3.0.0' }, + }) + + // Expecting the manifest to remain unchanged after running an update. + const updatedManifest = await addDependenciesToPackage( + projects['project1' as ProjectId], + ['is-positive'], + { + ...mutateOpts, + update: true, + }) + + expect(updatedManifest).toEqual({ + name: 'project1', + dependencies: { + 'is-positive': 'catalog:', + }, + }) + + // The lockfile should only contain 3.0.0 and not 3.1.0 (or a later version). + expect(readLockfile()).toEqual(expect.objectContaining({ + catalogs: { default: { 'is-positive': { specifier: '^3.0.0', version: '3.0.0' } } }, + packages: { 'is-positive@3.0.0': expect.objectContaining({}) }, + })) + }) +}) diff --git a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts index 30e72165c5..b5870a561c 100644 --- a/pkg-manager/resolve-dependencies/src/resolveDependencies.ts +++ b/pkg-manager/resolve-dependencies/src/resolveDependencies.ts @@ -530,6 +530,7 @@ async function resolveDependenciesOfImporterDependency ( throw result.error }, }) + const originalPref = extendedWantedDep.wantedDependency.pref if (catalogLookup != null) { // The lockfile from a previous installation may have already resolved this @@ -560,7 +561,10 @@ async function resolveDependenciesOfImporterDependency ( // If the catalog protocol was used, store metadata about the catalog // lookup to use in the lockfile. if (result.resolveDependencyResult != null && catalogLookup != null) { - result.resolveDependencyResult.catalogLookup = catalogLookup + result.resolveDependencyResult.catalogLookup = { + ...catalogLookup, + userSpecifiedPref: originalPref, + } } return result diff --git a/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts b/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts index ffd5970da0..661f727a15 100644 --- a/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts +++ b/pkg-manager/resolve-dependencies/src/resolveDependencyTree.ts @@ -62,6 +62,17 @@ export interface ResolvedDirectDependency { export interface CatalogLookupMetadata { readonly catalogName: string readonly specifier: string + + /** + * The catalog protocol pref the user wrote in package.json files or as a + * parameter to pnpm add. Ex: pnpm add foo@catalog: + * + * This will usually be 'catalog:', but can simply be 'catalog:' if + * users wrote the default catalog shorthand. This is different than the + * catalogName field, which would be 'default' regardless of whether users + * originally requested 'catalog:' or 'catalog:default'. + */ + readonly userSpecifiedPref: string } export interface Importer { diff --git a/pkg-manager/resolve-dependencies/src/updateProjectManifest.ts b/pkg-manager/resolve-dependencies/src/updateProjectManifest.ts index 3a4efc7de0..75becc8ebd 100644 --- a/pkg-manager/resolve-dependencies/src/updateProjectManifest.ts +++ b/pkg-manager/resolve-dependencies/src/updateProjectManifest.ts @@ -72,6 +72,7 @@ export async function updateProjectManifest ( function resolvedDirectDepToSpecObject ( { alias, + catalogLookup, isNew, name, normalizedPref, @@ -89,7 +90,9 @@ function resolvedDirectDepToSpecObject ( } ): PackageSpecObject { let pref!: string - if (normalizedPref) { + if (catalogLookup) { + pref = catalogLookup.userSpecifiedPref + } else if (normalizedPref) { pref = normalizedPref } else { const shouldUseWorkspaceProtocol = resolution.type === 'directory' &&