From 5ac03814f8313fc13c74e4175d11524e79f8ec87 Mon Sep 17 00:00:00 2001 From: rejifald Date: Fri, 7 Aug 2026 18:49:57 +0300 Subject: [PATCH] fix(repo): the size gate's externals match what the build actually emits (#709) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:size` re-bundles each scenario with `external: ['node:*']`. That pattern matches nothing in the package's own artifacts. The source is correct — `src/registry.ts` imports `node:fs`/`node:path`/`node:url` — but tsup strips the prefix, so `lib/` emits `from"fs"`, `from"path"`, `require("fs/promises")`. Across the 49 shipped JS files, zero static import or require specifiers carry a `node:` prefix; the only surviving `node:` strings sit inside `process?.getBuiltinModule?.("node:fs")`, a runtime lookup no bundler resolves. The gate passes today only because its three measured entries reach for no builtin at all, so the inert pattern has never been exercised. Point it at an entry that does and it fails outright: bundling `stitchapi/registry` under the gate's own externals reports `Could not resolve "fs"`, `"path"` and `"url"`. Under the bare specifiers it succeeds at 5593 B. The shadowing risk is real rather than theoretical. With trivial packages named `fs`, `path` and `url` planted in a `node_modules` beside a copy of `lib/`, a `platform: 'neutral'` build with `external: ['node:*']` succeeded and inlined the shadow package's body into the artifact — a measurement of something other than the library. A `node:`-prefixed specifier cannot be shadowed that way. So the list gains the bare specifiers the build actually emits. The prefixed forms stay: they cost nothing and keep this working if the build is ever changed to preserve `node:`, which is the better fix for the shadowing hazard and is left as a separate change with its own blast radius. A builtin the list misses still fails loudly rather than measuring the wrong bytes. No measured number moves — `--json` output is byte-identical before and after, so the `bundle-advertised-size` tether and the READMEs are untouched. Verified end-to-end by adding a temporary `registry` scenario: it measures 5.46 KB where it previously could not build at all. `Refs`, not `Closes`: this is §2 of the issue only. §1 — the gate's missing `splitting: true` — stays open. It is not a one-liner, because `measure()` reads `result.outputFiles[0]` and assumes a single output, so it needs an `outdir` plus chunk summing, and it moves an advertised figure the tether cross-checks against the READMEs and docs. That is a publishing decision, not a bug fix. Refs #709 Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 6 ++++++ packages/core/scripts/bundle-size.mjs | 18 +++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cbc0b623..325edfe9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1185,6 +1185,12 @@ npm release are grouped under the in-development version that introduced them. variable. See ADR 0005 Decision 1's addendum for why this was deferred and what implementing it would take; relaxing the guard later is non-breaking. +- **The bundle-size gate's externals now match what the build actually emits.** + ([#709](https://github.com/rejifald/StitchAPI/issues/709)) `check:size` externalised `node:*`, but + `tsup` strips the prefix — `lib/` emits bare `from"fs"` — so the pattern matched nothing, and + measuring a builtin-using entry failed outright with `Could not resolve "fs"`. No measured or + advertised number moves; the fix is repo tooling only. + ## [1.0.0-rc.7] — 2026-08-01 ### Added diff --git a/packages/core/scripts/bundle-size.mjs b/packages/core/scripts/bundle-size.mjs index 3ae98034..3eaea4f6 100644 --- a/packages/core/scripts/bundle-size.mjs +++ b/packages/core/scripts/bundle-size.mjs @@ -335,7 +335,23 @@ function measure(code) { format: 'esm', treeShaking: true, platform: 'neutral', - external: ['node:*'], // zero deps; the root entry is browser-safe + // Node builtins, in BOTH spellings — the package has zero deps, so builtins are + // the only thing that is ever external and the root entry stays browser-safe. + // + // The bare forms are not belt-and-braces: they are the ones that actually match. + // The source writes the prefix (`src/registry.ts` imports `node:fs`/`node:path`/ + // `node:url`) but tsup STRIPS it, so `lib/` emits `from"fs"`, `require("path")`. + // Across the shipped files the only surviving `node:` string is inside + // `process?.getBuiltinModule?.("node:fs")` — a runtime lookup no bundler resolves. + // `node:*` alone therefore matched NOTHING here: measuring a builtin-using entry + // (`stitchapi/registry`) failed outright with `Could not resolve "fs"`, and a bare + // `fs` left non-external can be shadowed by a stray `node_modules/fs` and silently + // inlined into the measurement. The prefixed forms are kept so this keeps working + // if the build is ever changed to preserve `node:` (the better fix for shadowing). + // + // A builtin the list misses fails loudly rather than measuring something wrong — + // extend it when the artifacts start reaching for a new one. + external: ['node:*', 'fs', 'fs/promises', 'path', 'url', 'http'], write: false, logLevel: 'silent', });