-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Eliminate createRequire/require from EXPORT_ES6 output
#26384
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
base: main
Are you sure you want to change the base?
Changes from all commits
c5f57b7
160122b
a47702b
a36955c
187d648
d612f3f
1efd494
d648fff
c1332e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,10 +5,18 @@ | |
| */ | ||
|
|
||
| addToLibrary({ | ||
| #if ENVIRONMENT_MAY_BE_NODE && EXPORT_ES6 | ||
| // In ESM mode, require() is not natively available. When SOCKFS is used, | ||
| // we need require() to lazily load the 'ws' npm package for WebSocket | ||
| // support on Node.js. Set up a createRequire-based polyfill. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we still need |
||
| $nodeRequire: `ENVIRONMENT_IS_NODE ? (await import('node:module')).createRequire(import.meta.url) : undefined`, | ||
| $SOCKFS__deps: ['$FS', '$nodeRequire'], | ||
| #else | ||
| $SOCKFS__deps: ['$FS'], | ||
| #endif | ||
| $SOCKFS__postset: () => { | ||
| addAtInit('SOCKFS.root = FS.mount(SOCKFS, {}, null);'); | ||
| }, | ||
| $SOCKFS__deps: ['$FS'], | ||
| $SOCKFS: { | ||
| #if expectToReceiveOnModule('websocket') | ||
| websocketArgs: {}, | ||
|
|
@@ -216,7 +224,11 @@ addToLibrary({ | |
| var WebSocketConstructor; | ||
| #if ENVIRONMENT_MAY_BE_NODE | ||
| if (ENVIRONMENT_IS_NODE) { | ||
| #if EXPORT_ES6 | ||
| WebSocketConstructor = /** @type{(typeof WebSocket)} */(nodeRequire('ws')); | ||
| #else | ||
| WebSocketConstructor = /** @type{(typeof WebSocket)} */(require('ws')); | ||
| #endif | ||
| } else | ||
| #endif // ENVIRONMENT_MAY_BE_NODE | ||
| { | ||
|
|
@@ -522,7 +534,11 @@ addToLibrary({ | |
| if (sock.server) { | ||
| throw new FS.ErrnoError({{{ cDefs.EINVAL }}}); // already listening | ||
| } | ||
| #if EXPORT_ES6 | ||
| var WebSocketServer = nodeRequire('ws').Server; | ||
| #else | ||
| var WebSocketServer = require('ws').Server; | ||
| #endif | ||
| var host = sock.saddr; | ||
| #if SOCKET_DEBUG | ||
| dbg(`websocket: listen: ${host}:${sock.sport}`); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,7 +55,7 @@ var ENVIRONMENT_IS_WEB = !ENVIRONMENT_IS_NODE; | |
|
|
||
| #if ENVIRONMENT_MAY_BE_NODE && (PTHREADS || WASM_WORKERS) | ||
| if (ENVIRONMENT_IS_NODE) { | ||
| var worker_threads = require('node:worker_threads'); | ||
| var worker_threads = {{{ makeNodeImport('node:worker_threads', false) }}}; | ||
| global.Worker = worker_threads.Worker; | ||
| } | ||
| #endif | ||
|
|
@@ -99,7 +99,7 @@ if (ENVIRONMENT_IS_NODE && ENVIRONMENT_IS_SHELL) { | |
| var defaultPrint = console.log.bind(console); | ||
| var defaultPrintErr = console.error.bind(console); | ||
| if (ENVIRONMENT_IS_NODE) { | ||
| var fs = require('node:fs'); | ||
| var fs = {{{ makeNodeImport('node:fs', false) }}}; | ||
| defaultPrint = (...args) => fs.writeSync(1, args.join(' ') + '\n'); | ||
| defaultPrintErr = (...args) => fs.writeSync(2, args.join(' ') + '\n'); | ||
| } | ||
|
|
@@ -181,13 +181,13 @@ if (!ENVIRONMENT_IS_PTHREAD) { | |
| // Wasm or Wasm2JS loading: | ||
|
|
||
| if (ENVIRONMENT_IS_NODE) { | ||
| var fs = require('node:fs'); | ||
| var fs = {{{ makeNodeImport('node:fs', false) }}}; | ||
| #if WASM == 2 | ||
| if (globalThis.WebAssembly) Module['wasm'] = fs.readFileSync(__dirname + '/{{{ TARGET_BASENAME }}}.wasm'); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this code (which uses If not, this seems like maybe a separate fix that we could land in isolation. e.g.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The The fix uses
I kept it in this PR because the changes are on the same lines as the
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If |
||
| else eval(fs.readFileSync(__dirname + '/{{{ TARGET_BASENAME }}}.wasm.js')+''); | ||
| if (globalThis.WebAssembly) Module['wasm'] = fs.readFileSync({{{ makeNodeFilePath(TARGET_BASENAME + '.wasm') }}}); | ||
| else eval(fs.readFileSync({{{ makeNodeFilePath(TARGET_BASENAME + '.wasm.js') }}})+''); | ||
| #else | ||
| #if !WASM2JS | ||
| Module['wasm'] = fs.readFileSync(__dirname + '/{{{ TARGET_BASENAME }}}.wasm'); | ||
| Module['wasm'] = fs.readFileSync({{{ makeNodeFilePath(TARGET_BASENAME + '.wasm') }}}); | ||
| #endif | ||
| #endif | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| // Polyfill require() for ESM mode so that EM_ASM/EM_JS code using | ||
| // require('fs'), require('path'), etc. works in both CJS and ESM. | ||
| // createRequire is available since Node 12.2.0. | ||
| if (typeof require === 'undefined') { | ||
| var { createRequire } = await import('module'); | ||
| var require = createRequire(import.meta.url); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5823,6 +5823,8 @@ def test_fs_base(self): | |
| def test_fs_nodefs_rw(self): | ||
| if not self.get_setting('NODERAWFS'): | ||
| self.setup_nodefs_test() | ||
| else: | ||
| self.add_require_polyfill() | ||
| self.maybe_closure() | ||
| self.do_runf('fs/test_nodefs_rw.c', 'success') | ||
|
|
||
|
|
@@ -5846,6 +5848,7 @@ def test_fs_nodefs_dup(self): | |
|
|
||
| @requires_node | ||
| def test_fs_nodefs_home(self): | ||
| self.add_require_polyfill() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't love having to inject the polyfill like this. Can this test instead be updated reference |
||
| self.do_runf('fs/test_nodefs_home.c', 'success', cflags=['-sFORCE_FILESYSTEM', '-lnodefs.js']) | ||
|
|
||
| @requires_node | ||
|
|
@@ -8656,7 +8659,14 @@ def test(assert_returncode=0): | |
| js = read_file(self.output_name('test_hello_world.support')) | ||
| else: | ||
| js = read_file(self.output_name('test_hello_world')) | ||
| assert ('require(' in js) == ('node' in self.get_setting('ENVIRONMENT')), 'we should have require() calls only if node js specified' | ||
| # In ESM mode we use dynamic import() instead of require() for node modules. | ||
| # MODULARIZE=instance implies EXPORT_ES6 which triggers ESM output. | ||
| is_esm = self.get_setting('EXPORT_ES6') or self.get_setting('WASM_ESM_INTEGRATION') or self.get_setting('MODULARIZE') == 'instance' | ||
| if is_esm: | ||
| has_node_imports = 'import(' in js | ||
| else: | ||
| has_node_imports = 'require(' in js | ||
| assert has_node_imports == ('node' in self.get_setting('ENVIRONMENT')), 'we should have node imports only if node js specified' | ||
|
|
||
| for engine in self.js_engines: | ||
| print(f'engine: {engine}') | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just call this
child_process? It seems unlikely enough to collide and is more in common with how most node porgrams name this (I assume)?