-
Notifications
You must be signed in to change notification settings - Fork 75
compartment-mapper: dynamic require of "node:"-prefixed builtins fails #2754
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Labels
bug
Something isn't working
Comments
Also: I would love a proper stack trace, but I'm not sure how to get any better; we're already using |
boneskull
added a commit
that referenced
this issue
Apr 3, 2025
closes #2754 Dynamically-required builtins were going through the entire candidate-specifier-guess-'n'-check loop, which shouldn't happen. Specifically, the call to a `MaybeReadNowFn` in this loop caused the original error as reported in #2754 because it uses a `FileUrlToPathFn`; the default implementation (Node.js' `url.fileURLToPath`) throws when provided `node:foo` _but does not throw_ when provided `foo`. Also exports `WILDCARD_POLICY_VALUE` from `policy-format` so we can use it instead of magic strings all over the place. Fixes #2754
boneskull
added a commit
that referenced
this issue
Apr 3, 2025
closes #2754 Dynamically-required builtins were going through the entire candidate-specifier-guess-'n'-check loop, which shouldn't happen. Specifically, the call to a `MaybeReadNowFn` in this loop caused the original error as reported in #2754 because it uses a `FileUrlToPathFn`; the default implementation (Node.js' `url.fileURLToPath`) throws when provided `node:foo` _but does not throw_ when provided `foo`. Also exports `WILDCARD_POLICY_VALUE` from `policy-format` so we can use it instead of magic strings all over the place. Fixes #2754
boneskull
added a commit
that referenced
this issue
Apr 5, 2025
closes #2754 Dynamically-required builtins were going through the entire candidate-specifier-guess-'n'-check loop, which shouldn't happen. Specifically, the call to a `MaybeReadNowFn` in this loop caused the original error as reported in #2754 because it uses a `FileUrlToPathFn`; the default implementation (Node.js' `url.fileURLToPath`) throws when provided `node:foo` _but does not throw_ when provided `foo`. Also exports `WILDCARD_POLICY_VALUE` from `policy-format` so we can use it instead of magic strings all over the place. Fixes #2754
boneskull
added a commit
that referenced
this issue
Apr 14, 2025
boneskull
added a commit
that referenced
this issue
Apr 14, 2025
closes #2754 Dynamically-required builtins were going through the entire candidate-specifier-guess-'n'-check loop, which shouldn't happen. Specifically, the call to a `MaybeReadNowFn` in this loop caused the original error as reported in #2754 because it uses a `FileUrlToPathFn`; the default implementation (Node.js' `url.fileURLToPath`) throws when provided `node:foo` _but does not throw_ when provided `foo`. Also exports `WILDCARD_POLICY_VALUE` from `policy-format` so we can use it instead of magic strings all over the place. Fixes #2754
boneskull
added a commit
that referenced
this issue
Apr 15, 2025
closes #2754 Dynamically-required builtins were going through the entire candidate-specifier-guess-'n'-check loop, which shouldn't happen. Specifically, the call to a `MaybeReadNowFn` in this loop caused the original error as reported in #2754 because it uses a `FileUrlToPathFn`; the default implementation (Node.js' `url.fileURLToPath`) throws when provided `node:foo` _but does not throw_ when provided `foo`. Also exports `WILDCARD_POLICY_VALUE` from `policy-format` so we can use it instead of magic strings all over the place. Fixes #2754
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
While I was careful to add a test case for "dynamic require of a builtin module", that test case only covers a bare module specifier (
cluster
).Changing
cluster
tonode:cluster
fails the test:This is reproducible outside of the
@endo/compartment-mapper
's test environment (I found it via@lavamoat/node
's testing environment; lol).Steps to reproduce
Modify
compartment-mapper/test/fixtures-dynamic/node_modules/hooked-app/index.js
:Execute test (from
packages/compartment-mapper
):npx ava test/dynamic-require.test.js -c 1 -m 'inter-package and exit module dynamic require works'
Observe failure akin to the trace pasted above.
Expected behavior
It should be the same behavior as exhibited sans-prefix.
Additional context
This is likely my fault, so I will fix it.
The text was updated successfully, but these errors were encountered: