feat(wasm): add @swc/nodejs-support-wasm#11975
Conversation
![kodiakhq[bot]](https://avatars.githubusercontent.com/in/29196?s=60&v=4){kind=small}
🦋 Changeset detectedLatest commit: 199b340 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cea0b7a1f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Merging this PR will degrade performance by 2.19%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | es/oxc/benches/assets/UserSettings.tsx/sourceMap=true/reactDev=false |
1.2 ms | 1.2 ms | -2.19% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing kdy1/nodejs-wasm-typescript-api (199b340) with main (9609b7f)
Footnotes
-
61 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48364c0d92
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6889ea8440
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28709ed22f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1de0d8e0f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.edits.push(Edit { | ||
| start, | ||
| end, | ||
| text: import_binding_access(binding), | ||
| }); |
There was a problem hiding this comment.
When a named or default import is called directly, e.g. import { check } from "./m.js"; check(), native module semantics call the function as a bare binding with this === undefined. This replacement turns the callee into a member expression like __nodeREPLImport0.check(), so any exported function that observes this receives the module namespace object instead; callee positions need a form such as (0, __nodeREPLImport0.check) or other context-aware handling.
Useful? React with 👍 / 👎.
| fn visit_assign_expr(&mut self, node: &AssignExpr) { | ||
| if self.import_binding_for_assign_target(&node.left).is_some() { | ||
| self.replace_expression_with_import_assignment_error(node.span); | ||
| return; |
There was a problem hiding this comment.
For inputs where a loop assigns to an imported binding, such as import { x } from "m"; for (x of values) {}, native module execution throws when the loop target writes to the immutable import binding. This visitor only replaces AssignExpr/UpdateExpr targets, while for...of/for...in left-hand patterns are not handled here, so the transformed script deletes the import and leaves a bare x loop target that can write an outer/global binding instead of failing.
Useful? React with 👍 / 👎.
| ModuleDecl::ExportNamed(export) => { | ||
| self.delete_span(export.span); |
There was a problem hiding this comment.
For local export lists without a from clause, e.g. export { missing };, native module linking fails because missing is not a declared local binding; the parser code only checks duplicate export names, so this branch deletes the statement and reports success. That makes invalid module input execute as an empty script instead of surfacing the missing-local-export error, so local named exports need binding validation before being dropped.
Useful? React with 👍 / 👎.
| with: Option<&ObjectLit>, | ||
| required_exports: &[String], | ||
| ) -> String { | ||
| let mut out = format!("{DYNAMIC_IMPORT_NAME}({}", json_string(specifier)); |
There was a problem hiding this comment.
When user input declares __nodeREPLDynamicImport, such as import "m"; const __nodeREPLDynamicImport = () => {};, the generated import calls this unqualified name in the same script scope. The later lexical declaration shadows the intended REPL helper for the whole scope, so the hoisted await __nodeREPLDynamicImport(...) hits the TDZ (or a hoisted var value) before loading the module; use a collision-proof helper binding or reject/rename conflicting user bindings.
Useful? React with 👍 / 👎.
| fn visit_assign_expr(&mut self, node: &AssignExpr) { | ||
| if self.import_binding_for_assign_target(&node.left).is_some() { | ||
| self.replace_expression_with_import_assignment_error(node.span); | ||
| return; |
There was a problem hiding this comment.
For assignments to imported bindings with side-effectful right-hand sides, such as import { x } from "m"; x = sideEffect(), module evaluation computes the RHS and then throws when writing the immutable import binding. This branch replaces the entire assignment and returns before visiting its children, so the transformed script throws without calling sideEffect(), changing observable behavior for these invalid-but-runtime-checked assignments.
Useful? React with 👍 / 👎.
| } else if code.starts_with('\n') || code.starts_with("\r\n") { | ||
| format!("{imports}{code}") | ||
| } else { | ||
| format!("{imports}\n{code}") |
There was a problem hiding this comment.
When the source begins with a directive such as "use strict"; followed by an import, this branch prepends the generated await __nodeREPLDynamicImport(...) before the original code. That moves the directive out of the directive prologue, so the script produced for inputs like "use strict"; import "m"; function f(){ return this; } no longer runs the following functions in strict mode; insert hoisted imports after the directive prologue or emit a replacement strict directive first.
Useful? React with 👍 / 👎.
kdy1
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a3f602f5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ModuleDecl::TsNamespaceExport(export) => { | ||
| self.delete_span(export.span); | ||
| } | ||
| ModuleDecl::TsExportAssignment(..) => {} |
There was a problem hiding this comment.
For TypeScript inputs such as export = value;, parse_module succeeds via TsSyntax, but this arm records no edit or hoisted import, so transform_module_syntax later sees an empty edit list and returns hadModuleSyntax: false with the original export = still in the output. Callers that trust the flag then execute invalid script syntax; this TS export assignment needs to be unwrapped, rejected, or otherwise treated as module syntax instead of a no-op.
Useful? React with 👍 / 👎.
| for stmt in &node.stmts { | ||
| if let Stmt::Decl(decl) = stmt { | ||
| collect_shadowed_decl_bindings(&mut shadowed, decl, bindings); | ||
| } |
There was a problem hiding this comment.
Fresh evidence after the shadowing fix: the collector only derives block scope from direct Stmt::Decl entries here and there is no visit_for* scope handling, so import { x } from "m"; for (let x of xs) { x; } rewrites the body use to __nodeREPLImport0.x even though the loop binding shadows the import. Any for/for...of/for...in header declaration with the same name changes semantics; include those bindings in the active scope before visiting the loop body.
Useful? React with 👍 / 👎.
| if self.import_binding_for_assign_target(&node.left).is_some() { | ||
| self.replace_expression_with_import_assignment_error(node.span); | ||
| return; |
There was a problem hiding this comment.
When the operator is ||=, &&=, or ??=, an imported binding is only assigned if the current imported value does not short-circuit; for example import { x } from "m"; x ||= sideEffect() should not throw or call sideEffect when x is already truthy. This branch replaces every assignment whose left side is imported with an unconditional throwing IIFE, so those valid short-circuit cases now fail before reading the namespace value; check the assignment operator and preserve the short-circuit path before forcing the immutable-binding error.
Useful? React with 👍 / 👎.
| if !self.code.get(start..end).is_some_and(|text| text == local) { | ||
| return; |
There was a problem hiding this comment.
When a reference to an imported local is written with an identifier escape, such as import { x as a } from "m"; \u0061;, SWC still reports the identifier symbol as a but the source slice is the raw escape text, so this guard skips the replacement. Since the import declaration is deleted, the transformed script leaves \u0061 without the imported lexical binding and can read a global or throw instead of accessing the module export; replace by span after symbol resolution rather than requiring the raw text to equal the symbol.
Useful? React with 👍 / 👎.
| } | ||
| out.push(')'); | ||
| if !required_exports.is_empty() { | ||
| out.push_str(".then((m) => { "); |
There was a problem hiding this comment.
When the imported module exports a callable then, dynamic import/await treats the namespace as a thenable, while native static imports can still bind its other exports. Because named/default imports are validated by chaining .then((m) => ...) on the helper result here, import { x } from "./then.mjs" can receive whatever the exported then resolves instead of the module namespace and report x missing even though the static import is valid; the helper protocol needs to avoid awaiting/chaining the namespace object as a thenable.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 199b340838
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| fn visit_function(&mut self, node: &Function) { | ||
| let shadowed = function_scope_shadowed_bindings(node, self.bindings); |
There was a problem hiding this comment.
When an imported name is reused as the self-name of a function expression, e.g. import { x } from "m"; const f = function x(){ return x; }, the body should resolve x to the function's inner binding. This scope only records parameters and var declarations, so visiting the body rewrites return x to return __nodeREPLImport0.x, changing valid module semantics; add the FnExpr name to the shadowing scope before visiting the function body.
Useful? React with 👍 / 👎.
| fn visit_block_stmt(&mut self, node: &BlockStmt) { | ||
| let shadowed = direct_block_shadowed_bindings(node, self.bindings); | ||
| self.with_scope(shadowed, |this| node.visit_children_with(this)); |
There was a problem hiding this comment.
Fresh evidence in this revision is that shadow scopes are still only created for BlockStmts, but a switch statement has its own lexical environment for let/class/function declarations in cases. For import { x } from "m"; switch (n) { case 1: let x = 1; x; }, the case reference should read the local let x, yet it is visited without a switch scope and gets rewritten to __nodeREPLImport0.x; collect case declarations before visiting the switch body.
Useful? React with 👍 / 👎.
| }; | ||
|
|
||
| ModuleSyntaxTransformOutput { | ||
| code: prepend_hoisted_imports(apply_edits(code, edits), hoisted), |
There was a problem hiding this comment.
When any module syntax is accepted, the remaining body still needs module strict-mode semantics even if the source did not contain a "use strict" directive. This emits the rewritten code as a plain script, so inputs like import "m"; function f(){ return this; } f() run the function in sloppy mode and can return the global object instead of undefined; insert or preserve a strict directive while keeping hashbang/import ordering intact.
Useful? React with 👍 / 👎.
| for (local, export_name) in named { | ||
| self.import_bindings.push(ImportBinding { | ||
| local, | ||
| namespace: namespace_name.clone(), | ||
| export_name, |
There was a problem hiding this comment.
If two static imports bind the same local name, such as import { a as x } from "a"; import { b as x } from "b"; x;, native module instantiation fails with a duplicate lexical binding error. This path records both imports without validating the local name and later binding_for picks the first one, so the transformed script imports both modules and evaluates successfully with the wrong binding; detect duplicate imported locals before deleting the declarations.
Useful? React with 👍 / 👎.
|
|
||
| fn visit_expr(&mut self, node: &Expr) { | ||
| if let Expr::Ident(ident) = node { | ||
| self.replace_identifier(ident.span, ident.sym.as_ref()); |
There was a problem hiding this comment.
For TypeScript inputs that take the TS parse path, such as import { x } from "m"; const y: number = 0; delete x;, the original module still has the strict-mode early error for deleting an imported binding after types are stripped. This generic identifier rewrite turns the operand into delete __nodeREPLImport0.x, so the transformed script can run instead of rejecting the module; handle delete operands like other invalid writes to import bindings.
Useful? React with 👍 / 👎.
| } | ||
| out.push(')'); | ||
| if !required_exports.is_empty() { | ||
| out.push_str(".then((m) => { "); |
There was a problem hiding this comment.
Fresh evidence after the missing-export validation fix: the generated .then check only runs after the dynamic import has resolved, which means the requested module has already evaluated. Native static imports fail during linking before evaluating a module that lacks missing, so import { missing } from "./side-effect.mjs" now runs that module's side effects before throwing; missing export validation needs to happen before module evaluation or be delegated to a helper with static-import semantics.
Useful? React with 👍 / 👎.
1 participant
Description:
Adds an independent Rust/WebAssembly binding at
bindings/binding_nodejs_support_wasm, published as@swc/nodejs-support-wasm.The package has its own Rust crate, sources, build scripts, tests, README, Cargo entry, and npm workspace entry. It does not build from
binding_typescript_wasmand does not add features or exports to the existing@swc/wasm-typescriptor@swc/wasm-typescript-esmpackages.The new CommonJS package embeds its wasm binary and exposes these APIs at the top level:
transformtransformSynctransformModuleSyntaxgetFirstExpressionisValidSyntaxisRecoverableErrorIt also exports the related
Options,TransformOutput, andModuleSyntaxTransformOutputTypeScript types.The npm publish matrix, wasm CI matrix, release versioning, Cargo lockfile, and pnpm lockfile now reference the standalone
binding_nodejs_support_wasmcrate.Validation performed:
git submodule update --init --recursivecargo fmt --all -- --checkcargo test -p binding_typescript_wasm -p binding_nodejs_support_wasmcargo clippy --all --all-targets -- -D warnings(cd bindings/binding_nodejs_support_wasm && ./scripts/test.sh)(cd bindings/binding_typescript_wasm && ./scripts/test.sh)(cd bindings/binding_core_wasm && ./scripts/test.sh)(cd bindings/binding_minifier_wasm && ./scripts/test.sh)(cd bindings/binding_es_ast_viewer && ./scripts/test.sh)(cd bindings/binding_nodejs_support_wasm/pkg && npm pack --dry-run --json)pnpm install --frozen-lockfile --ignore-scriptsgo run github.com/rhysd/actionlint/cmd/actionlint@latest .github/workflows/CI.yml .github/workflows/publish-npm-package.ymlBREAKING CHANGE:
None. The existing TypeScript wasm packages and their public APIs are unchanged.
Related issue (if exists):