feat: better deduping of direct dependencies in workspace projects (#3614)

* feat: better deduping of direct dependencies in workspace projects

* fix: adding new deduped pkg

* test: fix

* test: getPreferredVersions()

.changeset/brown-cheetahs-protect.md

@@ -0,0 +1,5 @@
+---
+"supi": patch
+---
+
+When adding a new dependency to a workspace, and the dependency is already present in the workspace (in another project), use the already present spec.

packages/supi/src/install/getPreferredVersions.test.ts

@@ -0,0 +1,28 @@
+import { getAllUniqueSpecs } from './getPreferredVersions'
+
+test('getAllUniqueSpecs()', () => {
+  expect(getAllUniqueSpecs([
+    {
+      name: '',
+      version: '',
+      dependencies: {
+        foo: '1.0.0',
+        bar: '1.0.0',
+        qar: '1.0.0',
+        zoo: 'link:../zoo',
+        alpha: 'npm:beta@2',
+      },
+    },
+    {
+      name: '',
+      version: '',
+      dependencies: {
+        bar: '1.0.0',
+        qar: '2.0.0',
+      },
+    },
+  ])).toStrictEqual({
+    foo: '1.0.0',
+    bar: '1.0.0',
+  })
+})

packages/supi/src/install/getPreferredVersions.ts

@@ -1,9 +1,27 @@
 import { nameVerFromPkgSnapshot, PackageSnapshots } from '@pnpm/lockfile-utils'
 import { getAllDependenciesFromManifest } from '@pnpm/manifest-utils'
 import { PreferredVersions } from '@pnpm/resolver-base'
-import { Dependencies, ProjectManifest } from '@pnpm/types'
+import { Dependencies, DependencyManifest, ProjectManifest } from '@pnpm/types'
 import getVerSelType from 'version-selector-type'
 
+export function getAllUniqueSpecs (manifests: DependencyManifest[]) {
+  const allSpecs: Record<string, string> = {}
+  const ignored = new Set<string>()
+  for (const manifest of manifests) {
+    const specs = getAllDependenciesFromManifest(manifest)
+    for (const [name, spec] of Object.entries(specs)) {
+      if (ignored.has(name)) continue
+      if (allSpecs[name] != null && allSpecs[name] !== spec || spec.includes(':')) {
+        ignored.add(name)
+        delete allSpecs[name]
+        continue
+      }
+      allSpecs[name] = spec
+    }
+  }
+  return allSpecs
+}
+
 export default function getPreferredVersionsFromPackage (
   pkg: Pick<ProjectManifest, 'devDependencies' | 'dependencies' | 'optionalDependencies'>
 ): PreferredVersions {

packages/supi/src/install/index.ts

@@ -51,6 +51,7 @@ import rimraf from '@zkochan/rimraf'
 import isInnerLink from 'is-inner-link'
 import pFilter from 'p-filter'
 import pLimit from 'p-limit'
+import flatten from 'ramda/src/flatten'
 import fromPairs from 'ramda/src/fromPairs'
 import equals from 'ramda/src/equals'
 import isEmpty from 'ramda/src/isEmpty'
@@ -67,7 +68,7 @@ import extendOptions, {
   InstallOptions,
   StrictInstallOptions,
 } from './extendInstallOptions'
-import getPreferredVersionsFromPackage, { getPreferredVersionsFromLockfile } from './getPreferredVersions'
+import getPreferredVersionsFromPackage, { getPreferredVersionsFromLockfile, getAllUniqueSpecs } from './getPreferredVersions'
 import getWantedDependencies, {
   PinnedVersion,
   WantedDependency,
@@ -316,6 +317,8 @@ export async function mutateModules (
       unsafePerm: opts.unsafePerm || false,
     }
 
+    let preferredSpecs: Record<string, string> | null = null
+
     // TODO: make it concurrent
     for (const project of ctx.projects) {
       switch (project.mutation) {
@@ -431,6 +434,9 @@ export async function mutateModules (
       const currentPrefs = opts.ignoreCurrentPrefs ? {} : getAllDependenciesFromManifest(project.manifest)
       const optionalDependencies = project.targetDependenciesField ? {} : project.manifest.optionalDependencies || {}
       const devDependencies = project.targetDependenciesField ? {} : project.manifest.devDependencies || {}
+      if (preferredSpecs == null) {
+        preferredSpecs = getAllUniqueSpecs(flatten(Object.values(opts.workspacePackages).map(obj => Object.values(obj))).map(({ manifest }) => manifest))
+      }
       const wantedDeps = parseWantedDependencies(project.dependencySelectors, {
         allowNew: project.allowNew !== false,
         currentPrefs,
@@ -440,6 +446,7 @@ export async function mutateModules (
         optional: project.targetDependenciesField === 'optionalDependencies',
         optionalDependencies,
         updateWorkspaceDependencies: opts.update,
+        preferredSpecs,
       })
       projectsToInstall.push({
         pruneDirectDependencies: false,

packages/supi/src/parseWantedDependencies.ts

@@ -14,6 +14,7 @@ export default function parseWantedDependencies (
     optional: boolean
     optionalDependencies: Dependencies
     updateWorkspaceDependencies?: boolean
+    preferredSpecs?: Record<string, string>
   }
 ): WantedDependency[] {
   return rawWantedDependencies
@@ -35,14 +36,30 @@ export default function parseWantedDependencies (
         }
         pinnedVersion = guessPinnedVersionFromExistingSpec(opts.currentPrefs[alias])
       }
-      return {
+      const result = {
         alias,
         dev: Boolean(opts.dev || alias && !!opts.devDependencies[alias]),
         optional: Boolean(opts.optional || alias && !!opts.optionalDependencies[alias]),
         pinnedVersion,
-        pref: pref ?? opts.defaultTag,
         raw: rawWantedDependency,
       }
+      if (pref) {
+        return {
+          ...result,
+          pref,
+        }
+      }
+      if (alias && opts.preferredSpecs?.[alias]) {
+        return {
+          ...result,
+          pref: opts.preferredSpecs[alias],
+          raw: `${rawWantedDependency}@${opts.preferredSpecs[alias]}`,
+        }
+      }
+      return {
+        ...result,
+        pref: opts.defaultTag,
+      }
     })
     .filter((wd) => wd !== null) as WantedDependency[]
 }

packages/supi/test/install/multipleImporters.ts

@@ -3,6 +3,7 @@ import assertProject from '@pnpm/assert-project'
 import { LOCKFILE_VERSION } from '@pnpm/constants'
 import { readCurrentLockfile } from '@pnpm/lockfile-file'
 import { prepareEmpty, preparePackages } from '@pnpm/prepare'
+import { ProjectManifest } from '@pnpm/types'
 import {
   addDependenciesToPackage,
   MutatedProject,
@@ -1243,3 +1244,73 @@ test('resolve a subdependency from the workspace, when it uses the workspace pro
     workspacePackages,
   }))
 })
+
+test('install the dependency that is already present in the workspace when adding a new direct dependency', async () => {
+  await addDistTag('dep-of-pkg-with-1-dep', '100.0.0', 'latest')
+
+  const manifest1: ProjectManifest = {
+    name: 'project-1',
+    version: '1.0.0',
+    dependencies: {
+      'dep-of-pkg-with-1-dep': '^100.0.0',
+    },
+  }
+  const manifest2: ProjectManifest = { name: 'project-2' }
+
+  preparePackages([
+    {
+      location: 'project-1',
+      package: manifest1,
+    },
+    {
+      location: 'project-2',
+      package: manifest2,
+    },
+  ])
+
+  const importers: MutatedProject[] = [
+    {
+      buildIndex: 0,
+      manifest: manifest1,
+      mutation: 'install',
+      rootDir: path.resolve('project-1'),
+    },
+    {
+      buildIndex: 0,
+      manifest: manifest2,
+      mutation: 'install',
+      rootDir: path.resolve('project-2'),
+    },
+  ]
+  await mutateModules(importers, await testDefaults())
+
+  await addDistTag('dep-of-pkg-with-1-dep', '100.1.0', 'latest')
+
+  await mutateModules([
+    importers[0],
+    {
+      ...importers[1],
+      dependencySelectors: ['dep-of-pkg-with-1-dep'],
+      mutation: 'installSome',
+    },
+  ], await testDefaults({
+    lockfileDir: process.cwd(),
+    workspacePackages: {
+      'project-1': {
+        '1.0.0': {
+          dir: path.resolve('project-1'),
+          manifest: manifest1,
+        },
+      },
+    },
+  }))
+
+  const rootModules = assertProject(process.cwd())
+  const currentLockfile = await rootModules.readCurrentLockfile()
+
+  expect(currentLockfile.importers['project-1'].specifiers?.['dep-of-pkg-with-1-dep']).toBe('^100.0.0')
+  expect(currentLockfile.importers['project-2'].specifiers?.['dep-of-pkg-with-1-dep']).toBe('^100.0.0')
+
+  expect(currentLockfile.importers['project-1'].dependencies?.['dep-of-pkg-with-1-dep']).toBe('100.1.0')
+  expect(currentLockfile.importers['project-2'].dependencies?.['dep-of-pkg-with-1-dep']).toBe('100.1.0')
+})