Add support for -sEXPORT_ES6/*.mjs on Node.js

[kleisauke]

As described in #11792, require() and __dirname doesn't exist in
an ES6 module. Emscripten uses this to import built-in core Node.js
modules. For example, the node:fs module is used for synchronously
importing the *.wasm binary, when not linking with -sSINGLE_FILE.

To work around this, ES6 modules on Node.js may import
createRequire() from node:module to construct the require()
function, allowing modules to be imported in a CommonJS manner.

Emscripten targets a variety of environments, which can be
categorized as:

1. Multi-environment builds, which is the default when
      -sENVIRONMENT=* is not specified at link time.
2. Single-environment, e.g. only web or Node.js as target.

For use case (1), this commit ensures that an async function is
emitted, allowing Node.js modules to be dynamically imported. This is
necessary given that static import declarations cannot be put in
conditionals. Inside the module, for Node.js only, it's using the
above-mentioned createRequire()-construction.

For use case (2), when only Node.js is targeted, a static import
declaration utilize the same createRequire()-construction.

For both use cases, -sUSE_ES6_IMPORT_META=0 is not allowed, when
Node.js is among the targets, since it is not possible to mimic
__dirname when import.meta support is not available.

This commit does not change anything for use case (2), when only the
web is targeted (-sENVIRONMENT=web).

Resolves: #11792.
Resolves: #17797.

[kleisauke]

This should be ready for review, but I'm unsure how to proceed due to this FIXME:

emscripten/src/shell.js

Lines 205 to 208 in bc619d3

	// FIXME: How about the -sUSE_ES6_IMPORT_META=0 case? Is there a way of
	// getting the current absolute path of the module when support for
	// `import.meta.url` is not available?
	scriptDirectory = nodePath.dirname(fileURLToPath(import.meta.url));

Also, perhaps we should swap all require()'s with import()'s?

emscripten/emcc.py

Line 3596 in bc619d3

# TODO: Swap all `require()`'s with `import()`'s?

But AFAIK that requires a lot more changes, and I'm not sure it's worth it. For example, should we then eagerly import the crypto module too?

emscripten/src/library.js

Lines 2229 to 2238 in f9d26b0

	if (ENVIRONMENT_IS_NODE) {
	// for nodejs with or without crypto support included
	try {
	var crypto_module = require('crypto');
	// nodejs has crypto support
	return () => crypto_module['randomBytes'](1)[0];
	} catch (e) {
	// nodejs doesn't have crypto support
	}
	}

/cc @curiousdannii the creator of issue #11792.

[kripken]

cc @brendandahl who might know this ES6 stuff? I'm afraid I don't know much about it myself...

[sbc100]

According to the docs seem to exist on the url module, rather than at the top level..?

[brendandahl]

They are, but they're imported into the module's global scope (see emcc.py).

[sbc100]

But shouldn't closure be able to see that then? It seems odd to me that we should need to declare these in this global location like this? Perhaps a local annotation where they are declared/imported?

[brendandahl]

It does seem odd. Maybe closure doesn't understand await import('url')?

[kleisauke]

Indeed, it's a bit odd that this is needed. I think this was necessary when targeting both web/node as well as node only. That is:

if (typeof process == 'object' &&
    typeof process.versions == 'object' &&
    typeof process.versions.node == 'string') {
  const { createRequire } = await import('module');
  var { pathToFileURL, fileURLToPath } = await import('url');
  var require = createRequire(import.meta.url);
}

(-sENVIRONMENT=web,node)

import { createRequire } from 'module';
import { pathToFileURL, fileURLToPath } from 'url';
const require = createRequire(import.meta.url);

(-sENVIRONMENT=node)

Perhaps closure is run on an indeterminate file instead of the final JS file with these imports? Let me verify this.

[kleisauke]

It seems that Closure indeed has some limitations with dynamic import expressions.
https://github.com/google/closure-compiler/wiki/JS-Modules#dynamic-import-expressions

I moved these externals to the correct file with commit 68377b6 and removed the need of this URL import() with commit bae5bf5.

[sbc100]

Why pthreads here?

[kleisauke]

The test links with -pthread, so Node.js before version 16 needs the --experimental-wasm-bulk-memory --experimental-wasm-threads flags to successfully execute the subdir/hello_world.mjs script.

[sbc100]

lgtm with some comments

[curiousdannii]

This is a clever temporary solution, but I'm pretty sure it wouldn't help when the code is bundled, so it's not a full solution to #11792. Substantial parts of the JS output will need to be redesigned in order to be async, so as I've said before, before #11792 can be properly solved it will need direction from the core Emscripten team.

[sbc100]

This is a clever temporary solution, but I'm pretty sure it wouldn't help when the code is bundled, so it's not a full solution to #11792. Substantial parts of the JS output will need to be redesigned in order to be async, so as I've said before, before #11792 can be properly solved it will need direction from the core Emscripten team.

Are you ok with landing this as a temporary fix? It seems like have some test coverage is better than none.

[curiousdannii]

I haven't tested it myself, but the idea is a good one, and it would definitely be good to get some more test coverage.

[brendandahl]

cc @brendandahl who might know this ES6 stuff? I'm afraid I don't know much about it myself...

Looks and works how'd I'd expect.

[kleisauke]

but I'm pretty sure it wouldn't help when the code is bundled

Indeed, I think some bundlers will still fail on await import() (although that is only used when targeting both web/node) or import.meta.url (needed to get the current absolute path of the module, see the FIXME in #17915 (comment)).

Substantial parts of the JS output will need to be redesigned in order to be async

Is there a design document of how this would look like? I suppose Emscripten needs to change this emit:

var Module = (() => {
  var _scriptDir = import.meta.url;
  
  return (
function(Module) {
  Module = Module || {};

...

  return Module.ready
})();
export default Module;

to:

var Module = (() => {
  var _scriptDir = import.meta.url;
  
  return (
async function(Module) {
  Module = Module || {};

...

  return Module.ready
})();
export default Module;

(i.e., second function needs async, untested)

To allow await import() statements inside the module (annotated with ... in the above example). However, that wouldn't work for normal ES import statements (import ...), since that can't be put in conditionals. Currently, this (draft) PR uses this import syntax only when targeting Node (-sENVIRONMENT=node).

[kleisauke]

I resolved the above mentioned FIXME by ignoring -sUSE_ES6_IMPORT_META=0 when targeting Node, see commit bae5bf5. AFAIK, this option was introduced to fix compat on old browsers and bundlers.

[RReverser]

I suppose Emscripten needs to change this emit:

Actually... maybe best to do that as part of this PR? Then it wouldn't depend on top-level await, which is a relatively new feature in browsers (definitely newer than import.meta and we use conditional checks even for that) and still not supported by some tooling/bundlers.

[kleisauke]

Then it wouldn't depend on top-level await, which is a relatively new feature in browsers (definitely newer than import.meta and we use conditional checks even for that)

Ah, I think you're right. Let me fix that. I assumed that guarding the top-level await import for Node.js would prevent browsers from failing, but this probably didn't work for Node.js before v14.8 either.

[kleisauke]

... should be fixed with commit e8f83e9.

[RReverser]

This one I'm less sure about, but also worth checking if we can get away with just keeping the URL here.

[kleisauke]

Indeed, I wasn't sure about this. I removed the fileURLToPath construction with commit cff5945, however, that may cause the locateFile function to return file:// strings on Node.js.

emscripten/src/shell.js

Lines 152 to 161 in 84a6341

	// `/` should be present at the end if `scriptDirectory` is not empty
	var scriptDirectory = '';
	function locateFile(path) {
	#if expectToReceiveOnModule('locateFile')
	if (Module['locateFile']) {
	return Module['locateFile'](path, scriptDirectory);
	}
	#endif
	return scriptDirectory + path;
	}

Let's see if the CI likes that, though locally it still passes the other.test_emcc_output_mjs* other.test_export_es6* tests.

[RReverser]

however, that may cause the locateFile function to return file:// strings on Node.js.

Now I'm really curious, if file:// strings are not accepted directly, then how would same strings returned from locateFile work...

[kleisauke]

Now I'm really curious, if file:// strings are not accepted directly, then how would same strings returned from locateFile work...

IIUC, file:// strings are accepted after that isFileURI(filename) ? new URL(filename) ... change in read_, which will probably always be called on Node.js when reading files. But maybe it lacks some test coverage for this.

[RReverser]

Oh ok, that makes sense. (well, at least if nothing else uses the result of locateFile)

[kleisauke]

9074bb7 :)

[kleisauke]

Though, while doing that, I noticed this:

emscripten/src/shell.js

Lines 136 to 137 in 84a6341

	#if EXPORT_ES6
	var _scriptDir = import.meta.url;

... a couple of lines further ...

emscripten/src/shell.js

Lines 364 to 370 in 84a6341

	#if MODULARIZE
	// When MODULARIZE, this JS may be executed later, after document.currentScript
	// is gone, so we saved it, and we use it here instead of any other info.
	if (_scriptDir) {
	scriptDirectory = _scriptDir;
	}
	#endif

So, perhaps scriptDirectory was already a file://-string on Node.js when linking with -sEXPORT_ES6? Let me verify this.

[kleisauke]

... nvm, I didn't look carefully, that is guarded with #if SHARED_MEMORY && !MODULARIZE, though MODULARIZE is forcefully enabled when linking with -sEXPORT_ES6:

emscripten/emcc.py

Lines 2253 to 2257 in 89a494f

	if settings.EXPORT_ES6 and not settings.MODULARIZE:
	# EXPORT_ES6 requires output to be a module
	if 'MODULARIZE' in user_settings:
	exit_with_error('EXPORT_ES6 requires MODULARIZE to be set')
	settings.MODULARIZE = 1

So I'm not sure if this comment is up-to-date:

emscripten/src/shell.js

Lines 134 to 135 in 84a6341

	// In MODULARIZE mode _scriptDir needs to be captured already at the very top of the page immediately when the page is parsed, so it is generated there
	// before the page load. In non-MODULARIZE modes generate it here.

Edit: I need more coffee.

[kleisauke]

6e52016 :)

[RReverser]

Now you're in a danger zone :) But if it works, great!

[lgarron]

I do have a small nit, though, which is that the imports are still not prefixed with node:, e.g. node:module. This is necessary to distinguish the import from an npm package name. Would that be most appropriate to file a new issue for?

[lgarron]

I do have a small nit, though, which is that the imports are still not prefixed with node:, e.g. node:module. This is necessary to distinguish the import from an npm package name. Would that be most appropriate to file a new issue for?

I've created #18235 for this.

[RReverser]

I think node: prefix is a relatively recent addition in Node 18+, and not something supported in Node 14 that Emscripten ships with. Besides, I'm not sure what the bundler & other tooling support is like.

I'd rather err on the side of compatibility with more tooling for now.

[paulcdejean]

Hello this is a neat new feature but I don't see any documentation of it anywhere.