From ada3a83de6db43a90b3d7bd5200f343cf879a88f Mon Sep 17 00:00:00 2001 From: Rushbot Date: Fri, 4 Sep 2026 15:09:18 -0700 Subject: [PATCH 1/3] Fix change verification after version bumps Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...n-bump-change-verify_2026-09-04-14-49.json | 10 + .../rush-lib/src/cli/actions/ChangeAction.ts | 2 +- .../src/logic/ProjectChangeAnalyzer.ts | 216 +++++++++++++----- .../logic/test/ProjectChangeAnalyzer.test.ts | 198 +++++++++++++++- 4 files changed, 371 insertions(+), 55 deletions(-) create mode 100644 common/changes/@microsoft/rush/fix-version-bump-change-verify_2026-09-04-14-49.json diff --git a/common/changes/@microsoft/rush/fix-version-bump-change-verify_2026-09-04-14-49.json b/common/changes/@microsoft/rush/fix-version-bump-change-verify_2026-09-04-14-49.json new file mode 100644 index 0000000000..e225e6d08c --- /dev/null +++ b/common/changes/@microsoft/rush/fix-version-bump-change-verify_2026-09-04-14-49.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Fix `rush change --verify` to ignore dependency range rewrites generated by `rush version --bump` for locally bumped projects.", + "type": "patch" + } + ], + "packageName": "@microsoft/rush" +} diff --git a/libraries/rush-lib/src/cli/actions/ChangeAction.ts b/libraries/rush-lib/src/cli/actions/ChangeAction.ts index fed5bb1f8f..db1a71662e 100644 --- a/libraries/rush-lib/src/cli/actions/ChangeAction.ts +++ b/libraries/rush-lib/src/cli/actions/ChangeAction.ts @@ -428,7 +428,7 @@ export class ChangeAction extends BaseRushAction { includeExternalDependencies: false, // Since install may not have happened, cannot read rush-project.json enableFiltering: false, - // Exclude version-only changes to prevent 'rush version --bump' from triggering 'rush change --verify' + // Exclude version bump output to prevent 'rush version --bump' from triggering 'rush change --verify' excludeVersionOnlyChanges: true }); const projectHostMap: Map = this._generateHostMap(); diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index 3dc0fc6ed2..aa513aa712 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -6,7 +6,17 @@ import * as path from 'node:path'; import ignore, { type Ignore } from 'ignore'; import type { IReadonlyLookupByPath, LookupByPath, IPrefixMatch } from '@rushstack/lookup-by-path'; -import { Path, FileSystem, Async, AlreadyReportedError, Sort, JsonFile } from '@rushstack/node-core-library'; +import { + Path, + FileSystem, + Async, + AlreadyReportedError, + Sort, + JsonFile, + Objects, + type IPackageJson, + type IPackageJsonDependencyTable +} from '@rushstack/node-core-library'; import { getRepoChanges, getRepoRoot, @@ -24,6 +34,7 @@ import { BaseProjectShrinkwrapFile } from './base/BaseProjectShrinkwrapFile'; import { PnpmShrinkwrapFile } from './pnpm/PnpmShrinkwrapFile'; import { Git } from './Git'; import { DependencySpecifier, DependencySpecifierType } from './DependencySpecifier'; +import { PublishUtilities } from './PublishUtilities'; import type { IPnpmOptionsJson, PnpmOptionsConfiguration } from './pnpm/PnpmOptionsConfiguration'; import { type IInputsSnapshotProjectMetadata, @@ -55,7 +66,8 @@ export interface IGetChangedProjectsOptions { /** * If set to `true`, excludes projects where the only changes are: - * - A version-only change to `package.json` (only the "version" field differs) + * - Changes to `package.json` produced by a version bump, including dependency range updates for + * other locally bumped projects * - Changes to `CHANGELOG.md` and/or `CHANGELOG.json` files * * This prevents `rush version --bump` from triggering `rush change --verify` to request change files @@ -121,6 +133,15 @@ export class ProjectChangeAnalyzer { RushConfigurationProject, Map > = this.getChangesByProject(lookup, changedFiles); + const packageJsonChanges: Map = excludeVersionOnlyChanges + ? await getPackageJsonChangesAsync(changesByProject, repoRoot, this._git) + : new Map(); + const bumpedProjectVersions: Map = new Map(); + for (const [project, packageJsonChange] of packageJsonChanges) { + if (packageJsonChange.oldPackageJson.version !== packageJsonChange.newPackageJson.version) { + bumpedProjectVersions.set(project.packageName, packageJsonChange.newPackageJson.version); + } + } const changedProjects: Set = new Set(); @@ -142,8 +163,8 @@ export class ProjectChangeAnalyzer { return; } - // Filter out package.json with version-only changes, CHANGELOG.md, and CHANGELOG.json - for (const [filePath, diffStatus] of filteredChanges) { + // Filter out generated version bump changes in package.json and changelog files. + for (const filePath of filteredChanges.keys()) { // Use lookup to find the project-relative path const match: IPrefixMatch | undefined = lookup.findLongestPrefixMatch(filePath); @@ -160,15 +181,18 @@ export class ProjectChangeAnalyzer { continue; } - // Check if this is package.json at project root with version-only changes + // Check if this is package.json at project root with changes generated by the version bump if (projectRelativePath === '/package.json') { - const isVersionOnlyChange: boolean = await isVersionOnlyChangeAsync( - diffStatus, - repoRoot, - this._git - ); - if (isVersionOnlyChange) { - continue; // Skip version-only package.json changes + const packageJsonChange: IPackageJsonChange | undefined = packageJsonChanges.get(project); + const isVersionBumpChange: boolean = + !!packageJsonChange && + isPackageJsonVersionBumpChange( + packageJsonChange.oldPackageJson, + packageJsonChange.newPackageJson, + bumpedProjectVersions + ); + if (isVersionBumpChange) { + continue; } } @@ -632,37 +656,61 @@ export class ProjectChangeAnalyzer { } } -/** - * Checks if a diff represents a version-only change to package.json. - */ -async function isVersionOnlyChangeAsync( - diffStatus: IFileDiffStatus, +interface IPackageJsonChange { + oldPackageJson: IPackageJson; + newPackageJson: IPackageJson; +} + +const dependencyFieldNames: ReadonlyArray> = ['dependencies', 'devDependencies', 'peerDependencies']; + +async function getPackageJsonChangesAsync( + changesByProject: ReadonlyMap>, repoRoot: string, git: Git -): Promise { - try { - // Only check modified files, not additions or deletions - if (diffStatus.status !== 'M') { - return false; - } - - // Get both versions of package.json from Git in parallel - const [oldPackageJsonContent, currentPackageJsonContent] = await Promise.all([ - git.getBlobContentAsync({ - blobSpec: diffStatus.oldhash, - repositoryRoot: repoRoot - }), - git.getBlobContentAsync({ - blobSpec: diffStatus.newhash, - repositoryRoot: repoRoot - }) - ]); +): Promise> { + const packageJsonChanges: Map = new Map(); + await Async.forEachAsync( + changesByProject, + async ([project, projectChanges]) => { + const packageJsonPath: string = Path.convertToSlashes( + path.relative(repoRoot, path.join(project.projectFolder, 'package.json')) + ); + const diffStatus: IFileDiffStatus | undefined = projectChanges.get(packageJsonPath); + if (diffStatus?.status !== 'M') { + return; + } - return isPackageJsonVersionOnlyChange(oldPackageJsonContent, currentPackageJsonContent); - } catch (error) { - // If we can't read the file or parse it, assume it's not a version-only change - return false; - } + try { + const [oldPackageJsonContent, newPackageJsonContent] = await Promise.all([ + git.getBlobContentAsync({ + blobSpec: diffStatus.oldhash, + repositoryRoot: repoRoot + }), + git.getBlobContentAsync({ + blobSpec: diffStatus.newhash, + repositoryRoot: repoRoot + }) + ]); + const oldPackageJson: IPackageJson = JSON.parse(oldPackageJsonContent); + const newPackageJson: IPackageJson = JSON.parse(newPackageJsonContent); + if ( + oldPackageJson.name === project.packageName && + newPackageJson.name === project.packageName && + typeof oldPackageJson.version === 'string' && + typeof newPackageJson.version === 'string' + ) { + packageJsonChanges.set(project, { oldPackageJson, newPackageJson }); + } + } catch (error) { + // A package.json that cannot be read or parsed must remain a substantive change. + } + }, + { concurrency: 10 } + ); + return packageJsonChanges; } interface IAdditionalGlob { @@ -749,23 +797,85 @@ export function isPackageJsonVersionOnlyChange( newPackageJsonContent: string ): boolean { try { - // Parse both versions - use specific type since we only care about version field - const oldPackageJson: { version?: string } = JSON.parse(oldPackageJsonContent); - const newPackageJson: { version?: string } = JSON.parse(newPackageJsonContent); + return isPackageJsonVersionBumpChange( + JSON.parse(oldPackageJsonContent), + JSON.parse(newPackageJsonContent), + new Map() + ); + } catch (error) { + // If we can't parse the JSON, assume it's not a version-only change + return false; + } +} - // Ensure both have a version field - if (!oldPackageJson.version || !newPackageJson.version) { +/** + * Determines whether a package.json differs only by its version and dependency range rewrites that + * Rush would generate for other locally bumped projects. + */ +export function isPackageJsonVersionBumpChange( + oldPackageJson: IPackageJson, + newPackageJson: IPackageJson, + bumpedProjectVersions: ReadonlyMap +): boolean { + if ( + typeof oldPackageJson.version !== 'string' || + typeof newPackageJson.version !== 'string' || + oldPackageJson.version === newPackageJson.version || + oldPackageJson.name !== newPackageJson.name + ) { + return false; + } + + const oldPackageJsonWithoutBumpFields: Partial = { ...oldPackageJson }; + const newPackageJsonWithoutBumpFields: Partial = { ...newPackageJson }; + delete oldPackageJsonWithoutBumpFields.version; + delete newPackageJsonWithoutBumpFields.version; + + for (const dependencyFieldName of dependencyFieldNames) { + const oldDependencies: IPackageJsonDependencyTable | undefined = oldPackageJson[dependencyFieldName]; + const newDependencies: IPackageJsonDependencyTable | undefined = newPackageJson[dependencyFieldName]; + delete oldPackageJsonWithoutBumpFields[dependencyFieldName]; + delete newPackageJsonWithoutBumpFields[dependencyFieldName]; + + if (!oldDependencies && !newDependencies) { + continue; + } + if ( + !oldDependencies || + !newDependencies || + Object.keys(oldDependencies).length !== Object.keys(newDependencies).length + ) { return false; } - // Remove the version field from both (no need to clone, these are fresh objects from JSON.parse) - oldPackageJson.version = undefined; - newPackageJson.version = undefined; + for (const [dependencyName, oldDependencyVersion] of Object.entries(oldDependencies)) { + const newDependencyVersion: string | undefined = newDependencies[dependencyName]; + if (!newDependencyVersion) { + return false; + } + if (oldDependencyVersion === newDependencyVersion) { + continue; + } - // Compare the objects without the version field - return JSON.stringify(oldPackageJson) === JSON.stringify(newPackageJson); - } catch (error) { - // If we can't parse the JSON, assume it's not a version-only change - return false; + const bumpedProjectVersion: string | undefined = bumpedProjectVersions.get(dependencyName); + if (!bumpedProjectVersion) { + return false; + } + + try { + const expectedDependencyVersion: string = PublishUtilities.getNewDependencyVersion( + oldDependencies, + dependencyName, + bumpedProjectVersion + ); + if (newDependencyVersion !== expectedDependencyVersion) { + return false; + } + } catch (error) { + return false; + } + } } + + return Objects.areDeepEqual(oldPackageJsonWithoutBumpFields, newPackageJsonWithoutBumpFields); } diff --git a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts index bbc8274c26..efa014862c 100644 --- a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts +++ b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts @@ -117,7 +117,11 @@ import { resolve } from 'node:path'; import type { IDetailedRepoState, IFileDiffStatus } from '@rushstack/package-deps-hash'; import { StringBufferTerminalProvider, Terminal } from '@rushstack/terminal'; -import { ProjectChangeAnalyzer, isPackageJsonVersionOnlyChange } from '../ProjectChangeAnalyzer'; +import { + ProjectChangeAnalyzer, + isPackageJsonVersionBumpChange, + isPackageJsonVersionOnlyChange +} from '../ProjectChangeAnalyzer'; import { RushConfiguration } from '../../api/RushConfiguration'; import type { IInputsSnapshot, @@ -378,6 +382,47 @@ describe(ProjectChangeAnalyzer.name, () => { expect(changedProjects.has(rushConfiguration.getProjectByName('b')!)).toBe(true); }); + it('excludeVersionOnlyChanges excludes dependency ranges generated for locally bumped projects', async () => { + const rootDir: string = resolve(__dirname, 'repo'); + const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile( + resolve(rootDir, 'rush.json') + ); + + mockGetRepoChanges.mockReturnValue( + new Map([ + [ + 'a/package.json', + { mode: 'modified', newhash: 'newhash-a', oldhash: 'oldhash-a', status: 'M' } + ], + [ + 'b/package.json', + { mode: 'modified', newhash: 'newhash-b', oldhash: 'oldhash-b', status: 'M' } + ] + ]) + ); + const packageJsonByHash: Record = { + 'oldhash-a': { name: 'a', version: '1.0.0' }, + 'newhash-a': { name: 'a', version: '1.0.1' }, + 'oldhash-b': { name: 'b', version: '2.0.0', peerDependencies: { a: '1.0.0' } }, + 'newhash-b': { name: 'b', version: '2.0.1', peerDependencies: { a: '1.0.1' } } + }; + mockGetBlobContentAsync.mockImplementation(({ blobSpec }) => + Promise.resolve(JSON.stringify(packageJsonByHash[blobSpec]!)) + ); + + const projectChangeAnalyzer: ProjectChangeAnalyzer = new ProjectChangeAnalyzer(rushConfiguration); + const terminal: Terminal = new Terminal(new StringBufferTerminalProvider(true)); + const changedProjects = await projectChangeAnalyzer.getChangedProjectsAsync({ + enableFiltering: false, + includeExternalDependencies: false, + targetBranchName: 'main', + terminal, + excludeVersionOnlyChanges: true + }); + + expect(changedProjects.size).toBe(0); + }); + it('excludeVersionOnlyChanges does not exclude projects when package.json and other files changed', async () => { const rootDir: string = resolve(__dirname, 'repo'); const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile( @@ -1228,6 +1273,157 @@ describe(ProjectChangeAnalyzer.name, () => { expect(isPackageJsonVersionOnlyChange(oldContent, newContent)).toBe(true); }); }); + + describe('isPackageJsonVersionBumpChange', () => { + function expectGeneratedPeerDependencyRange(oldRange: string, newRange: string): void { + expect( + isPackageJsonVersionBumpChange( + { + name: 'consumer', + version: '1.0.0', + peerDependencies: { dependency: oldRange } + }, + { + name: 'consumer', + version: '1.0.1', + peerDependencies: { dependency: newRange } + }, + new Map([['dependency', '1.0.1']]) + ) + ).toBe(true); + } + + it('accepts a generated exact peer dependency range', () => { + expectGeneratedPeerDependencyRange('1.0.0', '1.0.1'); + }); + + it('accepts a generated caret peer dependency range', () => { + expectGeneratedPeerDependencyRange('^1.0.0', '^1.0.1'); + }); + + it('accepts generated ranges in every dependency section updated by VersionManager', () => { + expect( + isPackageJsonVersionBumpChange( + { + name: 'consumer', + version: '1.0.0', + dependencies: { exact: '1.0.0' }, + devDependencies: { tilde: '~2.0.0' }, + peerDependencies: { + range: '>=3.0.0 <4.0.0', + workspace: 'workspace:^4.0.0' + } + }, + { + name: 'consumer', + version: '1.0.1', + dependencies: { exact: '1.0.1' }, + devDependencies: { tilde: '~2.0.1' }, + peerDependencies: { + range: '>=3.0.1 <4.0.0', + workspace: 'workspace:^4.0.1' + } + }, + new Map([ + ['exact', '1.0.1'], + ['tilde', '2.0.1'], + ['range', '3.0.1'], + ['workspace', '4.0.1'] + ]) + ) + ).toBe(true); + }); + + it('rejects an unrelated dependency edit', () => { + expect( + isPackageJsonVersionBumpChange( + { + name: 'consumer', + version: '1.0.0', + dependencies: { dependency: '^1.0.0', unrelated: '^2.0.0' } + }, + { + name: 'consumer', + version: '1.0.1', + dependencies: { dependency: '^1.0.1', unrelated: '^2.1.0' } + }, + new Map([['dependency', '1.0.1']]) + ) + ).toBe(false); + }); + + it('rejects a semver-compatible range that Rush would not generate', () => { + expect( + isPackageJsonVersionBumpChange( + { + name: 'consumer', + version: '1.0.0', + dependencies: { dependency: '^1.0.0' } + }, + { + name: 'consumer', + version: '1.0.1', + dependencies: { dependency: '>=1.0.1 <2.0.0' } + }, + new Map([['dependency', '1.0.1']]) + ) + ).toBe(false); + }); + + it('rejects an optional dependency edit because VersionManager does not generate it', () => { + expect( + isPackageJsonVersionBumpChange( + { + name: 'consumer', + version: '1.0.0', + optionalDependencies: { dependency: '^1.0.0' } + }, + { + name: 'consumer', + version: '1.0.1', + optionalDependencies: { dependency: '^1.0.1' } + }, + new Map([['dependency', '1.0.1']]) + ) + ).toBe(false); + }); + + it('rejects a generated dependency range when the consumer version was not bumped', () => { + expect( + isPackageJsonVersionBumpChange( + { + name: 'consumer', + version: '1.0.0', + dependencies: { dependency: '^1.0.0' } + }, + { + name: 'consumer', + version: '1.0.0', + dependencies: { dependency: '^1.0.1' } + }, + new Map([['dependency', '1.0.1']]) + ) + ).toBe(false); + }); + + it('rejects a dependency edit when the referenced local project was not bumped', () => { + expect( + isPackageJsonVersionBumpChange( + { + name: 'consumer', + version: '1.0.0', + dependencies: { dependency: '^1.0.0' } + }, + { + name: 'consumer', + version: '1.0.1', + dependencies: { dependency: '^1.0.1' } + }, + new Map() + ) + ).toBe(false); + }); + }); }); /** From b9bea15ae1cb2b6fcedd14bcd80dde5f737426be Mon Sep 17 00:00:00 2001 From: Rushbot Date: Fri, 4 Sep 2026 15:23:58 -0700 Subject: [PATCH 2/3] Use undefined for ignored package fields Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index aa513aa712..361e497c0f 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -828,14 +828,14 @@ export function isPackageJsonVersionBumpChange( const oldPackageJsonWithoutBumpFields: Partial = { ...oldPackageJson }; const newPackageJsonWithoutBumpFields: Partial = { ...newPackageJson }; - delete oldPackageJsonWithoutBumpFields.version; - delete newPackageJsonWithoutBumpFields.version; + oldPackageJsonWithoutBumpFields.version = undefined; + newPackageJsonWithoutBumpFields.version = undefined; for (const dependencyFieldName of dependencyFieldNames) { const oldDependencies: IPackageJsonDependencyTable | undefined = oldPackageJson[dependencyFieldName]; const newDependencies: IPackageJsonDependencyTable | undefined = newPackageJson[dependencyFieldName]; - delete oldPackageJsonWithoutBumpFields[dependencyFieldName]; - delete newPackageJsonWithoutBumpFields[dependencyFieldName]; + oldPackageJsonWithoutBumpFields[dependencyFieldName] = undefined; + newPackageJsonWithoutBumpFields[dependencyFieldName] = undefined; if (!oldDependencies && !newDependencies) { continue; From 9d8be41f3091bcfde38897e2d1ed9589f7f91ad0 Mon Sep 17 00:00:00 2001 From: Rushbot Date: Fri, 4 Sep 2026 15:59:00 -0700 Subject: [PATCH 3/3] Simplify version bump peer dependency exclusion Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...n-bump-change-verify_2026-09-04-14-49.json | 2 +- .../src/logic/ProjectChangeAnalyzer.ts | 173 ++++-------------- .../logic/test/ProjectChangeAnalyzer.test.ts | 151 ++++----------- 3 files changed, 75 insertions(+), 251 deletions(-) diff --git a/common/changes/@microsoft/rush/fix-version-bump-change-verify_2026-09-04-14-49.json b/common/changes/@microsoft/rush/fix-version-bump-change-verify_2026-09-04-14-49.json index e225e6d08c..b8e2fdab60 100644 --- a/common/changes/@microsoft/rush/fix-version-bump-change-verify_2026-09-04-14-49.json +++ b/common/changes/@microsoft/rush/fix-version-bump-change-verify_2026-09-04-14-49.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@microsoft/rush", - "comment": "Fix `rush change --verify` to ignore dependency range rewrites generated by `rush version --bump` for locally bumped projects.", + "comment": "Fix `rush change --verify` to ignore peer dependency updates that accompany package version bumps.", "type": "patch" } ], diff --git a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts index 361e497c0f..08c559c6b7 100644 --- a/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts +++ b/libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts @@ -14,8 +14,7 @@ import { Sort, JsonFile, Objects, - type IPackageJson, - type IPackageJsonDependencyTable + type IPackageJson } from '@rushstack/node-core-library'; import { getRepoChanges, @@ -34,7 +33,6 @@ import { BaseProjectShrinkwrapFile } from './base/BaseProjectShrinkwrapFile'; import { PnpmShrinkwrapFile } from './pnpm/PnpmShrinkwrapFile'; import { Git } from './Git'; import { DependencySpecifier, DependencySpecifierType } from './DependencySpecifier'; -import { PublishUtilities } from './PublishUtilities'; import type { IPnpmOptionsJson, PnpmOptionsConfiguration } from './pnpm/PnpmOptionsConfiguration'; import { type IInputsSnapshotProjectMetadata, @@ -66,8 +64,7 @@ export interface IGetChangedProjectsOptions { /** * If set to `true`, excludes projects where the only changes are: - * - Changes to `package.json` produced by a version bump, including dependency range updates for - * other locally bumped projects + * - A version change to `package.json`, optionally accompanied by changes to `peerDependencies` * - Changes to `CHANGELOG.md` and/or `CHANGELOG.json` files * * This prevents `rush version --bump` from triggering `rush change --verify` to request change files @@ -133,15 +130,6 @@ export class ProjectChangeAnalyzer { RushConfigurationProject, Map > = this.getChangesByProject(lookup, changedFiles); - const packageJsonChanges: Map = excludeVersionOnlyChanges - ? await getPackageJsonChangesAsync(changesByProject, repoRoot, this._git) - : new Map(); - const bumpedProjectVersions: Map = new Map(); - for (const [project, packageJsonChange] of packageJsonChanges) { - if (packageJsonChange.oldPackageJson.version !== packageJsonChange.newPackageJson.version) { - bumpedProjectVersions.set(project.packageName, packageJsonChange.newPackageJson.version); - } - } const changedProjects: Set = new Set(); @@ -163,8 +151,8 @@ export class ProjectChangeAnalyzer { return; } - // Filter out generated version bump changes in package.json and changelog files. - for (const filePath of filteredChanges.keys()) { + // Filter out version bumps, peer dependency updates accompanying a version bump, and changelogs. + for (const [filePath, diffStatus] of filteredChanges) { // Use lookup to find the project-relative path const match: IPrefixMatch | undefined = lookup.findLongestPrefixMatch(filePath); @@ -181,16 +169,13 @@ export class ProjectChangeAnalyzer { continue; } - // Check if this is package.json at project root with changes generated by the version bump + // Check if this is package.json at project root with only an allowed version bump change. if (projectRelativePath === '/package.json') { - const packageJsonChange: IPackageJsonChange | undefined = packageJsonChanges.get(project); - const isVersionBumpChange: boolean = - !!packageJsonChange && - isPackageJsonVersionBumpChange( - packageJsonChange.oldPackageJson, - packageJsonChange.newPackageJson, - bumpedProjectVersions - ); + const isVersionBumpChange: boolean = await isVersionBumpChangeAsync( + diffStatus, + repoRoot, + this._git + ); if (isVersionBumpChange) { continue; } @@ -656,61 +641,30 @@ export class ProjectChangeAnalyzer { } } -interface IPackageJsonChange { - oldPackageJson: IPackageJson; - newPackageJson: IPackageJson; -} - -const dependencyFieldNames: ReadonlyArray> = ['dependencies', 'devDependencies', 'peerDependencies']; - -async function getPackageJsonChangesAsync( - changesByProject: ReadonlyMap>, +async function isVersionBumpChangeAsync( + diffStatus: IFileDiffStatus, repoRoot: string, git: Git -): Promise> { - const packageJsonChanges: Map = new Map(); - await Async.forEachAsync( - changesByProject, - async ([project, projectChanges]) => { - const packageJsonPath: string = Path.convertToSlashes( - path.relative(repoRoot, path.join(project.projectFolder, 'package.json')) - ); - const diffStatus: IFileDiffStatus | undefined = projectChanges.get(packageJsonPath); - if (diffStatus?.status !== 'M') { - return; - } +): Promise { + if (diffStatus.status !== 'M') { + return false; + } - try { - const [oldPackageJsonContent, newPackageJsonContent] = await Promise.all([ - git.getBlobContentAsync({ - blobSpec: diffStatus.oldhash, - repositoryRoot: repoRoot - }), - git.getBlobContentAsync({ - blobSpec: diffStatus.newhash, - repositoryRoot: repoRoot - }) - ]); - const oldPackageJson: IPackageJson = JSON.parse(oldPackageJsonContent); - const newPackageJson: IPackageJson = JSON.parse(newPackageJsonContent); - if ( - oldPackageJson.name === project.packageName && - newPackageJson.name === project.packageName && - typeof oldPackageJson.version === 'string' && - typeof newPackageJson.version === 'string' - ) { - packageJsonChanges.set(project, { oldPackageJson, newPackageJson }); - } - } catch (error) { - // A package.json that cannot be read or parsed must remain a substantive change. - } - }, - { concurrency: 10 } - ); - return packageJsonChanges; + try { + const [oldPackageJsonContent, newPackageJsonContent] = await Promise.all([ + git.getBlobContentAsync({ + blobSpec: diffStatus.oldhash, + repositoryRoot: repoRoot + }), + git.getBlobContentAsync({ + blobSpec: diffStatus.newhash, + repositoryRoot: repoRoot + }) + ]); + return isPackageJsonVersionOnlyChange(oldPackageJsonContent, newPackageJsonContent); + } catch (error) { + return false; + } } interface IAdditionalGlob { @@ -787,21 +741,18 @@ async function getAdditionalFilesFromRushProjectConfigurationAsync( } /** - * Compares two package.json file contents and determines if the only difference is the "version" field. + * Compares two package.json file contents and determines whether the package's version changed and + * all other changes are limited to peerDependencies. * @param oldPackageJsonContent - The old package.json content as a string * @param newPackageJsonContent - The new package.json content as a string - * @returns true if the only difference is the version field, false otherwise + * @returns true if the package version changed and every other field except peerDependencies is unchanged */ export function isPackageJsonVersionOnlyChange( oldPackageJsonContent: string, newPackageJsonContent: string ): boolean { try { - return isPackageJsonVersionBumpChange( - JSON.parse(oldPackageJsonContent), - JSON.parse(newPackageJsonContent), - new Map() - ); + return isPackageJsonVersionBumpChange(JSON.parse(oldPackageJsonContent), JSON.parse(newPackageJsonContent)); } catch (error) { // If we can't parse the JSON, assume it's not a version-only change return false; @@ -809,13 +760,11 @@ export function isPackageJsonVersionOnlyChange( } /** - * Determines whether a package.json differs only by its version and dependency range rewrites that - * Rush would generate for other locally bumped projects. + * Determines whether a package.json differs only by its version and peerDependencies. */ export function isPackageJsonVersionBumpChange( oldPackageJson: IPackageJson, - newPackageJson: IPackageJson, - bumpedProjectVersions: ReadonlyMap + newPackageJson: IPackageJson ): boolean { if ( typeof oldPackageJson.version !== 'string' || @@ -830,52 +779,8 @@ export function isPackageJsonVersionBumpChange( const newPackageJsonWithoutBumpFields: Partial = { ...newPackageJson }; oldPackageJsonWithoutBumpFields.version = undefined; newPackageJsonWithoutBumpFields.version = undefined; - - for (const dependencyFieldName of dependencyFieldNames) { - const oldDependencies: IPackageJsonDependencyTable | undefined = oldPackageJson[dependencyFieldName]; - const newDependencies: IPackageJsonDependencyTable | undefined = newPackageJson[dependencyFieldName]; - oldPackageJsonWithoutBumpFields[dependencyFieldName] = undefined; - newPackageJsonWithoutBumpFields[dependencyFieldName] = undefined; - - if (!oldDependencies && !newDependencies) { - continue; - } - if ( - !oldDependencies || - !newDependencies || - Object.keys(oldDependencies).length !== Object.keys(newDependencies).length - ) { - return false; - } - - for (const [dependencyName, oldDependencyVersion] of Object.entries(oldDependencies)) { - const newDependencyVersion: string | undefined = newDependencies[dependencyName]; - if (!newDependencyVersion) { - return false; - } - if (oldDependencyVersion === newDependencyVersion) { - continue; - } - - const bumpedProjectVersion: string | undefined = bumpedProjectVersions.get(dependencyName); - if (!bumpedProjectVersion) { - return false; - } - - try { - const expectedDependencyVersion: string = PublishUtilities.getNewDependencyVersion( - oldDependencies, - dependencyName, - bumpedProjectVersion - ); - if (newDependencyVersion !== expectedDependencyVersion) { - return false; - } - } catch (error) { - return false; - } - } - } + oldPackageJsonWithoutBumpFields.peerDependencies = undefined; + newPackageJsonWithoutBumpFields.peerDependencies = undefined; return Objects.areDeepEqual(oldPackageJsonWithoutBumpFields, newPackageJsonWithoutBumpFields); } diff --git a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts index efa014862c..c9138b3c85 100644 --- a/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts +++ b/libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts @@ -382,7 +382,7 @@ describe(ProjectChangeAnalyzer.name, () => { expect(changedProjects.has(rushConfiguration.getProjectByName('b')!)).toBe(true); }); - it('excludeVersionOnlyChanges excludes dependency ranges generated for locally bumped projects', async () => { + it('excludeVersionOnlyChanges excludes arbitrary peer dependency changes with a version bump', async () => { const rootDir: string = resolve(__dirname, 'repo'); const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile( resolve(rootDir, 'rush.json') @@ -390,10 +390,6 @@ describe(ProjectChangeAnalyzer.name, () => { mockGetRepoChanges.mockReturnValue( new Map([ - [ - 'a/package.json', - { mode: 'modified', newhash: 'newhash-a', oldhash: 'oldhash-a', status: 'M' } - ], [ 'b/package.json', { mode: 'modified', newhash: 'newhash-b', oldhash: 'oldhash-b', status: 'M' } @@ -401,10 +397,8 @@ describe(ProjectChangeAnalyzer.name, () => { ]) ); const packageJsonByHash: Record = { - 'oldhash-a': { name: 'a', version: '1.0.0' }, - 'newhash-a': { name: 'a', version: '1.0.1' }, - 'oldhash-b': { name: 'b', version: '2.0.0', peerDependencies: { a: '1.0.0' } }, - 'newhash-b': { name: 'b', version: '2.0.1', peerDependencies: { a: '1.0.1' } } + 'oldhash-b': { name: 'b', version: '2.0.0', peerDependencies: { external: '^1.0.0' } }, + 'newhash-b': { name: 'b', version: '2.0.1', peerDependencies: { external: '>=3.0.0' } } }; mockGetBlobContentAsync.mockImplementation(({ blobSpec }) => Promise.resolve(JSON.stringify(packageJsonByHash[blobSpec]!)) @@ -1275,151 +1269,76 @@ describe(ProjectChangeAnalyzer.name, () => { }); describe('isPackageJsonVersionBumpChange', () => { - function expectGeneratedPeerDependencyRange(oldRange: string, newRange: string): void { - expect( - isPackageJsonVersionBumpChange( - { - name: 'consumer', - version: '1.0.0', - peerDependencies: { dependency: oldRange } - }, - { - name: 'consumer', - version: '1.0.1', - peerDependencies: { dependency: newRange } - }, - new Map([['dependency', '1.0.1']]) - ) - ).toBe(true); - } - - it('accepts a generated exact peer dependency range', () => { - expectGeneratedPeerDependencyRange('1.0.0', '1.0.1'); - }); - - it('accepts a generated caret peer dependency range', () => { - expectGeneratedPeerDependencyRange('^1.0.0', '^1.0.1'); - }); - - it('accepts generated ranges in every dependency section updated by VersionManager', () => { + it('accepts arbitrary peer dependency changes with a version bump', () => { expect( isPackageJsonVersionBumpChange( { name: 'consumer', version: '1.0.0', - dependencies: { exact: '1.0.0' }, - devDependencies: { tilde: '~2.0.0' }, - peerDependencies: { - range: '>=3.0.0 <4.0.0', - workspace: 'workspace:^4.0.0' - } + peerDependencies: { dependency: '^1.0.0' } }, { name: 'consumer', version: '1.0.1', - dependencies: { exact: '1.0.1' }, - devDependencies: { tilde: '~2.0.1' }, peerDependencies: { - range: '>=3.0.1 <4.0.0', - workspace: 'workspace:^4.0.1' + anotherDependency: 'workspace:*', + dependency: 'file:../dependency' } - }, - new Map([ - ['exact', '1.0.1'], - ['tilde', '2.0.1'], - ['range', '3.0.1'], - ['workspace', '4.0.1'] - ]) + } ) ).toBe(true); }); - it('rejects an unrelated dependency edit', () => { + it('rejects peer dependency changes without a version bump', () => { expect( isPackageJsonVersionBumpChange( { name: 'consumer', version: '1.0.0', - dependencies: { dependency: '^1.0.0', unrelated: '^2.0.0' } + peerDependencies: { dependency: '^1.0.0' } }, - { - name: 'consumer', - version: '1.0.1', - dependencies: { dependency: '^1.0.1', unrelated: '^2.1.0' } - }, - new Map([['dependency', '1.0.1']]) - ) - ).toBe(false); - }); - - it('rejects a semver-compatible range that Rush would not generate', () => { - expect( - isPackageJsonVersionBumpChange( { name: 'consumer', version: '1.0.0', - dependencies: { dependency: '^1.0.0' } - }, - { - name: 'consumer', - version: '1.0.1', - dependencies: { dependency: '>=1.0.1 <2.0.0' } - }, - new Map([['dependency', '1.0.1']]) - ) - ).toBe(false); - }); - - it('rejects an optional dependency edit because VersionManager does not generate it', () => { - expect( - isPackageJsonVersionBumpChange( - { - name: 'consumer', - version: '1.0.0', - optionalDependencies: { dependency: '^1.0.0' } - }, - { - name: 'consumer', - version: '1.0.1', - optionalDependencies: { dependency: '^1.0.1' } - }, - new Map([['dependency', '1.0.1']]) + peerDependencies: { dependency: '^2.0.0' } + } ) ).toBe(false); }); - it('rejects a generated dependency range when the consumer version was not bumped', () => { - expect( - isPackageJsonVersionBumpChange( - { - name: 'consumer', - version: '1.0.0', - dependencies: { dependency: '^1.0.0' } - }, - { - name: 'consumer', - version: '1.0.0', - dependencies: { dependency: '^1.0.1' } - }, - new Map([['dependency', '1.0.1']]) - ) - ).toBe(false); - }); + it.each(['dependencies', 'devDependencies', 'optionalDependencies'] as const)( + 'rejects a version bump with a %s change', + (dependencyFieldName) => { + expect( + isPackageJsonVersionBumpChange( + { + name: 'consumer', + version: '1.0.0', + [dependencyFieldName]: { dependency: '^1.0.0' } + }, + { + name: 'consumer', + version: '1.0.1', + [dependencyFieldName]: { dependency: '^2.0.0' } + } + ) + ).toBe(false); + } + ); - it('rejects a dependency edit when the referenced local project was not bumped', () => { + it('rejects a version bump with an unrelated field change', () => { expect( isPackageJsonVersionBumpChange( { name: 'consumer', version: '1.0.0', - dependencies: { dependency: '^1.0.0' } + description: 'Old description' }, { name: 'consumer', version: '1.0.1', - dependencies: { dependency: '^1.0.1' } - }, - new Map() + description: 'New description' + } ) ).toBe(false); });