Skip to content

Commit e1abb30

Browse files
JavaScript: Package Manager to return the same marker if no changes (#7473)
1 parent 1df7d01 commit e1abb30

2 files changed

Lines changed: 93 additions & 1 deletion

File tree

rewrite-javascript/rewrite/src/javascript/package-manager.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import * as fsp from "fs/promises";
3434
import * as path from "path";
3535
import * as os from "os";
3636
import {spawnSync} from "child_process";
37+
import {isDeepStrictEqual} from "util";
3738
import * as YAML from "yaml";
3839

3940
/**
@@ -572,6 +573,16 @@ export async function updateNodeResolutionMarker<T extends BaseProjectUpdateInfo
572573
existingMarker.npmrcConfigs
573574
);
574575

576+
// If the new marker is structurally identical to the existing one (ignoring the
577+
// auto-generated id), return doc unchanged to avoid minting a fresh AST identity
578+
// for a no-op marker update. This prevents the empty-diff guard from firing on
579+
// recipes that don't modify package.json (e.g. lock-file-only fix strategies).
580+
const {id: _existingId, ...existingRest} = existingMarker;
581+
const {id: _newId, ...newRest} = newMarker;
582+
if (isDeepStrictEqual(existingRest, newRest)) {
583+
return doc;
584+
}
585+
575586
// Replace the marker in the document
576587
return {
577588
...doc,

rewrite-javascript/rewrite/test/javascript/package-manager.test.ts

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,23 @@
1515
*/
1616
import {withDir} from "tmp-promise";
1717
import * as fsp from "fs/promises";
18+
import * as fs from "fs";
1819
import * as path from "path";
1920
import {
21+
BaseProjectUpdateInfo,
22+
createDependencyRecipeAccumulator,
2023
detectPackageManager,
2124
getAllLockFileNames,
2225
getLockFileDetectionConfig,
2326
getLockFileFormat,
2427
getLockFileName,
2528
isYarnBerryLockFile,
29+
PackageJsonParser,
2630
PackageManager,
27-
runWorkspaceInstallInTempDir
31+
runWorkspaceInstallInTempDir,
32+
updateNodeResolutionMarker
2833
} from "../../src/javascript";
34+
import {Json} from "../../src/json";
2935

3036
describe("detectPackageManager", () => {
3137

@@ -325,3 +331,78 @@ describe("runWorkspaceInstallInTempDir", () => {
325331
expect(result.lockFileContent).toContain("is-number");
326332
}, 120000);
327333
});
334+
335+
describe("updateNodeResolutionMarker", () => {
336+
337+
test("returns same doc reference when accumulator has no updates", async () => {
338+
// given a parsed package.json with a NodeResolutionResult marker
339+
await withDir(async (dir) => {
340+
const packageJsonContent = JSON.stringify({
341+
name: "test-project",
342+
version: "1.0.0",
343+
dependencies: {
344+
"react": "^18.2.0"
345+
}
346+
}, null, 2);
347+
fs.writeFileSync(path.join(dir.path, "package.json"), packageJsonContent);
348+
349+
const parser = new PackageJsonParser({relativeTo: dir.path});
350+
const docs: Json.Document[] = [];
351+
for await (const result of parser.parse(path.join(dir.path, "package.json"))) {
352+
docs.push(result as Json.Document);
353+
}
354+
const doc = docs[0];
355+
356+
// when calling updateNodeResolutionMarker with an accumulator that has
357+
// no entries for this project (no package.json or lock file changes)
358+
const acc = createDependencyRecipeAccumulator<BaseProjectUpdateInfo>();
359+
const updateInfo = {
360+
packageJsonPath: "package.json",
361+
packageManager: PackageManager.Npm,
362+
originalPackageJson: packageJsonContent
363+
};
364+
365+
const result = await updateNodeResolutionMarker(doc, updateInfo, acc);
366+
367+
// then the same doc reference is returned (no-op marker update)
368+
expect(result).toBe(doc);
369+
}, {unsafeCleanup: true});
370+
});
371+
372+
test("returns same doc reference when updated content is structurally identical", async () => {
373+
// given a parsed package.json with a NodeResolutionResult marker
374+
await withDir(async (dir) => {
375+
const packageJsonContent = JSON.stringify({
376+
name: "test-project",
377+
version: "1.0.0",
378+
dependencies: {
379+
"react": "^18.2.0"
380+
}
381+
}, null, 2);
382+
fs.writeFileSync(path.join(dir.path, "package.json"), packageJsonContent);
383+
384+
const parser = new PackageJsonParser({relativeTo: dir.path});
385+
const docs: Json.Document[] = [];
386+
for await (const result of parser.parse(path.join(dir.path, "package.json"))) {
387+
docs.push(result as Json.Document);
388+
}
389+
const doc = docs[0];
390+
391+
// when calling updateNodeResolutionMarker with an accumulator whose
392+
// updatedPackageJsons contains content identical to the original
393+
const acc = createDependencyRecipeAccumulator<BaseProjectUpdateInfo>();
394+
acc.updatedPackageJsons.set("package.json", packageJsonContent);
395+
const updateInfo = {
396+
packageJsonPath: "package.json",
397+
packageManager: PackageManager.Npm,
398+
originalPackageJson: packageJsonContent
399+
};
400+
401+
const result = await updateNodeResolutionMarker(doc, updateInfo, acc);
402+
403+
// then the same doc reference is returned (structurally equal marker)
404+
expect(result).toBe(doc);
405+
}, {unsafeCleanup: true});
406+
});
407+
408+
});

0 commit comments

Comments
 (0)