refactor(ipc-codegen): keep the generator to erasable TypeScript - #25403
Open
charlielye wants to merge 1 commit into
Open
refactor(ipc-codegen): keep the generator to erasable TypeScript#25403charlielye wants to merge 1 commit into
charlielye wants to merge 1 commit into
Conversation
generate.ts advertises "runs with Node.js 22+ via --experimental-strip-types", but it could not: three parameter properties made Node reject it at module load with ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX, which takes out every --lang at once rather than only the TypeScript backend. Nothing noticed because all eleven call sites also pass --experimental-transform-types, which compiles those constructs, so the documented invocation was never exercised -- the one test that does run strip-only imports only schema_visitor.ts and never reaches generate.ts. Expanding the three into an explicit field plus an assignment in the constructor body -- what the transform emitted anyway -- makes the source erasable, so the transform flag comes off all eleven call sites and the generator runs under the type stripping that Node 23.6+ does by default. Generated output is byte-identical for all four languages, checked against the previous invocation. Keeping this property needs a test, since the failure mode is invisible while any caller passes the transform flag: scripts/strip_only_test.sh generates one of each --lang strip-only, and is wired into test_cmds. Reintroducing a parameter property fails it with the ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX above. --experimental-strip-types is kept even though it is a no-op on the pinned Node (.nvmrc is v24.15.0): it is what the Node 22 the header mentions still needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016einpgfthfjLYGwCB3iQqD
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ipc-codegen/src/generate.tssays:It could not. Three parameter properties made Node reject the module at load:
generate.tsimportstypescript_package_codegen.tsunconditionally, so this broke every--lang, not justts.Nothing noticed because all eleven call sites also pass
--experimental-transform-types, which compiles parameter properties. The documented invocation was never exercised: the only test that runs strip-only isschema_visitor.test.ts, whose sole local import isschema_visitor.ts— it never reachesgenerate.ts.Change
Expanded the three into an explicit field plus an assignment in the constructor body — what the transform emitted anyway:
That makes the source erasable, so
--experimental-transform-typescomes off all eleven call sites (3 CMakeLists,barretenberg/ts/bootstrap.sh,wsdb/bootstrap.sh, 5 echo_example bootstraps, the README). The generator now runs under the type stripping Node 23.6+ does by default.--experimental-strip-typesis kept: it is a no-op on the pinned Node (.nvmrcisv24.15.0) but is still required by the Node 22 the header mentions.A fourth parameter property at
typescript_package_codegen.ts:332is inside a template literal — emitted code, not source — so it is untouched.Verification
--experimental-strip-types --experimental-transform-types) overbb_schema.json. This is a no-op for consumers.--langvalues generate with the transform flag gone.ipc-codegen/test/schema_visitor.test.tspasses;echo_example/rust/bootstrap.sh(a real modified call site) generates fine.enum,const enum, andnamespaceare rejected; interfaces, type aliases, annotations, generics andimport typeare fine.Guard
The failure mode is invisible while any caller passes the transform flag, so the property needs a test.
ipc-codegen/scripts/strip_only_test.shgenerates one of each--langstrip-only and is wired intotest_cmds. Red/green checked — reintroducing a parameter property fails it:The stronger guard would be
erasableSyntaxOnlyin a tsconfig, which fails at typecheck rather than at load. Not done here:ipc-codegenhas notsconfig.jsonorpackage.json(the "zero npm dependencies" claim is literal), and the repo pins TypeScript^5.3.3while that flag landed in 5.8.🤖 Generated with Claude Code
https://claude.ai/code/session_016einpgfthfjLYGwCB3iQqD