From 0a381c5985da213becee647b33c317a5a6271b8b Mon Sep 17 00:00:00 2001 From: Ace Nassri Date: Fri, 12 Jul 2024 17:38:56 -0700 Subject: [PATCH 1/5] Prioritize explicit preferredVersions over implicit ones --- .../rush-lib/src/logic/pnpm/PnpmfileConfiguration.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libraries/rush-lib/src/logic/pnpm/PnpmfileConfiguration.ts b/libraries/rush-lib/src/logic/pnpm/PnpmfileConfiguration.ts index a7090c1746f..a4b97c89131 100644 --- a/libraries/rush-lib/src/logic/pnpm/PnpmfileConfiguration.ts +++ b/libraries/rush-lib/src/logic/pnpm/PnpmfileConfiguration.ts @@ -2,6 +2,7 @@ // See LICENSE in the project root for license information. import * as path from 'path'; +import * as semver from 'semver'; import { FileSystem, Import, type IPackageJson, JsonFile, MapExtensions } from '@rushstack/node-core-library'; import type { PnpmPackageManager } from '../../api/packageManager/PnpmPackageManager'; @@ -91,11 +92,16 @@ export class PnpmfileConfiguration { if ((rushConfiguration.packageManagerOptions as PnpmOptionsConfiguration).useWorkspaces) { const commonVersionsConfiguration: CommonVersionsConfiguration = subspace.getCommonVersions(); const preferredVersions: Map = new Map(); - MapExtensions.mergeFromMap(preferredVersions, commonVersionsConfiguration.getAllPreferredVersions()); MapExtensions.mergeFromMap( preferredVersions, rushConfiguration.getImplicitlyPreferredVersions(subspace) ); + commonVersionsConfiguration.getAllPreferredVersions().forEach((version, name) => { + // Use the most restrictive version range available + if (!preferredVersions.has(name) || semver.subset(version, preferredVersions.get(name)!)) { + preferredVersions.set(name, version); + } + }); allPreferredVersions = MapExtensions.toObject(preferredVersions); allowedAlternativeVersions = MapExtensions.toObject( commonVersionsConfiguration.allowedAlternativeVersions From 0d89644af739e77828457819c0809345d326054b Mon Sep 17 00:00:00 2001 From: ace-n-msft Date: Tue, 16 Jul 2024 12:23:19 -0700 Subject: [PATCH 2/5] Add tests --- .../pnpm/test/PnpmfileConfiguration.test.ts | 40 +++++++++++++++++++ .../pnpm/test/repo/apps/baz/package.json | 9 +++++ .../common/config/rush/common-versions.json | 8 ++++ .../src/logic/pnpm/test/repo/rush3.json | 14 +++++++ 4 files changed, 71 insertions(+) create mode 100644 libraries/rush-lib/src/logic/pnpm/test/PnpmfileConfiguration.test.ts create mode 100644 libraries/rush-lib/src/logic/pnpm/test/repo/apps/baz/package.json create mode 100644 libraries/rush-lib/src/logic/pnpm/test/repo/common/config/rush/common-versions.json create mode 100644 libraries/rush-lib/src/logic/pnpm/test/repo/rush3.json diff --git a/libraries/rush-lib/src/logic/pnpm/test/PnpmfileConfiguration.test.ts b/libraries/rush-lib/src/logic/pnpm/test/PnpmfileConfiguration.test.ts new file mode 100644 index 00000000000..ebde9895c27 --- /dev/null +++ b/libraries/rush-lib/src/logic/pnpm/test/PnpmfileConfiguration.test.ts @@ -0,0 +1,40 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { RushConfiguration } from '../../../api/RushConfiguration'; +import type { Subspace } from '../../../api/Subspace'; +import { PnpmfileConfiguration } from '../PnpmfileConfiguration'; +import { JsonFile, JsonObject } from '@rushstack/node-core-library'; +import { RushConstants } from '../../RushConstants'; +import { before } from 'node:test'; + +describe(PnpmfileConfiguration.name, () => { + const repoPath: string = `${__dirname}/repo`; + const rushFilename: string = `${repoPath}/rush3.json`; + const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile(rushFilename); + const shimPath: string = `${rushConfiguration.defaultSubspace.getSubspaceTempFolder()}/pnpmfileSettings.json`; + + beforeAll(async () => { + const subspace = rushConfiguration.defaultSubspace; + await PnpmfileConfiguration.writeCommonTempPnpmfileShimAsync( + rushConfiguration, + subspace.getSubspaceTempFolder(), + subspace + ); + }); + + it('should use the smallest-available SemVer range (preferredVersions)', async () => { + const shimJson: JsonObject = await JsonFile.loadAsync(shimPath); + expect(shimJson.allPreferredVersions).toHaveProperty('core-js', '3.6.5'); + }); + + it('should use the smallest-available SemVer range (per-project)', async () => { + const shimJson: JsonObject = await JsonFile.loadAsync(shimPath); + expect(shimJson.allPreferredVersions).toHaveProperty('delay', '5.0.0'); + }); + + it('should override preferredVersions when per-project versions conflict', async () => { + const shimJson: JsonObject = await JsonFile.loadAsync(shimPath); + expect(shimJson.allPreferredVersions).toHaveProperty('find-up', '5.0.0'); + }); +}); diff --git a/libraries/rush-lib/src/logic/pnpm/test/repo/apps/baz/package.json b/libraries/rush-lib/src/logic/pnpm/test/repo/apps/baz/package.json new file mode 100644 index 00000000000..524e9d4cb89 --- /dev/null +++ b/libraries/rush-lib/src/logic/pnpm/test/repo/apps/baz/package.json @@ -0,0 +1,9 @@ +{ + "name": "baz", + "version": "1.0.0", + "dependencies": { + "core-js": "^3.0.0", + "delay": "5.0.0", + "find-up": "*" + } +} diff --git a/libraries/rush-lib/src/logic/pnpm/test/repo/common/config/rush/common-versions.json b/libraries/rush-lib/src/logic/pnpm/test/repo/common/config/rush/common-versions.json new file mode 100644 index 00000000000..30d53549184 --- /dev/null +++ b/libraries/rush-lib/src/logic/pnpm/test/repo/common/config/rush/common-versions.json @@ -0,0 +1,8 @@ +{ + "$schema": "https://developer.microsoft.com/json-schemas/rush/v5/common-versions.schema.json", + "preferredVersions": { + "core-js": "3.6.5", + "delay": "4.0.0", + "find-up": "5.0.0" + } +} diff --git a/libraries/rush-lib/src/logic/pnpm/test/repo/rush3.json b/libraries/rush-lib/src/logic/pnpm/test/repo/rush3.json new file mode 100644 index 00000000000..d5b3a4e1d82 --- /dev/null +++ b/libraries/rush-lib/src/logic/pnpm/test/repo/rush3.json @@ -0,0 +1,14 @@ +{ + "$schema": "https://developer.microsoft.com/json-schemas/rush/v5/rush.schema.json", + "pnpmVersion": "7.0.0", + "rushVersion": "5.46.1", + "projects": [ + { + "packageName": "baz", + "projectFolder": "apps/baz" + } + ], + "pnpmOptions": { + "useWorkspaces": true + } +} From d0a18379f187d38197adb9f31e71f26827aa1dc9 Mon Sep 17 00:00:00 2001 From: ace-n-msft Date: Tue, 16 Jul 2024 18:52:29 -0700 Subject: [PATCH 3/5] Add changelog --- .../@microsoft/rush/fix-3205_2024-07-17-01-52.json | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 common/changes/@microsoft/rush/fix-3205_2024-07-17-01-52.json diff --git a/common/changes/@microsoft/rush/fix-3205_2024-07-17-01-52.json b/common/changes/@microsoft/rush/fix-3205_2024-07-17-01-52.json new file mode 100644 index 00000000000..b7b9c62884a --- /dev/null +++ b/common/changes/@microsoft/rush/fix-3205_2024-07-17-01-52.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Fix an issue where PreferredVersions are ignored when a project contains an overlapping dependency entry (https://github.com/microsoft/rushstack/issues/3205)", + "type": "none" + } + ], + "packageName": "@microsoft/rush" +} \ No newline at end of file From 92b7365837ff71031326ba55a909d39ca4a611ac Mon Sep 17 00:00:00 2001 From: ace-n-msft Date: Mon, 12 Aug 2024 19:00:18 -0700 Subject: [PATCH 4/5] Address feedback: foreach() function -> for...of loop --- libraries/rush-lib/src/logic/pnpm/PnpmfileConfiguration.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/rush-lib/src/logic/pnpm/PnpmfileConfiguration.ts b/libraries/rush-lib/src/logic/pnpm/PnpmfileConfiguration.ts index a4b97c89131..6dece515a8a 100644 --- a/libraries/rush-lib/src/logic/pnpm/PnpmfileConfiguration.ts +++ b/libraries/rush-lib/src/logic/pnpm/PnpmfileConfiguration.ts @@ -96,12 +96,12 @@ export class PnpmfileConfiguration { preferredVersions, rushConfiguration.getImplicitlyPreferredVersions(subspace) ); - commonVersionsConfiguration.getAllPreferredVersions().forEach((version, name) => { + for (const [name, version] of commonVersionsConfiguration.getAllPreferredVersions()) { // Use the most restrictive version range available if (!preferredVersions.has(name) || semver.subset(version, preferredVersions.get(name)!)) { preferredVersions.set(name, version); } - }); + } allPreferredVersions = MapExtensions.toObject(preferredVersions); allowedAlternativeVersions = MapExtensions.toObject( commonVersionsConfiguration.allowedAlternativeVersions From eb356ae1123495a9335875d415460025e29927bf Mon Sep 17 00:00:00 2001 From: ace-n-msft Date: Tue, 13 Aug 2024 11:48:50 -0700 Subject: [PATCH 5/5] Fix CI failures --- .../src/logic/pnpm/test/PnpmfileConfiguration.test.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/libraries/rush-lib/src/logic/pnpm/test/PnpmfileConfiguration.test.ts b/libraries/rush-lib/src/logic/pnpm/test/PnpmfileConfiguration.test.ts index ebde9895c27..b1875dbde8f 100644 --- a/libraries/rush-lib/src/logic/pnpm/test/PnpmfileConfiguration.test.ts +++ b/libraries/rush-lib/src/logic/pnpm/test/PnpmfileConfiguration.test.ts @@ -2,23 +2,20 @@ // See LICENSE in the project root for license information. import { RushConfiguration } from '../../../api/RushConfiguration'; -import type { Subspace } from '../../../api/Subspace'; import { PnpmfileConfiguration } from '../PnpmfileConfiguration'; -import { JsonFile, JsonObject } from '@rushstack/node-core-library'; -import { RushConstants } from '../../RushConstants'; -import { before } from 'node:test'; +import { JsonFile, type JsonObject } from '@rushstack/node-core-library'; describe(PnpmfileConfiguration.name, () => { const repoPath: string = `${__dirname}/repo`; const rushFilename: string = `${repoPath}/rush3.json`; const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile(rushFilename); - const shimPath: string = `${rushConfiguration.defaultSubspace.getSubspaceTempFolder()}/pnpmfileSettings.json`; + const shimPath: string = `${rushConfiguration.defaultSubspace.getSubspaceTempFolderPath()}/pnpmfileSettings.json`; beforeAll(async () => { const subspace = rushConfiguration.defaultSubspace; await PnpmfileConfiguration.writeCommonTempPnpmfileShimAsync( rushConfiguration, - subspace.getSubspaceTempFolder(), + subspace.getSubspaceTempFolderPath(), subspace ); });