Repository navigation
Embed the registry and the version in the bundle (#107) - #129
Conversation
The registry and the package version were read from files beside the module at run time. A library re-bundled into another application looked for registry/vaults.json beside the consumer's own bundle and read whatever was there. src/registry.ts and src/version.ts now import registry/vaults.json and package.json with JSON import attributes, so tsup and a consumer's bundler inline them. treasury --version reports the version the bundle was built from, and a registry change now rebuilds the bundle. - The boundary audit allows exactly these two JSON imports, by importing file and specifier; any other JSON import, or either of these from another file, still fails. - The broken-registry tests build the CLI from source with the registry import pointed at a broken copy, instead of swapping a file beside the bundle. - New tests run the committed bundle alone in a folder, with decoy registry and package.json files beside it, and a consumer's re-bundle of dist/index.js with a decoy registry beside that. - The comments that described the run-time read are corrected. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Kevin Chen <13785749+Kevin-Chen0@users.noreply.github.com>
Kevin-Chen0
left a comment
There was a problem hiding this comment.
Review at 5ecc316bdae6d2a779d39156a92aeb7aca8c1db9: changes requested, one finding.
Blocking
The CLI bundle now embeds the whole package.json, so a dev-dependency bump makes dist/treasury.mjs stale. import pkg from "../package.json" inlines the entire object, including scripts and devDependencies (the prepack script text is in the bundle; on release/v0.1.3 it is not). Measured: changing only devDependencies["@types/node"] and re-running tsup changes dist/treasury.mjs on this head and does not on the base. CI's "bundled CLI is current" step would then fail every dev-only dependency PR from Dependabot, which cannot rebuild the bundle.
Suggested fix: embed only the version in the bundle. For example, a small esbuild plugin in tsup.config.ts that loads package.json as export default { version }, or a define. Add a test that the bundle does not contain devDependencies or a dev-dependency name. The library build can keep the import as is, since it is not bundled.
Not blocking
On Node 22.0 to 22.11, importing the library prints ExperimentalWarning: Importing JSON modules is an experimental feature to stderr. It still works (7 vaults returned on 22.0.0 and 22.11.0). The CLI is unaffected because it inlines both files (--version is clean on 22.0.0). Either raise engines.node to >=22.12, or note it in the CHANGELOG entry.
Checked and fine
- The fix for #59 works. Both packages were packed and installed into an empty app, which was re-bundled with esbuild and run from a folder holding a decoy
registry/vaults.json:release/v0.1.3: reads the decoy and fails schema validation, and fails with ENOENT when no file is beside it.- This head: returns the package's own 7 vaults in both cases.
- Library emit.
tsckeepsimport … with { type: "json" }indist/registry.jsanddist/version.js. Fromdist/these resolve to the package's ownregistry/vaults.jsonandpackage.json, and both are in the published files. Annpm installof the packed tarball imports cleanly on Node 22.22. - The JSON allowlist is exact by importing file and specifier, and its negative controls cover the wrong importer, the wrong file, and outside the package.
- The broken-registry tests build from the same source and settings with only the embedded registry swapped, so they still prove what the file-swap tests proved.
- The suite and build. 456 passed, the build leaves the tree clean, and CI is green.
Importing package.json inlined its scripts and devDependencies into
dist/treasury.mjs, so a dev-only dependency bump (which Dependabot makes
without rebuilding) would have made the committed bundle stale.
tsup.config.ts now serves the root package.json to the bundler as
{ version }. The broken-registry test builds use the same plugin. A new
test checks the bundle carries no devDependency name and no script text.
The changelog notes the ExperimentalWarning that Node 22.0 to 22.11 print
when the library is imported.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Signed-off-by: Kevin Chen <13785749+Kevin-Chen0@users.noreply.github.com>
Importing package.json for its version inlined the whole manifest into
dist/treasury.mjs, scripts and devDependencies included, so any edit to
package.json (a dev-dependency bump) made the committed bundle stale and
failed the bundle check. An esbuild plugin in tsup.config.ts now serves
the root package.json to the bundle as { version } only.
A test holds the committed bundle to that: it contains the version, no
devDependencies, and no package.json script. Measured: a dev-dependency
and script edit leaves the bundle byte-identical; a version change still
reaches `treasury --version`.
The CHANGELOG also notes Node's ExperimentalWarning for JSON modules when
the library is imported unbundled on Node 22.0 to 22.11.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Kevin Chen <13785749+Kevin-Chen0@users.noreply.github.com>
Kevin-Chen0
left a comment
There was a problem hiding this comment.
Signed off at 75b20875466084d68d72fb554a7c441861ccc305.
What each review covers: this head has two parts, each reviewed by the reviewer who did not write it.
5ecc316b, the registry and version embedding: reviewed above. The decoy-registry re-bundle, the library emit and the JSON allowlist all pass.75b20875, the fix for the blocking finding: reviewed and re-run by the PR's original author. It embeds only{ version }frompackage.jsonin the CLI bundle, so a dev-dependency bump leavesdist/unchanged, while a version bump still reachestreasury --version.
The blocking finding is resolved, and the non-blocking Node 22.0–22.11 warning is noted in the CHANGELOG.
Embeds the vault registry and the package version in the bundle, so nothing is read from beside it (option B of the design note in
docs/design-embed-registry-and-version.md). Closes #107.What changes
src/registry.tsandsrc/version.tsimportregistry/vaults.jsonandpackage.jsonwithwith { type: "json" }instead of reading them from the module's directory.tsupand a consumer's bundler inline them;tsc -bemits the import unchanged.treasury --versionreports the version the bundle was built from. A registry change now also changesdist/treasury.mjsand the plugin copy, so this PR carries the rebuilt bundle.Tests
tests/boundary.unit.test.ts: aJSON_IMPORTS_ALLOWEDmap names the two imports by importing file and exact specifier. Both reach audits use it. A control test checks each entry is really imported, and that another JSON file, the same JSON from another file, and a path outside the package are refused.tests/entrypoint.unit.test.ts: the broken-registry cases now build the CLI from source with the registry import pointed at a broken copy (sametsupsettings). They keep what they proved: the CLI starts, and the command names the broken rule.tests/embedded.unit.test.ts(new): the committed bundle alone in a folder reports its version and the shipped registry; with decoyregistry/vaults.jsonandpackage.jsonbeside it and its parent, it ignores them; a consumer's bundle ofdist/index.jslists the shipped registry with a decoy beside it.npm test: 456 passed, 40 skipped.npm run buildleaves the tree as committed;check-versions,sync-plugin --check,notices:checkand the offline.github/scripts/*.test.shpass.Not in this PR
Removing
plugin/registry/vaults.jsonandplugin/package.json(the bundle no longer needs them) changesscripts/sync-plugin.sh,scripts/check-versions.shand their offline tests, so it is a separate PR.Known consequences
tests/embedded.unit.test.tsneedsdist/index.js, whichnpm testbuilds first; barevitestsays so in its failure message.