-
Notifications
You must be signed in to change notification settings - Fork 76
fix(release): bump package-lock.json alongside package.json #290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,66 @@ | ||||||
| #!/usr/bin/env node | ||||||
| // Sets the release version in package.json AND package-lock.json. | ||||||
| // | ||||||
| // prerelease.yml and promote.yml used to `sed` package.json alone, so every | ||||||
| // release shipped a lockfile whose root version disagreed with the package it | ||||||
| // locks: | ||||||
| // | ||||||
| // v1.7.0 package.json=1.7.0 lock=1.6.0 | ||||||
| // v1.8.0 package.json=1.8.0 lock=1.8.0-rc.4 | ||||||
| // v1.9.0 package.json=1.9.0 lock=1.8.0 | ||||||
| // | ||||||
| // Nothing caught it for three releases because `npm ci` only fails on | ||||||
| // dependency drift, never on this field — the mismatch is inert until someone | ||||||
| // reads the diff, which is how it was eventually noticed. | ||||||
| // | ||||||
| // Both files are tab-indented JSON that JSON.stringify round-trips byte for | ||||||
| // byte, so rewriting them whole still produces a one-line-per-file diff. The | ||||||
| // test pins that: if npm ever changes how it formats a lockfile, a release | ||||||
| // commit would otherwise silently become a 40k-line reformat. | ||||||
|
|
||||||
| import { readFileSync, writeFileSync } from "node:fs"; | ||||||
| import { join } from "node:path"; | ||||||
| import { argv } from "node:process"; | ||||||
|
|
||||||
| /** | ||||||
| * @param {string} version Version to write, e.g. "1.9.0" or "1.9.0-rc.2". | ||||||
| * @param {string} dir Directory holding package.json and package-lock.json. | ||||||
| */ | ||||||
| export function setReleaseVersion(version, dir) { | ||||||
| if (!version) throw new Error("a version is required"); | ||||||
|
|
||||||
| const edit = (name, mutate) => { | ||||||
| const file = join(dir, name); | ||||||
| const json = JSON.parse(readFileSync(file, "utf8")); | ||||||
| mutate(json); | ||||||
| writeFileSync(file, `${JSON.stringify(json, null, "\t")}\n`); | ||||||
| }; | ||||||
|
|
||||||
| edit("package.json", (pkg) => { | ||||||
| pkg.version = version; | ||||||
| }); | ||||||
|
|
||||||
| edit("package-lock.json", (lock) => { | ||||||
| lock.version = version; | ||||||
| // lockfileVersion 3 repeats the root version inside packages[""]. Optional | ||||||
| // chaining would quietly skip it if the shape ever changed — the same | ||||||
| // silent half-bump this script exists to end — so demand it instead. | ||||||
| if (!lock.packages?.[""]) { | ||||||
| throw new Error( | ||||||
| 'package-lock.json has no packages[""] entry; the lockfile format changed and this script needs updating', | ||||||
| ); | ||||||
| } | ||||||
| lock.packages[""].version = version; | ||||||
| }); | ||||||
| } | ||||||
|
|
||||||
| // Only run when invoked directly, so the test can import the function. | ||||||
| if (import.meta.filename === argv[1]) { | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Repo files matching script:"
fd -a 'set-release-version\.mjs$' . || true
echo
echo "Script size:"
wc -l .github/scripts/set-release-version.mjs 2>/dev/null || true
echo
echo "Relevant script lines:"
sed -n '1,120p' .github/scripts/set-release-version.mjs 2>/dev/null || true
echo
echo "Usages of script:"
rg -n "set-release-version\.mjs|release-version|releaseVersion|manifest" .github package.json . 2>/dev/null | head -200Repository: getopenscreen/openscreen Length of output: 9633 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json, sys, pathlib, tempfile, os
script = pathlib.Path(".github/scripts/set-release-version.mjs")
if not script.exists():
print("missing script")
sys.exit(0)
source = script.read_text()
for needle in ["import.meta.filename", "argv[1]", "resolve(", "join("]:
print(f"{needle!r}: {source.count(needle)}")
print("contains current comparison:")
print("if (import.meta.filename === argv[1])" in source)
with tempfile.TemporaryDirectory() as d:
dpath = pathlib.Path(d)
(dpath / "package.json").write_text("{}")
os.chdir(dpath)
os.environ["GITHUB_REPOSITORY"] = "owner/repo"
os.environ["GITHUB_WORKSPACE"] = str(dpath)
os.environ["GITHUB_REF_NAME"] = "main"
os.environ["GITHUB_REF"] = "refs/heads/main"
full = script.resolve()
rel = pathlib.Path(".github/scripts/set-release-version.mjs")
print("\nPaths in temporary repo at root:")
print("GITHUB_WORKSPACE:", dpath)
print("script resolve():", full)
print("cli argv[1] =", str(rel))
print("absolute argv:", os.path.abspath(str(rel)))
print("current equality:", str(full) == str(rel))
print("current equality with resolved argv:", str(full) == os.path.abspath(str(rel)))
PYRepository: getopenscreen/openscreen Length of output: 596 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "Lines 95-117 of prerelease.yml:"
sed -n '95,117p' .github/workflows/prerelease.yml 2>/dev/null || true
echo
echo "Lines 65-83 of promote.yml:"
sed -n '65,83p' .github/workflows/promote.yml 2>/dev/null || true
echo
echo "Relevant tests:"
sed -n '1,140p' .github/scripts/set-release-version.test.mjs 2>/dev/null || trueRepository: getopenscreen/openscreen Length of output: 6268 Normalize the direct-invocation path check.
Proposed fix-import { join } from "node:path";
+import { join, resolve } from "node:path";
-if (import.meta.filename === argv[1]) {
+if (argv[1] && import.meta.filename === resolve(argv[1])) {Add a CLI test that invokes 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| const version = argv[2]; | ||||||
| if (!version) { | ||||||
| console.error("usage: node .github/scripts/set-release-version.mjs <version>"); | ||||||
| process.exit(1); | ||||||
| } | ||||||
| setReleaseVersion(version, process.cwd()); | ||||||
| console.log(`version ${version} set in package.json and package-lock.json`); | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; | ||
| import { tmpdir } from "node:os"; | ||
| import { join } from "node:path"; | ||
| import { afterEach, beforeEach, describe, expect, it } from "vitest"; | ||
| import { setReleaseVersion } from "./set-release-version.mjs"; | ||
|
|
||
| let dir; | ||
|
|
||
| // Tab-indented, like the real files, and shaped like lockfileVersion 3 — the | ||
| // point of most of these assertions is formatting, so the fixtures have to be | ||
| // byte-faithful rather than merely structurally right. | ||
| const pkg = [ | ||
| "{", | ||
| '\t"name": "openscreen",', | ||
| '\t"version": "1.8.0",', | ||
| '\t"private": true', | ||
| "}", | ||
| "", | ||
| ].join("\n"); | ||
|
|
||
| const lock = [ | ||
| "{", | ||
| '\t"name": "openscreen",', | ||
| '\t"version": "1.8.0",', | ||
| '\t"lockfileVersion": 3,', | ||
| '\t"requires": true,', | ||
| '\t"packages": {', | ||
| '\t\t"": {', | ||
| '\t\t\t"name": "openscreen",', | ||
| '\t\t\t"version": "1.8.0",', | ||
| '\t\t\t"dependencies": {', | ||
| '\t\t\t\t"zod": "^4.0.0"', | ||
| "\t\t\t}", | ||
| "\t\t},", | ||
| '\t\t"node_modules/zod": {', | ||
| '\t\t\t"version": "4.0.0"', | ||
| "\t\t}", | ||
| "\t}", | ||
| "}", | ||
| "", | ||
| ].join("\n"); | ||
|
|
||
| beforeEach(() => { | ||
| dir = mkdtempSync(join(tmpdir(), "set-release-version-")); | ||
| writeFileSync(join(dir, "package.json"), pkg); | ||
| writeFileSync(join(dir, "package-lock.json"), lock); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| rmSync(dir, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| const read = (name) => readFileSync(join(dir, name), "utf8"); | ||
|
|
||
| describe("setReleaseVersion", () => { | ||
| it("sets the version in package.json and both lockfile roots", () => { | ||
| setReleaseVersion("1.9.0", dir); | ||
|
|
||
| expect(JSON.parse(read("package.json")).version).toBe("1.9.0"); | ||
| const written = JSON.parse(read("package-lock.json")); | ||
| expect(written.version).toBe("1.9.0"); | ||
| expect(written.packages[""].version).toBe("1.9.0"); | ||
| }); | ||
|
|
||
| it("accepts a prerelease version", () => { | ||
| setReleaseVersion("2.0.0-rc.3", dir); | ||
|
|
||
| expect(JSON.parse(read("package.json")).version).toBe("2.0.0-rc.3"); | ||
| expect(JSON.parse(read("package-lock.json")).packages[""].version).toBe("2.0.0-rc.3"); | ||
| }); | ||
|
|
||
| // The reason the script may rewrite these files wholesale: anything else in | ||
| // them must come back out byte for byte. If npm changes its lockfile | ||
| // formatting, this fails here rather than turning a release commit into a | ||
| // 40k-line reformat nobody reviews. | ||
| it("changes only the version lines, leaving formatting untouched", () => { | ||
| setReleaseVersion("1.9.0", dir); | ||
|
|
||
| const diff = (before, after) => { | ||
| const a = before.split("\n"); | ||
| const b = after.split("\n"); | ||
| expect(b.length).toBe(a.length); | ||
| return a.map((line, i) => [line, b[i]]).filter(([x, y]) => x !== y); | ||
| }; | ||
|
|
||
| expect(diff(pkg, read("package.json"))).toEqual([ | ||
| ['\t"version": "1.8.0",', '\t"version": "1.9.0",'], | ||
| ]); | ||
| expect(diff(lock, read("package-lock.json"))).toEqual([ | ||
| ['\t"version": "1.8.0",', '\t"version": "1.9.0",'], | ||
| ['\t\t\t"version": "1.8.0",', '\t\t\t"version": "1.9.0",'], | ||
| ]); | ||
| }); | ||
|
|
||
| it("leaves dependency versions alone", () => { | ||
| setReleaseVersion("1.9.0", dir); | ||
|
|
||
| const written = JSON.parse(read("package-lock.json")); | ||
| expect(written.packages["node_modules/zod"].version).toBe("4.0.0"); | ||
| }); | ||
|
|
||
| // A lockfile format change must stop the release, not half-bump it. | ||
| it("throws rather than half-bumping when the lockfile shape is unknown", () => { | ||
| writeFileSync( | ||
| join(dir, "package-lock.json"), | ||
| `${JSON.stringify({ name: "openscreen", version: "1.8.0" }, null, "\t")}\n`, | ||
| ); | ||
|
|
||
| expect(() => setReleaseVersion("1.9.0", dir)).toThrow(/packages/); | ||
| }); | ||
|
|
||
| it("requires a version", () => { | ||
| expect(() => setReleaseVersion("", dir)).toThrow(/version is required/); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate the lockfile before writing either manifest.
When
package-lock.jsonlackspackages[""], Line 40 writespackage.jsonand Lines 48-51 then throw. This leaves the manifests out of sync on the error path that must prevent partial updates.Parse and validate both files first. Write either file only after validation succeeds. Extend the invalid-lockfile test to assert that
package.jsonremains at1.8.0.🤖 Prompt for AI Agents