Remove JS wrapper and instead export necessary functions for wrapper implementers

[phated]

This is an alternative to #6192 in which Binaryen removes the JS wrapper and exports the functions needed by implementers. This helps in that the JS wrapper is not useful in all situations, such as js_of_ocaml bindings, and it is missing some newer functionality, like GC. These are the changes the Grain team needs to add try/catch and GC to our Binaryen.ml bindings.

With this change, the work in #6192 can just be moved to https://github.com/AssemblyScript/binaryen.js where the wrapper can be maintained in isolation (such as being written in TypeScript or anything else they need to do).

I've also removed the wrapper tests which should be moved to https://github.com/AssemblyScript/binaryen.js. If Binaryen would like to test the API exported by Emscripten, the C tests can be compiled with it and run in Node.

cc @ospencer @kripken @ericvergnaud

[ericvergnaud]

I like the idea of separating concerns i.e. 1) generate the wasm and 2) provide a convenient API for the wasm. But the latter should not move out of this repo to a repo not managed by the WebAssembly organisation (rather the opposite actually, the packaging and publishing to npm should move under the WebAssembly organisation).
The observed discrepancy between the binaryen.js API and the underlying wasm is imho a direct consequence of it.
Also, it's impossible to get the wrapper correct without referring to the c++ code.
I would support making the wrapper target a separate one, living in this repo, such that the wasm is as lean as can be, but the source of truth remains in one place.
And sorry for being straightforward, but removing tests is generally a bad idea. Without them, how does one ensure that the generated wasm behaves as expected ? Without the wrapper, many wasm entry points are unusable.

[ericvergnaud]

FYI I've added GC to #6192 (WIP), and maybe if you add EH that would meet Grain requirements ?

[ericvergnaud]

If you look at src/ts/binaryen_post.ts in #6192 you'll find that it's down to almost nothing, so Grain could maybe adopt the now quite lean generated wasm, and build their own wrapper as desired ?

[phated]

We are at an impasse @ericvergnaud. I've tried to contribute to the JS and @kripken is not confident in any changes. It's better to rely on emscripten to be correct and wrapper authors can just rely on the low-level API

[CountBleck]

Frankly, AssemblyScript itself doesn't use the high-level APIs that are in binaryen.js, as far as I'm aware. It uses its own high-level wrappers found here. That means AS would do perfectly fine without the high-level API being removed here.

At the same time, I do know there are users that do use the high-level API for their own projects, but, as suggested, moving it to the binaryen.js repo would work just fine.

@dcodeIO WDYT?

[dcodeIO]

Indeed, at AS we are using the C-API directly. So far, I considered maintaining the accompanying JS+documentation as a nice-to-have for those within the community who rely on it.

I would disagree on the priorly assumed causes of discrepancies. How this typically goes is that new/experimental features are added to the C++ API, sometimes also to the C-API, and sometimes also to the JS API - with decreasing probability. Binaryen.js provides typings and packages for what's in this repo's JS bindings, but can of course only do so for what exists.

[ericvergnaud]

I would disagree on the priorly assumed causes of discrepancies. How this typically goes is that new/experimental features are added to the C++ API, sometimes also to the C-API, and sometimes also to the JS API - with decreasing probability. Binaryen.js provides typings and packages for what's in this repo's JS bindings, but can of course only do so for what exists.

Maybe you're missing the bit where binaryen.js is not even compatible with what's in this repo's JS bindings ? that is definitely caused by the dual repos (and is what triggered #6192 in the first place).

(that said many genuine thanks to the individuals like yourself who went the extra mile and did make something happen).

I guess we've moved from a technical PR to a broader governance one, which maybe deserves a dedicated discussion. I created one here: #6225 .

[CountBleck]

Maybe you're missing the bit where binaryen.js is not even compatible with what's in this repo's JS bindings ?

I don't follow. Are you referring to the .d.ts file in the binaryen.js repo? That's the only thing that could be considered incompatible, as that's practically the only thing unique to the binaryen.js repo (everything else is from this repo).

[ericvergnaud]

Are you referring to the .d.ts file in the binaryen.js repo?

Yes that's what I'm referring to

[dcodeIO]

Maybe you're missing the bit where binaryen.js is not even compatible with what's in this repo's JS bindings ? that is definitely caused by the dual repos (and is what triggered #6192 in the first place).

I am struggling to see what exactly is not compatible. Historically, Binaryen.js wraps both Binaryen-compiled-to-JS (which once was the default and does not need to be awaited) and Binaryen-compiled-to-Wasm (which now needs to be awaited as it instantiates) in a common way, awaiting and providing common typings for both. Is that what you are referring to?

[CountBleck]

I think he's referring to the typings in the binaryen.js repo becoming out of sync with changes in the binaryen.js-post.js file here. That can only be avoided if binaryen.js-post.js here became typed, as constantly syncing API changes between this repo and a second repo providing types is difficult. That is what @ericvergnaud is advocating, in #6192.

Meanwhile, if the JS wrapper is removed as per this PR (which likely has its own merits), then this syncing problem will persist, but instead of syncing changes in the types of JS functions, it would be syncing changes to the Binaryen C API. That would make the problem worse, unless one's willing to sacrifice nightly builds.