Skip to content

Commit 2ee05e6

Browse files
fix: [rush] Parse pnpm-config.json with JsonSyntax.JsonWithComments (#5820)
Co-authored-by: Bart van den Ende <bartvandenende-wm@users.noreply.github.com>
1 parent fb68fb6 commit 2ee05e6

3 files changed

Lines changed: 45 additions & 6 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"comment": "Fix a regression where Rush change-detection treated any `pnpm-config.json` containing comments as unparseable, causing every project to be flagged as impacted.",
5+
"type": "patch",
6+
"packageName": "@microsoft/rush"
7+
}
8+
],
9+
"packageName": "@microsoft/rush",
10+
"email": "bartvandenende-wm@users.noreply.github.com"
11+
}

libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as path from 'node:path';
66
import ignore, { type Ignore } from 'ignore';
77

88
import type { IReadonlyLookupByPath, LookupByPath, IPrefixMatch } from '@rushstack/lookup-by-path';
9-
import { Path, FileSystem, Async, AlreadyReportedError, Sort } from '@rushstack/node-core-library';
9+
import { Path, FileSystem, Async, AlreadyReportedError, Sort, JsonFile } from '@rushstack/node-core-library';
1010
import {
1111
getRepoChanges,
1212
getRepoRoot,
@@ -545,16 +545,16 @@ export class ProjectChangeAnalyzer {
545545
blobSpec: `${mergeCommit}:${pnpmConfigRelativePath}`,
546546
repositoryRoot: repoRoot
547547
});
548-
const oldPnpmConfig: IPnpmOptionsJson = JSON.parse(oldPnpmConfigText);
548+
const oldPnpmConfig: IPnpmOptionsJson = JsonFile.parseString(oldPnpmConfigText);
549549
oldCatalogs = oldPnpmConfig.globalCatalogs ?? {};
550550
} catch {
551551
// Old file didn't exist or was unparseable — treat all packages in all current catalogs as changed
552552
if (rushConfiguration.subspacesFeatureEnabled) {
553-
terminal.writeLine(
553+
terminal.writeWarningLine(
554554
`"${subspace.subspaceName}" subspace pnpm-config.json was created or unparseable. Assuming all projects are affected.`
555555
);
556556
} else {
557-
terminal.writeLine(
557+
terminal.writeWarningLine(
558558
`pnpm-config.json was created or unparseable. Assuming all projects are affected.`
559559
);
560560
}
@@ -608,8 +608,7 @@ export class ProjectChangeAnalyzer {
608608
// Check each project in the subspace to see if it depends on a changed catalog package
609609
const subspaceProjects: RushConfigurationProject[] = subspace.getProjects();
610610
subspaceProjects.forEach((project) => {
611-
const { dependencies, devDependencies, optionalDependencies, peerDependencies } =
612-
project.packageJson;
611+
const { dependencies, devDependencies, optionalDependencies, peerDependencies } = project.packageJson;
613612
const allDependencies: Set<[string, string]> = new Set(
614613
[dependencies, devDependencies, optionalDependencies, peerDependencies].flatMap((deps) =>
615614
Object.entries(deps ?? {})

libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -842,6 +842,35 @@ describe(ProjectChangeAnalyzer.name, () => {
842842
// 'c' has no catalog deps
843843
expect(changedProjects.has(rushConfiguration.getProjectByName('c')!)).toBe(false);
844844
});
845+
846+
it('parses pnpm-config.json blobs containing comments (regression #5813)', async () => {
847+
const rushConfiguration: RushConfiguration = getCatalogRushConfiguration();
848+
mockPnpmConfigChanged();
849+
mockGetBlobContentAsync.mockImplementation(() => {
850+
return Promise.resolve(
851+
`// banner comment from rush init template\n` +
852+
`/* block comment */\n` +
853+
JSON.stringify({
854+
globalCatalogs: {
855+
default: { react: '^18.0.0', semver: '^7.5.4' },
856+
tools: { typescript: '~5.3.0', eslint: '^8.50.0' }
857+
}
858+
})
859+
);
860+
});
861+
862+
const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(rushConfiguration);
863+
const terminal: Terminal = new Terminal(new StringBufferTerminalProvider(true));
864+
865+
const changedProjects = await projectChangeAnalyzer.getChangedProjectsAsync({
866+
enableFiltering: false,
867+
includeExternalDependencies: false,
868+
targetBranchName: 'main',
869+
terminal
870+
});
871+
872+
expect(changedProjects.size).toBe(0);
873+
});
845874
});
846875

847876
describe('subspace catalog change detection', () => {

0 commit comments

Comments
 (0)