feat(core)!: apply rs.mock to non-literal dynamic import()#1485
feat(core)!: apply rs.mock to non-literal dynamic import()#1485fi3ework wants to merge 5 commits into
Conversation
Deploying rstest with
|
| Latest commit: |
c94dd6f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b5d55947.rstest.pages.dev |
| Branch Preview URL: | https://fix-mock-nonliteral-dynamic.rstest.pages.dev |
![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: 3af4f1714a
ℹ️ 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".
fi3ework
force-pushed
the
fix/mock-nonliteral-dynamic-import
branch
from
3af4f17 to
6da09a5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6da09a5bda
ℹ️ 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".
fi3ework
force-pushed
the
fix/mock-nonliteral-dynamic-import
branch
from
6da09a5 to
9cd0343
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cd0343d18
ℹ️ 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".
fi3ework
force-pushed
the
fix/mock-nonliteral-dynamic-import
branch
from
9cd0343 to
232c665
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 232c6654c7
ℹ️ 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".
fi3ework
force-pushed
the
fix/mock-nonliteral-dynamic-import
branch
from
232c665 to
2da800c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2da800cb2c
ℹ️ 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".
fi3ework
force-pushed
the
fix/mock-nonliteral-dynamic-import
branch
from
2da800c to
12df18a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12df18a9f4
ℹ️ 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".
fi3ework
force-pushed
the
fix/mock-nonliteral-dynamic-import
branch
from
12df18a to
99131b0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99131b0f20
ℹ️ 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".
fi3ework
force-pushed
the
fix/mock-nonliteral-dynamic-import
branch
from
99131b0 to
e157937
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e157937dfe
ℹ️ 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".
fi3ework
force-pushed
the
fix/mock-nonliteral-dynamic-import
branch
from
e157937 to
f3ce8c1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3ce8c1a05
ℹ️ 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".
fi3ework
force-pushed
the
fix/mock-nonliteral-dynamic-import
branch
from
f3ce8c1 to
9f09037
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f090372e0
ℹ️ 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".
fi3ework
force-pushed
the
fix/mock-nonliteral-dynamic-import
branch
from
9f09037 to
cb04e34
Compare
A dynamic `import()` with a non-literal specifier is routed to the worker's native import hook and loaded by Node outside the webpack runtime, so `rs.mock` was bypassed (#1454). - Route a non-literal `import(request)` whose request names a mocked module to the bundle's mock instance via a shared-global resolver (all Node), fixing `const s = 'node:os'; import(s)`. A builtin's two spellings (`os` / `node:os`) resolve to one mock, and an unmock with either spelling clears both. - Install in-thread `module.registerHooks` (Node >= 22.15 / >= 23.5, feature-detected with silent fallback) so a natively-loaded module's inner import of a mocked module is redirected to a worker-realm mock registry — fixing the reported repro (a local module reached via a variable `import()`) and externalized deps that import a mocked module. The synchronous hooks also fire for `require()`; those resolutions are left to Node so a natively-loaded CommonJS module gets the real module instead of the ESM synthetic. - Publish each mock's exports to the native registry as a LAZY producer: it is evaluated once, only when a natively-loaded module actually imports the mock, so an `rs.mock`/`doMock` factory with side effects keeps its lazy semantics and never runs at registration. The synthetic native module mirrors the bundle's CJS interop: object and callable (default-function) results are served, a whole-object default is synthesized only for a non-`__esModule` mock, and an async result is detected by its `Promise` tag (so a `then` export is preserved and an async factory instead falls through to the real module). A throwing factory propagates its error so a broken mock surfaces on the native path, matching the bundled path. - Skip the mock router for `importActual` (its internal `with: { rstest }` attribute), so a non-literal `importActual` of a mocked module returns the real module. - Strip the Node-only native-mock bridge from the browser runtime bundle via the RSTEST_TARGET_BROWSER build flag, so its import.meta.resolve / pathToFileURL use never reaches browser mode. Limitations documented in the mock guide: Node version floor, TS/JSX native targets, function-factory instance divergence, async factories falling through to the real module, native named-export identity, `mockRequire` being CJS-only, `require()` on the native path staying unmocked, isolate:false ESM cache, the shared non-literal resolver under isolate:false with multiple projects, and relative specifiers resolved against the test file.
Zoom-out cleanup of the native-mock runtime after the #1454 review. - Remove the `request === undefined` guards in publishNativeMock, unpublishNativeMock, rstest_unmock, and registerRequestAlias. They guarded a pre-request-literal @rspack/core; @rsbuild/core is pinned (~2.1.1), so the RstestPlugin always emits the request literal — the whole non-literal `import()` mock resolution depends on it — making the branches dead. `request` is now a guaranteed string. - Align the bundle's `isPromise` with the worker registry's realm-safe `[object Promise]` tag check (a cross-realm Promise fails `instanceof`), so both realms apply one async-detection predicate. No behavior change: the full e2e mock suite stays green.
Consume the {o, r} identity newer rspack appends to generated
rstest_mock/rstest_unmock calls (emitMockResolvedInfo), feature-detected
by argument presence so older rspack keeps the previous behavior:
- mockRegistry entries become alias groups: every successful key
derivation (build-resolved target, declaring-module resolution,
test-file resolution) registers the same entry, and removal from any
spelling clears the whole group.
- key derivation lives next to the resolve hook that consumes it
(nativeMockHooks), injected through the registry like the hook
installer; the browser build injects no resolver, so registrations
no-op there and the RSTEST_TARGET_BROWSER define machinery is gone.
- a derivingKey guard keeps the derivation's import.meta.resolve from
being captured by our own resolve hook (a re-mock of an already
mocked module would otherwise be keyed by its virtual URL).
- the bundle runtime registers every request spelling (raw, alt builtin
spelling, build-resolved target) at the one write point, so a literal
import('os') of a node:os mock now hits and every reader is a single
lookup.
- pin the non-string-literal import() rewrite callee via
injectDynamicImportOrigin.functionName instead of piggybacking on
output.importFunctionName.
fi3ework
force-pushed
the
fix/mock-nonliteral-dynamic-import
branch
from
a70571d to
c94dd6f
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c94dd6fe76
ℹ️ 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".
| }; | ||
|
|
||
| installFactory(finalModFactory); | ||
| publishNativeMock(request, modFactory, info); |
There was a problem hiding this comment.
getMockImplementation is shared by rs.mock and rs.mockRequire, but these unconditional publishNativeMock calls also register mockRequire factories with the native ESM module.registerHooks path. When a natively loaded ESM module imports the same specifier, it will now receive the CJS mockRequire mock even though mockRequire is supposed to affect only require()/CJS entries. Guard native publication for !isMockRequire (including the other publish branches).
Useful? React with 👍 / 👎.
BREAKING CHANGE: module mocking now requires @rspack/core >= 2.1.3.
Every generated rstest_mock/rstest_unmock call must carry the
build-resolved identity {o, r} (emitted unconditionally by rspack since
web-infra-dev/rspack#14665); an older rspack now fails loudly at runtime
with an upgrade hint instead of silently missing mock aliases.
On top of the hard contract, non-literal dynamic import() matching
becomes resolved-first:
- relative mock requests are no longer keyed by their raw spelling
(ambiguous across directories) — only by their build-resolved
absolute target;
- on a raw-spelling miss, the worker resolves a relative runtime
specifier against the importing module's directory (the
rspack-injected origin) and the lookup retries by that absolute path,
so relative, absolute, and file:// spellings all meet the absolute
keys; two import('./foo') calls from different directories are
matched independently, and a relative rs.mock('./dep') declared in a
shared helper resolves against the helper's own directory (info.o) —
the testPath-based fallback derivation is gone;
- alias / tsconfig-paths spellings are matched verbatim only, never
expanded at runtime (an unmocked one fails loudly — Node cannot
natively load such a specifier either).
Temporary override so CI runs against the canary that emits the build-resolved mock identity (web-infra-dev/rspack#14665). Drop this commit once a stable @rspack/core release ships the emission and @rsbuild/core picks it up.
fi3ework
changed the title
1 participant
Summary
A dynamic
import()whose specifier is not a static string literal (e.g. a variable) is routed to the worker's native import hook and loaded by Node outside the webpack runtime, sors.mock()was bypassed (#1454).import(request)whose request names a mocked module to the bundle's mock instance via a shared-global resolver (all Node), fixingconst s = 'node:os'; import(s).module.registerHooks(Node >= 22.15 / >= 23.5, feature-detected with silent fallback, installed lazily on the first mock) so a natively-loaded module's inner import of a mocked module is redirected to a worker-realm mock registry; this also covers externalized deps that import a mocked module.BREAKING: requires @rspack/core >= 2.1.3
Per the release plan (rspack ships first, no old-rspack compatibility): every generated
rstest_mock/rstest_unmockcall must carry the build-resolved identity{o, r}, emitted unconditionally by rspack since web-infra-dev/rspack#14665. On an older rspack,rs.mock()fails loudly at runtime with an upgrade hint instead of silently missing.On top of the hard contract, non-literal
import(variable)matching is resolved-first:./foois ambiguous across directories); on a raw-spelling miss the worker resolves a relative runtime specifier against the importing module's directory (the rspack-injected origin) and retries by absolute path — relative, absolute, andfile://spellings all match, twoimport('./foo')calls from different directories are matched independently, and a relativers.mock('./dep')declared in a shared helper resolves against the helper's own directory (info.o);Limitations are documented in the mock guide: rspack version floor, Node version floor, compiled (TS/JSX) native targets, function-factory instance divergence, alias spellings, and
isolate: falseESM cache.Merge gate
The last commit pins the rspack canary (
2.1.3-canary-7caa6eff) so CI runs against the emitting rspack. Keep this PR draft until a stable @rspack/core release ships the emission and @rsbuild/core picks it up; then drop the pin commit and bump the dependency.Related Links
Checklist