fix(core): add transformed ESM fallback for ESM server bundle execution - #8290
fix(core): add transformed ESM fallback for ESM server bundle execution#8290SyMind wants to merge 22 commits into
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac9db0accd
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2159a4aec0
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9a94c3b5eb
ℹ️ 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.
Pull request overview
Adds a new execution path for ESM server bundles when the runtime does not support vm.SourceTextModule, by evaluating bundles via a SystemJS (System.register) transform + runtime. This enhances compatibility across supported Node.js versions while keeping the existing ESM VM-based runner when available.
Changes:
- Make the runner factory async and select between
CommonJsRunner,EsmRunner, and the newSystemJsRunnerbased on bundle format andvm.SourceTextModuleavailability. - Introduce
SystemJsRunner/SystemJsEvaluatorplus an SWC-basedtransformToSystemJshelper for lazy ESM→SystemJS transformation with inline source maps. - Add unit tests covering import.meta behavior, source-map stack mapping, and missing static external export diagnostics.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/tests/systemJsRunner.test.ts | Adds tests validating SystemJS runner behavior (import.meta, source maps, missing exports). |
| packages/core/src/server/runner/type.ts | Updates runner factory interface to return Promise<Runner>. |
| packages/core/src/server/runner/systemJsTransform.ts | Adds SWC-based ESM→SystemJS transform with inline source map handling. |
| packages/core/src/server/runner/systemJs.ts | Implements SystemJS evaluator/runtime, module lifecycle, and external resolution. |
| packages/core/src/server/runner/index.ts | Updates runner selection logic to include CommonJS and SystemJS fallback, and awaits factory creation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
8a10b6d to
14dd571
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14dd5716c2
ℹ️ 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".
chenjiahan
left a comment
There was a problem hiding this comment.
I’m concerned about two things:
- The potential edge cases introduced by using a plugin to transform ESM into a custom module system
- The various trade-offs of introducing a Wasm plugin, including bundle size, performance, and maintenance costs
Summary
This PR adds a transformed-ESM fallback for evaluating ESM server bundles when Node.js does not expose
vm.SourceTextModule.The server runner now selects an execution strategy based on the bundle format and runtime capabilities:
CommonJsRunnervm.SourceTextModuleavailableEsmRunnervm.SourceTextModuleunavailableTransformedEsmRunnerTransformedEsmRunnerlazily transforms emitted ESM modules into a small async runtime protocol and evaluates them withAsyncFunction. Bundle modules are managed by a runner-scoped module graph, while external dependencies are resolved relative to the importer and loaded through nativeimport().This allows Rsbuild to execute ESM server output without requiring
--experimental-vm-modules.Design
This PR:
TransformedEsmRunneras the fallback whenvm.SourceTextModuleis unavailable;await;import.meta.url,import.meta.dirname, andimport.meta.filename;Why not SystemJS
An earlier implementation used SWC's SystemJS transform and evaluated the resulting
System.registermodules. This works for a closed SystemJS module graph, but it cannot reliably preserve ESM semantics when the graph also contains native external modules.SystemJS propagates imported values through dependency setters. For a native external module loaded with
import(), the setter can only be invoked with the module namespace once. If the external module later mutates an exported binding, the native namespace reflects the new value, but the value copied into the SystemJS-generated local binding remains stale.For example:
This also affects re-exports and multiple bundle modules importing the same external binding. Native ESM does not expose a subscription mechanism that would let the SystemJS runtime notify dependency setters whenever an external export changes.
The generic SystemJS transform introduced additional complexity:
The transformed-ESM approach avoids this impedance mismatch. Imported identifiers are rewritten to read directly from the native module namespace, preserving live bindings without requiring external modules to participate in a SystemJS runtime.
Why not
module.registerHooks()A native ESM loader hook could serve bundle output directly from memfs, but it cannot provide safe cache invalidation across rebuilds:
module.registerHooks()was added in Node.js 22.15.0, while@rsbuild/coresupports Node.js^20.19.0 || >=22.12.0.require.cache. Reusing a stable URL such as/dist/index.jsafter a rebuild returns the previous module without calling theloadhook again./${timestamp}/dist/index.js, loads the latest source but creates a new cached module graph on every rebuild. Node.js exposes no public API for evicting these ESM cache entries, so old module namespaces and their retained state remain alive and memory usage can grow throughout a long-running watch session.SystemJsEvaluatorinstead owns a runner-scoped module cache. Replacing the runner releases the whole module graph for garbage collection, while still allowing bundle source to be read from memfs.References: Node.js
module.registerHooks(), ES modules are cached as URLs, and ESM uses a separate cache.Related Links