Fix createRequire so it works for local node_modules.
#23516
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sbc100
force-pushed
the
fix_createRequire
branch
from
aeeca2b to
c80239f
Compare
kripken
reviewed
Jan 28, 2025
| @@ -105,7 +105,7 @@ if (ENVIRONMENT_IS_NODE) { | |||
| // We need to use `createRequire()` to construct the require()` function. | |||
| const { createRequire } = await import('module'); | |||
| /** @suppress{duplicate} */ | |||
| var require = createRequire('/'); | |||
| var require = createRequire(import.meta.url); | |||
There was a problem hiding this comment.
I don't know enough about support for this - didn't we have an issue with it breaking in some bundler or environment?
There was a problem hiding this comment.
I believe you are thinking of the use of import.meta.url of in new Worker. This usage is perfectly fine AFAIK. See #23169 which removed it prematurely.
sbc100
force-pushed
the
fix_createRequire
branch
from
c80239f to
946733f
Compare
kripken
approved these changes
Jan 28, 2025
For most uses of `require()` in emscripten this does not matter since we load mostly system libraries. However for the `ws` module it is needed. This bug was being masked by the fact that we were setting `NODE_PATH` in our socket test running. This is no longer needed now that we run (non-parallel) tests in the emscripten tree (in out/test). This is followup to emscripten-core#23265 which itself was an attempt to revert 23169. Fixes: emscripten-core#23503
sbc100
force-pushed
the
fix_createRequire
branch
from
946733f to
3c07854
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For most uses of
require()in emscripten this does not matter since we load mostly system libraries. However for thewsmodule it is needed. This bug was being masked by the fact that we were settingNODE_PATHin our socket test running. This is no longer needed now that we run (non-parallel) tests in the emscripten tree (in out/test).This is followup to #23265 which itself was an attempt to revert #23169.
Fixes: #23503