Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@microsoft/rush",
"comment": "Fix `rush change --verify` to ignore peer dependency updates that accompany package version bumps.",
"type": "patch"
}
],
"packageName": "@microsoft/rush"
}
2 changes: 1 addition & 1 deletion libraries/rush-lib/src/cli/actions/ChangeAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<RushConfigurationProject, string> = this._generateHostMap();
Expand Down
91 changes: 53 additions & 38 deletions libraries/rush-lib/src/logic/ProjectChangeAnalyzer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,16 @@ 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
} from '@rushstack/node-core-library';
import {
getRepoChanges,
getRepoRoot,
Expand Down Expand Up @@ -55,7 +64,7 @@ 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)
* - 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
Expand Down Expand Up @@ -142,7 +151,7 @@ export class ProjectChangeAnalyzer {
return;
}

// Filter out package.json with version-only changes, CHANGELOG.md, and CHANGELOG.json
// 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<RushConfigurationProject> | undefined =
Expand All @@ -160,15 +169,15 @@ 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 only an allowed version bump change.
if (projectRelativePath === '/package.json') {
const isVersionOnlyChange: boolean = await isVersionOnlyChangeAsync(
const isVersionBumpChange: boolean = await isVersionBumpChangeAsync(
diffStatus,
repoRoot,
this._git
);
if (isVersionOnlyChange) {
continue; // Skip version-only package.json changes
if (isVersionBumpChange) {
continue;
}
}

Expand Down Expand Up @@ -632,22 +641,17 @@ export class ProjectChangeAnalyzer {
}
}

/**
* Checks if a diff represents a version-only change to package.json.
*/
async function isVersionOnlyChangeAsync(
async function isVersionBumpChangeAsync(
diffStatus: IFileDiffStatus,
repoRoot: string,
git: Git
): Promise<boolean> {
try {
// Only check modified files, not additions or deletions
if (diffStatus.status !== 'M') {
return false;
}
if (diffStatus.status !== 'M') {
return false;
}

// Get both versions of package.json from Git in parallel
const [oldPackageJsonContent, currentPackageJsonContent] = await Promise.all([
try {
const [oldPackageJsonContent, newPackageJsonContent] = await Promise.all([
git.getBlobContentAsync({
blobSpec: diffStatus.oldhash,
repositoryRoot: repoRoot
Expand All @@ -657,10 +661,8 @@ async function isVersionOnlyChangeAsync(
repositoryRoot: repoRoot
})
]);

return isPackageJsonVersionOnlyChange(oldPackageJsonContent, currentPackageJsonContent);
return isPackageJsonVersionOnlyChange(oldPackageJsonContent, newPackageJsonContent);
} catch (error) {
// If we can't read the file or parse it, assume it's not a version-only change
return false;
}
}
Expand Down Expand Up @@ -739,33 +741,46 @@ 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 {
// 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);

// Ensure both have a version field
if (!oldPackageJson.version || !newPackageJson.version) {
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;

// Compare the objects without the version field
return JSON.stringify(oldPackageJson) === JSON.stringify(newPackageJson);
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;
}
}

/**
* Determines whether a package.json differs only by its version and peerDependencies.
*/
export function isPackageJsonVersionBumpChange(
oldPackageJson: IPackageJson,
newPackageJson: IPackageJson
): boolean {
if (
typeof oldPackageJson.version !== 'string' ||
typeof newPackageJson.version !== 'string' ||
oldPackageJson.version === newPackageJson.version ||
oldPackageJson.name !== newPackageJson.name
) {
return false;
}

const oldPackageJsonWithoutBumpFields: Partial<IPackageJson> = { ...oldPackageJson };
const newPackageJsonWithoutBumpFields: Partial<IPackageJson> = { ...newPackageJson };
oldPackageJsonWithoutBumpFields.version = undefined;
newPackageJsonWithoutBumpFields.version = undefined;
oldPackageJsonWithoutBumpFields.peerDependencies = undefined;
newPackageJsonWithoutBumpFields.peerDependencies = undefined;

return Objects.areDeepEqual(oldPackageJsonWithoutBumpFields, newPackageJsonWithoutBumpFields);
}
117 changes: 116 additions & 1 deletion libraries/rush-lib/src/logic/test/ProjectChangeAnalyzer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -378,6 +382,41 @@ describe(ProjectChangeAnalyzer.name, () => {
expect(changedProjects.has(rushConfiguration.getProjectByName('b')!)).toBe(true);
});

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')
);

mockGetRepoChanges.mockReturnValue(
new Map<string, IFileDiffStatus>([
[
'b/package.json',
{ mode: 'modified', newhash: 'newhash-b', oldhash: 'oldhash-b', status: 'M' }
]
])
);
const packageJsonByHash: Record<string, object> = {
'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]!))
);

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(
Expand Down Expand Up @@ -1228,6 +1267,82 @@ describe(ProjectChangeAnalyzer.name, () => {
expect(isPackageJsonVersionOnlyChange(oldContent, newContent)).toBe(true);
});
});

describe('isPackageJsonVersionBumpChange', () => {
it('accepts arbitrary peer dependency changes with a version bump', () => {
expect(
isPackageJsonVersionBumpChange(
{
name: 'consumer',
version: '1.0.0',
peerDependencies: { dependency: '^1.0.0' }
},
{
name: 'consumer',
version: '1.0.1',
peerDependencies: {
anotherDependency: 'workspace:*',
dependency: 'file:../dependency'
}
}
)
).toBe(true);
});

it('rejects peer dependency changes without a version bump', () => {
expect(
isPackageJsonVersionBumpChange(
{
name: 'consumer',
version: '1.0.0',
peerDependencies: { dependency: '^1.0.0' }
},
{
name: 'consumer',
version: '1.0.0',
peerDependencies: { dependency: '^2.0.0' }
}
)
).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 version bump with an unrelated field change', () => {
expect(
isPackageJsonVersionBumpChange(
{
name: 'consumer',
version: '1.0.0',
description: 'Old description'
},
{
name: 'consumer',
version: '1.0.1',
description: 'New description'
}
)
).toBe(false);
});
});
});

/**
Expand Down