-
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 17 commits
86d219c
c85c69e
6d7219d
ba0aa88
56b9a6d
19d4d53
cb5bec6
79183e0
d6d7268
7474309
5dd4a01
6221db1
23d0a46
80e0bd5
44f7423
b9075a4
2c7e271
06b07f2
4311074
2642a5d
f9a9bc8
548e32d
c62836c
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
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. sync context requires createRequire, can't use await import()
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. Can you explain more here? How/why does libsocksf require "sync context" whereas other places in the codebase don't? Do we need to update how we use the
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 sync context at connect()/listen() is real (sync syscall callbacks across the JS↔Wasm boundary — no await without ASYNCIFY). More importantly, ws is a user-installed peer-dep: many programs link libsockets transitively without ever calling WebSocket ops, so loading ws at init would regress those programs vs. the CJS build. createRequire gives us a sync require() scoped to import.meta.url; the inner await import('node:module') is a cheap Node builtin that can't fail. The actual ws load stays lazy — same behavior as the CJS build. A bigger refactor (async-friendly WS library, or asyncify the socket syscall layer) could remove createRequire, but that's out of scope here. You can analyze failures by change addressing your request: |
||
| $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; | ||
|
vogel76 marked this conversation as resolved.
|
||
| #endif | ||
| var host = sock.saddr; | ||
| #if SOCKET_DEBUG | ||
| dbg(`websocket: listen: ${host}:${sock.sport}`); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -953,6 +953,27 @@ function makeModuleReceiveWithVar(localName, moduleName, defaultValue) { | |
| return ret; | ||
| } | ||
|
|
||
| function makeNodeImport(module, guard = true) { | ||
| assert(ENVIRONMENT_MAY_BE_NODE, 'makeNodeImport called when environment can never be node'); | ||
|
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. Should we have this assert in |
||
| var expr; | ||
| if (EXPORT_ES6) { | ||
| expr = `await import(/* webpackIgnore: true */ /* @vite-ignore */ '${module}')`; | ||
|
vogel76 marked this conversation as resolved.
Outdated
|
||
| } else { | ||
| expr = `require('${module}')`; | ||
| } | ||
| if (guard) { | ||
| return `ENVIRONMENT_IS_NODE ? ${expr} : undefined`; | ||
| } | ||
| return expr; | ||
| } | ||
|
|
||
| function makeNodeFilePath(filename) { | ||
| if (EXPORT_ES6) { | ||
| return `new URL('${filename}', import.meta.url)`; | ||
| } | ||
| return `__dirname + '/${filename}'`; | ||
| } | ||
|
|
||
| function makeRemovedFSAssert(fsName) { | ||
| assert(ASSERTIONS); | ||
| const lower = fsName.toLowerCase(); | ||
|
|
@@ -1243,6 +1264,8 @@ addToCompileTimeContext({ | |
| makeModuleReceive, | ||
| makeModuleReceiveExpr, | ||
| makeModuleReceiveWithVar, | ||
| makeNodeFilePath, | ||
| makeNodeImport, | ||
| makeRemovedFSAssert, | ||
| makeRetainedCompilerSettings, | ||
| makeReturn64, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,7 +59,7 @@ var ENVIRONMENT_IS_WORKER = !!globalThis.WorkerGlobalScope; | |
|
|
||
| #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) }}}; | ||
| globalThis.Worker = worker_threads.Worker; | ||
| ENVIRONMENT_IS_WORKER = !worker_threads.isMainThread; | ||
| } | ||
|
|
@@ -104,9 +104,14 @@ 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'); | ||
| #if (ASSERTIONS || RUNTIME_DEBUG || AUTODEBUG) | ||
| var utils = {{{ makeNodeImport('node:util', false) }}}; | ||
| var dbg_node_fs = fs; | ||
| var dbg_node_utils = utils; | ||
| #endif | ||
| } | ||
| var out = defaultPrint; | ||
| var err = defaultPrintErr; | ||
|
|
@@ -115,6 +120,16 @@ var out = (...args) => console.log(...args); | |
| var err = (...args) => console.error(...args); | ||
| #endif | ||
|
|
||
| #if !PTHREADS && WASM_WORKERS && ENVIRONMENT_MAY_BE_NODE && (ASSERTIONS || RUNTIME_DEBUG || AUTODEBUG) | ||
| // Initialize dbg() node module references for WASM_WORKERS without PTHREADS. | ||
| // (With PTHREADS these are set in the print setup block above.) | ||
| var dbg_node_fs, dbg_node_utils; | ||
| if (ENVIRONMENT_IS_NODE) { | ||
| dbg_node_fs = {{{ makeNodeImport('node:fs', false) }}}; | ||
| dbg_node_utils = {{{ makeNodeImport('node:util', false) }}}; | ||
| } | ||
| #endif | ||
|
|
||
| // Override this function in a --pre-js file to get a signal for when | ||
| // compilation is ready. In that callback, call the function run() to start | ||
| // the program. | ||
|
|
@@ -182,13 +197,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); | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.