fix: dedupe commands of direct dependencies (#7359)

.changeset/tasty-bugs-approve.md

@@ -0,0 +1,10 @@
+---
+"@pnpm/plugin-commands-installation": patch
+"@pnpm/resolve-dependencies": patch
+"@pnpm/modules-cleaner": patch
+"@pnpm/headless": patch
+"@pnpm/core": patch
+"pnpm": patch
+---
+
+When `dedupe-direct-deps` is set to `true`, commands of dependencies should be deduplicated [#7359](https://github.com/pnpm/pnpm/pull/7359).

patching/plugin-commands-patching/test/patch.test.ts

@@ -601,6 +601,7 @@ describe('patch and commit in workspaces', () => {
     await install.handler({
       ...DEFAULT_OPTS,
       cacheDir,
+      dedupeDirectDeps: true,
       storeDir,
       allProjects,
       allProjectsGraph,
@@ -633,6 +634,7 @@ describe('patch and commit in workspaces', () => {
       allProjects,
       allProjectsGraph,
       selectedProjectsGraph,
+      dedupeDirectDeps: true,
       dir: process.cwd(),
       rootProjectManifestDir: process.cwd(),
       cacheDir,

patching/plugin-commands-patching/test/utils/index.ts

@@ -11,6 +11,7 @@ export const DEFAULT_OPTS = {
   ca: undefined,
   cacheDir: '../cache',
   cert: undefined,
+  dedupeDirectDeps: false,
   extraEnv: {},
   cliOptions: {},
   fetchRetries: 2,

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

@@ -1009,6 +1009,7 @@ const _installInContext: InstallFunction = async (projects, ctx, opts) => {
       autoInstallPeers: opts.autoInstallPeers,
       currentLockfile: ctx.currentLockfile,
       defaultUpdateDepth: opts.depth,
+      dedupeDirectDeps: opts.dedupeDirectDeps,
       dedupePeerDependents: opts.dedupePeerDependents,
       dryRun: opts.lockfileOnly,
       engineStrict: opts.engineStrict,

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

@@ -106,6 +106,7 @@ export async function linkPackages (
   depGraph = Object.fromEntries(depNodes.map((depNode) => [depNode.depPath, depNode]))
   const removedDepPaths = await prune(projects, {
     currentLockfile: opts.currentLockfile,
+    dedupeDirectDeps: opts.dedupeDirectDeps,
     hoistedDependencies: opts.hoistedDependencies,
     hoistedModulesDir: (opts.hoistPattern != null) ? opts.hoistedModulesDir : undefined,
     include: opts.include,

pkg-manager/core/test/install/autoInstallPeers.ts

@@ -342,6 +342,7 @@ test('auto install peer deps in a workspace. test #1', async () => {
         rootDir: path.resolve('project'),
       },
     ],
+    dedupeDirectDeps: false,
   }))
 })
 
@@ -381,6 +382,7 @@ test('auto install peer deps in a workspace. test #2', async () => {
         rootDir: path.resolve('project'),
       },
     ],
+    dedupeDirectDeps: false,
   }))
 })
 

pkg-manager/core/test/install/dedupeDirectDeps.ts

@@ -58,7 +58,7 @@ test('dedupe direct dependencies', async () => {
         version: '1.0.0',
 
         dependencies: {
-          'is-negative': '1.0.0',
+          '@pnpm.e2e/hello-world-js-bin': '1.0.0',
         },
       },
       rootDir: path.resolve('project-2'),
@@ -70,37 +70,39 @@ test('dedupe direct dependencies', async () => {
         version: '1.0.0',
 
         dependencies: {
-          'is-negative': '1.0.0',
+          '@pnpm.e2e/hello-world-js-bin': '1.0.0',
         },
       },
       rootDir: path.resolve('project-3'),
     },
   ]
   await mutateModules(importers, await testDefaults({ allProjects, dedupeDirectDeps: true }))
-  await projects['project-2'].has('is-negative')
-  await projects['project-3'].has('is-negative')
+  await projects['project-2'].has('@pnpm.e2e/hello-world-js-bin')
+  await projects['project-3'].has('@pnpm.e2e/hello-world-js-bin')
 
-  allProjects[0].manifest.dependencies['is-negative'] = '1.0.0'
+  allProjects[0].manifest.dependencies['@pnpm.e2e/hello-world-js-bin'] = '1.0.0'
   allProjects[1].manifest.dependencies['is-positive'] = '1.0.0'
   allProjects[1].manifest.dependencies['is-odd'] = '2.0.0'
   await mutateModules(importers, await testDefaults({ allProjects, dedupeDirectDeps: true }))
 
   expect(Array.from(fs.readdirSync('node_modules').sort())).toEqual([
+    '.bin',
     '.modules.yaml',
     '.pnpm',
+    '@pnpm.e2e',
     'foo',
-    'is-negative',
     'is-odd',
     'is-positive',
   ])
+  expect(Array.from(fs.readdirSync('node_modules/@pnpm.e2e'))).toEqual(['hello-world-js-bin'])
   expect(fs.readdirSync('project-2/node_modules').sort()).toEqual(['is-odd'])
-  await projects['project-3'].hasNot('is-negative')
+  await projects['project-3'].hasNot('@pnpm.e2e/hello-world-js-bin')
   expect(fs.existsSync('project-3/node_modules')).toBeFalsy()
 
   // Test the same with headless install
   await mutateModules(importers, await testDefaults({ allProjects, dedupeDirectDeps: true, frozenLockfile: true }))
   expect(fs.readdirSync('project-2/node_modules').sort()).toEqual(['is-odd'])
-  await projects['project-3'].hasNot('is-negative')
+  await projects['project-3'].hasNot('@pnpm.e2e/hello-world-js-bin')
   expect(fs.existsSync('project-3/node_modules')).toBeFalsy()
 })
 

pkg-manager/headless/src/index.ts

@@ -235,6 +235,7 @@ export async function headlessInstall (opts: HeadlessOptions): Promise<Installat
         selectedProjects,
         {
           currentLockfile,
+          dedupeDirectDeps: opts.dedupeDirectDeps,
           dryRun: false,
           hoistedDependencies: opts.hoistedDependencies,
           hoistedModulesDir: (opts.hoistPattern == null) ? undefined : hoistedModulesDir,
@@ -522,6 +523,7 @@ export async function headlessInstall (opts: HeadlessOptions): Promise<Installat
   })
 
   if (opts.enableModulesDir !== false) {
+    const rootProjectDeps = !opts.dedupeDirectDeps ? {} : (directDependenciesByImporterId['.'] ?? {})
     /** Skip linking and due to no project manifest */
     if (!opts.ignorePackageManifest) {
       await Promise.all(selectedProjects.map(async (project) => {
@@ -531,7 +533,17 @@ export async function headlessInstall (opts: HeadlessOptions): Promise<Installat
             preferSymlinkedExecutables: opts.preferSymlinkedExecutables,
           })
         } else {
-          const directPkgDirs = Object.values(directDependenciesByImporterId[project.id])
+          let directPkgDirs: string[]
+          if (project.id === '.') {
+            directPkgDirs = Object.values(directDependenciesByImporterId[project.id])
+          } else {
+            directPkgDirs = []
+            for (const [alias, dir] of Object.entries(directDependenciesByImporterId[project.id])) {
+              if (rootProjectDeps[alias] !== dir) {
+                directPkgDirs.push(dir)
+              }
+            }
+          }
           await linkBinsOfPackages(
             (
               await Promise.all(
@@ -670,6 +682,13 @@ async function symlinkDirectDependencies (
       }),
     }]))
   ))
+  const rootProject = projectsToLink['.']
+  if (rootProject && dedupe) {
+    const rootDeps = Object.fromEntries(rootProject.dependencies.map((dep: LinkedDirectDep) => [dep.alias, dep.dir]))
+    for (const project of Object.values(omit(['.'], projectsToLink))) {
+      project.dependencies = project.dependencies.filter((dep: LinkedDirectDep) => dep.dir !== rootDeps[dep.alias])
+    }
+  }
   return linkDirectDeps(projectsToLink, { dedupe: Boolean(dedupe) })
 }
 

pkg-manager/modules-cleaner/src/prune.ts

@@ -26,7 +26,7 @@ import difference from 'ramda/src/difference'
 import equals from 'ramda/src/equals'
 import mergeAll from 'ramda/src/mergeAll'
 import pickAll from 'ramda/src/pickAll'
-import { removeDirectDependency } from './removeDirectDependency'
+import { removeDirectDependency, removeIfEmpty } from './removeDirectDependency'
 
 export async function prune (
   importers: Array<{
@@ -38,6 +38,7 @@ export async function prune (
     rootDir: string
   }>,
   opts: {
+    dedupeDirectDeps?: boolean
     dryRun?: boolean
     include: { [dependenciesField in DependenciesField]: boolean }
     hoistedDependencies: HoistedDependencies
@@ -58,6 +59,8 @@ export async function prune (
     include: opts.include,
     skipped: opts.skipped,
   })
+  const rootImporter = wantedLockfile.importers['.'] ?? {} as ProjectSnapshot
+  const wantedRootPkgs = mergeDependencies(rootImporter)
   await Promise.all(importers.map(async ({ binsDir, id, modulesDir, pruneDirectDependencies, removePackages, rootDir }) => {
     const currentImporter = opts.currentLockfile.importers[id] || {} as ProjectSnapshot
     const currentPkgs = Object.entries(mergeDependencies(currentImporter))
@@ -72,7 +75,11 @@ export async function prune (
       (removePackages ?? []).filter((removePackage) => allCurrentPackages.has(removePackage))
     )
     currentPkgs.forEach(([depName, depVersion]) => {
-      if (!wantedPkgs[depName] || wantedPkgs[depName] !== depVersion) {
+      if (
+        !wantedPkgs[depName] ||
+        wantedPkgs[depName] !== depVersion ||
+        opts.dedupeDirectDeps && id !== '.' && wantedPkgs[depName] === wantedRootPkgs[depName]
+      ) {
         depsToRemove.add(depName)
       }
     })
@@ -87,7 +94,12 @@ export async function prune (
       }
     }
 
-    return Promise.all(Array.from(depsToRemove).map(async (depName) => {
+    const removedFromScopes = new Set<string>()
+    await Promise.all(Array.from(depsToRemove).map(async (depName) => {
+      const scope = getScopeFromPackageName(depName)
+      if (scope) {
+        removedFromScopes.add(scope)
+      }
       return removeDirectDependency({
         dependenciesField: currentImporter.devDependencies?.[depName] != null && 'devDependencies' ||
           currentImporter.optionalDependencies?.[depName] != null && 'optionalDependencies' ||
@@ -101,6 +113,8 @@ export async function prune (
         rootDir,
       })
     }))
+    await Promise.all(Array.from(removedFromScopes).map((scope) => removeIfEmpty(path.join(modulesDir, scope))))
+    await removeIfEmpty(modulesDir)
   }))
 
   const selectedImporterIds = importers.map((importer) => importer.id).sort()
@@ -172,6 +186,13 @@ export async function prune (
   return new Set(orphanDepPaths)
 }
 
+function getScopeFromPackageName (pkgName: string): string | undefined {
+  if (pkgName[0] === '@') {
+    return pkgName.substring(0, pkgName.indexOf('/'))
+  }
+  return undefined
+}
+
 async function readVirtualStoreDir (virtualStoreDir: string, lockfileDir: string) {
   try {
     return await fs.readdir(virtualStoreDir)

pkg-manager/modules-cleaner/src/removeDirectDependency.ts

@@ -1,7 +1,9 @@
 import path from 'path'
+import { promises as fs } from 'fs'
 import { rootLogger } from '@pnpm/core-loggers'
 import { removeBin, removeBinsOfDependency } from '@pnpm/remove-bins'
 import { type DependenciesField } from '@pnpm/types'
+import rimraf from '@zkochan/rimraf'
 
 export async function removeDirectDependency (
   dependency: {
@@ -21,6 +23,7 @@ export async function removeDirectDependency (
     removeBinsOfDependency(dependencyDir, opts),
     !opts.dryRun && removeBin(dependencyDir) as any, // eslint-disable-line @typescript-eslint/no-explicit-any
   ])
+  await removeIfEmpty(opts.binsDir)
 
   const uninstalledPkg = results[0]
   if (!opts.muteLogs) {
@@ -37,3 +40,18 @@ export async function removeDirectDependency (
     })
   }
 }
+
+export async function removeIfEmpty (dir: string) {
+  if (await dirIsEmpty(dir)) {
+    await rimraf(dir)
+  }
+}
+
+async function dirIsEmpty (dir: string) {
+  try {
+    const fileNames = await fs.readdir(dir)
+    return fileNames.length === 0
+  } catch {
+    return false
+  }
+}

pkg-manager/plugin-commands-installation/src/install.ts

@@ -250,6 +250,7 @@ export type InstallCommandOptions = Pick<Config,
 | 'bail'
 | 'bin'
 | 'cliOptions'
+| 'dedupeDirectDeps'
 | 'dedupePeerDependents'
 | 'deployAllFiles'
 | 'depth'

pkg-manager/resolve-dependencies/src/index.ts

@@ -95,6 +95,7 @@ export async function resolveDependencies (
   opts: ResolveDependenciesOptions & {
     defaultUpdateDepth: number
     dedupePeerDependents?: boolean
+    dedupeDirectDeps?: boolean
     excludeLinksFromLockfile?: boolean
     preserveWorkspaceProtocol: boolean
     saveWorkspaceProtocol: 'rolling' | boolean
@@ -220,6 +221,7 @@ export async function resolveDependencies (
   } = resolvePeers({
     dependenciesTree,
     dedupePeerDependents: opts.dedupePeerDependents,
+    dedupeDirectDeps: opts.dedupeDirectDeps,
     lockfileDir: opts.lockfileDir,
     projects: projectsToLink,
     virtualStoreDir: opts.virtualStoreDir,

pkg-manager/resolve-dependencies/src/resolvePeers.ts

@@ -63,6 +63,7 @@ export function resolvePeers<T extends PartialResolvedPackage> (
     lockfileDir: string
     resolvePeersFromWorkspaceRoot?: boolean
     dedupePeerDependents?: boolean
+    dedupeDirectDeps?: boolean
   }
 ): {
     dependenciesGraph: GenericDependenciesGraph<T>
@@ -108,8 +109,28 @@ export function resolvePeers<T extends PartialResolvedPackage> (
   })
 
   const dependenciesByProjectId: { [id: string]: Record<string, string> } = {}
-  for (const { directNodeIdsByAlias, id } of opts.projects) {
-    dependenciesByProjectId[id] = mapValues((nodeId) => pathsByNodeId.get(nodeId)!, directNodeIdsByAlias)
+  if (opts.dedupeDirectDeps) {
+    const rootProject = opts.projects.find(({ id }) => id === '.')
+    if (rootProject) {
+      dependenciesByProjectId['.'] = mapValues((nodeId) => pathsByNodeId.get(nodeId)!, rootProject.directNodeIdsByAlias)
+    }
+    const rootDeps = dependenciesByProjectId['.'] ?? {}
+    for (const { directNodeIdsByAlias, id } of opts.projects) {
+      if (id !== '.') {
+        const deps: Record<string, string> = {}
+        for (const [alias, nodeId] of Object.entries(directNodeIdsByAlias)) {
+          const depPath = pathsByNodeId.get(nodeId)!
+          if (rootDeps[alias] !== depPath) {
+            deps[alias] = depPath
+          }
+        }
+        dependenciesByProjectId[id] = deps
+      }
+    }
+  } else {
+    for (const { directNodeIdsByAlias, id } of opts.projects) {
+      dependenciesByProjectId[id] = mapValues((nodeId) => pathsByNodeId.get(nodeId)!, directNodeIdsByAlias)
+    }
   }
   if (opts.dedupePeerDependents) {
     const duplicates = Array.from(depPathsByPkgId.values()).filter((item) => item.size > 1)

pnpm-lock.yaml

@@ -9332,8 +9332,8 @@ packages:
       '@types/lodash': 4.14.197
       '@types/semver': 7.5.3
       '@types/treeify': 1.0.3
-      '@yarnpkg/fslib': 3.0.0-rc.45
-      '@yarnpkg/libzip': 3.0.0-rc.45(@yarnpkg/fslib@3.0.0-rc.45)
+      '@yarnpkg/fslib': 3.0.0-rc.25
+      '@yarnpkg/libzip': 3.0.0-rc.25(@yarnpkg/fslib@3.0.0-rc.25)
       '@yarnpkg/parsers': 3.0.0-rc.45
       '@yarnpkg/shell': 4.0.0-rc.45(typanion@3.14.0)
       camelcase: 5.3.1
@@ -9371,22 +9371,22 @@ packages:
     engines: {node: '>=14.15.0'}
     dependencies:
       tslib: 2.6.2
-    dev: false
 
   /@yarnpkg/fslib@3.0.0-rc.45:
     resolution: {integrity: sha512-XarEtHTbeO4+rgZQObn16+uFzb1dXiVHuoqDNGoMywwBm/FF7DeCqSANeKiNYbi79WMgoLk8l3lO4g0vRYkJBg==}
     engines: {node: '>=14.15.0'}
     dependencies:
       tslib: 2.6.2
+    dev: false
 
-  /@yarnpkg/libzip@3.0.0-rc.45(@yarnpkg/fslib@3.0.0-rc.45):
-    resolution: {integrity: sha512-ZsYi6Y01yMJOLnJ5ISZgOFvCEXzp4EScrM91D7bvCx0lIfH3DZ40H4M5nGNeVFk7jXUHOXuJkNYlNoXixSconA==}
+  /@yarnpkg/libzip@3.0.0-rc.25(@yarnpkg/fslib@3.0.0-rc.25):
+    resolution: {integrity: sha512-YmG+oTBCyrAoMIx5g2I9CfyurSpHyoan+9SCj7laaFKseOe3lFEyIVKvwRBQMmSt8uzh+eY5RWeQnoyyOs6AbA==}
     engines: {node: '>=14.15.0'}
     peerDependencies:
-      '@yarnpkg/fslib': 3.0.0-rc.45
+      '@yarnpkg/fslib': 3.0.0-rc.25
     dependencies:
       '@types/emscripten': 1.39.10
-      '@yarnpkg/fslib': 3.0.0-rc.45
+      '@yarnpkg/fslib': 3.0.0-rc.25
       tslib: 2.6.2
 
   /@yarnpkg/lockfile@1.1.0:
@@ -9459,7 +9459,7 @@ packages:
     engines: {node: '>=14.15.0'}
     hasBin: true
     dependencies:
-      '@yarnpkg/fslib': 3.0.0-rc.45
+      '@yarnpkg/fslib': 3.0.0-rc.25
       '@yarnpkg/parsers': 3.0.0-rc.45
       chalk: 3.0.0
       clipanion: 3.2.0-rc.6(typanion@3.14.0)