[dcodeIO]

This PR adds another build target to build-js.sh building a WebAssembly version using the JS wrapper as binaryen-wasm.js.

It already works (can run the kitchen-sink.js test) under the following conditions:

• When using node.js, binaryen-wasm.js must be required from the ./bin directory because it otherwise can't find binaryen-wasm.wasm next to it (looks in the current working directory instead?).
• When not using node.js (which seems to synchronously instantiate the module working around this issue), there is no mechanism yet to tell when the module is ready.

Opening this PR so we can figure something out :)

See also: #1380

[dcodeIO]

One way to make loading lazily work could be to provide the .then interface under all conditions, making it resolve instantly when it's not a lazy build. A non-lazy build would then initialize instantly while a lazy-build would require that the user uses .then. Both versions work like MODULARIZE_INSTANCE, and the safest way to use it would be to always use .then (though not necesary in a non-lazy-build), so things are interchangable. Hope that makes sense :)

[dcodeIO]

Ok, this one should "just work" (except for the cwd issue above), but .then is named .ready here so it doesn't conflict for now.

var Binaryen = ...;

// can test if already ready
console.log(Binaryen.isReady);

// or just use a promise and wait for it, no matter if already ready
Binaryen.ready.then(function() {
  ...
}, function(err) { throw err; });

// or if you like it rather fancy
(async function() {
  await Binaryen.ready;
  ...
})();

[dcodeIO]

Actually I figured that .ready should probably remain a binaryen.js-only thing because it requires specific knowledge of which parts in post-js must be delayed (here: initializeConstants). Generalizing this functionality would most likely result in more confusion than it'd be useful. At least "you should probably use MODULARIZE instead" seems like a more useful advice to me than "you first have to find out which parts of your code would fail in an asynchronous instantiation scenario, then wrap that in a function and do X, then Y and maybe Z".

Remains the cwd issue.

[kripken]

For the cwd, you can use Module.locateFile to customize where to look. I think that could work, but it would need to check if the file is in the current dir or under bin/ manually I suppose.

[kripken]

Maybe we could use SINGLE_FILE to embed the wasm? Not great for size, though.

How about disabling async compilation, does that work in node? (I know it would fail in chrome, but maybe v8 in node.js is more tolerant of sync compilation?)

[dcodeIO]

Hmm, I really thought Module.locateFile or Module.filePackagePrefixURL could solve this (if the latter also works for non-prepackaged files, just trying it as well), but it seems that both aren't actually accessed when I put the following in binaryen.js-pre.js and include it in just the wasm build:

Module['locateFile'] = function(file) {
  throw "locateFile called: " + file;
};
Object.defineProperty(Module, 'filePackagePrefixURL', {
  get: function() {
    throw "filePackagePrefixURL accessed";
  }
});

Any ideas?

[kripken]

The place where it should call locateFile is here: https://github.com/kripken/emscripten/blob/incoming/src/preamble.js#L1990 maybe some debugging logging around that location can help.

[dcodeIO]

Seems integrateWasmJS is executed before the pre-js code is. Is there a way to insert code even earlier?

[kripken]

That.. would be very odd if true :) the pre-js goes before the "prelude" file which is where the integrate method is.

Is the pre-js queuing code to happen asynchronously later perhaps?

[dcodeIO]

Scratch that, I have two --post-jses ... nooo

[dcodeIO]

Without, there is a warning when running binaryen-wasm.js. Is that correct?

[kripken]

Something about atexits? That's expected when assertions are on, but not in an optimized build. So we shouldn't see it here, I don't think. Or was it something else?

[dcodeIO]

Let me double check

[dcodeIO]

You are right, this must have been caused by something else.

[kripken]

as this is a pre-js, you can actually just check the ENVIRONMENT_IS_NODE variable

[dcodeIO]

I tried, but it wasn't defined already at this point.

[dcodeIO]

Just checked again without a minifier. Appears pre-js goes right after if (!Module) Module = typeof Binaryen !== 'undefined' ? Binaryen : {}; (starts at line 21) while ENVIRONMENT_IS_NODE is defined a bit later (line 63).

var Binaryen = function(Binaryen) {
  Binaryen = Binaryen || {};

// The Module object: Our interface to the outside world. We import
// and export values on it. There are various ways Module can be used:
// 1. Not defined. We create it here
// 2. A function parameter, function(Module) { ..generated code.. }
// 3. pre-run appended it, var Module = {}; ..generated code..
// 4. External script tag defines var Module.
// We need to check if Module already exists (e.g. case 3 above).
// Substitution will be replaced with actual code on later stage of the build,
// this way Closure Compiler will not mangle it (e.g. case 4. above).
// Note that if you want to run closure, and also to use Module
// after the generated code, you will need to define   var Module = {};
// before the code. Then that object will be used in the code, and you
// can continue to use Module afterwards as well.
// if (!Module)` is crucial for Closure Compiler here as it will otherwise replace every `Module` occurrence with a string
var Module;
if (!Module) Module = typeof Binaryen !== 'undefined' ? Binaryen : {};

// --pre-jses are emitted after the Module integration code, so that they can
// refer to Module (if they choose; they can also define Module)
// Make sure node is able to find the .wasm next to the .js file. Only relevant
// when building binaryen-wasm.js (not used for binaryen.js currently).
if (
  typeof process === 'object' && typeof require === 'function' && // IS_NODE
  typeof window !== 'object' &&                                   // !IS_WEB
  typeof importScripts !== 'function'                             // !IS_WORKER
) {
  Module['locateFile'] = function(file) {
    var nodePath = require('path');
    return nodePath.join(__dirname, file);
  };
}



// Sometimes an existing Module object exists with properties
// meant to overwrite the default module functionality. Here
// we collect those properties and reapply _after_ we configure
// the current environment's defaults to avoid having to be so
// defensive during initialization.
var moduleOverrides = {};
var key;
for (key in Module) {
  if (Module.hasOwnProperty(key)) {
    moduleOverrides[key] = Module[key];
  }
}

Module['arguments'] = [];
Module['thisProgram'] = './this.program';
Module['quit'] = function(status, toThrow) {
  throw toThrow;
};
Module['preRun'] = [];
Module['postRun'] = [];

// The environment setup code below is customized to use Module.
// *** Environment setup code ***
var ENVIRONMENT_IS_WEB = false;
var ENVIRONMENT_IS_WORKER = false;
var ENVIRONMENT_IS_NODE = false;
var ENVIRONMENT_IS_SHELL = false;
...

That's here and here.

[kripken]

Oh right, the pre-js goes before everything...

[dcodeIO]

Would it make sense to move just the ENVIRONMENT_IS_XY definitions up so pre-js can use/modify them? Could create a PR for this if it makes sense.

[dcodeIO]

See: emscripten-core/emscripten#6137

[dcodeIO]

Just figured that a.js has to be generated to bin/ now by check.py because it, like binaryen-wasm.js that is a part of it, looks for binaryen-wasm.wasm in the same directory. That's expected and actually an indicator that it is functioning as intended.

[kripken]

adding files to bin/ should be avoided. one option is to clean up those files, but then there's a risk of not reaching the cleanup code. how about copying the js and wasm files into the current dir? if we forget them there, that's less bad than leaving them in the bin dir. Even better would be to do this in a new subdir just for testing, perhaps, like test/temp/.

[dcodeIO]

The current dir is the project's base directory, where a.js was generated before, which wasn't that great either. Hence, a temp directory sounds ideal, will take a look.

[kripken]

About the synchronous/asynchronous issues,

1. Did we check if node.js can handle synchronous compilation? that would be best.
2. Is there some way in module loaders to handle this? What i mean is, when you do require() in node.js, is that an async or a sync operation from the perspective of the module being loaded? If node allowed an async callback for the require to complete, that would be really cool...
3. If we can't do either of those things to keep the API synchronous, I think we should consider changing it to be asynchronous everywhere - the asm.js and wasm versions being different seems confusing. But i'm not sure.

[dcodeIO]

Did we check if node.js can handle synchronous compilation? that would be best.

I know that node.js supports new WebAssembly.Instance(new WebAssembly.Module(...), {}), if that counts as synchronous. Though, as node.js is all about doing things asynchronous, I'd say that designing things to do compilation in the background is even better.

Is there some way in module loaders to handle this? What i mean is, when you do require() in node.js, is that an async or a sync operation from the perspective of the module being loaded? If node allowed an async callback for the require to complete, that would be really cool...

require is a synchronous operation that usually returns something (like at least a handle). In binaryen-wasm's case, it returns the same thing a browser would get when accessing Binaryen before it is ready. In all cases, the recommended (and universal) usage is:

Binaryen.ready.then(() => {
  ... use it ...
});

If we can't do either of those things to keep the API synchronous, I think we should consider changing it to be asynchronous everywhere - the asm.js and wasm versions being different seems confusing. But i'm not sure.

If I understood correctly, there is no way to do synchronous loading in browsers anyway if one wants to use an additional .wasm file (not embedded as base64 or something), is this right? If so, the default case should be asynchronous anyway because there's at least one important target that doesn't support synchronous instantiation.

From binaryen-wasm' vs. binaryen.js perspective, the key difference is that binaryen.js can, technically, be used right away (conveniently keeps backwards compatibility). BUT: In the docs I'd always recommend the asynchronous way (see example above) from this point onwards, to minimize confusion.

Or, in other words: Switching synchronous binaryen.js with asynchronous binaryen-wasm.js and vice-versa will just work as long as Binaryen.ready is used as shown above. That's what the updated tests are doing as well so these work with both versions. Again, I hope this makes sense :)

[kripken]

Yeah, makes sense overall.

About .ready.then(), could it be just .then() like MODULARIZE does now? Or is .ready better for some reason?

I see that Parcel allows synchronous syntax for loading wasm, by translating it to async at bundling time. We probably don't want to depend on a specific bundler, though, but that would have been nice...

[dcodeIO]

Currently it looks like this:

import { Binaryen } from "./binaryen-wasm.js";
Binaryen.ready.then(...);

or

import { Binaryen } from "./binaryen-wasm.js";
await Binaryen.ready;
...

About .ready.then(), could it be just .then() like MODULARIZE does now? Or is .ready better for some reason?

.ready has the advantage that it is a promise and thus can be used with await. It also does some binaryen.js specific things as outlined above, which conveniently retains backwards-compatibility when continuing to use just binaryen.js. It might be possible to make .then work similar, but that'd require that an Emscripten module instance extends promise and that it also does that in MODULARIZE_INSTANCE. Would look like:

import { Binaryen } from "./binaryen-wasm.js";
Binaryen.then(...);

or

import { Binaryen } from "./binaryen-wasm.js";
await Binaryen;
...

I see that Parcel allows synchronous syntax for loading wasm, by translating it to async at bundling time. We probably don't want to depend on a specific bundler, though, but that would have been nice...

I don't think that we can hook to import like in Parcel's examples, because when using binaryen.js or binaryen-wasm.js one is actually importing a .js file, not a .wasm. Unfortunately, I don't know what happens when the imported element in an async import(...) scenario is a promise itself. If it propagates that to where async import(...) is happening, changing .then as above might actually make it possible, but I'd not bet that this is how it's done.

[dcodeIO]

Looked this up pretty quick: It seems we can actually do that import thing. The only thing necessary is to return a promise as the module's export. Does this already work with just .then because it resembles one? Is anything with a .then property awaitable?

[kripken]

I think the Modularize .then() just looks like a promise, but isn't actually one. This caused some issues in emscripten-core/emscripten#5820 so maybe as discussed there we should change it.

Anyhow, I don't want to block this PR on all these larger issues. But picking the API (with .ready or not) is maybe something we should figure out before landing. Or at least we should have an idea of a way to change things later without breakage.

[dcodeIO]

I've worked something out here, which essentially makes every module a valid promise by means of var Module = new PromisedModule() and then works as usual (using .then if asynchronous, using nothing at all if synchronous). That's a node-version for easy testing, but one could also replace require with import, for example when using Parcel. Let me know if that's something you'd tolerate in Emscripten or not, as it's certainly doing... well... JavaScript-y things.

Btw, if we stuff PromisedModule into pre-js for now, it sould already work that way in binaryen.js/binaryen-wasm.js. Might be useful as a testing ground for this functionality, and if it turns out to be good enough, could translate to Emscripten.

Edit: Tweaked it a bit so it behaves correctly in chaining scenarios.

[dcodeIO]

The obvious alternative here is to simply provide a common .ready property on everything.

• MODULARIZE .ready promises the instantiated module (what the outer wrapper function did) and should then propagate to...
• MODULARIZE_INSTANCE .ready promises the module instance itself once it's ready (what onRuntimeInitialized did).

In any case, the result of .ready would be a ready to use module. In MODULARIZE it is a unique one, with its own memory etc., while in MODULARIZE_INSTANCE it's always the same one.

In this case, binaryen.js in its current state could be used as a test case, without changes, as it implemements the second. Btw, libwabt.js uses such a ready promise.

[dcodeIO]

I think I'd favor the .ready promise because of its simplicity. Less code and no need for promise shenanigans. Has zero drawbacks except style.

[kripken]

Interesting about wabt using .ready too - is that a general convention in the JS space these days? If so, definitely let's do that.

[dcodeIO]

is that a general convention in the JS space these days?

Not that I'm aware of. I guess it just makes sense to use a Promise, and to use ready as its name :)

[kripken]

I see. Well, I don't really have strong thoughts on the API one way or another, just trying to keep us consistent with other stuff... :) I'm ok to merge this in unless you think there's more to discuss.

I am somewhat worried about us already having a non-ready method on MODULARIZE, so maybe it's worth opening an issue on emscripten to discuss what to do more generally there.

[dcodeIO]

Superseded by #2503