Skip to content

[rush] Relocate pnpm settings to pnpm-workspace.yaml for pnpm 11#5838

Open
brunojppb wants to merge 4 commits into
microsoft:mainfrom
brunojppb:pnpm11-relocate-global-settings
Open

[rush] Relocate pnpm settings to pnpm-workspace.yaml for pnpm 11#5838
brunojppb wants to merge 4 commits into
microsoft:mainfrom
brunojppb:pnpm11-relocate-global-settings

Conversation

@brunojppb

@brunojppb brunojppb commented Jun 18, 2026

Copy link
Copy Markdown

Summary

Proposal fix for #5837

pnpm 11 stopped reading configuration from the pnpm field of package.json. Those settings must now live in pnpm-workspace.yaml (see the pnpm 11 release notes and settings docs).

Rush still serialized most of its pnpm-config.json settings into the pnpm field of the generated common/temp/package.json. After bumping pnpmVersion to 11.x, those settings were silently dropped: pnpm prints a warning, but the install still reports success. The most consequential of them, globalOverrides and globalPatchedDependencies, are commonly used to pin CVE remediations on transitive dependencies, so a rush update that re-resolves can quietly revert them to vulnerable versions.

#5817 already relocated allowBuilds to pnpm-workspace.yaml for pnpm 11. This PR applies the same mechanism to the remaining affected settings.

Changes

For pnpm >= 11.0.0, the following are now written to the generated common/temp/pnpm-workspace.yaml instead of the package.json pnpm field:

pnpm-config.json key pnpm-workspace.yaml key
globalOverrides overrides
globalPackageExtensions packageExtensions
globalPeerDependencyRules peerDependencyRules
globalAllowedDeprecatedVersions allowedDeprecatedVersions
globalPatchedDependencies patchedDependencies
  • PnpmWorkspaceFile: added fields, setters, and serialization for the five settings.
  • WorkspaceInstallManager: populates them on the workspace file when pnpmVersion >= 11.0.0.
  • InstallHelpers: skips writing the corresponding package.json pnpm entries when pnpmVersion >= 11.0.0.

Behavior for pnpm 10 and earlier is unchanged.

Out of scope

minimumReleaseAge (tracked in #5752 / #5798), trustPolicy*, and ignoredOptionalDependencies are also written to the pnpm field but are left for follow-up PRs to keep this change focused.

Could potentially include that here too if you folks want.

pnpm 11 no longer reads the "pnpm" field of package.json, so the settings
Rush serialized there were silently ignored. Relocate globalOverrides,
globalPackageExtensions, globalPeerDependencyRules,
globalAllowedDeprecatedVersions, and globalPatchedDependencies to the
generated common/temp/pnpm-workspace.yaml for pnpm >= 11.0.0, mirroring the
allowBuilds relocation from microsoft#5817. Behavior for older pnpm is unchanged.

Fixes microsoft#5837
@brunojppb

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

Two fixes from adversarial review of the pnpm 11 settings relocation:

1. rush-pnpm patch-commit / patch-remove read patchedDependencies from
   common/temp/package.json, which pnpm 11 no longer populates. They now read
   patchedDependencies from common/temp/pnpm-workspace.yaml for pnpm >= 11
   (mirroring the approve-builds allowBuilds path), preserving the package.json
   path for pnpm < 11 and the existing subspace + patches-folder logic.

2. Added integration coverage:
   - InstallHelpers.generateCommonPackageJson omits the relocated settings from
     package.json for pnpm 11 and still writes them for pnpm < 11.
   - RushPnpmCommandLineParser reads patchedDependencies from pnpm-workspace.yaml
     for pnpm 11 and from package.json for pnpm < 11.

Refs microsoft#5837
…obal-settings

# Conflicts:
#	libraries/rush-lib/src/logic/installManager/WorkspaceInstallManager.ts
#	libraries/rush-lib/src/logic/pnpm/PnpmWorkspaceFile.ts
#	libraries/rush-lib/src/logic/pnpm/test/PnpmWorkspaceFile.test.ts
#	libraries/rush-lib/src/logic/pnpm/test/__snapshots__/PnpmWorkspaceFile.test.ts.snap
Comment on lines +236 to +254
if (this._overrides && Object.keys(this._overrides).length > 0) {
workspaceYaml.overrides = this._overrides;
}

if (this._packageExtensions && Object.keys(this._packageExtensions).length > 0) {
workspaceYaml.packageExtensions = this._packageExtensions;
}

if (this._peerDependencyRules && Object.keys(this._peerDependencyRules).length > 0) {
workspaceYaml.peerDependencyRules = this._peerDependencyRules;
}

if (this._allowedDeprecatedVersions && Object.keys(this._allowedDeprecatedVersions).length > 0) {
workspaceYaml.allowedDeprecatedVersions = this._allowedDeprecatedVersions;
}

if (this._patchedDependencies && Object.keys(this._patchedDependencies).length > 0) {
workspaceYaml.patchedDependencies = this._patchedDependencies;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guards aren't necessary. See #5859 (comment)

Suggested change
if (this._overrides && Object.keys(this._overrides).length > 0) {
workspaceYaml.overrides = this._overrides;
}
if (this._packageExtensions && Object.keys(this._packageExtensions).length > 0) {
workspaceYaml.packageExtensions = this._packageExtensions;
}
if (this._peerDependencyRules && Object.keys(this._peerDependencyRules).length > 0) {
workspaceYaml.peerDependencyRules = this._peerDependencyRules;
}
if (this._allowedDeprecatedVersions && Object.keys(this._allowedDeprecatedVersions).length > 0) {
workspaceYaml.allowedDeprecatedVersions = this._allowedDeprecatedVersions;
}
if (this._patchedDependencies && Object.keys(this._patchedDependencies).length > 0) {
workspaceYaml.patchedDependencies = this._patchedDependencies;
}
workspaceYaml.overrides = this._overrides;
workspaceYaml.packageExtensions = this._packageExtensions;
workspaceYaml.peerDependencyRules = this._peerDependencyRules;
workspaceYaml.allowedDeprecatedVersions = this._allowedDeprecatedVersions;
workspaceYaml.patchedDependencies = this._patchedDependencies;

}
);
const { stdout: fileLog, exitCode, signal } = await Executable.waitForExitAsync(gitProcess, {
const {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change?

@@ -140,11 +140,9 @@ describe(PublishUtilities.findChangeRequestsAsync.name, () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these changes look unrelated.


public constructor(options: IShellOperationRunnerOptions) {
const { phase, displayName, rushProject, initialCommand, incrementalCommand, commandForHash, ignoredParameterValues } = options;
const {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change?

// See https://github.com/microsoft/rushstack/issues/5837
const isPnpm11OrNewer: boolean = semver.gte(pnpmVersion, '11.0.0');

if (!isPnpm11OrNewer && pnpmOptions.globalOverrides) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of repeatedly checking isPnpm11OrNewer, just organize all of these in a single check.

Comment on lines +18 to +37
interface IPnpmOptionsStub {
globalPatchedDependencies: Record<string, string> | undefined;
updateGlobalPatchedDependencies: jest.Mock;
}

interface ISubspaceStub {
getSubspaceTempFolderPath(): string;
getSubspaceConfigFolderPath(): string;
getSubspacePnpmPatchesFolderPath(): string;
getPnpmOptions(): IPnpmOptionsStub | undefined;
}

interface IPostExecuteInternals {
_commandName?: string;
_subspace: ISubspaceStub;
_rushConfiguration: { packageManagerToolVersion: string };
_terminal: { writeWarningLine: jest.Mock; writeErrorLine: jest.Mock };
_doRushUpdateAsync: jest.Mock;
_postExecuteAsync(): Promise<void>;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These shouldn't be necessary.

});
});

describe('RushPnpmCommandLineParser patch-commit patchedDependencies sync', () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
describe('RushPnpmCommandLineParser patch-commit patchedDependencies sync', () => {
describe(`${RushPnpmCommandLineParser.name} patch-commit patchedDependencies sync`, () => {

Comment on lines +87 to +91
afterEach(() => {
readFileAsyncSpy?.mockRestore();
existsSpy?.mockRestore();
jsonLoadSpy?.mockRestore();
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this happen automatically?

'patchedDependencies:\n' +
' lodash@4.17.21: patches/lodash@4.17.21.patch\n';
readFileAsyncSpy = jest.spyOn(FileSystem, 'readFileAsync').mockResolvedValue(workspaceYaml);
existsSpy = jest.spyOn(FileSystem, 'exists').mockReturnValue(false);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is never called.

};
const parser: IPostExecuteInternals = createPostExecuteParser('patch-commit', '10.27.0', pnpmOptions);

existsSpy = jest.spyOn(FileSystem, 'exists').mockReturnValue(false);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is never called.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs triage

Development

Successfully merging this pull request may close these issues.

3 participants