fix(core): handle builtin imports in linked esm loader#1070
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 078c91971a
ℹ️ 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".
| : isBuiltinSpecifier(specifier) | ||
| ? specifier |
There was a problem hiding this comment.
Keep bare builtins canonicalized to
node:
Returning specifier here means bare builtins like fs and path no longer go through import.meta.resolve, so modulePath stays 'fs' instead of 'node:fs'. With the default interopDefault: true, that makes the later !modulePath.startsWith('node:') guard miss these modules and wraps each await import('fs') in a fresh interop Proxy instead of returning Node’s cached module namespace object. Any ESM test code that compares repeated builtin imports or uses them as identity-stable keys will now behave differently from native Node semantics.
Useful? React with 👍 / 👎.
Summary
Background
The worker ESM loader resolves non-absolute specifiers during module linking. Linked modules that statically import Node builtins can fall into that path and fail when builtin specifiers are treated like regular file imports.
Implementation
node:imports and bare builtin module names, before callingimport.meta.resolve.node:fsandnode:path.User Impact
Linked ESM chunks that import Node builtins now load correctly in
@rstest/coreworkers.Related Links
Checklist