From 1072ec12869fcad320459754b5fbbac09d7787a0 Mon Sep 17 00:00:00 2001 From: Zoltan Kochan Date: Sun, 22 Jan 2023 20:03:30 +0200 Subject: [PATCH] fix: don't remove hoisted dependencies from .pnpm on repeat install (#5971) --- .changeset/rare-dragons-crash.md | 7 +++++++ pkg-manager/core/src/install/link.ts | 6 ++++-- pkg-manager/core/test/install/hoist.ts | 15 ++++++++++++++- pkg-manager/modules-cleaner/src/prune.ts | 2 +- 4 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 .changeset/rare-dragons-crash.md diff --git a/.changeset/rare-dragons-crash.md b/.changeset/rare-dragons-crash.md new file mode 100644 index 0000000000..ca3a59a7fd --- /dev/null +++ b/.changeset/rare-dragons-crash.md @@ -0,0 +1,7 @@ +--- +"@pnpm/modules-cleaner": patch +"@pnpm/core": patch +"pnpm": patch +--- + +Packages hoisted to the virtual store are not removed on repeat install, when the non-headless algorithm runs the installation. diff --git a/pkg-manager/core/src/install/link.ts b/pkg-manager/core/src/install/link.ts index 7db9d9ae34..8b6838d152 100644 --- a/pkg-manager/core/src/install/link.ts +++ b/pkg-manager/core/src/install/link.ts @@ -245,7 +245,9 @@ export async function linkPackages ( } let newHoistedDependencies!: HoistedDependencies - if ((opts.hoistPattern != null || opts.publicHoistPattern != null) && (newDepPaths.length > 0 || removedDepPaths.size > 0)) { + if (opts.hoistPattern == null && opts.publicHoistPattern == null) { + newHoistedDependencies = {} + } else if (newDepPaths.length > 0 || removedDepPaths.size > 0) { // It is important to keep the skipped packages in the lockfile which will be saved as the "current lockfile". // pnpm is comparing the current lockfile to the wanted one and they should match. // But for hoisting, we need a version of the lockfile w/o the skipped packages, so we're making a copy. @@ -264,7 +266,7 @@ export async function linkPackages ( virtualStoreDir: opts.virtualStoreDir, }) } else { - newHoistedDependencies = {} + newHoistedDependencies = opts.hoistedDependencies } return { diff --git a/pkg-manager/core/test/install/hoist.ts b/pkg-manager/core/test/install/hoist.ts index 46805b0fd0..9b1ce85d5d 100644 --- a/pkg-manager/core/test/install/hoist.ts +++ b/pkg-manager/core/test/install/hoist.ts @@ -20,7 +20,7 @@ import { testDefaults } from '../utils' test('should hoist dependencies', async () => { const project = prepareEmpty() - await addDependenciesToPackage({}, ['express', '@foo/has-dep-from-same-scope'], await testDefaults({ fastUnpack: false, hoistPattern: '*' })) + const manifest = await addDependenciesToPackage({}, ['express', '@foo/has-dep-from-same-scope'], await testDefaults({ fastUnpack: false, hoistPattern: '*' })) await project.has('express') await project.has('.pnpm/node_modules/debug') @@ -31,6 +31,19 @@ test('should hoist dependencies', async () => { // should also hoist bins await project.isExecutable('.pnpm/node_modules/.bin/mime') + + const modules = await project.readModulesManifest() + expect(Object.keys(modules!.hoistedDependencies).length > 0).toBeTruthy() + + // On repeat install the hoisted packages are preserved (non-headless install) + await install(manifest, await testDefaults({ fastUnpack: false, hoistPattern: '*', preferFrozenLockfile: false, modulesCacheMaxAge: 0 })) + await project.has('.pnpm/node_modules/debug') + expect((await project.readModulesManifest())!.hoistedDependencies).toStrictEqual(modules!.hoistedDependencies) + + // On repeat install the hoisted packages are preserved (headless install) + await install(manifest, await testDefaults({ fastUnpack: false, hoistPattern: '*', frozenLockfile: true, modulesCacheMaxAge: 0 })) + await project.has('.pnpm/node_modules/debug') + expect((await project.readModulesManifest())!.hoistedDependencies).toStrictEqual(modules!.hoistedDependencies) }) test('should hoist dependencies to the root of node_modules when publicHoistPattern is used', async () => { diff --git a/pkg-manager/modules-cleaner/src/prune.ts b/pkg-manager/modules-cleaner/src/prune.ts index d1dc89d670..379956c8e3 100644 --- a/pkg-manager/modules-cleaner/src/prune.ts +++ b/pkg-manager/modules-cleaner/src/prune.ts @@ -156,7 +156,7 @@ export async function prune ( .map((orphanDepPath) => depPathToFilename(orphanDepPath)) .map(async (orphanDepPath) => _tryRemovePkg(orphanDepPath)) ) - const neededPkgs: Set = new Set() + const neededPkgs: Set = new Set(['node_modules']) for (const depPath of Object.keys(opts.wantedLockfile.packages ?? {})) { if (opts.skipped.has(depPath)) continue neededPkgs.add(depPathToFilename(depPath))