Eliminate createRequire/require from EXPORT_ES6 output#26384
Eliminate createRequire/require from EXPORT_ES6 output#26384vogel76 wants to merge 2 commits intoemscripten-core:mainfrom
createRequire/require from EXPORT_ES6 output#26384Conversation
sbc100
left a comment
There was a problem hiding this comment.
Wow, thanks for working on this! Nice work.
src/lib/libwasi.js
Outdated
| if (ENVIRONMENT_IS_NODE) { | ||
| #if EXPORT_ES6 | ||
| return (view) => nodeCrypto.randomFillSync(view); | ||
| #else |
There was a problem hiding this comment.
To keep thing simpler could we just always use the nodeCrypto global via __deps? (i.e. make the difference only in how nodeCrypto is globally defined?
src/lib/libnode_imports.js
Outdated
| #if EXPORT_ES6 && ENVIRONMENT_MAY_BE_NODE | ||
| addToLibrary({ | ||
| $nodeOs: "ENVIRONMENT_IS_NODE ? await import('node:os') : undefined", | ||
| }); |
There was a problem hiding this comment.
Instead of creating and entirely new file for this one entry maybe just tack this onto libcore.js?
There was a problem hiding this comment.
sure, I will consider this change
src/shell_minimal.js
Outdated
| #if EXPORT_ES6 | ||
| var fs = await import('node:fs'); | ||
| #else | ||
| var fs = require('node:fs'); |
There was a problem hiding this comment.
Maybe we can make a helper macro here and do something like:
var fs = {{{ makeNodeImport('node:fs') }}};
Then them be could change if/when we use await import easily later. For example, we maybe want to use await import in all MODULARIZE use cases, not just EXPORT_ES6?
src/shell_minimal.js
Outdated
| Module['wasm'] = fs.readFileSync(new URL('{{{ TARGET_BASENAME }}}.wasm', import.meta.url)); | ||
| #endif | ||
| #endif | ||
| #else |
There was a problem hiding this comment.
This seems like a lot of extra complexity/duplication. Is there some way to avoid it maybe?
createRequire/require from EXPORT_ES6 output
0ec991e to
6b74d2a
Compare
|
@sbc100 I hope your remarks have been addressed in added fixup commits. Once you accept it, I will do autosquash rebase to leave only actual commits in clean history. |
6b74d2a to
8f585b7
Compare
|
Is it possible to write a test for this? When you say "This breaks bundlers", do you know why our current bundler tests in test_browser.py are working? (see |
|
Re-commiting, we always squash all changes in the emscripten repo. If you think this change can be split into to commits then please send a two separate PRs. This is just how we do things in emscripten. It helps keep the tests passing on every commit (for the benefit of bisectors). |
src/lib/libcore.js
Outdated
|
|
||
| var cp = require('node:child_process'); | ||
| var cp = nodeChildProcess; | ||
| var ret = cp.spawnSync(cmdstr, [], {shell:true, stdio:'inherit'}); |
There was a problem hiding this comment.
I think you can just drop the cp variable here and use nodeChildProcess.spawnSync?
src/preamble.js
Outdated
| #endif | ||
|
|
||
| #if NODE_CODE_CACHING && ENVIRONMENT_MAY_BE_NODE && EXPORT_ES6 | ||
| var nodeV8 = ENVIRONMENT_IS_NODE ? await import('node:v8') : undefined; |
There was a problem hiding this comment.
Sure, I will deeper review await import usage to encapsulate it.
src/preamble.js
Outdated
| var v8 = nodeV8; | ||
| #else | ||
| var v8 = require('node:v8'); | ||
| #endif |
There was a problem hiding this comment.
No need for this var anymore if you use makeNodeImport I thinik?
I think you for this globalVar its reasonable to call it v8?
src/runtime_common.js
Outdated
| #if EXPORT_ES6 | ||
| global.performance ??= (await import('perf_hooks')).performance; | ||
| #else | ||
| global.performance ??= require('perf_hooks').performance; |
| var fs = require('node:fs'); | ||
| var fs = {{{ makeNodeImport('node:fs') }}}; | ||
| #if WASM == 2 | ||
| if (globalThis.WebAssembly) Module['wasm'] = fs.readFileSync(__dirname + '/{{{ TARGET_BASENAME }}}.wasm'); |
There was a problem hiding this comment.
Does this code (which uses __dirname) simply not work with EXPORT_EST today?
If not, this seems like maybe a separate fix that we could land in isolation. e.g. Fix for EXPORT_ES6 + MINIMAL_RUNTIME + ???
Sure - will add such tests. The sake of such work are maintenance problems of our Hive blockchain interfacing library: wax which compiles blockchain protocol C++ code into wasm and uses it at TS/JS level. We have set of tests related to bundlers and different frameworks where we got problems while loading wasm etc. https://gitlab.syncad.com/hive/wax/-/tree/develop/examples/ts?ref_type=heads |
When EXPORT_ES6 is enabled, the generated JS used createRequire() to
polyfill require(), which breaks bundlers (webpack, Rollup, esbuild)
and Electron's renderer process. Since EXPORT_ES6 requires MODULARIZE,
the module body is wrapped in an async function where await is valid.
- shell.js: Remove createRequire block entirely. Use await import()
for worker_threads, fs, path, url, util. Replace __dirname with
import.meta.url for path resolution.
- shell_minimal.js: Same pattern for worker_threads and fs. Replace
__dirname with new URL(..., import.meta.url) for wasm file loading.
- runtime_debug.js: Skip local require() for fs/util when EXPORT_ES6,
reuse outer-scope variables from shell.js instead.
- runtime_common.js: Guard perf_hooks require() with EXPORT_ES6
alternative.
- preamble.js: Hoist await import('node:v8') above instantiateSync()
for NODE_CODE_CACHING since await can't be used inside sync functions.
Library functions run in synchronous context where await is unavailable.
Define top-level library symbols that use await import() at module init
time, then reference them via __deps from synchronous functions.
- Add libnode_imports.js with shared $nodeOs symbol, register in
modules.mjs when EXPORT_ES6 is enabled.
- libatomic.js, libwasm_worker.js: Use $nodeOs for os.cpus().length
instead of require('node:os').
- libwasi.js: Define $nodeCrypto for crypto.randomFillSync in
$initRandomFill. Combine conditional __deps to avoid override.
- libcore.js: Define $nodeChildProcess for _emscripten_system.
- libnodepath.js: Switch $nodePath initializer to await import().
- libsockfs.js: Define $nodeWs ((await import('ws')).default) for
WebSocket constructor in connect() and Server in listen().
8f585b7 to
6c822be
Compare
When building with
-sEXPORT_ES6, the generated JavaScript currently usescreateRequire(import.meta.url)to polyfillrequire(), then callsrequire()for Node.js built-in modules. This breaks bundlers (webpack, Rollup, esbuild, Vite) and Electron's renderer process, forcing users to post-process emscripten output withsedor custom plugins to strip out the offending calls.This PR replaces all
require()calls withawait import()whenEXPORT_ES6is enabled, eliminating the need forcreateRequireentirely.Why this is safe
EXPORT_ES6requiresMODULARIZE(enforced intools/link.py).MODULARIZEwraps the module body in anasync function, soawaitis valid at the top level of the generated code.await import(...)is transformed to anEMSCRIPTEN$AWAIT$IMPORTplaceholder during preprocessing (parseTools.mjs:83) and restored during linking (link.py:2136). This mechanism is already used elsewhere in the codebase.(await import('node:fs')).readFileSyncworks identically torequire('node:fs').readFileSync. For CJS npm packages likews,.defaultgives themodule.exportsvalue.#if EXPORT_ES6/#elsepreprocessor conditionals. The existingrequire()code path is untouched.Approach
The changes split naturally into two categories:
Shell and runtime files (top-level async context where
awaitworks directly):src/shell.js— Remove thecreateRequireblock. Useawait import()forworker_threads,fs,path,url,util.src/shell_minimal.js— Same pattern forworker_threadsandfs. Replace__dirnamewithnew URL(..., import.meta.url)for wasm file loading.src/runtime_debug.js— Skip localrequire()forfs/utilwhenEXPORT_ES6; reuse outer-scope variables fromshell.js.src/runtime_common.js— Guardperf_hooksrequire()withEXPORT_ES6alternative.src/preamble.js— Hoistawait import('node:v8')aboveinstantiateSync()forNODE_CODE_CACHING(can't useawaitinside a sync function).Library files (synchronous function bodies where
awaitis unavailable):Library functions execute in synchronous context, so
await import()cannot be called inline. Instead, we define library symbols initialized at module top level (async context) and reference them via__deps:$nodeOsnode:oslibatomic.js,libwasm_worker.js$nodeCryptonode:cryptolibwasi.js($initRandomFill)$nodeChildProcessnode:child_processlibcore.js(_emscripten_system)$nodeWsws(.default)libsockfs.js(WebSocket connect + listen)$nodePathnode:pathlibnodepath.js(NODERAWFS)Shared symbols live in a new
src/lib/libnode_imports.js, registered insrc/modules.mjswhenEXPORT_ES6is enabled.Intentionally skipped
src/lib/libembind_gen.js— Build-time code running in Node.js directly, not in emitted runtime JS.src/closure-externs/closure-externs.js—createRequireextern kept; still needed for the non-ES6 path.What changes in the output
Before (EXPORT_ES6):
After (EXPORT_ES6):
The non-ES6 path remains unchanged:
Files modified
13 files modified, 1 new file — 123 insertions, 12 deletions (source files only).
Testing
All existing ESM tests pass:
test_esm(default, node, pthreads variants)test_esm_worker(default, node variants)test_esm_worker_single_filetest_esm_closuretest_esm_implies_modularizetest_esm_requires_modularizetest_pthread_export_es6(default, trusted variants)Additionally verified:
emcc -sEXPORT_ES6 -sMODULARIZEoutput contains zerocreateRequireorrequire(occurrencesemcc -sEXPORT_ES6 -sMODULARIZE -pthreadoutput contains zerocreateRequireorrequire(occurrencesrequire()as before (no regression)Related issues
EXPORT_ES6output containsrequire()which breaks bundlersEXPORT_ES6use only ES6 semantics? #20503 —createRequirebreaks Electron rendererrequire()require()in ESM output breaks RollupRelated PRs
nodebuilt-in module imports withnode:. #18235 — Earlier attempt to address this (partial)