From 5898daee69b00a9e3bbf47d5c4d70089751be925 Mon Sep 17 00:00:00 2001 From: Mark Murray Date: Fri, 2 Oct 2026 09:54:58 +0100 Subject: [PATCH 1/7] Add configurable bundle size budgets and explicit PR acceptance --- .ci/bundle-size-budgets.json | 8 + .ci/bundle-size-budgets.md | 81 ++++++ .github/scripts/bundle-size-budgets.cjs | 257 +++++++++++++++++ .github/scripts/bundle-size-budgets.test.cjs | 180 ++++++++++++ .github/scripts/measure-bundle-size.test.cjs | 69 +++++ .github/scripts/measure-package-size | 24 +- .github/scripts/publish-bundle-size.cjs | 279 +++++++++++++++++++ .github/scripts/publish-bundle-size.test.cjs | 277 ++++++++++++++++++ .github/workflows/bundle-size-budgets.yml | 66 +++++ .github/workflows/ci.yml | 6 + .github/workflows/package-size.yml | 88 +++--- dev.yml | 1 + 12 files changed, 1291 insertions(+), 45 deletions(-) create mode 100644 .ci/bundle-size-budgets.json create mode 100644 .ci/bundle-size-budgets.md create mode 100644 .github/scripts/bundle-size-budgets.cjs create mode 100644 .github/scripts/bundle-size-budgets.test.cjs create mode 100644 .github/scripts/measure-bundle-size.test.cjs create mode 100644 .github/scripts/publish-bundle-size.cjs create mode 100644 .github/scripts/publish-bundle-size.test.cjs create mode 100644 .github/workflows/bundle-size-budgets.yml diff --git a/.ci/bundle-size-budgets.json b/.ci/bundle-size-budgets.json new file mode 100644 index 000000000..ff58ed745 --- /dev/null +++ b/.ci/bundle-size-budgets.json @@ -0,0 +1,8 @@ +{ + "web": { + "javascript": { + "softKiB": 35, + "hardKiB": 50 + } + } +} diff --git a/.ci/bundle-size-budgets.md b/.ci/bundle-size-budgets.md new file mode 100644 index 000000000..5eea5bf87 --- /dev/null +++ b/.ci/bundle-size-budgets.md @@ -0,0 +1,81 @@ +# Bundle size budgets + +`.ci/bundle-size-budgets.json` assigns soft and hard limits to individual platform +metrics. Values are KiB (1,024 bytes), including fractions. Comparisons use exact +bytes, not the rounded numbers displayed in PR reports. + +Web starts with a 35 KiB soft limit and 50 KiB hard limit on shipped JavaScript. +The report also includes deterministic gzip size and package sizes. These remain +informational until a budget is configured for their metric. + +## Accepting an increase + +The **Size budgets** check combines all configured metrics for affected platforms: + +- At or below the soft limit: pass. +- Already above a limit on the PR base, and unchanged or smaller: pass. +- Growing above the soft limit, but at or below the hard limit: require acceptance. +- Growing above the hard limit: require a reviewed budget-file change. +- Missing a configured head measurement: fail. Missing base measurements do not + exempt an increase from its limits. + +Once the current revision's report is posted, a repository writer (including the +PR author) can add a new PR comment: + +```text +/accept-size web Required for the new checkout capability. +``` + +The reason is mandatory. The command accepts every currently unaccepted soft-limit +breach for that platform, recording an independent byte cap for each metric, the +actor, and a link to the reason. Multiple commands may share one comment: + +```text +/accept-size web Includes the new checkout capability. +/accept-size android Includes its native implementation. +``` + +Every breached metric must be satisfied before the aggregate check passes. +Acceptance survives subsequent commits when the accepted metric stays the same +size or gets smaller. Further growth or a newly breached metric requires a new +comment. A comment cannot override a hard limit, failed build, or missing data. +It does not modify the configured budget. Edited comments are not processed; post +a new command instead. Editing or deleting an already accepted reason does not +erase the recorded decision in the bot report. + +Commands posted before the current report is ready cannot pre-approve future +measurements. If the PR head or base has changed, rerun **Package Size** and wait +for the updated report. Expired measurement artifacts also require a rerun. + +## Metrics and additional platforms + +| Platform key | Metric key | Measurement | +| --- | --- | --- | +| `web` | `javascript` | Sum of shipped `.js`, `.mjs`, and `.cjs` files in `dist` | +| `web` | `javascriptGzip` | Sum of those files compressed individually with `gzip -n -9` | +| `web` | `npmTarball` | Compressed published package | +| `react-native` | `npmTarball` | Compressed published wrapper package | +| `android` | `aar` | Release AAR | + +Add budgets for existing metrics with the same `{ "softKiB": 35, "hardKiB": 50 }` +shape. Unknown platforms, metric names, units, and invalid limits fail configuration +validation. Adding a new measurement (for example a Swift framework) requires a +reproducible collector in `measure-package-size`, its changed-path detection and +build setup in `package-size.yml`, and an adapter in `bundle-size-budgets.cjs`. +The evaluator, comment commands, and aggregate check need no platform-specific policy. + +## CI integration + +**Package Size** builds the explicit PR head and base SHAs with a read-only token, +including on drafts. It uploads measurements and the informational file breakdown. +**Bundle Size Budgets** runs trusted default-branch code to read that data, check +commenter permissions, and publish the report and **Size budgets** check. It never +executes code from a PR in the job with write permissions. Saved acceptances are +read only from the GitHub Actions bot's report, and concurrent updates are serialized +per PR. Older runs cannot overwrite a newer revision's result. + +The publisher and comment commands become available after this workflow lands on +the default branch. At that point, add **Size budgets** (GitHub Actions) to the +repository's required status checks, alongside **CI Required**. Requiring it before +then would block PRs waiting for a check that cannot yet run. Keep it separate from +the build gate so accepting an increase can update its result without rebuilding. diff --git a/.github/scripts/bundle-size-budgets.cjs b/.github/scripts/bundle-size-budgets.cjs new file mode 100644 index 000000000..fdde4e0b6 --- /dev/null +++ b/.github/scripts/bundle-size-budgets.cjs @@ -0,0 +1,257 @@ +const fs = require("node:fs"); + +// Measurement adapters map the existing artifact report to stable config keys. +// Policy and acceptance handling below do not depend on a particular platform. +const platforms = { + web: { + label: "Web", + metrics: { + javascript: "JavaScript", + javascriptGzip: "JavaScript (gzip)", + npmTarball: "npm tarball", + }, + }, + "react-native": { label: "React Native", metrics: { npmTarball: "npm tarball" } }, + android: { label: "Android", metrics: { aar: "release AAR" } }, +}; +const marker = ""; +const statePattern = //; + +function object(value) { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function validateBudgets(budgets) { + if (!object(budgets)) throw new Error("Budgets must be an object"); + for (const [platform, metrics] of Object.entries(budgets)) { + if (!Object.hasOwn(platforms, platform) || !object(metrics)) + throw new Error(`Unknown platform: ${platform}`); + for (const [metric, budget] of Object.entries(metrics)) { + if (!Object.hasOwn(platforms[platform].metrics, metric)) + throw new Error(`Unknown metric: ${platform}.${metric}`); + if ( + !object(budget) || + Object.keys(budget).sort().join(",") !== "hardKiB,softKiB" || + !Number.isFinite(budget.softKiB) || + !Number.isFinite(budget.hardKiB) || + budget.softKiB <= 0 || + budget.hardKiB < budget.softKiB || + budget.hardKiB * 1024 > Number.MAX_SAFE_INTEGER + ) { + throw new Error(`Invalid budget for ${platform}.${metric}: require 0 < softKiB <= hardKiB`); + } + } + } + return budgets; +} + +function measurements(tsv) { + const values = {}; + for (const line of tsv.split("\n").filter(Boolean)) { + const columns = line.split("\t"); + if (columns.length >= 4) continue; // Per-file details are informational. + const [platform, metric, bytes] = columns; + if (columns.length !== 3 || !/^\d+$/.test(bytes) || !Number.isSafeInteger(Number(bytes))) { + throw new Error("Invalid measurement row"); + } + const key = `${platform}\t${metric}`; + if (Object.hasOwn(values, key)) throw new Error(`Duplicate measurement: ${key}`); + values[key] = Number(bytes); + } + return values; +} + +function evaluate({ budgets, base, head, measuredPlatforms }, acceptances = {}) { + validateBudgets(budgets); + if ( + !Array.isArray(measuredPlatforms) || + measuredPlatforms.some((p) => !Object.hasOwn(platforms, p)) + ) { + throw new Error("Invalid measured platforms"); + } + const rows = []; + for (const [platform, metrics] of Object.entries(budgets)) { + if (!measuredPlatforms.includes(platform)) continue; + for (const [metric, budget] of Object.entries(metrics)) { + const key = `${platform}.${metric}`; + const measurementKey = `${platforms[platform].label}\t${platforms[platform].metrics[metric]}`; + const before = base[measurementKey]; + const after = head[measurementKey]; + const acceptance = acceptances[key]; + let status; + if (!Number.isSafeInteger(after) || after <= 0) status = "missing"; + else if (after <= budget.softKiB * 1024) status = "within"; + else if (before !== undefined && after <= before) status = "no-growth"; + else if (after > budget.hardKiB * 1024) status = "hard"; + else if (acceptance && after <= acceptance.bytes) status = "accepted"; + else status = "soft"; + rows.push({ key, platform, metric, before, after, ...budget, status, acceptance }); + } + } + return rows; +} + +function parseCommands(body) { + const commands = []; + for (const line of body.split(/\r?\n/)) { + if (!line.startsWith("/accept-size")) continue; + const match = /^\/accept-size ([a-z][a-z-]*) (\S.*)$/.exec(line); + if (!match || !Object.hasOwn(platforms, match[1]) || match[2].trim().length > 1000) { + throw new Error("Use /accept-size (reason: 1–1000 characters)"); + } + commands.push({ platform: match[1], reason: match[2].trim() }); + } + return commands; +} + +function accept(rows, commands, actor, comment, previous = {}) { + const accepted = { ...previous }; + const notes = []; + for (const { platform, reason } of commands) { + const eligible = rows.filter((row) => row.platform === platform && row.status === "soft"); + for (const row of eligible) { + accepted[row.key] = { + bytes: row.after, + actor, + reason, + commentId: comment.id, + url: comment.html_url, + }; + } + const hard = rows.some((row) => row.platform === platform && row.status === "hard"); + notes.push( + hard + ? `${platform}: hard budget exceeded; edit the budget file for review.` + : eligible.length + ? `${platform}: accepted ${eligible.length} exceeded metric(s).` + : `${platform}: no soft-budget breaches to accept.`, + ); + } + return { accepted, notes }; +} + +function readState(body) { + const match = body?.match(statePattern); + if (!match) return null; + const state = JSON.parse(Buffer.from(match[1], "base64").toString("utf8")); + if ( + state.version !== 1 || + !object(state.acceptances) || + !Array.isArray(state.processedComments) + ) { + throw new Error("Invalid saved budget report"); + } + return state; +} + +function escape(value) { + return String(value) + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/\|/g, "|") + .replace(/\r?\n/g, " ") + .replace(/([\\`*_[\]])/g, "\\$1"); +} + +function kib(bytes) { + return bytes === undefined ? "unavailable" : `${(bytes / 1024).toFixed(2)} KiB`; +} + +function render(rows, state, packageComment, notes = []) { + const labels = { + within: "Within budget", + "no-growth": "Above budget; no increase", + soft: "Acceptance required", + hard: "Budget change required", + missing: "Measurement missing", + }; + const lines = [ + marker, + "## Size budgets", + "", + `Measured head: \`${state.headSha}\`; base: \`${state.baseSha}\`.`, + "", + "| Platform / metric | Base | Head | Delta | Soft | Hard | Result |", + "| --- | ---: | ---: | ---: | ---: | ---: | --- |", + ]; + for (const row of rows) { + const status = + row.status === "accepted" + ? `Accepted by @${escape(row.acceptance.actor)}: ${escape(row.acceptance.reason)} ([comment](${row.acceptance.url}))` + : labels[row.status]; + const delta = + row.before === undefined || row.after === undefined + ? "unavailable" + : `${row.after > row.before ? "+" : ""}${kib(row.after - row.before)}`; + lines.push( + `| ${row.platform} / ${row.metric} | ${kib(row.before)} | ${kib(row.after)} | ${delta} | ${row.softKiB} KiB | ${row.hardKiB} KiB | ${status} |`, + ); + } + if (!rows.length) lines.push("| — | — | — | — | — | — | No configured budgets affected |"); + lines.push( + "", + "Repository writers, including the PR author, can accept current soft-budget breaches with a reason:", + "", + "```text", + "/accept-size web Explain why this increase is necessary.", + "```", + "", + "Use one command per platform; several lines can share a comment. Post after this report is ready for the current head. Commands in edited comments are not accepted.", + "", + "Acceptance applies to each currently exceeded metric up to its recorded size. Further growth or a newly exceeded metric needs fresh acceptance. Hard-budget increases require a reviewed change to `.ci/bundle-size-budgets.json`.", + "", + ); + for (const note of notes) lines.push(`- ${escape(note)}`); + lines.push( + "", + packageComment.replace(//g, ""), + "", + ``, + ); + const body = lines.join("\n"); + if (body.length > 65000) throw new Error("Size report exceeds GitHub comment limit"); + return body; +} + +function conclusion(rows) { + return rows.some((row) => ["soft", "hard", "missing"].includes(row.status)) + ? "failure" + : "success"; +} + +module.exports = { + platforms, + marker, + validateBudgets, + measurements, + evaluate, + parseCommands, + accept, + readState, + render, + conclusion, +}; + +if (require.main === module) { + const [basePath, headPath, budgetsPath, outputPath] = process.argv.slice(2); + const event = JSON.parse(fs.readFileSync(process.env.GITHUB_EVENT_PATH, "utf8")); + const report = { + version: 1, + pr: event.pull_request.number, + headSha: event.pull_request.head.sha, + baseSha: process.env.BASE_SHA || event.pull_request.base.sha, + budgets: validateBudgets(JSON.parse(fs.readFileSync(budgetsPath, "utf8"))), + base: measurements(fs.readFileSync(basePath, "utf8")), + head: measurements(fs.readFileSync(headPath, "utf8")), + measuredPlatforms: Object.entries({ + web: process.env.MEASURE_WEB, + "react-native": process.env.MEASURE_REACT_NATIVE, + android: process.env.MEASURE_ANDROID, + }) + .filter(([, enabled]) => enabled === "true") + .map(([platform]) => platform), + }; + evaluate(report); // Reject malformed configuration before publishing an artifact. + fs.writeFileSync(outputPath, JSON.stringify(report)); +} diff --git a/.github/scripts/bundle-size-budgets.test.cjs b/.github/scripts/bundle-size-budgets.test.cjs new file mode 100644 index 000000000..f9cc572c6 --- /dev/null +++ b/.github/scripts/bundle-size-budgets.test.cjs @@ -0,0 +1,180 @@ +const { test } = require("node:test"); +const assert = require("node:assert/strict"); +const policy = require("./bundle-size-budgets.cjs"); + +const KiB = 1024; +const report = (size, base = 34 * KiB) => ({ + budgets: { web: { javascript: { softKiB: 35, hardKiB: 50 } } }, + base: base === undefined ? {} : { "Web\tJavaScript": base }, + head: { "Web\tJavaScript": size }, + measuredPlatforms: ["web"], +}); +const comment = { id: 42, html_url: "https://github.com/example/repo/pull/1#issuecomment-42" }; + +test("compares exact bytes at both limits, including fractional KiB", () => { + for (const [bytes, expected] of [ + [35 * KiB, "within"], + [35 * KiB + 1, "soft"], + [50 * KiB, "soft"], + [50 * KiB + 1, "hard"], + ]) { + assert.equal(policy.evaluate(report(bytes))[0].status, expected); + } + const input = report(35.5 * KiB); + input.budgets.web.javascript.softKiB = 35.5; + assert.equal(policy.evaluate(input)[0].status, "within"); + input.head["Web\tJavaScript"]++; + assert.equal(policy.evaluate(input)[0].status, "soft"); +}); + +test("unchanged or reduced artifacts above either limit pass", () => { + for (const size of [40 * KiB, 60 * KiB]) { + assert.equal(policy.evaluate(report(size, size))[0].status, "no-growth"); + assert.equal(policy.evaluate(report(size - 1, size))[0].status, "no-growth"); + } + assert.equal(policy.evaluate(report(60 * KiB + 1, 60 * KiB))[0].status, "hard"); +}); + +test("missing or zero head measurements fail; missing base is not an exemption", () => { + assert.equal(policy.evaluate(report(undefined))[0].status, "missing"); + assert.equal(policy.evaluate(report(0))[0].status, "missing"); + const input = report(40 * KiB); + input.base = {}; + assert.equal(policy.evaluate(input)[0].status, "soft"); + input.head["Web\tJavaScript"] = 51 * KiB; + assert.equal(policy.evaluate(input)[0].status, "hard"); +}); + +test("only affected platforms are gated; a docs-only report passes", () => { + const input = report(undefined); + input.measuredPlatforms = []; + assert.deepEqual(policy.evaluate(input), []); + assert.equal(policy.conclusion([]), "success"); +}); + +test("one platform command accepts each current soft breach, leaving other platforms unresolved", () => { + const input = report(40 * KiB); + input.budgets.web.javascriptGzip = { softKiB: 10, hardKiB: 15 }; + input.head["Web\tJavaScript (gzip)"] = 11 * KiB; + input.budgets.android = { aar: { softKiB: 100, hardKiB: 200 } }; + input.head["Android\trelease AAR"] = 150 * KiB; + input.measuredPlatforms.push("android"); + const web = policy.accept( + policy.evaluate(input), + [{ platform: "web", reason: "New capability" }], + "writer", + comment, + ); + assert.equal(Object.keys(web.accepted).length, 2); + assert.deepEqual( + policy.evaluate(input, web.accepted).map((row) => row.status), + ["accepted", "accepted", "soft"], + ); + assert.equal(policy.conclusion(policy.evaluate(input, web.accepted)), "failure"); + const all = policy.accept( + policy.evaluate(input, web.accepted), + [{ platform: "android", reason: "Native support" }], + "writer", + comment, + web.accepted, + ); + assert.equal(policy.conclusion(policy.evaluate(input, all.accepted)), "success"); +}); + +test("acceptance caps survive unchanged or smaller sizes; further growth needs acceptance", () => { + const input = report(40 * KiB); + const { accepted } = policy.accept( + policy.evaluate(input), + [{ platform: "web", reason: "New capability" }], + "writer", + comment, + ); + assert.equal(accepted["web.javascript"].bytes, 40 * KiB); + assert.equal(accepted["web.javascript"].reason, "New capability"); + for (const [size, expected] of [ + [40 * KiB, "accepted"], + [39 * KiB, "accepted"], + [40 * KiB + 1, "soft"], + ]) { + assert.equal(policy.evaluate(report(size), accepted)[0].status, expected); + } + input.budgets.web.javascriptGzip = { softKiB: 10, hardKiB: 15 }; + input.head["Web\tJavaScript (gzip)"] = 11 * KiB; + assert.equal(policy.evaluate(input, accepted)[1].status, "soft"); +}); + +test("a comment or a prior acceptance cannot override a hard limit", () => { + const input = report(51 * KiB); + const result = policy.accept( + policy.evaluate(input), + [{ platform: "web", reason: "Please override" }], + "writer", + comment, + ); + assert.deepEqual(result.accepted, {}); + assert.match(result.notes[0], /hard budget/); + assert.equal(policy.evaluate(input, { "web.javascript": { bytes: 60 * KiB } })[0].status, "hard"); +}); + +test("rejects unknown keys and malformed limits", () => { + for (const budgets of [ + null, + [], + { ios: {} }, + { web: { typo: { softKiB: 1, hardKiB: 2 } } }, + { web: { javascript: { soft: 35, hard: 50 } } }, + { web: { javascript: { softKiB: "35", hardKiB: 50 } } }, + { web: { javascript: { softKiB: 51, hardKiB: 50 } } }, + { web: { javascript: { softKiB: 0, hardKiB: 50 } } }, + { web: { javascript: { softKiB: 35, hardKiB: Infinity } } }, + { toString: {} }, + ]) { + assert.throws(() => policy.validateBudgets(budgets)); + } +}); + +test("parses multiple commands with mandatory reasons, ignoring ordinary quoted prose", () => { + assert.deepEqual( + policy.parseCommands("/accept-size web New capability\n/accept-size android Native support"), + [ + { platform: "web", reason: "New capability" }, + { platform: "android", reason: "Native support" }, + ], + ); + assert.deepEqual( + policy.parseCommands("I could use /accept-size web a reason\n> /accept-size web quoted"), + [], + ); + for (const body of ["/accept-size web", "/accept-size web ", "/accept-size unknown A reason"]) { + assert.throws(() => policy.parseCommands(body)); + } +}); + +test("summary parser ignores file rows and rejects duplicate or malformed measurements", () => { + assert.deepEqual( + policy.measurements("Web\tJavaScript\t34073\nWeb\tnpm tarball\t42000\tdist/index.js\n"), + { "Web\tJavaScript": 34073 }, + ); + for (const text of [ + "Web\tJavaScript\t-1", + "Web\tJavaScript\t1.5", + "Web\tJavaScript\t12\nWeb\tJavaScript\t13", + ]) { + assert.throws(() => policy.measurements(text)); + } +}); + +test("report round-trips trusted acceptance data without artifact marker injection", () => { + const state = { + version: 1, + headSha: "abc", + baseSha: "def", + acceptances: {}, + processedComments: [], + }; + const injected = ""; + const body = policy.render(policy.evaluate(report(40 * KiB)), state, injected); + assert.deepEqual(policy.readState(body), state); + assert.match(body, /Acceptance required/); + assert.match(body, /\+6.00 KiB/); +}); diff --git a/.github/scripts/measure-bundle-size.test.cjs b/.github/scripts/measure-bundle-size.test.cjs new file mode 100644 index 000000000..a4679c221 --- /dev/null +++ b/.github/scripts/measure-bundle-size.test.cjs @@ -0,0 +1,69 @@ +const { test } = require("node:test"); +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); +const { execFileSync } = require("node:child_process"); +const { measurements } = require("./bundle-size-budgets.cjs"); + +test("collector measures all shipped JS chunks, excludes maps/types, and ignores timestamps in gzip", () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), "bundle-collector-test-")); + try { + const dist = path.join(directory, "package/dist"); + const bin = path.join(directory, "bin"); + fs.mkdirSync(dist, { recursive: true }); + fs.mkdirSync(bin); + fs.mkdirSync(path.join(directory, "platforms/web"), { recursive: true }); + const files = { + "index.js": "export const answer = 42;\n", + "chunk.mjs": 'export default "chunk";\n', + "legacy.cjs": "module.exports = 42;\n", + "index.d.ts": "export declare const answer: number;", + "index.js.map": '{"sources":[]}', + }; + for (const [name, content] of Object.entries(files)) + fs.writeFileSync(path.join(dist, name), content); + const fakePnpm = path.join(bin, "pnpm"); + fs.writeFileSync( + fakePnpm, + '#!/usr/bin/env bash\nset -euo pipefail\nif [[ "$1" == pack ]]; then\n tar -czf "$3/package.tgz" -C "$PACKAGE_SIZE_REPO_ROOT" package\nfi\n', + ); + fs.chmodSync(fakePnpm, 0o755); + const env = { + ...process.env, + PATH: `${bin}:${process.env.PATH}`, + TMPDIR: directory, + PACKAGE_SIZE_REPO_ROOT: directory, + MEASURE_WEB: "true", + MEASURE_REACT_NATIVE: "false", + MEASURE_ANDROID: "false", + }; + const output = path.join(directory, "sizes.tsv"); + const collect = () => { + execFileSync("bash", [path.join(__dirname, "measure-package-size"), "collect", output], { + env, + }); + return measurements(fs.readFileSync(output, "utf8")); + }; + const first = collect(); + const scripts = ["index.js", "chunk.mjs", "legacy.cjs"]; + assert.equal( + first["Web\tJavaScript"], + scripts.reduce((sum, file) => sum + Buffer.byteLength(files[file]), 0), + ); + assert.equal( + first["Web\tJavaScript (gzip)"], + scripts.reduce( + (sum, file) => sum + execFileSync("gzip", ["-n", "-9", "-c", path.join(dist, file)]).length, + 0, + ), + ); + for (const name of scripts) fs.utimesSync(path.join(dist, name), 1234567890, 1234567890); + const second = collect(); + assert.equal(second["Web\tJavaScript (gzip)"], first["Web\tJavaScript (gzip)"]); + for (const name of scripts) fs.unlinkSync(path.join(dist, name)); + assert.throws(collect, /No shipped web JavaScript found/); + } finally { + fs.rmSync(directory, { recursive: true, force: true }); + } +}); diff --git a/.github/scripts/measure-package-size b/.github/scripts/measure-package-size index 99123ef29..d13ab5915 100755 --- a/.github/scripts/measure-package-size +++ b/.github/scripts/measure-package-size @@ -99,6 +99,28 @@ measure_web_package() { ) tarball=$(find "$pack_dir" -name "*.tgz" -type f -print -quit) + # Measure every shipped JS chunk, excluding declarations and source maps. + # Sum gzip sizes per file, matching separately compressed HTTP responses. + local js_dir + local js_file + local javascript_bytes=0 + local gzip_bytes=0 + local js_count=0 + js_dir=$(mktemp -d "${TMPDIR:-/tmp}/checkout-kit-web-js.XXXXXX") + tar -xzf "$tarball" -C "$js_dir" + while IFS= read -r js_file; do + javascript_bytes=$((javascript_bytes + $(size_bytes "$js_file"))) + gzip_bytes=$((gzip_bytes + $(gzip -n -9 -c "$js_file" | wc -c))) + js_count=$((js_count + 1)) + done < <(find "$js_dir/package/dist" -type f \( -name '*.js' -o -name '*.mjs' -o -name '*.cjs' \) | sort) + if [[ "$js_count" -eq 0 ]]; then + echo "No shipped web JavaScript found" >&2 + rm -rf "$js_dir" + return 1 + fi + printf "Web\tJavaScript\t%s\n" "$javascript_bytes" >> "$output_file" + printf "Web\tJavaScript (gzip)\t%s\n" "$gzip_bytes" >> "$output_file" + rm -rf "$js_dir" record_artifact "Web" "npm tarball" "$tarball" record_tarball_breakdown "Web" "npm tarball" "$tarball" } @@ -333,7 +355,7 @@ render_comment() { } print ""; - print "_Measured from the PR base SHA and PR head SHA. The file breakdown shows uncompressed sizes within each package artifact, so individual files do not sum to the compressed artifact total. This comment reports package artifact sizes only; it is not a final app binary-size report._"; + print "_Measured from the PR base SHA and PR head SHA. The file breakdown shows uncompressed sizes within each package artifact, so individual files do not sum to the compressed artifact total. JavaScript rows sum shipped runtime files; gzip uses deterministic per-file compression. Package artifacts are not final app binary sizes._"; } ' "$base_file" "$head_file" > "$comment_file" } diff --git a/.github/scripts/publish-bundle-size.cjs b/.github/scripts/publish-bundle-size.cjs new file mode 100644 index 000000000..647995d13 --- /dev/null +++ b/.github/scripts/publish-bundle-size.cjs @@ -0,0 +1,279 @@ +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); +const { execFileSync } = require("node:child_process"); +const policy = require("./bundle-size-budgets.cjs"); + +const checkName = "Size budgets"; +const workflow = "package-size.yml"; +const isReport = (comment) => + comment.user?.login === "github-actions[bot]" && + comment.user.type === "Bot" && + comment.body?.startsWith(policy.marker); + +async function resolvePR({ github, context }) { + if (context.eventName === "issue_comment") + return context.payload.issue.pull_request ? context.payload.issue.number : null; + const run = context.payload.workflow_run; + if (run.event !== "pull_request" || run.path !== `.github/workflows/${workflow}`) return null; + // workflow_run.pull_requests can be empty for a fork PR. + const prs = run.pull_requests.length + ? run.pull_requests + : await github.paginate(github.rest.repos.listPullRequestsAssociatedWithCommit, { + ...context.repo, + commit_sha: run.head_sha, + per_page: 100, + }); + const pr = prs.find( + (pr) => pr.head.sha === run.head_sha && pr.head.repo?.id === run.head_repository?.id, + ); + return pr?.number ?? null; +} + +async function readArtifact(github, repo, runId) { + const artifacts = await github.paginate(github.rest.actions.listWorkflowRunArtifacts, { + ...repo, + run_id: runId, + per_page: 100, + }); + const artifact = artifacts.find((item) => item.name === "bundle-size-report" && !item.expired); + if (!artifact || artifact.size_in_bytes > 5 * 1024 * 1024) + throw new Error("Missing or oversized measurement artifact; rerun Package Size."); + const { data } = await github.rest.actions.downloadArtifact({ + ...repo, + artifact_id: artifact.id, + archive_format: "zip", + }); + const directory = fs.mkdtempSync(path.join(os.tmpdir(), "bundle-size-report-")); + try { + const archive = path.join(directory, "report.zip"); + fs.writeFileSync(archive, Buffer.from(data)); + // Read only known text entries. Never extract paths or execute PR-provided code. + const read = (name) => + execFileSync("unzip", ["-p", archive, name], { encoding: "utf8", maxBuffer: 1024 * 1024 }); + return { report: JSON.parse(read("report.json")), packageComment: read("comment.md") }; + } finally { + fs.rmSync(directory, { recursive: true, force: true }); + } +} + +function validateReport(report, pr, run) { + if ( + report.version !== 1 || + report.pr !== pr.number || + report.headSha !== pr.head.sha || + report.baseSha !== pr.base.sha || + run.head_sha !== pr.head.sha + ) { + throw new Error("Measurements do not match the current PR head and base; rerun Package Size."); + } + for (const values of [report.base, report.head]) { + if ( + !values || + typeof values !== "object" || + Array.isArray(values) || + Object.values(values).some((value) => !Number.isSafeInteger(value) || value < 0) + ) { + throw new Error("Invalid size measurements"); + } + } + policy.evaluate(report); +} + +async function latestRun(github, repo, pr) { + const runs = await github.paginate(github.rest.actions.listWorkflowRuns, { + ...repo, + workflow_id: workflow, + event: "pull_request", + head_sha: pr.head.sha, + per_page: 100, + }); + return runs + .filter((run) => run.head_repository?.id === pr.head.repo.id && run.head_branch === pr.head.ref) + .sort((a, b) => b.id - a.id)[0]; +} + +async function updateCheck(github, repo, pr, status, summary, detailsUrl) { + const { data: current } = await github.rest.pulls.get({ ...repo, pull_number: pr.number }); + if ( + current.head.sha !== pr.head.sha || + current.base.sha !== pr.base.sha || + current.state !== "open" + ) + return; + const checks = await github.paginate(github.rest.checks.listForRef, { + ...repo, + ref: pr.head.sha, + check_name: checkName, + per_page: 100, + }); + const externalId = `bundle-size-budgets:${pr.number}`; + const existing = checks.find( + (check) => check.external_id === externalId && check.app?.slug === "github-actions", + ); + const params = { + ...repo, + name: checkName, + external_id: externalId, + status: status === "pending" ? "in_progress" : "completed", + output: { + title: + status === "success" + ? "All size budgets satisfied or accepted" + : status === "pending" + ? "Waiting for size measurements" + : "Size budgets need attention", + summary, + }, + details_url: detailsUrl, + }; + if (status !== "pending") { + params.conclusion = status; + params.completed_at = new Date().toISOString(); + } + if (existing) await github.rest.checks.update({ ...params, check_run_id: existing.id }); + else await github.rest.checks.create({ ...params, head_sha: pr.head.sha }); +} + +async function processCommands({ github, repo, comments, state, report, mayAccept }) { + let acceptances = { ...state.acceptances }; + const processed = new Set(state.processedComments); + const notes = []; + const permissions = new Map(); + for (const comment of comments) { + if ( + processed.has(comment.id) || + !comment.body?.split(/\r?\n/).some((line) => line.startsWith("/accept-size")) + ) + continue; + processed.add(comment.id); + if (!mayAccept || comment.created_at < state.publishedAt) { + notes.push( + `Comment ${comment.id}: wait for the current size report, then post a new command.`, + ); + continue; + } + if (comment.created_at !== comment.updated_at) { + notes.push(`Comment ${comment.id}: post a new command; edited comments are not accepted.`); + continue; + } + const actor = comment.user.login; + if (comment.user.type !== "User") continue; + if (!permissions.has(actor)) { + const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ + ...repo, + username: actor, + }); + permissions.set( + actor, + ["admin", "maintain", "write"].includes(data.permission) || + data.user?.permissions?.push === true, + ); + } + if (!permissions.get(actor)) { + notes.push(`Comment ${comment.id}: repository write access is required.`); + continue; + } + let commands; + try { + commands = policy.parseCommands(comment.body); + } catch (error) { + notes.push(`Comment ${comment.id}: ${error.message}`); + continue; + } + const result = policy.accept( + policy.evaluate(report, acceptances), + commands, + actor, + comment, + acceptances, + ); + acceptances = result.accepted; + notes.push(...result.notes); + } + return { acceptances, processedComments: [...processed], notes: notes.slice(-10) }; +} + +async function publish({ github, context, core, prNumber, loadArtifact = readArtifact }) { + const repo = context.repo; + const { data: pr } = await github.rest.pulls.get({ ...repo, pull_number: prNumber }); + if (pr.state !== "open") return; + const run = await latestRun(github, repo, pr); + if (!run) return; // A comment cannot create an approval before measurements exist. + if (context.eventName === "workflow_run" && context.payload.workflow_run.id !== run.id) return; + try { + if (run.status !== "completed") { + await updateCheck( + github, + repo, + pr, + "pending", + "Package Size is measuring this revision. Wait for its report before accepting an increase.", + run.html_url, + ); + return; + } + if (run.conclusion !== "success") + throw new Error( + "Package Size did not succeed. Fix or rerun the measurement workflow; a comment cannot bypass build failures.", + ); + const { report, packageComment } = await loadArtifact(github, repo, run.id); + validateReport(report, pr, run); + const comments = await github.paginate(github.rest.issues.listComments, { + ...repo, + issue_number: pr.number, + per_page: 100, + }); + const existing = comments.find(isReport); + const previous = existing ? policy.readState(existing.body) : null; + // A command only accepts sizes already shown for this exact measurement run. + // Saved per-metric caps survive subsequent runs and unrelated commits. + const mayAccept = + previous?.headSha === report.headSha && + previous?.baseSha === report.baseSha && + previous?.runId === run.id && + previous?.runAttempt === run.run_attempt; + const state = previous ?? { + version: 1, + acceptances: {}, + processedComments: [], + publishedAt: new Date().toISOString(), + }; + const processed = await processCommands({ github, repo, comments, state, report, mayAccept }); + const next = { + ...state, + ...processed, + notes: undefined, + headSha: report.headSha, + baseSha: report.baseSha, + runId: run.id, + runAttempt: run.run_attempt, + publishedAt: mayAccept ? state.publishedAt : new Date().toISOString(), + }; + const rows = policy.evaluate(report, next.acceptances); + const body = policy.render(rows, next, packageComment, processed.notes); + const { data: current } = await github.rest.pulls.get({ ...repo, pull_number: pr.number }); + if ( + current.head.sha !== pr.head.sha || + current.base.sha !== pr.base.sha || + current.state !== "open" + ) + return; + const { data: comment } = existing + ? await github.rest.issues.updateComment({ ...repo, comment_id: existing.id, body }) + : await github.rest.issues.createComment({ ...repo, issue_number: pr.number, body }); + await updateCheck( + github, + repo, + pr, + policy.conclusion(rows), + body.replace(//gs, "").slice(0, 60000), + comment.html_url, + ); + } catch (error) { + await updateCheck(github, repo, pr, "failure", error.message, run.html_url); + core.setFailed(error.message); + } +} + +module.exports = { resolvePR, publish, processCommands, validateReport, isReport }; diff --git a/.github/scripts/publish-bundle-size.test.cjs b/.github/scripts/publish-bundle-size.test.cjs new file mode 100644 index 000000000..ce3ff8569 --- /dev/null +++ b/.github/scripts/publish-bundle-size.test.cjs @@ -0,0 +1,277 @@ +const { test } = require("node:test"); +const assert = require("node:assert/strict"); +const policy = require("./bundle-size-budgets.cjs"); +const publisher = require("./publish-bundle-size.cjs"); + +function fixture() { + const pr = { + number: 1, + state: "open", + head: { sha: "a".repeat(40), ref: "feature", repo: { id: 123 } }, + base: { sha: "b".repeat(40) }, + }; + const run = { + id: 10, + run_attempt: 1, + event: "pull_request", + path: ".github/workflows/package-size.yml", + head_sha: pr.head.sha, + head_branch: "feature", + head_repository: { id: 123 }, + pull_requests: [pr], + status: "completed", + conclusion: "success", + html_url: "https://github.com/example/repo/actions/runs/10", + }; + const report = { + version: 1, + pr: 1, + headSha: pr.head.sha, + baseSha: pr.base.sha, + budgets: { web: { javascript: { softKiB: 35, hardKiB: 50 } } }, + base: { "Web\tJavaScript": 34000 }, + head: { "Web\tJavaScript": 40000 }, + measuredPlatforms: ["web"], + }; + const comments = []; + const checks = []; + const errors = []; + const permission = { writer: "write", reader: "read" }; + let pullReads = 0; + const rest = { + pulls: { + get: async () => { + pullReads++; + return { data: pr }; + }, + }, + actions: { listWorkflowRuns: "runs" }, + repos: { + listPullRequestsAssociatedWithCommit: "prs", + getCollaboratorPermissionLevel: async ({ username }) => ({ + data: { permission: permission[username] ?? "none" }, + }), + }, + issues: { + listComments: "comments", + createComment: async ({ body }) => { + const item = { + id: 100, + body, + user: { login: "github-actions[bot]", type: "Bot" }, + html_url: "https://github.com/example/repo/pull/1#issuecomment-100", + }; + comments.push(item); + return { data: item }; + }, + updateComment: async ({ comment_id, body }) => { + const item = comments.find((c) => c.id === comment_id); + item.body = body; + return { data: item }; + }, + }, + checks: { + listForRef: "checks", + create: async (params) => { + checks.push({ ...params, id: 200 + checks.length, app: { slug: "github-actions" } }); + }, + update: async (params) => { + Object.assign( + checks.find((c) => c.id === params.check_run_id), + params, + ); + }, + }, + }; + const github = { + rest, + paginate: async (route, params) => + route === "checks" + ? checks.filter((check) => check.head_sha === params.ref) + : { runs: [run], prs: [pr], comments }[route], + }; + const context = { + repo: { owner: "example", repo: "repo" }, + eventName: "workflow_run", + payload: { workflow_run: run }, + }; + const args = { + github, + context, + core: { setFailed: (message) => errors.push(message) }, + prNumber: 1, + loadArtifact: async () => ({ + report, + packageComment: policy.marker + "\nPackage measurements", + }), + }; + const command = (body, actor = "writer", overrides = {}) => { + const date = new Date(Date.now() + 2000).toISOString(); + const item = { + id: comments.length + 101, + body, + user: { login: actor, type: "User" }, + created_at: date, + updated_at: date, + html_url: "https://github.com/example/repo/pull/1#issuecomment-101", + ...overrides, + }; + comments.push(item); + context.eventName = "issue_comment"; + context.payload = { issue: { number: 1, pull_request: {} }, comment: item }; + return item; + }; + return { + pr, + run, + report, + comments, + checks, + errors, + permission, + github, + context, + args, + command, + reads: () => pullReads, + }; +} + +test("publishes a failing gate, accepts a writer reason, then reopens it after further growth", async () => { + const f = fixture(); + await publisher.publish(f.args); + assert.equal(f.checks.at(-1).conclusion, "failure"); + f.command("/accept-size web New checkout capability"); + await publisher.publish(f.args); + assert.equal(f.checks.length, 1); + assert.equal(f.checks.at(-1).conclusion, "success"); + assert.match(f.comments[0].body, /Accepted by @writer: New checkout capability/); + f.pr.head.sha = f.report.headSha = f.run.head_sha = "c".repeat(40); + f.run.id++; + f.context.eventName = "workflow_run"; + f.context.payload = { workflow_run: f.run }; + await publisher.publish(f.args); + assert.equal(f.checks.at(-1).conclusion, "success"); + f.report.head["Web\tJavaScript"]++; + f.run.id++; + await publisher.publish(f.args); + assert.equal(f.checks.at(-1).conclusion, "failure"); + assert.deepEqual(f.errors, []); +}); + +test("readers, malformed commands and edited comments cannot accept an increase", async () => { + for (const [body, actor, overrides, expected] of [ + ["/accept-size web Please accept", "reader", {}, /write access/], + ["/accept-size web", "writer", {}, /reason/], + [ + "/accept-size web Changed reason", + "writer", + { updated_at: "2099-01-01T00:00:00Z" }, + /edited comments/, + ], + ]) { + const f = fixture(); + await publisher.publish(f.args); + f.command(body, actor, overrides); + await publisher.publish(f.args); + assert.equal(f.checks.at(-1).conclusion, "failure"); + assert.match(f.comments[0].body, expected); + assert.deepEqual(policy.readState(f.comments[0].body).acceptances, {}); + } +}); + +test("commands cannot pre-approve unreported sizes or new measurements", async () => { + const f = fixture(); + f.command("/accept-size web Accept whatever it becomes"); + await publisher.publish(f.args); + assert.equal(f.checks.at(-1).conclusion, "failure"); + f.command("/accept-size web Report was for an older commit"); + f.pr.head.sha = f.report.headSha = f.run.head_sha = "c".repeat(40); + f.run.id++; + await publisher.publish(f.args); + assert.equal(f.checks.at(-1).conclusion, "failure"); + assert.deepEqual(policy.readState(f.comments.find(publisher.isReport).body).acceptances, {}); +}); + +test("coalesced comment events still process every pending platform command", async () => { + const f = fixture(); + f.report.budgets.android = { aar: { softKiB: 100, hardKiB: 200 } }; + f.report.head["Android\trelease AAR"] = 150 * 1024; + f.report.measuredPlatforms.push("android"); + await publisher.publish(f.args); + f.command("/accept-size web New checkout capability"); + f.command("/accept-size android Native support"); + await publisher.publish(f.args); + assert.equal(f.checks.at(-1).conclusion, "success"); + assert.equal(Object.keys(policy.readState(f.comments[0].body).acceptances).length, 2); +}); + +test("a forged report from a human cannot supply approvals", async () => { + const f = fixture(); + f.comments.push({ + id: 2, + user: { login: "writer", type: "User" }, + body: policy.render( + [], + { + version: 1, + acceptances: { "web.javascript": { bytes: 999999 } }, + processedComments: [], + }, + "", + ), + }); + await publisher.publish(f.args); + assert.equal(f.checks.at(-1).conclusion, "failure"); + assert.equal(f.comments.length, 2); +}); + +test("hard budgets, missing data, failed builds and expired artifacts fail closed", async () => { + for (const scenario of ["hard", "missing", "failure", "artifact"]) { + const f = fixture(); + if (scenario === "hard") f.report.head["Web\tJavaScript"] = 60000; + if (scenario === "missing") delete f.report.head["Web\tJavaScript"]; + if (scenario === "failure") f.run.conclusion = "failure"; + if (scenario === "artifact") + f.args.loadArtifact = async () => { + throw new Error("Artifact expired"); + }; + await publisher.publish(f.args); + f.command("/accept-size web Please override"); + await publisher.publish(f.args); + assert.equal(f.checks.at(-1).conclusion, "failure", scenario); + } +}); + +test("stale head/base or wrong PR artifacts are rejected", async () => { + for (const field of ["headSha", "baseSha", "pr"]) { + const f = fixture(); + f.report[field] = "wrong"; + await publisher.publish(f.args); + assert.equal(f.comments.length, 0); + assert.equal(f.checks.at(-1).conclusion, "failure"); + assert.match(f.errors[0], /current PR head and base/); + } +}); + +test("pending builds cannot be accepted; stale workflow completions are ignored", async () => { + const f = fixture(); + f.run.status = "in_progress"; + f.command("/accept-size web Please accept"); + await publisher.publish(f.args); + assert.equal(f.checks.at(-1).status, "in_progress"); + assert.equal(f.comments.length, 1); + f.context.eventName = "workflow_run"; + f.context.payload = { workflow_run: { ...f.run, id: 9 } }; + const reads = f.reads(); + await publisher.publish(f.args); + assert.equal(f.reads(), reads + 1); +}); + +test("fork PR resolution works when workflow_run has no pull_requests", async () => { + const f = fixture(); + f.run.pull_requests = []; + assert.equal(await publisher.resolvePR(f.args), 1); + f.run.event = "push"; + assert.equal(await publisher.resolvePR(f.args), null); +}); diff --git a/.github/workflows/bundle-size-budgets.yml b/.github/workflows/bundle-size-budgets.yml new file mode 100644 index 000000000..331267f78 --- /dev/null +++ b/.github/workflows/bundle-size-budgets.yml @@ -0,0 +1,66 @@ +name: Bundle Size Budgets + +on: + workflow_run: + workflows: [Package Size] + types: [requested, completed] + issue_comment: + types: [created] + +permissions: + contents: read + +jobs: + resolve: + if: >- + github.event_name == 'workflow_run' || + (github.event.issue.pull_request && contains(github.event.comment.body, '/accept-size')) + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + pull-requests: read + outputs: + pr: ${{ steps.resolve.outputs.pr }} + steps: + # These events run trusted default-branch code. Never check out the PR here. + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + id: resolve + with: + script: | + const {resolvePR} = require('./.github/scripts/publish-bundle-size.cjs'); + const pr = await resolvePR({github, context}); + if (pr) core.setOutput('pr', pr); + + report: + name: Publish size budgets + needs: resolve + if: needs.resolve.outputs.pr != '' + runs-on: ubuntu-latest + timeout-minutes: 5 + permissions: + contents: read + actions: read + checks: write + pull-requests: write + # Serialize report writes and acceptance comments for this PR. Each run + # processes all pending commands, including events coalesced by concurrency. + concurrency: + group: bundle-size-report-${{ needs.resolve.outputs.pr }} + cancel-in-progress: false + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ github.event.repository.default_branch }} + persist-credentials: false + - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + PR_NUMBER: ${{ needs.resolve.outputs.pr }} + with: + script: | + const {publish} = require('./.github/scripts/publish-bundle-size.cjs'); + await publish({github, context, core, prNumber: Number(process.env.PR_NUMBER)}); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 945f5fe1c..bf0b4664e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -113,6 +113,11 @@ jobs: - '.ci/changed-file-filters.yml' - '.github/workflows/ci.yml' scripts: + - '.github/scripts/measure-package-size' + - '.github/scripts/*bundle-size*' + - '.github/workflows/package-size.yml' + - '.github/workflows/bundle-size-budgets.yml' + - '.ci/bundle-size-budgets.json' - 'platforms/swift/Scripts/api' - 'platforms/swift/Scripts/normalize-api.jq' - 'scripts/lib/**' @@ -245,6 +250,7 @@ jobs: - uses: ruby/setup-ruby@e8944e80fb94b20106697132f8c20c665fab29e9 # v1.325.0 with: ruby-version: .ruby-version + - run: node --test .github/scripts/*bundle-size*.test.cjs - run: ./scripts/test_ruby - run: ./e2e/scripts/check_hide_keyboard_usage diff --git a/.github/workflows/package-size.yml b/.github/workflows/package-size.yml index 4e01b020f..8ef0e869c 100644 --- a/.github/workflows/package-size.yml +++ b/.github/workflows/package-size.yml @@ -2,11 +2,11 @@ name: Package Size on: pull_request: - types: [opened, synchronize, reopened, ready_for_review] + types: [opened, synchronize, reopened, ready_for_review, edited] permissions: contents: read - pull-requests: write + pull-requests: read concurrency: group: package-size-${{ github.ref }} @@ -15,7 +15,6 @@ concurrency: jobs: changes: name: Detect Changed Packages - if: github.event_name == 'pull_request' && github.event.pull_request.draft == false runs-on: ubuntu-latest timeout-minutes: 5 outputs: @@ -37,39 +36,60 @@ jobs: filters: | android: - '.github/scripts/measure-package-size' + - '.github/scripts/*bundle-size*' + - '.ci/bundle-size-budgets.json' + - '.github/workflows/bundle-size-budgets.yml' - '.ci/changed-file-filters.yml' - '.github/workflows/package-size.yml' reactNative: - '.github/actions/setup/**' - '.github/scripts/measure-package-size' + - '.github/scripts/*bundle-size*' + - '.ci/bundle-size-budgets.json' + - '.github/workflows/bundle-size-budgets.yml' - '.ci/changed-file-filters.yml' - '.github/workflows/package-size.yml' web: - '.github/actions/setup/**' - '.github/scripts/measure-package-size' + - '.github/scripts/*bundle-size*' + - '.ci/bundle-size-budgets.json' + - '.github/workflows/bundle-size-budgets.yml' - '.ci/changed-file-filters.yml' - '.github/workflows/package-size.yml' measure: name: Measure Package Size needs: changes - if: | - needs.changes.outputs.android == 'true' || - needs.changes.outputs.reactNative == 'true' || - needs.changes.outputs.web == 'true' runs-on: ubuntu-latest timeout-minutes: 30 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: + ref: ${{ github.event.pull_request.head.sha }} submodules: true + persist-credentials: false + + - name: Check budget policy + run: node --test .github/scripts/*bundle-size*.test.cjs + + - name: Resolve current PR base + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + id: base + with: + script: | + const {data: pr} = await github.rest.pulls.get({ + ...context.repo, pull_number: context.issue.number, + }); + core.setOutput('sha', pr.base.sha); - name: Checkout PR base uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - ref: ${{ github.event.pull_request.base.sha }} + ref: ${{ steps.base.outputs.sha }} path: .package-size-base submodules: true + persist-credentials: false - name: Setup base Web dependencies if: needs.changes.outputs.web == 'true' @@ -136,41 +156,21 @@ jobs: touch /tmp/package-size-base.tsv .github/scripts/measure-package-size render /tmp/package-size-base.tsv /tmp/package-size-head.tsv /tmp/package-size-comment.md - - name: Comment on PR - uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + - name: Prepare budget report env: - COMMENT_FILE: /tmp/package-size-comment.md - PR_NUMBER: ${{ github.event.pull_request.number }} - with: - script: | - const fs = require('fs'); - - const marker = ''; - const body = fs.readFileSync(process.env.COMMENT_FILE, 'utf8'); - const issue_number = Number(process.env.PR_NUMBER); - const {owner, repo} = context.repo; - - const comments = await github.paginate(github.rest.issues.listComments, { - owner, - repo, - issue_number, - per_page: 100, - }); + BASE_SHA: ${{ steps.base.outputs.sha }} + MEASURE_ANDROID: ${{ needs.changes.outputs.android }} + MEASURE_REACT_NATIVE: ${{ needs.changes.outputs.reactNative }} + MEASURE_WEB: ${{ needs.changes.outputs.web }} + run: | + mkdir -p /tmp/bundle-size-report + node .github/scripts/bundle-size-budgets.cjs /tmp/package-size-base.tsv /tmp/package-size-head.tsv .ci/bundle-size-budgets.json /tmp/bundle-size-report/report.json + cp /tmp/package-size-comment.md /tmp/bundle-size-report/comment.md - const existing = comments.find((comment) => comment.body?.includes(marker)); - - if (existing) { - await github.rest.issues.updateComment({ - owner, - repo, - comment_id: existing.id, - body, - }); - } else { - await github.rest.issues.createComment({ - owner, - repo, - issue_number, - body, - }); - } + - name: Upload measurements + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: bundle-size-report + path: /tmp/bundle-size-report/ + if-no-files-found: error + retention-days: 30 diff --git a/dev.yml b/dev.yml index 4694d9899..b27ef8170 100644 --- a/dev.yml +++ b/dev.yml @@ -69,6 +69,7 @@ open: "PRs": "https://github.com/Shopify/checkout-kit/pulls" check: + bundle-size-budget-tests: node --test .github/scripts/*bundle-size*.test.cjs ejson-plaintext: ./scripts/ejson_lint generate-env-tests: ./scripts/test_generate_env_files storefront-env-tests: ./scripts/test_setup_storefront_env From b230343035c2dae250a60c53d718e4fa9ce6fe28 Mon Sep 17 00:00:00 2001 From: Mark Murray Date: Fri, 2 Oct 2026 09:58:53 +0100 Subject: [PATCH 2/7] Handle acceptance commands from non-collaborators --- .github/scripts/publish-bundle-size.cjs | 23 ++++++++++++-------- .github/scripts/publish-bundle-size.test.cjs | 14 ++++++++++++ 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/.github/scripts/publish-bundle-size.cjs b/.github/scripts/publish-bundle-size.cjs index 647995d13..d2ccf45b1 100644 --- a/.github/scripts/publish-bundle-size.cjs +++ b/.github/scripts/publish-bundle-size.cjs @@ -160,15 +160,20 @@ async function processCommands({ github, repo, comments, state, report, mayAccep const actor = comment.user.login; if (comment.user.type !== "User") continue; if (!permissions.has(actor)) { - const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ - ...repo, - username: actor, - }); - permissions.set( - actor, - ["admin", "maintain", "write"].includes(data.permission) || - data.user?.permissions?.push === true, - ); + try { + const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ + ...repo, + username: actor, + }); + permissions.set( + actor, + ["admin", "maintain", "write"].includes(data.permission) || + data.user?.permissions?.push === true, + ); + } catch (error) { + if (error.status !== 404) throw error; + permissions.set(actor, false); + } } if (!permissions.get(actor)) { notes.push(`Comment ${comment.id}: repository write access is required.`); diff --git a/.github/scripts/publish-bundle-size.test.cjs b/.github/scripts/publish-bundle-size.test.cjs index ce3ff8569..017c2e4d9 100644 --- a/.github/scripts/publish-bundle-size.test.cjs +++ b/.github/scripts/publish-bundle-size.test.cjs @@ -275,3 +275,17 @@ test("fork PR resolution works when workflow_run has no pull_requests", async () f.run.event = "push"; assert.equal(await publisher.resolvePR(f.args), null); }); + +test("a non-collaborator lookup cannot turn a passing budget check into a failure", async () => { + const f = fixture(); + f.report.head["Web\tJavaScript"] = 34000; + await publisher.publish(f.args); + f.github.rest.repos.getCollaboratorPermissionLevel = async () => { + throw Object.assign(new Error("Not found"), { status: 404 }); + }; + f.command("/accept-size web Please accept", "outsider"); + await publisher.publish(f.args); + assert.equal(f.checks.at(-1).conclusion, "success"); + assert.match(f.comments[0].body, /write access is required/); + assert.deepEqual(f.errors, []); +}); From 3d2f5b8bad179181d50a05ba633cd9b668f20326 Mon Sep 17 00:00:00 2001 From: Mark Murray Date: Fri, 2 Oct 2026 11:09:45 +0100 Subject: [PATCH 3/7] Focus bundle budget tests on policy and acceptance rules --- .github/scripts/bundle-size-budgets.test.cjs | 131 +++----- .github/scripts/measure-bundle-size.test.cjs | 69 ---- .github/scripts/publish-bundle-size.test.cjs | 325 ++++--------------- 3 files changed, 111 insertions(+), 414 deletions(-) delete mode 100644 .github/scripts/measure-bundle-size.test.cjs diff --git a/.github/scripts/bundle-size-budgets.test.cjs b/.github/scripts/bundle-size-budgets.test.cjs index f9cc572c6..93b015c93 100644 --- a/.github/scripts/bundle-size-budgets.test.cjs +++ b/.github/scripts/bundle-size-budgets.test.cjs @@ -5,105 +5,77 @@ const policy = require("./bundle-size-budgets.cjs"); const KiB = 1024; const report = (size, base = 34 * KiB) => ({ budgets: { web: { javascript: { softKiB: 35, hardKiB: 50 } } }, - base: base === undefined ? {} : { "Web\tJavaScript": base }, + base: base === null ? {} : { "Web\tJavaScript": base }, head: { "Web\tJavaScript": size }, measuredPlatforms: ["web"], }); const comment = { id: 42, html_url: "https://github.com/example/repo/pull/1#issuecomment-42" }; -test("compares exact bytes at both limits, including fractional KiB", () => { - for (const [bytes, expected] of [ - [35 * KiB, "within"], - [35 * KiB + 1, "soft"], - [50 * KiB, "soft"], - [50 * KiB + 1, "hard"], +test("enforces exact limits, exempts no growth, and handles missing measurements", () => { + for (const [size, base, status] of [ + [35 * KiB, 34 * KiB, "within"], + [35 * KiB + 1, 34 * KiB, "soft"], + [50 * KiB, 34 * KiB, "soft"], + [50 * KiB + 1, 34 * KiB, "hard"], + [60 * KiB, 60 * KiB, "no-growth"], + [59 * KiB, 60 * KiB, "no-growth"], + [40 * KiB, null, "soft"], + [undefined, 34 * KiB, "missing"], + [0, 34 * KiB, "missing"], ]) { - assert.equal(policy.evaluate(report(bytes))[0].status, expected); + assert.equal(policy.evaluate(report(size, base))[0].status, status, `${base} → ${size}`); } const input = report(35.5 * KiB); input.budgets.web.javascript.softKiB = 35.5; assert.equal(policy.evaluate(input)[0].status, "within"); input.head["Web\tJavaScript"]++; assert.equal(policy.evaluate(input)[0].status, "soft"); -}); - -test("unchanged or reduced artifacts above either limit pass", () => { - for (const size of [40 * KiB, 60 * KiB]) { - assert.equal(policy.evaluate(report(size, size))[0].status, "no-growth"); - assert.equal(policy.evaluate(report(size - 1, size))[0].status, "no-growth"); - } - assert.equal(policy.evaluate(report(60 * KiB + 1, 60 * KiB))[0].status, "hard"); -}); - -test("missing or zero head measurements fail; missing base is not an exemption", () => { - assert.equal(policy.evaluate(report(undefined))[0].status, "missing"); - assert.equal(policy.evaluate(report(0))[0].status, "missing"); - const input = report(40 * KiB); - input.base = {}; - assert.equal(policy.evaluate(input)[0].status, "soft"); - input.head["Web\tJavaScript"] = 51 * KiB; - assert.equal(policy.evaluate(input)[0].status, "hard"); -}); - -test("only affected platforms are gated; a docs-only report passes", () => { - const input = report(undefined); input.measuredPlatforms = []; - assert.deepEqual(policy.evaluate(input), []); - assert.equal(policy.conclusion([]), "success"); + assert.equal(policy.conclusion(policy.evaluate(input)), "success"); }); -test("one platform command accepts each current soft breach, leaving other platforms unresolved", () => { +test("platform acceptance covers each current breach, with independent caps", () => { const input = report(40 * KiB); input.budgets.web.javascriptGzip = { softKiB: 10, hardKiB: 15 }; input.head["Web\tJavaScript (gzip)"] = 11 * KiB; input.budgets.android = { aar: { softKiB: 100, hardKiB: 200 } }; input.head["Android\trelease AAR"] = 150 * KiB; input.measuredPlatforms.push("android"); - const web = policy.accept( + const { accepted } = policy.accept( policy.evaluate(input), [{ platform: "web", reason: "New capability" }], "writer", comment, ); - assert.equal(Object.keys(web.accepted).length, 2); assert.deepEqual( - policy.evaluate(input, web.accepted).map((row) => row.status), + policy.evaluate(input, accepted).map((row) => row.status), ["accepted", "accepted", "soft"], ); - assert.equal(policy.conclusion(policy.evaluate(input, web.accepted)), "failure"); + assert.equal(policy.conclusion(policy.evaluate(input, accepted)), "failure"); const all = policy.accept( - policy.evaluate(input, web.accepted), + policy.evaluate(input, accepted), [{ platform: "android", reason: "Native support" }], "writer", comment, - web.accepted, - ); - assert.equal(policy.conclusion(policy.evaluate(input, all.accepted)), "success"); -}); - -test("acceptance caps survive unchanged or smaller sizes; further growth needs acceptance", () => { - const input = report(40 * KiB); - const { accepted } = policy.accept( - policy.evaluate(input), - [{ platform: "web", reason: "New capability" }], - "writer", - comment, - ); - assert.equal(accepted["web.javascript"].bytes, 40 * KiB); - assert.equal(accepted["web.javascript"].reason, "New capability"); - for (const [size, expected] of [ - [40 * KiB, "accepted"], + accepted, + ).accepted; + assert.equal(policy.conclusion(policy.evaluate(input, all)), "success"); + for (const [size, status] of [ [39 * KiB, "accepted"], + [40 * KiB, "accepted"], [40 * KiB + 1, "soft"], ]) { - assert.equal(policy.evaluate(report(size), accepted)[0].status, expected); + assert.equal(policy.evaluate(report(size), all)[0].status, status); } - input.budgets.web.javascriptGzip = { softKiB: 10, hardKiB: 15 }; - input.head["Web\tJavaScript (gzip)"] = 11 * KiB; - assert.equal(policy.evaluate(input, accepted)[1].status, "soft"); + input.budgets.web.npmTarball = { softKiB: 100, hardKiB: 200 }; + input.head["Web\tnpm tarball"] = 120 * KiB; + assert.equal( + policy.evaluate(input, all).find((row) => row.metric === "npmTarball").status, + "soft", + ); }); -test("a comment or a prior acceptance cannot override a hard limit", () => { +test("comments and existing acceptances cannot override a hard budget", () => { const input = report(51 * KiB); const result = policy.accept( policy.evaluate(input), @@ -112,45 +84,22 @@ test("a comment or a prior acceptance cannot override a hard limit", () => { comment, ); assert.deepEqual(result.accepted, {}); - assert.match(result.notes[0], /hard budget/); assert.equal(policy.evaluate(input, { "web.javascript": { bytes: 60 * KiB } })[0].status, "hard"); }); -test("rejects unknown keys and malformed limits", () => { +test("rejects unknown configuration keys and invalid limits", () => { for (const budgets of [ - null, - [], { ios: {} }, { web: { typo: { softKiB: 1, hardKiB: 2 } } }, { web: { javascript: { soft: 35, hard: 50 } } }, { web: { javascript: { softKiB: "35", hardKiB: 50 } } }, { web: { javascript: { softKiB: 51, hardKiB: 50 } } }, { web: { javascript: { softKiB: 0, hardKiB: 50 } } }, - { web: { javascript: { softKiB: 35, hardKiB: Infinity } } }, - { toString: {} }, - ]) { + ]) assert.throws(() => policy.validateBudgets(budgets)); - } }); -test("parses multiple commands with mandatory reasons, ignoring ordinary quoted prose", () => { - assert.deepEqual( - policy.parseCommands("/accept-size web New capability\n/accept-size android Native support"), - [ - { platform: "web", reason: "New capability" }, - { platform: "android", reason: "Native support" }, - ], - ); - assert.deepEqual( - policy.parseCommands("I could use /accept-size web a reason\n> /accept-size web quoted"), - [], - ); - for (const body of ["/accept-size web", "/accept-size web ", "/accept-size unknown A reason"]) { - assert.throws(() => policy.parseCommands(body)); - } -}); - -test("summary parser ignores file rows and rejects duplicate or malformed measurements", () => { +test("measurement parsing excludes file details and rejects duplicate or invalid sizes", () => { assert.deepEqual( policy.measurements("Web\tJavaScript\t34073\nWeb\tnpm tarball\t42000\tdist/index.js\n"), { "Web\tJavaScript": 34073 }, @@ -164,7 +113,7 @@ test("summary parser ignores file rows and rejects duplicate or malformed measur } }); -test("report round-trips trusted acceptance data without artifact marker injection", () => { +test("artifact text cannot inject saved acceptance state into the report", () => { const state = { version: 1, headSha: "abc", @@ -172,9 +121,11 @@ test("report round-trips trusted acceptance data without artifact marker injecti acceptances: {}, processedComments: [], }; - const injected = ""; - const body = policy.render(policy.evaluate(report(40 * KiB)), state, injected); + const body = policy.render( + policy.evaluate(report(40 * KiB)), + state, + "", + ); assert.deepEqual(policy.readState(body), state); assert.match(body, /Acceptance required/); - assert.match(body, /\+6.00 KiB/); }); diff --git a/.github/scripts/measure-bundle-size.test.cjs b/.github/scripts/measure-bundle-size.test.cjs deleted file mode 100644 index a4679c221..000000000 --- a/.github/scripts/measure-bundle-size.test.cjs +++ /dev/null @@ -1,69 +0,0 @@ -const { test } = require("node:test"); -const assert = require("node:assert/strict"); -const fs = require("node:fs"); -const os = require("node:os"); -const path = require("node:path"); -const { execFileSync } = require("node:child_process"); -const { measurements } = require("./bundle-size-budgets.cjs"); - -test("collector measures all shipped JS chunks, excludes maps/types, and ignores timestamps in gzip", () => { - const directory = fs.mkdtempSync(path.join(os.tmpdir(), "bundle-collector-test-")); - try { - const dist = path.join(directory, "package/dist"); - const bin = path.join(directory, "bin"); - fs.mkdirSync(dist, { recursive: true }); - fs.mkdirSync(bin); - fs.mkdirSync(path.join(directory, "platforms/web"), { recursive: true }); - const files = { - "index.js": "export const answer = 42;\n", - "chunk.mjs": 'export default "chunk";\n', - "legacy.cjs": "module.exports = 42;\n", - "index.d.ts": "export declare const answer: number;", - "index.js.map": '{"sources":[]}', - }; - for (const [name, content] of Object.entries(files)) - fs.writeFileSync(path.join(dist, name), content); - const fakePnpm = path.join(bin, "pnpm"); - fs.writeFileSync( - fakePnpm, - '#!/usr/bin/env bash\nset -euo pipefail\nif [[ "$1" == pack ]]; then\n tar -czf "$3/package.tgz" -C "$PACKAGE_SIZE_REPO_ROOT" package\nfi\n', - ); - fs.chmodSync(fakePnpm, 0o755); - const env = { - ...process.env, - PATH: `${bin}:${process.env.PATH}`, - TMPDIR: directory, - PACKAGE_SIZE_REPO_ROOT: directory, - MEASURE_WEB: "true", - MEASURE_REACT_NATIVE: "false", - MEASURE_ANDROID: "false", - }; - const output = path.join(directory, "sizes.tsv"); - const collect = () => { - execFileSync("bash", [path.join(__dirname, "measure-package-size"), "collect", output], { - env, - }); - return measurements(fs.readFileSync(output, "utf8")); - }; - const first = collect(); - const scripts = ["index.js", "chunk.mjs", "legacy.cjs"]; - assert.equal( - first["Web\tJavaScript"], - scripts.reduce((sum, file) => sum + Buffer.byteLength(files[file]), 0), - ); - assert.equal( - first["Web\tJavaScript (gzip)"], - scripts.reduce( - (sum, file) => sum + execFileSync("gzip", ["-n", "-9", "-c", path.join(dist, file)]).length, - 0, - ), - ); - for (const name of scripts) fs.utimesSync(path.join(dist, name), 1234567890, 1234567890); - const second = collect(); - assert.equal(second["Web\tJavaScript (gzip)"], first["Web\tJavaScript (gzip)"]); - for (const name of scripts) fs.unlinkSync(path.join(dist, name)); - assert.throws(collect, /No shipped web JavaScript found/); - } finally { - fs.rmSync(directory, { recursive: true, force: true }); - } -}); diff --git a/.github/scripts/publish-bundle-size.test.cjs b/.github/scripts/publish-bundle-size.test.cjs index 017c2e4d9..6e89ba0f0 100644 --- a/.github/scripts/publish-bundle-size.test.cjs +++ b/.github/scripts/publish-bundle-size.test.cjs @@ -3,289 +3,104 @@ const assert = require("node:assert/strict"); const policy = require("./bundle-size-budgets.cjs"); const publisher = require("./publish-bundle-size.cjs"); -function fixture() { - const pr = { - number: 1, - state: "open", - head: { sha: "a".repeat(40), ref: "feature", repo: { id: 123 } }, - base: { sha: "b".repeat(40) }, - }; - const run = { - id: 10, - run_attempt: 1, - event: "pull_request", - path: ".github/workflows/package-size.yml", - head_sha: pr.head.sha, - head_branch: "feature", - head_repository: { id: 123 }, - pull_requests: [pr], - status: "completed", - conclusion: "success", - html_url: "https://github.com/example/repo/actions/runs/10", - }; +function fixture(permission = "write") { const report = { version: 1, pr: 1, - headSha: pr.head.sha, - baseSha: pr.base.sha, + headSha: "head", + baseSha: "base", budgets: { web: { javascript: { softKiB: 35, hardKiB: 50 } } }, base: { "Web\tJavaScript": 34000 }, head: { "Web\tJavaScript": 40000 }, measuredPlatforms: ["web"], }; - const comments = []; - const checks = []; - const errors = []; - const permission = { writer: "write", reader: "read" }; - let pullReads = 0; - const rest = { - pulls: { - get: async () => { - pullReads++; - return { data: pr }; - }, - }, - actions: { listWorkflowRuns: "runs" }, - repos: { - listPullRequestsAssociatedWithCommit: "prs", - getCollaboratorPermissionLevel: async ({ username }) => ({ - data: { permission: permission[username] ?? "none" }, - }), - }, - issues: { - listComments: "comments", - createComment: async ({ body }) => { - const item = { - id: 100, - body, - user: { login: "github-actions[bot]", type: "Bot" }, - html_url: "https://github.com/example/repo/pull/1#issuecomment-100", - }; - comments.push(item); - return { data: item }; - }, - updateComment: async ({ comment_id, body }) => { - const item = comments.find((c) => c.id === comment_id); - item.body = body; - return { data: item }; - }, - }, - checks: { - listForRef: "checks", - create: async (params) => { - checks.push({ ...params, id: 200 + checks.length, app: { slug: "github-actions" } }); - }, - update: async (params) => { - Object.assign( - checks.find((c) => c.id === params.check_run_id), - params, - ); - }, - }, + const comment = { + id: 42, + body: "/accept-size web New checkout capability", + user: { login: "writer", type: "User" }, + created_at: "2026-10-02T12:01:00Z", + updated_at: "2026-10-02T12:01:00Z", + html_url: "https://github.com/example/repo/pull/1#issuecomment-42", }; + const state = { acceptances: {}, processedComments: [], publishedAt: "2026-10-02T12:00:00Z" }; const github = { - rest, - paginate: async (route, params) => - route === "checks" - ? checks.filter((check) => check.head_sha === params.ref) - : { runs: [run], prs: [pr], comments }[route], - }; - const context = { - repo: { owner: "example", repo: "repo" }, - eventName: "workflow_run", - payload: { workflow_run: run }, + rest: { + repos: { + getCollaboratorPermissionLevel: async () => { + if (permission === "missing") + throw Object.assign(new Error("Not found"), { status: 404 }); + return { data: { permission } }; + }, + }, + }, }; - const args = { - github, - context, - core: { setFailed: (message) => errors.push(message) }, - prNumber: 1, - loadArtifact: async () => ({ + const process = (overrides = {}) => + publisher.processCommands({ + github, + repo: {}, + comments: [comment], + state, report, - packageComment: policy.marker + "\nPackage measurements", - }), - }; - const command = (body, actor = "writer", overrides = {}) => { - const date = new Date(Date.now() + 2000).toISOString(); - const item = { - id: comments.length + 101, - body, - user: { login: actor, type: "User" }, - created_at: date, - updated_at: date, - html_url: "https://github.com/example/repo/pull/1#issuecomment-101", + mayAccept: true, ...overrides, - }; - comments.push(item); - context.eventName = "issue_comment"; - context.payload = { issue: { number: 1, pull_request: {} }, comment: item }; - return item; - }; - return { - pr, - run, - report, - comments, - checks, - errors, - permission, - github, - context, - args, - command, - reads: () => pullReads, - }; + }); + return { report, comment, state, process }; } -test("publishes a failing gate, accepts a writer reason, then reopens it after further growth", async () => { +test("writer acceptance records the actor, reason and size, and is processed only once", async () => { const f = fixture(); - await publisher.publish(f.args); - assert.equal(f.checks.at(-1).conclusion, "failure"); - f.command("/accept-size web New checkout capability"); - await publisher.publish(f.args); - assert.equal(f.checks.length, 1); - assert.equal(f.checks.at(-1).conclusion, "success"); - assert.match(f.comments[0].body, /Accepted by @writer: New checkout capability/); - f.pr.head.sha = f.report.headSha = f.run.head_sha = "c".repeat(40); - f.run.id++; - f.context.eventName = "workflow_run"; - f.context.payload = { workflow_run: f.run }; - await publisher.publish(f.args); - assert.equal(f.checks.at(-1).conclusion, "success"); + const result = await f.process(); + assert.deepEqual(result.acceptances["web.javascript"], { + bytes: 40000, + actor: "writer", + reason: "New checkout capability", + commentId: 42, + url: f.comment.html_url, + }); + assert.equal(policy.conclusion(policy.evaluate(f.report, result.acceptances)), "success"); + Object.assign(f.state, result); f.report.head["Web\tJavaScript"]++; - f.run.id++; - await publisher.publish(f.args); - assert.equal(f.checks.at(-1).conclusion, "failure"); - assert.deepEqual(f.errors, []); + const replay = await f.process(); + assert.deepEqual(replay.acceptances, result.acceptances); + assert.equal(policy.conclusion(policy.evaluate(f.report, replay.acceptances)), "failure"); }); -test("readers, malformed commands and edited comments cannot accept an increase", async () => { - for (const [body, actor, overrides, expected] of [ - ["/accept-size web Please accept", "reader", {}, /write access/], - ["/accept-size web", "writer", {}, /reason/], - [ - "/accept-size web Changed reason", - "writer", - { updated_at: "2099-01-01T00:00:00Z" }, - /edited comments/, - ], - ]) { - const f = fixture(); - await publisher.publish(f.args); - f.command(body, actor, overrides); - await publisher.publish(f.args); - assert.equal(f.checks.at(-1).conclusion, "failure"); - assert.match(f.comments[0].body, expected); - assert.deepEqual(policy.readState(f.comments[0].body).acceptances, {}); +test("unauthorized, malformed, edited and premature commands cannot accept sizes", async () => { + for (const scenario of ["read", "missing", "reason", "edited", "stale", "premature"]) { + const f = fixture(["read", "missing"].includes(scenario) ? scenario : "write"); + if (scenario === "reason") f.comment.body = "/accept-size web"; + if (scenario === "edited") f.comment.updated_at = "2026-10-02T12:02:00Z"; + if (scenario === "premature") f.state.publishedAt = "2026-10-02T12:02:00Z"; + const result = await f.process({ mayAccept: scenario !== "stale" }); + assert.deepEqual(result.acceptances, {}, scenario); + assert.deepEqual(result.processedComments, [42], scenario); + assert.ok(result.notes.length, scenario); } }); -test("commands cannot pre-approve unreported sizes or new measurements", async () => { - const f = fixture(); - f.command("/accept-size web Accept whatever it becomes"); - await publisher.publish(f.args); - assert.equal(f.checks.at(-1).conclusion, "failure"); - f.command("/accept-size web Report was for an older commit"); - f.pr.head.sha = f.report.headSha = f.run.head_sha = "c".repeat(40); - f.run.id++; - await publisher.publish(f.args); - assert.equal(f.checks.at(-1).conclusion, "failure"); - assert.deepEqual(policy.readState(f.comments.find(publisher.isReport).body).acceptances, {}); -}); - -test("coalesced comment events still process every pending platform command", async () => { +test("all pending platform commands are processed when comment events are coalesced", async () => { const f = fixture(); f.report.budgets.android = { aar: { softKiB: 100, hardKiB: 200 } }; f.report.head["Android\trelease AAR"] = 150 * 1024; f.report.measuredPlatforms.push("android"); - await publisher.publish(f.args); - f.command("/accept-size web New checkout capability"); - f.command("/accept-size android Native support"); - await publisher.publish(f.args); - assert.equal(f.checks.at(-1).conclusion, "success"); - assert.equal(Object.keys(policy.readState(f.comments[0].body).acceptances).length, 2); + const android = { ...f.comment, id: 43, body: "/accept-size android Native support" }; + const result = await f.process({ comments: [f.comment, android] }); + assert.equal(policy.conclusion(policy.evaluate(f.report, result.acceptances)), "success"); + assert.deepEqual(result.processedComments, [42, 43]); }); -test("a forged report from a human cannot supply approvals", async () => { - const f = fixture(); - f.comments.push({ - id: 2, - user: { login: "writer", type: "User" }, - body: policy.render( - [], - { - version: 1, - acceptances: { "web.javascript": { bytes: 999999 } }, - processedComments: [], - }, - "", - ), - }); - await publisher.publish(f.args); - assert.equal(f.checks.at(-1).conclusion, "failure"); - assert.equal(f.comments.length, 2); -}); - -test("hard budgets, missing data, failed builds and expired artifacts fail closed", async () => { - for (const scenario of ["hard", "missing", "failure", "artifact"]) { - const f = fixture(); - if (scenario === "hard") f.report.head["Web\tJavaScript"] = 60000; - if (scenario === "missing") delete f.report.head["Web\tJavaScript"]; - if (scenario === "failure") f.run.conclusion = "failure"; - if (scenario === "artifact") - f.args.loadArtifact = async () => { - throw new Error("Artifact expired"); - }; - await publisher.publish(f.args); - f.command("/accept-size web Please override"); - await publisher.publish(f.args); - assert.equal(f.checks.at(-1).conclusion, "failure", scenario); - } -}); - -test("stale head/base or wrong PR artifacts are rejected", async () => { +test("rejects stale or mismatched reports and trusts only the Actions bot's saved state", () => { + const { report } = fixture(); + const pr = { number: 1, head: { sha: "head" }, base: { sha: "base" } }; + const run = { head_sha: "head" }; + assert.doesNotThrow(() => publisher.validateReport(report, pr, run)); for (const field of ["headSha", "baseSha", "pr"]) { - const f = fixture(); - f.report[field] = "wrong"; - await publisher.publish(f.args); - assert.equal(f.comments.length, 0); - assert.equal(f.checks.at(-1).conclusion, "failure"); - assert.match(f.errors[0], /current PR head and base/); + assert.throws(() => publisher.validateReport({ ...report, [field]: "wrong" }, pr, run)); + } + for (const [login, type, trusted] of [ + ["github-actions[bot]", "Bot", true], + ["writer", "User", false], + ]) { + assert.equal(publisher.isReport({ user: { login, type }, body: policy.marker }), trusted); } -}); - -test("pending builds cannot be accepted; stale workflow completions are ignored", async () => { - const f = fixture(); - f.run.status = "in_progress"; - f.command("/accept-size web Please accept"); - await publisher.publish(f.args); - assert.equal(f.checks.at(-1).status, "in_progress"); - assert.equal(f.comments.length, 1); - f.context.eventName = "workflow_run"; - f.context.payload = { workflow_run: { ...f.run, id: 9 } }; - const reads = f.reads(); - await publisher.publish(f.args); - assert.equal(f.reads(), reads + 1); -}); - -test("fork PR resolution works when workflow_run has no pull_requests", async () => { - const f = fixture(); - f.run.pull_requests = []; - assert.equal(await publisher.resolvePR(f.args), 1); - f.run.event = "push"; - assert.equal(await publisher.resolvePR(f.args), null); -}); - -test("a non-collaborator lookup cannot turn a passing budget check into a failure", async () => { - const f = fixture(); - f.report.head["Web\tJavaScript"] = 34000; - await publisher.publish(f.args); - f.github.rest.repos.getCollaboratorPermissionLevel = async () => { - throw Object.assign(new Error("Not found"), { status: 404 }); - }; - f.command("/accept-size web Please accept", "outsider"); - await publisher.publish(f.args); - assert.equal(f.checks.at(-1).conclusion, "success"); - assert.match(f.comments[0].body, /write access is required/); - assert.deepEqual(f.errors, []); }); From 8792e708f3df5bfd02f68f2bb3cc6dcbf1337dab Mon Sep 17 00:00:00 2001 From: Mark Murray Date: Fri, 2 Oct 2026 11:15:42 +0100 Subject: [PATCH 4/7] Make bundle budget measurement scope explicit --- .ci/bundle-size-budgets.json | 1 + .ci/bundle-size-budgets.md | 54 ++++++++++---- .github/scripts/bundle-size-budgets.cjs | 78 ++++++++++++++------ .github/scripts/bundle-size-budgets.test.cjs | 78 +++++++++++++++++--- .github/scripts/publish-bundle-size.test.cjs | 7 +- 5 files changed, 169 insertions(+), 49 deletions(-) diff --git a/.ci/bundle-size-budgets.json b/.ci/bundle-size-budgets.json index ff58ed745..5166016f7 100644 --- a/.ci/bundle-size-budgets.json +++ b/.ci/bundle-size-budgets.json @@ -1,6 +1,7 @@ { "web": { "javascript": { + "measurement": "shippedJavaScript", "softKiB": 35, "hardKiB": 50 } diff --git a/.ci/bundle-size-budgets.md b/.ci/bundle-size-budgets.md index 5eea5bf87..88bae3c50 100644 --- a/.ci/bundle-size-budgets.md +++ b/.ci/bundle-size-budgets.md @@ -1,10 +1,14 @@ # Bundle size budgets -`.ci/bundle-size-budgets.json` assigns soft and hard limits to individual platform -metrics. Values are KiB (1,024 bytes), including fractions. Comparisons use exact +`.ci/bundle-size-budgets.json` defines named budgets for each platform. Each budget +explicitly selects a `measurement` and may select a `file` within a package. +Limits are KiB (1,024 bytes), including fractions. Comparisons use exact bytes, not the rounded numbers displayed in PR reports. -Web starts with a 35 KiB soft limit and 50 KiB hard limit on shipped JavaScript. +Web starts with a 35 KiB soft limit and 50 KiB hard limit on all shipped JavaScript, +using `"measurement": "shippedJavaScript"`. This currently measures `dist/index.js` +and will include any additional JavaScript chunks shipped in `dist`. Declarations, +source maps, and other package contents are excluded from this measurement. The report also includes deterministic gzip size and package sizes. These remain informational until a budget is configured for their metric. @@ -36,9 +40,10 @@ actor, and a link to the reason. Multiple commands may share one comment: ``` Every breached metric must be satisfied before the aggregate check passes. -Acceptance survives subsequent commits when the accepted metric stays the same +Acceptance survives subsequent commits when the accepted measurement stays the same size or gets smaller. Further growth or a newly breached metric requires a new -comment. A comment cannot override a hard limit, failed build, or missing data. +comment. Changing a budget's measurement or file also requires fresh acceptance. +A comment cannot override a hard limit, failed build, or missing data. It does not modify the configured budget. Edited comments are not processed; post a new command instead. Editing or deleting an already accepted reason does not erase the recorded decision in the bot report. @@ -49,16 +54,37 @@ for the updated report. Expired measurement artifacts also require a rerun. ## Metrics and additional platforms -| Platform key | Metric key | Measurement | +| Platform key | `measurement` | Scope | | --- | --- | --- | -| `web` | `javascript` | Sum of shipped `.js`, `.mjs`, and `.cjs` files in `dist` | -| `web` | `javascriptGzip` | Sum of those files compressed individually with `gzip -n -9` | -| `web` | `npmTarball` | Compressed published package | -| `react-native` | `npmTarball` | Compressed published wrapper package | -| `android` | `aar` | Release AAR | - -Add budgets for existing metrics with the same `{ "softKiB": 35, "hardKiB": 50 }` -shape. Unknown platforms, metric names, units, and invalid limits fail configuration +| `web` | `shippedJavaScript` | Sum of raw shipped `.js`, `.mjs`, and `.cjs` files in `dist` | +| `web` | `shippedJavaScriptGzip` | Sum of those files compressed individually with `gzip -n -9` | +| `web` | `package` | Whole compressed npm tarball | +| `react-native` | `package` | Whole compressed wrapper npm tarball | +| `android` | `package` | Whole compressed release AAR | + +With `measurement: "package"`, an optional `file` selects that exact file's +**uncompressed** size inside the package. Paths are relative to the published +package root (or AAR root); globs are not supported. A missing file fails the check. +For example, this budgets only the entry file, rather than every JavaScript chunk: + +```json +{ + "web": { + "entryPoint": { + "measurement": "package", + "file": "dist/index.js", + "softKiB": 35, + "hardKiB": 50 + } + } +} +``` + +Omit `file` from a `package` budget to measure the complete compressed artifact. +Budget names such as `entryPoint` are labels; the selector fields determine what +is measured, and the PR report displays that scope explicitly. + +Unknown platforms, measurements, units, and invalid limits fail configuration validation. Adding a new measurement (for example a Swift framework) requires a reproducible collector in `measure-package-size`, its changed-path detection and build setup in `package-size.yml`, and an adapter in `bundle-size-budgets.cjs`. diff --git a/.github/scripts/bundle-size-budgets.cjs b/.github/scripts/bundle-size-budgets.cjs index fdde4e0b6..f37edf056 100644 --- a/.github/scripts/bundle-size-budgets.cjs +++ b/.github/scripts/bundle-size-budgets.cjs @@ -1,18 +1,18 @@ const fs = require("node:fs"); -// Measurement adapters map the existing artifact report to stable config keys. +// Measurement adapters map the existing artifact report to explicit selectors. // Policy and acceptance handling below do not depend on a particular platform. const platforms = { web: { label: "Web", - metrics: { - javascript: "JavaScript", - javascriptGzip: "JavaScript (gzip)", - npmTarball: "npm tarball", + measurements: { + shippedJavaScript: "JavaScript", + shippedJavaScriptGzip: "JavaScript (gzip)", + package: "npm tarball", }, }, - "react-native": { label: "React Native", metrics: { npmTarball: "npm tarball" } }, - android: { label: "Android", metrics: { aar: "release AAR" } }, + "react-native": { label: "React Native", measurements: { package: "npm tarball" } }, + android: { label: "Android", measurements: { package: "release AAR" } }, }; const marker = ""; const statePattern = //; @@ -27,11 +27,27 @@ function validateBudgets(budgets) { if (!Object.hasOwn(platforms, platform) || !object(metrics)) throw new Error(`Unknown platform: ${platform}`); for (const [metric, budget] of Object.entries(metrics)) { - if (!Object.hasOwn(platforms[platform].metrics, metric)) - throw new Error(`Unknown metric: ${platform}.${metric}`); + if (!/^[a-zA-Z][a-zA-Z0-9-]*$/.test(metric)) + throw new Error(`Invalid budget name: ${platform}.${metric}`); if ( !object(budget) || - Object.keys(budget).sort().join(",") !== "hardKiB,softKiB" || + typeof budget.measurement !== "string" || + !Object.hasOwn(platforms[platform].measurements, budget.measurement) + ) + throw new Error(`Unknown measurement for ${platform}.${metric}`); + if ( + Object.hasOwn(budget, "file") && + (budget.measurement !== "package" || + typeof budget.file !== "string" || + !budget.file || + /[\\\t\r\n*?[\]{}]/.test(budget.file) || + budget.file.split("/").some((part) => !part || part === "." || part === "..")) + ) + throw new Error(`File must be an exact package-relative path for ${platform}.${metric}`); + if ( + Object.keys(budget).some( + (key) => !["measurement", "file", "softKiB", "hardKiB"].includes(key), + ) || !Number.isFinite(budget.softKiB) || !Number.isFinite(budget.hardKiB) || budget.softKiB <= 0 || @@ -49,12 +65,16 @@ function measurements(tsv) { const values = {}; for (const line of tsv.split("\n").filter(Boolean)) { const columns = line.split("\t"); - if (columns.length >= 4) continue; // Per-file details are informational. - const [platform, metric, bytes] = columns; - if (columns.length !== 3 || !/^\d+$/.test(bytes) || !Number.isSafeInteger(Number(bytes))) { + const [platform, metric, bytes, file] = columns; + if ( + ![3, 4].includes(columns.length) || + (columns.length === 4 && !file) || + !/^\d+$/.test(bytes) || + !Number.isSafeInteger(Number(bytes)) + ) { throw new Error("Invalid measurement row"); } - const key = `${platform}\t${metric}`; + const key = `${platform}\t${metric}${file ? `\t${file}` : ""}`; if (Object.hasOwn(values, key)) throw new Error(`Duplicate measurement: ${key}`); values[key] = Number(bytes); } @@ -74,16 +94,23 @@ function evaluate({ budgets, base, head, measuredPlatforms }, acceptances = {}) if (!measuredPlatforms.includes(platform)) continue; for (const [metric, budget] of Object.entries(metrics)) { const key = `${platform}.${metric}`; - const measurementKey = `${platforms[platform].label}\t${platforms[platform].metrics[metric]}`; + const measurementKey = `${platforms[platform].label}\t${platforms[platform].measurements[budget.measurement]}${budget.file ? `\t${budget.file}` : ""}`; const before = base[measurementKey]; const after = head[measurementKey]; const acceptance = acceptances[key]; let status; - if (!Number.isSafeInteger(after) || after <= 0) status = "missing"; + if (!Number.isSafeInteger(after) || after < 0 || (after === 0 && !budget.file)) + status = "missing"; else if (after <= budget.softKiB * 1024) status = "within"; else if (before !== undefined && after <= before) status = "no-growth"; else if (after > budget.hardKiB * 1024) status = "hard"; - else if (acceptance && after <= acceptance.bytes) status = "accepted"; + else if ( + acceptance && + acceptance.measurement === budget.measurement && + acceptance.file === budget.file && + after <= acceptance.bytes + ) + status = "accepted"; else status = "soft"; rows.push({ key, platform, metric, before, after, ...budget, status, acceptance }); } @@ -112,6 +139,8 @@ function accept(rows, commands, actor, comment, previous = {}) { for (const row of eligible) { accepted[row.key] = { bytes: row.after, + measurement: row.measurement, + ...(row.file ? { file: row.file } : {}), actor, reason, commentId: comment.id, @@ -172,8 +201,8 @@ function render(rows, state, packageComment, notes = []) { "", `Measured head: \`${state.headSha}\`; base: \`${state.baseSha}\`.`, "", - "| Platform / metric | Base | Head | Delta | Soft | Hard | Result |", - "| --- | ---: | ---: | ---: | ---: | ---: | --- |", + "| Platform / budget | Measurement | Base | Head | Delta | Soft | Hard | Result |", + "| --- | --- | ---: | ---: | ---: | ---: | ---: | --- |", ]; for (const row of rows) { const status = @@ -184,11 +213,18 @@ function render(rows, state, packageComment, notes = []) { row.before === undefined || row.after === undefined ? "unavailable" : `${row.after > row.before ? "+" : ""}${kib(row.after - row.before)}`; + const scope = row.file + ? `${escape(row.file)} (uncompressed)` + : { + shippedJavaScript: "All shipped JavaScript (raw)", + shippedJavaScriptGzip: "All shipped JavaScript (gzip)", + package: "Whole package (compressed)", + }[row.measurement]; lines.push( - `| ${row.platform} / ${row.metric} | ${kib(row.before)} | ${kib(row.after)} | ${delta} | ${row.softKiB} KiB | ${row.hardKiB} KiB | ${status} |`, + `| ${row.platform} / ${row.metric} | ${scope} | ${kib(row.before)} | ${kib(row.after)} | ${delta} | ${row.softKiB} KiB | ${row.hardKiB} KiB | ${status} |`, ); } - if (!rows.length) lines.push("| — | — | — | — | — | — | No configured budgets affected |"); + if (!rows.length) lines.push("| — | — | — | — | — | — | — | No configured budgets affected |"); lines.push( "", "Repository writers, including the PR author, can accept current soft-budget breaches with a reason:", diff --git a/.github/scripts/bundle-size-budgets.test.cjs b/.github/scripts/bundle-size-budgets.test.cjs index 93b015c93..d19b1824f 100644 --- a/.github/scripts/bundle-size-budgets.test.cjs +++ b/.github/scripts/bundle-size-budgets.test.cjs @@ -4,7 +4,7 @@ const policy = require("./bundle-size-budgets.cjs"); const KiB = 1024; const report = (size, base = 34 * KiB) => ({ - budgets: { web: { javascript: { softKiB: 35, hardKiB: 50 } } }, + budgets: { web: { javascript: { measurement: "shippedJavaScript", softKiB: 35, hardKiB: 50 } } }, base: base === null ? {} : { "Web\tJavaScript": base }, head: { "Web\tJavaScript": size }, measuredPlatforms: ["web"], @@ -36,9 +36,13 @@ test("enforces exact limits, exempts no growth, and handles missing measurements test("platform acceptance covers each current breach, with independent caps", () => { const input = report(40 * KiB); - input.budgets.web.javascriptGzip = { softKiB: 10, hardKiB: 15 }; + input.budgets.web.javascriptGzip = { + measurement: "shippedJavaScriptGzip", + softKiB: 10, + hardKiB: 15, + }; input.head["Web\tJavaScript (gzip)"] = 11 * KiB; - input.budgets.android = { aar: { softKiB: 100, hardKiB: 200 } }; + input.budgets.android = { aar: { measurement: "package", softKiB: 100, hardKiB: 200 } }; input.head["Android\trelease AAR"] = 150 * KiB; input.measuredPlatforms.push("android"); const { accepted } = policy.accept( @@ -67,7 +71,7 @@ test("platform acceptance covers each current breach, with independent caps", () ]) { assert.equal(policy.evaluate(report(size), all)[0].status, status); } - input.budgets.web.npmTarball = { softKiB: 100, hardKiB: 200 }; + input.budgets.web.npmTarball = { measurement: "package", softKiB: 100, hardKiB: 200 }; input.head["Web\tnpm tarball"] = 120 * KiB; assert.equal( policy.evaluate(input, all).find((row) => row.metric === "npmTarball").status, @@ -84,25 +88,75 @@ test("comments and existing acceptances cannot override a hard budget", () => { comment, ); assert.deepEqual(result.accepted, {}); - assert.equal(policy.evaluate(input, { "web.javascript": { bytes: 60 * KiB } })[0].status, "hard"); + assert.equal( + policy.evaluate(input, { + "web.javascript": { bytes: 60 * KiB, measurement: "shippedJavaScript" }, + })[0].status, + "hard", + ); +}); + +test("selects whole packages or individual files without reusing acceptance for another scope", () => { + const input = report(40000); + Object.assign( + input.head, + policy.measurements( + "Web\tnpm tarball\t90000\nWeb\tnpm tarball\t40000\tdist/index.js\nWeb\tnpm tarball\t39999\tdist/other.js\n", + ), + ); + const budget = input.budgets.web.javascript; + budget.measurement = "package"; + assert.equal(policy.evaluate(input)[0].after, 90000); + budget.file = "dist/index.js"; + assert.equal(policy.evaluate(input)[0].after, 40000); + const { accepted } = policy.accept( + policy.evaluate(input), + [{ platform: "web", reason: "New capability" }], + "writer", + comment, + ); + assert.equal(policy.evaluate(input, accepted)[0].status, "accepted"); + budget.file = "dist/other.js"; + assert.equal(policy.evaluate(input, accepted)[0].status, "soft"); + budget.file = "dist/missing.js"; + assert.equal(policy.evaluate(input, accepted)[0].status, "missing"); + input.head["Web\tnpm tarball\tdist/missing.js"] = 0; + assert.equal(policy.evaluate(input, accepted)[0].status, "within"); + delete budget.file; + budget.measurement = "shippedJavaScript"; + assert.equal(policy.evaluate(input, accepted)[0].status, "soft"); }); test("rejects unknown configuration keys and invalid limits", () => { for (const budgets of [ { ios: {} }, - { web: { typo: { softKiB: 1, hardKiB: 2 } } }, - { web: { javascript: { soft: 35, hard: 50 } } }, - { web: { javascript: { softKiB: "35", hardKiB: 50 } } }, - { web: { javascript: { softKiB: 51, hardKiB: 50 } } }, - { web: { javascript: { softKiB: 0, hardKiB: 50 } } }, + { web: { javascript: { softKiB: 35, hardKiB: 50 } } }, + { web: { typo: { measurement: "unknown", softKiB: 1, hardKiB: 2 } } }, + { web: { javascript: { measurement: "shippedJavaScript", soft: 35, hard: 50 } } }, + { web: { javascript: { measurement: "shippedJavaScript", softKiB: "35", hardKiB: 50 } } }, + { web: { javascript: { measurement: "shippedJavaScript", softKiB: 51, hardKiB: 50 } } }, + { web: { javascript: { measurement: "shippedJavaScript", softKiB: 0, hardKiB: 50 } } }, + ...["", "/index.js", "../index.js", "dist/*.js", "dist\\index.js", 42].map((file) => ({ + web: { entry: { measurement: "package", file, softKiB: 35, hardKiB: 50 } }, + })), + { + web: { + entry: { + measurement: "shippedJavaScript", + file: "dist/index.js", + softKiB: 35, + hardKiB: 50, + }, + }, + }, ]) assert.throws(() => policy.validateBudgets(budgets)); }); -test("measurement parsing excludes file details and rejects duplicate or invalid sizes", () => { +test("measurement parsing keeps package and file sizes separate and rejects invalid rows", () => { assert.deepEqual( policy.measurements("Web\tJavaScript\t34073\nWeb\tnpm tarball\t42000\tdist/index.js\n"), - { "Web\tJavaScript": 34073 }, + { "Web\tJavaScript": 34073, "Web\tnpm tarball\tdist/index.js": 42000 }, ); for (const text of [ "Web\tJavaScript\t-1", diff --git a/.github/scripts/publish-bundle-size.test.cjs b/.github/scripts/publish-bundle-size.test.cjs index 6e89ba0f0..629f61233 100644 --- a/.github/scripts/publish-bundle-size.test.cjs +++ b/.github/scripts/publish-bundle-size.test.cjs @@ -9,7 +9,9 @@ function fixture(permission = "write") { pr: 1, headSha: "head", baseSha: "base", - budgets: { web: { javascript: { softKiB: 35, hardKiB: 50 } } }, + budgets: { + web: { javascript: { measurement: "shippedJavaScript", softKiB: 35, hardKiB: 50 } }, + }, base: { "Web\tJavaScript": 34000 }, head: { "Web\tJavaScript": 40000 }, measuredPlatforms: ["web"], @@ -52,6 +54,7 @@ test("writer acceptance records the actor, reason and size, and is processed onl const result = await f.process(); assert.deepEqual(result.acceptances["web.javascript"], { bytes: 40000, + measurement: "shippedJavaScript", actor: "writer", reason: "New checkout capability", commentId: 42, @@ -80,7 +83,7 @@ test("unauthorized, malformed, edited and premature commands cannot accept sizes test("all pending platform commands are processed when comment events are coalesced", async () => { const f = fixture(); - f.report.budgets.android = { aar: { softKiB: 100, hardKiB: 200 } }; + f.report.budgets.android = { aar: { measurement: "package", softKiB: 100, hardKiB: 200 } }; f.report.head["Android\trelease AAR"] = 150 * 1024; f.report.measuredPlatforms.push("android"); const android = { ...f.comment, id: 43, body: "/accept-size android Native support" }; From 9dd4451151b47ce48d6acd5be16b1a6bb21a589d Mon Sep 17 00:00:00 2001 From: Mark Murray Date: Fri, 2 Oct 2026 11:21:14 +0100 Subject: [PATCH 5/7] Use platform-neutral bundle measurement names --- .ci/bundle-size-budgets.json | 2 +- .ci/bundle-size-budgets.md | 8 +++++--- .github/scripts/bundle-size-budgets.cjs | 8 ++++---- .github/scripts/bundle-size-budgets.test.cjs | 18 +++++++++--------- .github/scripts/publish-bundle-size.test.cjs | 4 ++-- 5 files changed, 21 insertions(+), 19 deletions(-) diff --git a/.ci/bundle-size-budgets.json b/.ci/bundle-size-budgets.json index 5166016f7..ec3f9b0f1 100644 --- a/.ci/bundle-size-budgets.json +++ b/.ci/bundle-size-budgets.json @@ -1,7 +1,7 @@ { "web": { "javascript": { - "measurement": "shippedJavaScript", + "measurement": "bundle", "softKiB": 35, "hardKiB": 50 } diff --git a/.ci/bundle-size-budgets.md b/.ci/bundle-size-budgets.md index 88bae3c50..f2dd235f9 100644 --- a/.ci/bundle-size-budgets.md +++ b/.ci/bundle-size-budgets.md @@ -2,11 +2,13 @@ `.ci/bundle-size-budgets.json` defines named budgets for each platform. Each budget explicitly selects a `measurement` and may select a `file` within a package. +The platform adapter defines what its `bundle` contains; for web, that is all +shipped JavaScript under `dist`. Limits are KiB (1,024 bytes), including fractions. Comparisons use exact bytes, not the rounded numbers displayed in PR reports. Web starts with a 35 KiB soft limit and 50 KiB hard limit on all shipped JavaScript, -using `"measurement": "shippedJavaScript"`. This currently measures `dist/index.js` +using `"measurement": "bundle"`. This currently measures `dist/index.js` and will include any additional JavaScript chunks shipped in `dist`. Declarations, source maps, and other package contents are excluded from this measurement. The report also includes deterministic gzip size and package sizes. These remain @@ -56,8 +58,8 @@ for the updated report. Expired measurement artifacts also require a rerun. | Platform key | `measurement` | Scope | | --- | --- | --- | -| `web` | `shippedJavaScript` | Sum of raw shipped `.js`, `.mjs`, and `.cjs` files in `dist` | -| `web` | `shippedJavaScriptGzip` | Sum of those files compressed individually with `gzip -n -9` | +| `web` | `bundle` | Sum of raw shipped `.js`, `.mjs`, and `.cjs` files in `dist` | +| `web` | `bundleGzip` | Sum of those files compressed individually with `gzip -n -9` | | `web` | `package` | Whole compressed npm tarball | | `react-native` | `package` | Whole compressed wrapper npm tarball | | `android` | `package` | Whole compressed release AAR | diff --git a/.github/scripts/bundle-size-budgets.cjs b/.github/scripts/bundle-size-budgets.cjs index f37edf056..c438cf309 100644 --- a/.github/scripts/bundle-size-budgets.cjs +++ b/.github/scripts/bundle-size-budgets.cjs @@ -6,8 +6,8 @@ const platforms = { web: { label: "Web", measurements: { - shippedJavaScript: "JavaScript", - shippedJavaScriptGzip: "JavaScript (gzip)", + bundle: "JavaScript", + bundleGzip: "JavaScript (gzip)", package: "npm tarball", }, }, @@ -216,8 +216,8 @@ function render(rows, state, packageComment, notes = []) { const scope = row.file ? `${escape(row.file)} (uncompressed)` : { - shippedJavaScript: "All shipped JavaScript (raw)", - shippedJavaScriptGzip: "All shipped JavaScript (gzip)", + bundle: "Bundle (raw)", + bundleGzip: "Bundle (gzip)", package: "Whole package (compressed)", }[row.measurement]; lines.push( diff --git a/.github/scripts/bundle-size-budgets.test.cjs b/.github/scripts/bundle-size-budgets.test.cjs index d19b1824f..b6c6966e1 100644 --- a/.github/scripts/bundle-size-budgets.test.cjs +++ b/.github/scripts/bundle-size-budgets.test.cjs @@ -4,7 +4,7 @@ const policy = require("./bundle-size-budgets.cjs"); const KiB = 1024; const report = (size, base = 34 * KiB) => ({ - budgets: { web: { javascript: { measurement: "shippedJavaScript", softKiB: 35, hardKiB: 50 } } }, + budgets: { web: { javascript: { measurement: "bundle", softKiB: 35, hardKiB: 50 } } }, base: base === null ? {} : { "Web\tJavaScript": base }, head: { "Web\tJavaScript": size }, measuredPlatforms: ["web"], @@ -37,7 +37,7 @@ test("enforces exact limits, exempts no growth, and handles missing measurements test("platform acceptance covers each current breach, with independent caps", () => { const input = report(40 * KiB); input.budgets.web.javascriptGzip = { - measurement: "shippedJavaScriptGzip", + measurement: "bundleGzip", softKiB: 10, hardKiB: 15, }; @@ -90,7 +90,7 @@ test("comments and existing acceptances cannot override a hard budget", () => { assert.deepEqual(result.accepted, {}); assert.equal( policy.evaluate(input, { - "web.javascript": { bytes: 60 * KiB, measurement: "shippedJavaScript" }, + "web.javascript": { bytes: 60 * KiB, measurement: "bundle" }, })[0].status, "hard", ); @@ -123,7 +123,7 @@ test("selects whole packages or individual files without reusing acceptance for input.head["Web\tnpm tarball\tdist/missing.js"] = 0; assert.equal(policy.evaluate(input, accepted)[0].status, "within"); delete budget.file; - budget.measurement = "shippedJavaScript"; + budget.measurement = "bundle"; assert.equal(policy.evaluate(input, accepted)[0].status, "soft"); }); @@ -132,17 +132,17 @@ test("rejects unknown configuration keys and invalid limits", () => { { ios: {} }, { web: { javascript: { softKiB: 35, hardKiB: 50 } } }, { web: { typo: { measurement: "unknown", softKiB: 1, hardKiB: 2 } } }, - { web: { javascript: { measurement: "shippedJavaScript", soft: 35, hard: 50 } } }, - { web: { javascript: { measurement: "shippedJavaScript", softKiB: "35", hardKiB: 50 } } }, - { web: { javascript: { measurement: "shippedJavaScript", softKiB: 51, hardKiB: 50 } } }, - { web: { javascript: { measurement: "shippedJavaScript", softKiB: 0, hardKiB: 50 } } }, + { web: { javascript: { measurement: "bundle", soft: 35, hard: 50 } } }, + { web: { javascript: { measurement: "bundle", softKiB: "35", hardKiB: 50 } } }, + { web: { javascript: { measurement: "bundle", softKiB: 51, hardKiB: 50 } } }, + { web: { javascript: { measurement: "bundle", softKiB: 0, hardKiB: 50 } } }, ...["", "/index.js", "../index.js", "dist/*.js", "dist\\index.js", 42].map((file) => ({ web: { entry: { measurement: "package", file, softKiB: 35, hardKiB: 50 } }, })), { web: { entry: { - measurement: "shippedJavaScript", + measurement: "bundle", file: "dist/index.js", softKiB: 35, hardKiB: 50, diff --git a/.github/scripts/publish-bundle-size.test.cjs b/.github/scripts/publish-bundle-size.test.cjs index 629f61233..4523d9d9c 100644 --- a/.github/scripts/publish-bundle-size.test.cjs +++ b/.github/scripts/publish-bundle-size.test.cjs @@ -10,7 +10,7 @@ function fixture(permission = "write") { headSha: "head", baseSha: "base", budgets: { - web: { javascript: { measurement: "shippedJavaScript", softKiB: 35, hardKiB: 50 } }, + web: { javascript: { measurement: "bundle", softKiB: 35, hardKiB: 50 } }, }, base: { "Web\tJavaScript": 34000 }, head: { "Web\tJavaScript": 40000 }, @@ -54,7 +54,7 @@ test("writer acceptance records the actor, reason and size, and is processed onl const result = await f.process(); assert.deepEqual(result.acceptances["web.javascript"], { bytes: 40000, - measurement: "shippedJavaScript", + measurement: "bundle", actor: "writer", reason: "New checkout capability", commentId: 42, From 058f48b0b1b6889c63f2b81c8a10ae6089ed2705 Mon Sep 17 00:00:00 2001 From: Mark Murray Date: Fri, 2 Oct 2026 11:27:44 +0100 Subject: [PATCH 6/7] Clarify bundle and package compression in size reports --- .github/scripts/bundle-size-budgets.cjs | 17 +++++++--- .github/scripts/measure-package-size | 41 ++++++++++++++++++++----- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/.github/scripts/bundle-size-budgets.cjs b/.github/scripts/bundle-size-budgets.cjs index c438cf309..d690944b1 100644 --- a/.github/scripts/bundle-size-budgets.cjs +++ b/.github/scripts/bundle-size-budgets.cjs @@ -5,14 +5,23 @@ const fs = require("node:fs"); const platforms = { web: { label: "Web", + packageLabel: "Whole npm package (gzip)", measurements: { bundle: "JavaScript", bundleGzip: "JavaScript (gzip)", package: "npm tarball", }, }, - "react-native": { label: "React Native", measurements: { package: "npm tarball" } }, - android: { label: "Android", measurements: { package: "release AAR" } }, + "react-native": { + label: "React Native", + packageLabel: "Whole npm package (gzip)", + measurements: { package: "npm tarball" }, + }, + android: { + label: "Android", + packageLabel: "Whole AAR package (ZIP)", + measurements: { package: "release AAR" }, + }, }; const marker = ""; const statePattern = //; @@ -216,9 +225,9 @@ function render(rows, state, packageComment, notes = []) { const scope = row.file ? `${escape(row.file)} (uncompressed)` : { - bundle: "Bundle (raw)", + bundle: "Bundle (uncompressed)", bundleGzip: "Bundle (gzip)", - package: "Whole package (compressed)", + package: platforms[row.platform].packageLabel, }[row.measurement]; lines.push( `| ${row.platform} / ${row.metric} | ${scope} | ${kib(row.before)} | ${kib(row.after)} | ${delta} | ${row.softKiB} KiB | ${row.hardKiB} KiB | ${status} |`, diff --git a/.github/scripts/measure-package-size b/.github/scripts/measure-package-size index d13ab5915..13915765f 100755 --- a/.github/scripts/measure-package-size +++ b/.github/scripts/measure-package-size @@ -202,6 +202,26 @@ render_comment() { return (bytes == "") ? "—" : human(bytes); } + function measurement(artifact) { + if (artifact == "JavaScript" || artifact == "JavaScript (gzip)") { + return "JavaScript bundle"; + } + if (artifact == "npm tarball") { + return "npm package (`.tgz`)"; + } + if (artifact == "release AAR") { + return "Library package (`.aar`)"; + } + return artifact; + } + + function compression(artifact) { + if (artifact == "JavaScript") return "Uncompressed"; + if (artifact == "JavaScript (gzip)" || artifact == "npm tarball") return "gzip"; + if (artifact == "release AAR") return "ZIP"; + return "Unspecified"; + } + function delta(head, base, diff) { if (head == "" || base == "") { return "unavailable"; @@ -285,19 +305,22 @@ render_comment() { cap = 20; print ""; - print "## Package Size"; + print "## Bundle and package size"; print ""; - print "| Platform | Artifact | Base | Head | Delta |"; - print "| --- | --- | ---: | ---: | ---: |"; + print "Web bundle sizes cover shipped runtime JavaScript. Package sizes cover the full published archive, including any source maps, declarations, and documentation it contains."; + print ""; + print "| Platform | Measurement | Compression | Base | Head | Delta |"; + print "| --- | --- | --- | ---: | ---: | ---: |"; if (order_count == 0) { - print "| - | - | - | - | - |"; + print "| - | - | - | - | - | - |"; } else { for (i = 1; i <= order_count; i++) { key = order[i]; - printf "| %s | %s | %s | %s | %s |\n", + printf "| %s | %s | %s | %s | %s | %s |\n", platforms[key], - artifacts[key], + measurement(artifacts[key]), + compression(artifacts[key]), human(base_bytes[key]), human(head_bytes[key]), delta(head_bytes[key], base_bytes[key]); @@ -332,7 +355,9 @@ render_comment() { } print ""; - print "
" platform " file breakdown"; + print "
" platform " package files (uncompressed)"; + print ""; + print "These are uncompressed file sizes; they do not sum to the compressed package size above."; print ""; print "| File | Base | Head | Delta |"; print "| --- | ---: | ---: | ---: |"; @@ -355,7 +380,7 @@ render_comment() { } print ""; - print "_Measured from the PR base SHA and PR head SHA. The file breakdown shows uncompressed sizes within each package artifact, so individual files do not sum to the compressed artifact total. JavaScript rows sum shipped runtime files; gzip uses deterministic per-file compression. Package artifacts are not final app binary sizes._"; + print "_Measured from the PR base SHA and PR head SHA. Web bundle rows sum shipped `.js`, `.mjs`, and `.cjs` files under `dist/`, excluding source maps and declarations. The gzip bundle size sums files compressed individually with `gzip -n -9`. npm package sizes are gzip-compressed `.tgz` archives; Android AAR sizes are ZIP archives. Package sizes are not final app binary sizes._"; } ' "$base_file" "$head_file" > "$comment_file" } From 948cefd90e9b6f61da0d279210d8779e79de8c03 Mon Sep 17 00:00:00 2001 From: Mark Murray Date: Fri, 2 Oct 2026 11:32:32 +0100 Subject: [PATCH 7/7] Retain bundle size reports for 90 days --- .github/workflows/package-size.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/package-size.yml b/.github/workflows/package-size.yml index 8ef0e869c..2787ef269 100644 --- a/.github/workflows/package-size.yml +++ b/.github/workflows/package-size.yml @@ -173,4 +173,4 @@ jobs: name: bundle-size-report path: /tmp/bundle-size-report/ if-no-files-found: error - retention-days: 30 + retention-days: 90