diff --git a/.changeset/tall-zebras-accept.md b/.changeset/tall-zebras-accept.md new file mode 100644 index 0000000000..d758330064 --- /dev/null +++ b/.changeset/tall-zebras-accept.md @@ -0,0 +1,6 @@ +--- +"@pnpm/core": patch +"pnpm": patch +--- + +Dedupe direct dependencies after hoisting. diff --git a/pkg-manager/core/src/install/link.ts b/pkg-manager/core/src/install/link.ts index 296545950a..e8de84562e 100644 --- a/pkg-manager/core/src/install/link.ts +++ b/pkg-manager/core/src/install/link.ts @@ -157,52 +157,6 @@ export async function linkPackages ( stage: 'importing_done', }) - if (opts.symlink) { - const projectsToLink = Object.fromEntries(await Promise.all( - projects.map(async ({ id, manifest, modulesDir, rootDir }) => { - const deps = opts.dependenciesByProjectId[id] - const importerFromLockfile = newCurrentLockfile.importers[id] - return [id, { - dir: rootDir, - modulesDir, - dependencies: await Promise.all([ - ...Object.entries(deps) - .filter(([rootAlias]) => importerFromLockfile.specifiers[rootAlias]) - .map(([rootAlias, depPath]) => ({ rootAlias, depGraphNode: depGraph[depPath] })) - .filter(({ depGraphNode }) => depGraphNode) - .map(async ({ rootAlias, depGraphNode }) => { - const isDev = Boolean(manifest.devDependencies?.[depGraphNode.name]) - const isOptional = Boolean(manifest.optionalDependencies?.[depGraphNode.name]) - return { - alias: rootAlias, - name: depGraphNode.name, - version: depGraphNode.version, - dir: depGraphNode.dir, - id: depGraphNode.id, - dependencyType: (isDev && 'dev' || isOptional && 'optional' || 'prod') as 'dev' | 'optional' | 'prod', - latest: opts.outdatedDependencies[depGraphNode.id], - isExternalLink: false, - } - }), - ...opts.linkedDependenciesByProjectId[id].map(async (linkedDependency) => { - const dir = resolvePath(rootDir, linkedDependency.resolution.directory) - return { - alias: linkedDependency.alias, - name: linkedDependency.name, - version: linkedDependency.version, - dir, - id: linkedDependency.resolution.directory, - dependencyType: (linkedDependency.dev && 'dev' || linkedDependency.optional && 'optional' || 'prod') as 'dev' | 'optional' | 'prod', - isExternalLink: true, - } - }), - ]), - }] - })) - ) - await linkDirectDeps(projectsToLink, { dedupe: opts.dedupeDirectDeps }) - } - let currentLockfile: Lockfile const allImportersIncluded = equals(projectIds.sort(), Object.keys(opts.wantedLockfile.importers).sort()) if ( @@ -269,6 +223,52 @@ export async function linkPackages ( newHoistedDependencies = opts.hoistedDependencies } + if (opts.symlink) { + const projectsToLink = Object.fromEntries(await Promise.all( + projects.map(async ({ id, manifest, modulesDir, rootDir }) => { + const deps = opts.dependenciesByProjectId[id] + const importerFromLockfile = newCurrentLockfile.importers[id] + return [id, { + dir: rootDir, + modulesDir, + dependencies: await Promise.all([ + ...Object.entries(deps) + .filter(([rootAlias]) => importerFromLockfile.specifiers[rootAlias]) + .map(([rootAlias, depPath]) => ({ rootAlias, depGraphNode: depGraph[depPath] })) + .filter(({ depGraphNode }) => depGraphNode) + .map(async ({ rootAlias, depGraphNode }) => { + const isDev = Boolean(manifest.devDependencies?.[depGraphNode.name]) + const isOptional = Boolean(manifest.optionalDependencies?.[depGraphNode.name]) + return { + alias: rootAlias, + name: depGraphNode.name, + version: depGraphNode.version, + dir: depGraphNode.dir, + id: depGraphNode.id, + dependencyType: (isDev && 'dev' || isOptional && 'optional' || 'prod') as 'dev' | 'optional' | 'prod', + latest: opts.outdatedDependencies[depGraphNode.id], + isExternalLink: false, + } + }), + ...opts.linkedDependenciesByProjectId[id].map(async (linkedDependency) => { + const dir = resolvePath(rootDir, linkedDependency.resolution.directory) + return { + alias: linkedDependency.alias, + name: linkedDependency.name, + version: linkedDependency.version, + dir, + id: linkedDependency.resolution.directory, + dependencyType: (linkedDependency.dev && 'dev' || linkedDependency.optional && 'optional' || 'prod') as 'dev' | 'optional' | 'prod', + isExternalLink: true, + } + }), + ]), + }] + })) + ) + await linkDirectDeps(projectsToLink, { dedupe: opts.dedupeDirectDeps }) + } + return { currentLockfile, newDepPaths, diff --git a/pkg-manager/core/test/install/dedupeDirectDeps.ts b/pkg-manager/core/test/install/dedupeDirectDeps.ts index 243dec7bd6..788791ecb0 100644 --- a/pkg-manager/core/test/install/dedupeDirectDeps.ts +++ b/pkg-manager/core/test/install/dedupeDirectDeps.ts @@ -2,6 +2,7 @@ import fs from 'fs' import path from 'path' import { preparePackages } from '@pnpm/prepare' import { mutateModules, type MutatedProject } from '@pnpm/core' +import rimraf from '@zkochan/rimraf' import { testDefaults } from '../utils' test('dedupe direct dependencies', async () => { @@ -102,3 +103,77 @@ test('dedupe direct dependencies', async () => { await projects['project-3'].hasNot('is-negative') expect(fs.existsSync('project-3/node_modules')).toBeFalsy() }) + +test('dedupe direct dependencies after public hoisting', async () => { + const projects = preparePackages([ + { + location: '', + package: { name: 'project-1' }, + }, + { + location: 'project-2', + package: { name: 'project-2' }, + }, + ]) + + const importers: MutatedProject[] = [ + { + mutation: 'install', + rootDir: process.cwd(), + }, + { + mutation: 'install', + rootDir: path.resolve('project-2'), + }, + ] + const allProjects = [ + { + buildIndex: 0, + manifest: { + name: 'project-1', + version: '1.0.0', + + dependencies: { + '@pnpm.e2e/pkg-with-1-dep': '100.0.0', + }, + }, + rootDir: process.cwd(), + }, + { + buildIndex: 0, + manifest: { + name: 'project-2', + version: '1.0.0', + + dependencies: { + '@pnpm.e2e/dep-of-pkg-with-1-dep': '100.0.0', + }, + }, + rootDir: path.resolve('project-2'), + }, + ] + const opts = await testDefaults({ + allProjects, + dedupeDirectDeps: true, + publicHoistPattern: ['@pnpm.e2e/dep-of-pkg-with-1-dep'], + }) + await mutateModules(importers, opts) + await projects['project-1'].has('@pnpm.e2e/dep-of-pkg-with-1-dep') + await projects['project-2'].hasNot('@pnpm.e2e/dep-of-pkg-with-1-dep') + expect(Array.from(fs.readdirSync('node_modules/@pnpm.e2e').sort())).toEqual([ + 'dep-of-pkg-with-1-dep', + 'pkg-with-1-dep', + ]) + expect(fs.existsSync('project-2/node_modules')).toBeFalsy() + + // Test the same with headless install + await rimraf('node_modules') + await mutateModules(importers, { ...opts, frozenLockfile: true }) + await projects['project-1'].has('@pnpm.e2e/dep-of-pkg-with-1-dep') + await projects['project-2'].hasNot('@pnpm.e2e/dep-of-pkg-with-1-dep') + expect(Array.from(fs.readdirSync('node_modules/@pnpm.e2e').sort())).toEqual([ + 'dep-of-pkg-with-1-dep', + 'pkg-with-1-dep', + ]) + expect(fs.existsSync('project-2/node_modules')).toBeFalsy() +}) diff --git a/pnpm/test/install/hoist.ts b/pnpm/test/install/hoist.ts index ebca1f98d5..2b5b59ff0d 100644 --- a/pnpm/test/install/hoist.ts +++ b/pnpm/test/install/hoist.ts @@ -58,12 +58,13 @@ test('shamefully-hoist: applied to all the workspace projects when set to true i ]) await writeYamlFile('pnpm-workspace.yaml', { packages: ['**', '!store/**'] }) - await fs.writeFile('.npmrc', 'shamefully-hoist', 'utf8') + await fs.writeFile('.npmrc', 'shamefully-hoist=true', 'utf8') - await execPnpm(['recursive', 'install']) + await execPnpm(['install']) await projects.root.has('@pnpm.e2e/dep-of-pkg-with-1-dep') await projects.root.has('@pnpm.e2e/foo') + await projects.root.has('@pnpm.e2e/foobar') await projects.project.hasNot('@pnpm.e2e/foo') - await projects.project.has('@pnpm.e2e/foobar') + await projects.project.hasNot('@pnpm.e2e/foobar') })