fix: should use the one with parent syntax when both override rules match the same target (#6266)

close #6210

.changeset/few-wasps-enjoy.md

@@ -0,0 +1,6 @@
+---
+"@pnpm/hooks.read-package-hook": patch
+"pnpm": patch
+---
+
+Should use most specific override rule when multiple rules match the same target [#6210](https://github.com/pnpm/pnpm/issues/6210).

hooks/read-package-hook/src/createVersionsOverrider.ts

@@ -32,12 +32,14 @@ export function createVersionsOverrider (
       })
   ) as [VersionOverrideWithParent[], VersionOverride[]]
   return ((manifest: PackageManifest, dir?: string) => {
-    overrideDepsOfPkg({ manifest, dir }, versionOverrides.filter(({ parentPkg }) => {
-      return parentPkg.name === manifest.name && (
-        !parentPkg.pref || semver.satisfies(manifest.version, parentPkg.pref)
+    const versionOverridesWithParent = versionOverrides.filter(({ parentPkg }) => {
+      return (
+        parentPkg.name === manifest.name &&
+        (!parentPkg.pref || semver.satisfies(manifest.version, parentPkg.pref))
       )
-    }))
-    overrideDepsOfPkg({ manifest, dir }, genericVersionOverrides)
+    })
+    overrideDepsOfPkg({ manifest, dir }, versionOverridesWithParent, genericVersionOverrides)
+
     return manifest
   }) as ReadPackageHook
 }
@@ -73,27 +75,52 @@ interface VersionOverrideWithParent extends VersionOverride {
 
 function overrideDepsOfPkg (
   { manifest, dir }: { manifest: PackageManifest, dir: string | undefined },
-  versionOverrides: VersionOverride[]
+  versionOverrides: VersionOverrideWithParent[],
+  genericVersionOverrides: VersionOverride[]
 ) {
-  if (manifest.dependencies != null) overrideDeps(versionOverrides, manifest.dependencies, dir)
-  if (manifest.optionalDependencies != null) overrideDeps(versionOverrides, manifest.optionalDependencies, dir)
-  if (manifest.devDependencies != null) overrideDeps(versionOverrides, manifest.devDependencies, dir)
-  return manifest
+  if (manifest.dependencies != null) overrideDeps(versionOverrides, genericVersionOverrides, manifest.dependencies, dir)
+  if (manifest.optionalDependencies != null) overrideDeps(versionOverrides, genericVersionOverrides, manifest.optionalDependencies, dir)
+  if (manifest.devDependencies != null) overrideDeps(versionOverrides, genericVersionOverrides, manifest.devDependencies, dir)
 }
 
-function overrideDeps (versionOverrides: VersionOverride[], deps: Dependencies, dir: string | undefined) {
-  for (const versionOverride of versionOverrides) {
-    const actual = deps[versionOverride.targetPkg.name]
-    if (actual == null) continue
-    if (!isSubRange(versionOverride.targetPkg.pref, actual)) continue
+function overrideDeps (
+  versionOverrides: VersionOverrideWithParent[],
+  genericVersionOverrides: VersionOverride[],
+  deps: Dependencies,
+  dir: string | undefined
+) {
+  for (const [name, pref] of Object.entries(deps)) {
+    const versionOverride =
+    pickMostSpecificVersionOverride(
+      versionOverrides.filter(
+        ({ targetPkg }) =>
+          targetPkg.name === name && isSubRange(targetPkg.pref, pref)
+      )
+    ) ??
+    pickMostSpecificVersionOverride(
+      genericVersionOverrides.filter(
+        ({ targetPkg }) =>
+          targetPkg.name === name && isSubRange(targetPkg.pref, pref)
+      )
+    )
+    if (!versionOverride) continue
+
     if (versionOverride.linkTarget && dir) {
-      deps[versionOverride.targetPkg.name] = `link:${normalizePath(path.relative(dir, versionOverride.linkTarget))}`
+      deps[versionOverride.targetPkg.name] = `link:${normalizePath(
+        path.relative(dir, versionOverride.linkTarget)
+      )}`
       continue
     }
     if (versionOverride.linkFileTarget) {
-      deps[versionOverride.targetPkg.name] = `file:${versionOverride.linkFileTarget}`
+      deps[
+        versionOverride.targetPkg.name
+      ] = `file:${versionOverride.linkFileTarget}`
       continue
     }
     deps[versionOverride.targetPkg.name] = versionOverride.newPref
   }
 }
+
+function pickMostSpecificVersionOverride (versionOverrides: VersionOverride[]): VersionOverride | undefined {
+  return versionOverrides.sort((a, b) => isSubRange(b.targetPkg.pref ?? '', a.targetPkg.pref ?? '') ? -1 : 1)[0]
+}

hooks/read-package-hook/test/createVersionOverrider.test.ts

@@ -292,6 +292,79 @@ test('createVersionsOverrider() overrides dependencies with file specified with
   })
 })
 
+test('createVersionOverride() should use the most specific rule when both override rules match the same target', () => {
+  const overrider = createVersionsOverrider({
+    foo: '3.0.0',
+    'foo@3': '4.0.0',
+    'foo@2': '2.12.0',
+    'bar>foo@2': 'github:org/foo',
+    'bar>foo@3': '5.0.0',
+  }, process.cwd())
+  expect(
+    overrider({
+      dependencies: {
+        foo: '^3.0.0',
+      },
+    })
+  ).toStrictEqual({
+    dependencies: {
+      foo: '4.0.0',
+    },
+  })
+  expect(
+    overrider({
+      dependencies: {
+        foo: '^4.0.0',
+      },
+    })
+  ).toStrictEqual({
+    dependencies: {
+      foo: '3.0.0',
+    },
+  })
+  expect(
+    overrider({
+      dependencies: {
+        foo: '^2.0.0',
+      },
+    })
+  ).toStrictEqual({
+    dependencies: {
+      foo: '2.12.0',
+    },
+  })
+  expect(
+    overrider({
+      name: 'bar',
+      version: '1.0.0',
+      dependencies: {
+        foo: '^2.0.0',
+      },
+    })
+  ).toStrictEqual({
+    name: 'bar',
+    version: '1.0.0',
+    dependencies: {
+      foo: 'github:org/foo',
+    },
+  })
+  expect(
+    overrider({
+      name: 'bar',
+      version: '1.0.0',
+      dependencies: {
+        foo: '^3.0.0',
+      },
+    })
+  ).toStrictEqual({
+    name: 'bar',
+    version: '1.0.0',
+    dependencies: {
+      foo: '5.0.0',
+    },
+  })
+})
+
 test('createVersionOverrider() throws error when supplied an invalid selector', () => {
   expect(() => createVersionsOverrider({
     'foo > bar': '2',