Repository navigation
fix(nodejs): complete Amaro REPL support APIs - #12395
Conversation
🦋 Changeset detectedLatest commit: 2aa0242 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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2aa02422b8
ℹ️ 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 (@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 (@codex) address that feedback".
| Some(format!( | ||
| "const {{ {} }} = await {import};", | ||
| bindings.join(", ") | ||
| )) |
There was a problem hiding this comment.
Avoid shadowing the dynamic-import helper
When a valid import binds the name __nodeREPLDynamicImport, such as import { x as __nodeREPLDynamicImport } from "m", this emits const { x: __nodeREPLDynamicImport } = await __nodeREPLDynamicImport(...). The newly declared lexical binding shadows the caller-supplied helper while its initializer is evaluated, so the transformed input fails instead of importing the value. Preserve access to the helper through a collision-free temporary before declaring user bindings.
Useful? React with 👍 / 👎.
Merging this PR will not alter performance
Comparing Footnotes
|
Description:
Complete the four Node.js/Amaro support requests in
@swc/nodejs-support-wasm. An input such asimport fs from "node:fs"now declaresfs, so later REPL inputs can use it. Static imports use local declarations instead of rewriting references within a single input; existing import hoisting, export validation, and import attributes are preserved.Add
tokenizefor compact highlighting tokens andfindTopLevelAwaitsfor module-scope await locations, includingfor awaitandawait using. Both support strings and UTF-8Uint8Arrayinputs, with JavaScript UTF-16 coordinates. Also recognizeSyntaxError::Unexpectedwith<eof>as recoverable. Include TypeScript declarations, documentation, shared Rust/WASM fixtures, and executable REPL coverage.BREAKING CHANGE:
In
@swc/nodejs-support-wasm, transformed default and named imports now capture initial export values in localconstbindings rather than rewriting reads into live namespace property accesses. Namespace imports retain the module namespace object. This matches the requested REPL behavior.Validation:
cargo fmt --allcargo clippy --all --all-targets -- -D warningscargo test -p binding_nodejs_support_wasm: 20 passednightlyfeature buildspackages/core:pnpm build:devandpnpm test; 119 passed, 3 skipped by the existing configurationRust commands ran with
RUSTC_WRAPPER=because the configuredsccacheexecutable is unavailable locally.Related issue (if exists):
Closes #12378.