fix(txe): emit the CLI bundle at a path tsc-only rebuilds cannot clobber - #38
fix(txe): emit the CLI bundle at a path tsc-only rebuilds cannot clobber#38vezenovm wants to merge 5 commits into
Conversation
Node 24 rejects dynamic imports of JSON modules without
`with { type: 'json' }`, so the TXE crashed at startup with
ERR_IMPORT_ATTRIBUTE_MISSING when dispatcher_pool loaded the Schnorr
account artifact through @aztec/accounts/schnorr/lazy. The static
imports already carry the attribute; this adds it to the lazy dynamic
imports as well.
…t imports" This reverts commit ee340c8.
esbuild used to overwrite the swc-emitted dest/bin/index.js, so a tsc-only rebuild (e.g. `yarn build` at the yarn-project root) re-emitted the unbundled entrypoint over the bundle. The unbundled tree reaches dynamic JSON imports that Node's ESM loader rejects (ERR_IMPORT_ATTRIBUTE_MISSING via @aztec/accounts/schnorr/lazy in dispatcher_pool), crashing TXE at startup. Emit the CLI bundle as dest/bin/index.bundle.js instead — a path tsc never writes, matching worker.bundle.js and server.bundle.js — and point bin, start/dev scripts, the publish files list, the size guard, and the root bootstrap TXE launcher at it. Also reword the stale comments on the lazy JSON imports: "cannot assert this import" used the pre-attributes terminology, and the attribute cannot be added because vite's dev server serves JSON as a JS module and only strips attributes from static imports, leaving these imports Node-incompatible unless bundled.
vezenovm
changed the title
A root-only `yarn build` (tsgo + swc, no package esbuild steps) leaves the esbuild bundles — including worker.bundle.js, where foreign calls actually execute — stale. Rebundling takes ~150ms, negligible against TXE cold start, so run esbuild.config.mjs before launching in the start and dev scripts.
…he attribute syntax
Say `with { type: 'json' }` explicitly instead of "assert this import" so
the comment names the actual syntax that can't be added, without otherwise
changing the reasoning.
nchamo
left a comment
nchamo
left a comment
There was a problem hiding this comment.
This is not my strong suit, so the review is mostly AI-driven. I would be more comfortable if we got another set of eyes on this PR
| } | ||
| }, | ||
| "bin": "./dest/bin/index.js", | ||
| "bin": "./dest/bin/index.bundle.js", |
There was a problem hiding this comment.
yarn.lock still has txe: ./dest/bin/index.js. Should we commit the regenerated lockfile? yarn install is immutable in CI, so it'd fail on the first node_modules cache miss rather than here.
| port=$((txe_base_port + i)) | ||
| kill_port $port | ||
| dump_fail "LOG_LEVEL=info TXE_PORT=$port retry 'node --no-warnings ./yarn-project/txe/dest/bin/index.js'" & | ||
| dump_fail "LOG_LEVEL=info TXE_PORT=$port retry 'node --no-warnings ./yarn-project/txe/dest/bin/index.bundle.js'" & |
There was a problem hiding this comment.
This one doesn't go through yarn start, so it misses the rebundle you added. After a root-only yarn build it runs whatever the last esbuild left behind, and after a yarn clean the file isn't there at all. Should start_txes bundle first too?
| // yarn-project root) must not be able to replace a bundle with unbundled output, because the | ||
| // unbundled tree reaches dynamic JSON imports that Node's ESM loader rejects | ||
| // (ERR_IMPORT_ATTRIBUTE_MISSING) — TXE only runs correctly from the bundles. | ||
| 'bin/index.bundle': 'src/bin/index.ts', |
There was a problem hiding this comment.
Now that esbuild stops overwriting it, swc's dest/bin/index.js sticks around as a broken copy of the CLI. Should we stop emitting it, so the old path fails as a missing file instead of a confusing crash?
| }, | ||
| "files": [ | ||
| "dest/bin/index.js", | ||
| "dest/bin/index.bundle.js", |
There was a problem hiding this comment.
"dest" on line 109 already publishes the whole directory. Should we drop the redundant entries rather than keep renaming them?
| */ | ||
| export async function getStubSchnorrAccountContractArtifact() { | ||
| // Cannot assert this import as it's incompatible with bundlers like vite | ||
| // Cannot add `with { type: 'json' }` to this import as it's incompatible with bundlers like vite |
There was a problem hiding this comment.
Nit: the generator that emits this same block was missed — generate_client_artifacts_helper.ts:100 and :150 still say "Cannot assert this import". They're the last two copies of the old wording left.
| // https://github.com/vitejs/vite/issues/19095#issuecomment-2566074352 | ||
| // Even if now supported by all major browsers, the MIME type is replaced with | ||
| // "text/javascript" | ||
| // In the meantime, this lazy import is INCOMPATIBLE WITH NODEJS |
There was a problem hiding this comment.
@aztec/noir-protocol-circuits-types is external, so its client/lazy JSON imports still reach Node's loader unbundled — same error, just not called today. Should the generator emit with { type: 'json' }? One change covers ~150 sites.
| * inside `dest/`) or bundled into `dest/bin/index.js` (one directory deeper). `import.meta.url` | ||
| * refers to whichever module the calling code actually lives in; we try both relative locations | ||
| * and use whichever exists. | ||
| * inside `dest/`) or bundled into `dest/bin/index.bundle.js` (one directory deeper). |
There was a problem hiding this comment.
With splitting on, this code ends up in a dest/chunk-*.js at the dest/ root rather than dest/bin/, so the ../worker.bundle.js candidate never wins. Is it still needed?
2 participants
Problem
TXE crashed at startup with
ERR_IMPORT_ATTRIBUTE_MISSINGloading the Schnorr artifact via@aztec/accounts/schnorr/lazy.Root cause: a build-artifact collision, not a missing import attribute. TXE only runs correctly from its esbuild bundles — esbuild resolves dynamic JSON imports at build time, so Node's loader never sees them. But the CLI bundle was emitted at
dest/bin/index.js, the same path swc writes the unbundled entrypoint to. A root-onlyyarn build(tsgo + swc, no package esbuild) overwrote the bundle with unbundled output, which does hit Node's loader. CI never sees this because bootstrap always runs each package's fullbuildscript.Why not add
with { type: 'json' }insteadRemoved on purpose in AztecProtocol/aztec-packages#12352: vite's dev server serves JSON as
text/javascriptand only strips import attributes from static imports, so an attributed dynamic import fails the browser's MIME check (vitejs/vite#19095, still open). The playground and external wallet apps consume these lazy entrypoints through vite, so they stay browser-only by design.Fix
dest/bin/index.bundle.js(matchesworker.bundle.js/server.bundle.js) — a path swc never writes — and repointbin,start/dev, thefileslist, the size guard, and the bootstrap TXE launcher.start/dev(~150ms) so a root-onlyyarn buildcan never leave a launched TXE on stale bundles — also fixesworker.bundle.jssilently going stale in that case.