Conversation
The build script shared crates/trusted-server-js/dist with every other cargo build and with manual npm runs, so overlapping builds could embed a partial or empty bundle set and still exit 0. build-all.mjs now accepts --out-dir, and build.rs builds into OUT_DIR/tsjs-dist and fails unless it holds exactly core plus every lib/src/integrations/<id>/index.ts, each non-empty. Silent reuse of dist is gone: TSJS_SKIP_BUILD and a missing npm now fail with instructions, and TSJS_PREBUILT_DIR embeds prebuilt bundles after the same check. Stale node_modules fails instead of reinstalling, npm ci on a missing node_modules is serialized with a file lock, TSJS_TEST failures fail the build, rerun-if-env-changed covers every variable read, and rerun-if-changed is narrowed to the sources. Fixes #1200 Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
Cargo reruns a build script on every invocation while a watched path is missing, so always watching lib/node_modules/.package-lock.json made every build rerun under TSJS_PREBUILT_DIR without node_modules. Watch it only on the npm build path, after the freshness check has confirmed it exists. Watch lib/test and lib/vitest.config.ts when TSJS_TEST=1 so edited tests rerun, and note in the error reference that the timestamp-based freshness check also fires when a checkout rewrites an unchanged lockfile. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
The build script now writes bundles to OUT_DIR/tsjs-dist, so the GPT module lookup in template-cache-local-test.sh must search out/tsjs-dist/tsjs-gpt.js. Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
ChristianPavilonis
left a comment
There was a problem hiding this comment.
Summary
The private OUT_DIR bundle builds and completeness validation fix the original concurrency race. Concurrent debug and release builds produced separate complete 13-bundle sets, and an incomplete prebuilt set failed before code generation. I found one medium-severity recovery issue, included inline.
| .lock() | ||
| .unwrap_or_else(|err| panic!("tsjs: failed to lock {}: {err}", lock_path.display())); | ||
|
|
||
| if node_modules.exists() { |
There was a problem hiding this comment.
P2: A failed automatic npm ci leaves retries stuck on its partial directory
Issue: npm ci creates node_modules before installation finishes. If it then exits unsuccessfully, the assertion below panics without removing that partial directory. The next Cargo build acquires the lock, sees that node_modules exists here, and skips npm ci.
Impact: A transient install failure, or a concurrent build waiting behind it, cannot recover through the documented automatic installation path. It instead fails later with the misleading stale or missing hidden-lockfile error until someone manually removes or repairs node_modules.
Evidence: In a temporary npm fixture whose dependency install script exits 7, npm ci --foreground-scripts exited 7 while leaving node_modules present and node_modules/.package-lock.json absent. The next invocation returns here, then ensure_dependencies_fresh rejects the incomplete tree.
Suggested fix: While holding the lock, remove node_modules when the spawned npm ci fails before reporting the error. Add a fake-npm regression test that creates node_modules, exits nonzero, and verifies the next invocation attempts installation again.
Summary
crates/trusted-server-js/dist, whichbuild-all.mjsdeletes and rewrites. Overlapping builds (dev + release in one target dir, two target dirs, or a manualnpm run build) could exit 0 having embedded a partial or empty bundle set, including nocore. The build script now builds into a privateOUT_DIR/tsjs-distand never readsdist.build.rsderives the expected module set the same waybuild-all.mjsdoes (core+ everylib/src/integrations/<id>/index.ts) and fails on any missing, empty or unexpected bundle.distare gone:TSJS_SKIP_BUILDand a missingnpmfail with instructions, and the newTSJS_PREBUILT_DIRembeds prebuilt bundles after the same check.Changes
crates/trusted-server-js/lib/build-all.mjs--out-dir <dir>; default stays../dist, sonpm run build, Playwright and CI are unchangedcrates/trusted-server-js/build/bundle_set.rscheck_bundle_set(missing / empty / unexpected), with unit testscrates/trusted-server-js/build.rsOUT_DIR/tsjs-distand validate before codegen;TSJS_PREBUILT_DIRpath;TSJS_SKIP_BUILDand missingnpmfail with instructions; stalenode_modules(hidden lockfile older thanpackage-lock.json) fails instead of reinstalling;npm cion a missingnode_modulesserialized with a file lock;TSJS_TESTfailures fail the build;rerun-if-env-changedfor every variable read;rerun-if-changednarrowed from ~34k paths to the sources and lockfilescrates/trusted-server-js/src/lib.rsbundle_set.rsunder#[cfg(test)]so its tests run with the cratecrates/trusted-server-js/lib/.gitignorenpm cilock filescripts/template-cache-local-test.shout/tsjs-dist/pathdocs/guide/error-reference.mdTSJS_SKIP_BUILDtip withTSJS_PREBUILT_DIR; document each new build-script errorcrates/trusted-server-js/README.md,docs/guide/creative-processing.mdOUT_DIRanddistis only written bynpm run buildBehavior changes
TSJS_SKIP_BUILD=1now fails; useTSJS_PREBUILT_DIR=<dir with tsjs-*.js>.package-lock.json(for example switching branches), the build asks fornpm ciinstead of building against out-of-date dependencies.node_modulesis still installed automatically (the Axum, Cloudflare, Spin and clippy CI jobs rely on this), but a failednpm cinow fails the build.Coordination
#1180 and #855 also touch the code
build.rsgenerates. This PR changes only theinclude_str!path in that output (/tsjs-dist/tsjs-<id>.js), so rebasing either should be small.Closes
Closes #1200
Test plan
cargo test-fastly && cargo test-axumcargo clippy-fastly && cargo clippy-axumcargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest runcd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute servemainand this branch on the same machinemainnpm run buildloops ondistnode_modulesmissing, second started mid-npm cinpm ciAlso checked by hand:
TSJS_SKIP_BUILD=1, nonpmonPATH, stalenode_modules, and aTSJS_PREBUILT_DIRwith one missing and one empty bundle each fail with the documented message; a validTSJS_PREBUILT_DIRbuilds; a no-change rebuild stays fresh.Checklist
unwrap()in production code — useexpect("should ...")tracingmacros (notprintln!)