EXPORT_ES6: Add testing and support for node.js (replace require(), __dirname)

[curiousdannii]

So I'm finally getting around to trying EXPORT_ES6 mode, and it just doesn't work in node (when loaded as an actual ES6 Module), because it still refers to __dirname. I assume other people have only been using it Webpack/rollup/whatever?

It doesn't look like there are any tests for EXPORT_ES6 either.

[kripken]

I think we don't have tests for it because at the time there wasn't a version of node that supported it. If there is now, we should add some.

I think we have 12.10 on CI, and can maybe add another version. Another option is to use v8, which we already install a new version of, if it has ES6 module support.

It would be good if someone could write up a test for this and then we can check the options.

[curiousdannii]

Node 12.10 will definitely be enough.

[sbc100]

Just FYI, it looks like out current version of node that we use in emsdk and in all our testing is 12.18.1.

[jvail]

Could this issue be extended to cover also "require" vs "import"? I wonder if replacing all lines in the output such as

var nodeFS = require('fs');`

with

#if EXPORT_ES6
const { default: nodeFS } = await import('fs');
#else
var nodeFS = require('fs');

would be sufficient.

Unfortunately my insight in the build system is very limited. But if someone could start then I could help at least with testing.

[curiousdannii]

Realised this is going to be difficult to handle for multi-environment builds. ES import statements can't be put in conditionals, and while the import function can, it needs to be handled async.

[RReverser]

ES import statements can't be put in conditionals, and while the import function can, it needs to be handled async.

They don't need to, Emscripten could emit static import declarations instead.

[curiousdannii]

I said it would be difficult for multi-environment builds. A static import declaration for node will fail in the browser.

[RReverser]

Ah, right, I see. In that case, the async import sounds fine. The fact that it's async shouldn't be an issue - Emscripten already has a set of async runtime dependencies and a way to wait on them.

Take a look at usages of addRunDependency from

emscripten/src/preamble.js

Line 488 in 0cc4a66

function addRunDependency(id) {

.

[matthewp]

@RReverser You mentioned working on this in #13571 , do you have a test case that you were using?

[RReverser]

I said I stopped working on it and just waiting for someone else to fix it :)

do you have a test case that you were using?

Absolutely any file in EXPORT_ES6 mode. Try to generate emcc temp.c -o temp.mjs (it will export ES6 module) and then

import Module from './temp.mjs';
Module();

somewhere else. You'll run into the code trying to use __dirname.

The fix for just one configuration wouldn't be hard, but the general issue is that scriptDirectory is assigned from multiple places under different modes, and there needs to be some more systematic fix to align them correctly.

[lovasoa]

What is the best workaround for this at the moment ? Not using EXPORT_ES6 ?

[curiousdannii]

Yes, just don't use EXPORT_ES6. If you're targeting Node that won't be a problem, even if the rest of your app uses ES modules you'll be able to make it work (possibly by changing the file extension to .cjs, I can't remember the details.)

[matthewp]

@lovasoa I still use EXPORT_ES6, I just have a post-build script that removes the commonjs-isms. I do wind up producing separate Node.js and Browser entry points as a result though.

[dahuang37]

@matthewp I'm using EXPORT_ES6 as well, but I haven't figure out a way to get it compiled. I am able to get pass the __dirname error, but then it complained about require.
I'm curious how you did it. Can you point me to resources you used or the github repo with a working sample? I'm fairly new to this. many thanks in advanced :)

[surma]

In case it’s helpful, this has been my workaround for making EXPORT_ES6 files work in Node.js:

import {dirname} from "path";
globalThis.__dirname = dirname(import.meta.url);
import { createRequire } from 'module';
globalThis.require = createRequire(import.meta.url);

import emscriptenModule from "./my-emscripten-module.js";
// ... business as usual

[matthewp]

@yhung119 I just use sed to find and replace the require calls with references to path, etc. and then append appropriate import statements to the top. It does mean that I have separate JS files for Node and Browser which isn't great. Perhaps there is a better way. You can see what I'm doing here: https://github.com/lucydsl/liblucy/blob/77fb82fa6c72a3621de60da61c9d972da0659e6e/Makefile#L19-L26

[thomasballinger]

Here's smaller example of roughly the workaround described by @matthewp above along with a minimal repro (make node-repro) of the issue. https://github.com/thomasballinger/embind-ES6-for-node-and-browser

[kripken]

It looks like this is the best we have to a tracking issue for the general problems with the current EXPORT_ES6. See also the linked issues, and also some relevant discussion in WebAssembly/binaryen#3304 (comment)

Following that, some offline discussion raised the following ideas:

• For proper ES6 module support, we don't want MODULARIZE mode - we don't need a factory. We just want to emit the code without a factory, directly - being emitted in an ES6 module file takes care of the code being "modular".
• It seems hard to change the current relevant flags without breaking changes, EXPORT_ES6 and MODULARIZE, and we used to have MODULARIZE_INSTANCE.
• Instead, perhaps we can deprecate EXPORT_ES6, and add a new flag ESM (or ES_MODULE or such). The goal would be for that to emit exactly the right contents for an ES6 module: no factory, the exports are the expected ones, etc.
• When that is stable, we could remove the deprecated EXPORT_ES6.

Thoughts?

[curiousdannii]

that doesn't rely on injecting globals

I'm not really sure what you're referring to there, sorry. Nothing in that issue is talking about injecting globals?

[konsumer]

I just mean, again, if you look at the linked js, at the very end I do

export { writeAsciiToMemory, UTF8ToString }

so it's possible to actually use the library.

Otherwise it's relying on the fact that ES5 on web has a default window global-scope. ES6 doesn't work like this (and neither does node commonjs) so it won't work like that, but exporting them does.

[curiousdannii]

You should be listing those in EXPORTED_RUNTIME_METHODS so then they'll be exposed on the Module.

[elalish]

+1 for getting this solved. As a WASM library author, it's key that the library work transparently on both browser and server (nodeJS also simplifies CI testing considerably). As is I'll just publish as a CommonJS module, since that works, but I'd prefer to move to a modern ES6 module.

[kleisauke]

This will be fixed with PR #17915, see #17915 (comment) for the emsdk install instructions to test this.

[curiousdannii]

I don't think this should be closed yet as I don't think web+node builds will work when the output is put through a bundler. #17915 was great progress but it's not enough.

[RReverser]

as I don't think web+node builds will work when the output is put through a bundler

Did you try / run into specific issues? As far as I can tell from my usage, it does work, but you need to tweak the bundler config obviously to either ignore or polyfill Node modules (I chose ignoring as it's a better option to save code size, but either one should work).

But then, you have to do that either way. I suggest to track any bundler-specific improvements in separate issues. Native ES6 works now in both Node and browsers, and that's the primary goal.

[curiousdannii]

I haven't tested it. But for it to continue working a bundler would have to be able to output require()s when in ESM output mode, and I've never heard of one that does.

And no, users shouldn't need to configure their bundler beyond selecting ESM output. We need to move to dynamic import().

[RReverser]

But for it to continue working a bundler would have to be able to output require()s when in ESM output mode, and I've never heard of one that does.

Hm, why would you want require in your output? Are we both talking about bundling from Node.js code for the browser?

[dasa]

Could this help? https://github.com/rollup/plugins/tree/master/packages/commonjs#transformmixedesmodules

I agree that ideally the JS file should still workin in Node and the browser after being bundled by Rollup, Webpack, etc.

[curiousdannii]

Hm, why would you want require in your output? Are we both talking about bundling from Node.js code for the browser?

Because that's how #17915 works. It will work if you just use the output as is, but if you try to bundle it the bundler will convert the require calls into imports and then it won't work in both web and node.

[curiousdannii]

Could this help? https://github.com/rollup/plugins/tree/master/packages/commonjs#transformmixedesmodules

I agree that ideally the JS file should still workin in Node and the browser after being bundled by Rollup, Webpack, etc.

Oh, interesting. So yes, there is at least one bundler that could handle it.

[rojvv]

Yes, dynamic import() should be used.

[dasa]

Could the new Node API process.getBuiltinModule(id) help with this by removing the need to use require or import('module') to load the "fs" and "path" modules.

Doc: https://nodejs.org/docs/latest-v20.x/api/process.html#processgetbuiltinmoduleid

Maybe the code could use this when the min Node version is 20.16 or greater.