Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new 'none' ENVIRONMENT #12184

Open
curiousdannii opened this issue Sep 12, 2020 · 23 comments
Open

Add new 'none' ENVIRONMENT #12184

curiousdannii opened this issue Sep 12, 2020 · 23 comments

Comments

@curiousdannii
Copy link
Contributor

curiousdannii commented Sep 12, 2020

So in my project I'm manually downloading the WASM file, mainly so that I can do it in parallel with other large files, but also potentially so that I could add a download progress meter.

What would you think about adding a new ENVIRONMENT option of 'none' which would stop all shell type code from being emitted, would prevent the JS from trying to locate and download the WASM, would make the resulting code perfectly platform-agnostic, with no require calls, etc?

Things that would be slightly less optimal under this situation:

  • _emscripten_get_now might be less accurate in Node
  • something would need to be done for the TTY get_char in Node
  • crypto in Node. Would it just have to abort? Fall back to Math.random? (I assume there are good reasons not to, as it currently aborts rather than falling back to Math.random.)
@kripken
Copy link
Member

kripken commented Sep 12, 2020

For your use case is standalone wasm the best option? That emits no JS at all, just wasm, that you download yourself.

If not, then I think we could make -s ENVIRONMENT='' (empty list of environments) work, yeah, using just 100% generic JS, no node or web details, etc. That code would have no way to download the wasm, so it would depend on a hook from the user to get the wasm binary (or compiled WebAssembly.Module) I suppose?

@curiousdannii
Copy link
Contributor Author

No, I need the JS because I use FS, EM_JS, Asyncify, etc.

I thought adding 'none' would be clearer. The default is already currently ''.

@kripken
Copy link
Member

kripken commented Sep 15, 2020

Ah yes, the empty string means "everything" right now, so we do need something like none.

@curiousdannii
Copy link
Contributor Author

With the new potential way of doing environment stuff, I'm wondering if a setting like MANUALLY_PROVIDE_WASM might be better instead - it would just cut out all of the loading code in expectation that wasmBinary would be provided on Module.

@kripken
Copy link
Member

kripken commented Sep 18, 2020

@curiousdannii I think that makes sense, since the core of your idea is around loading the wasm.

We already have a Module.instantiateWasm hook, does that do what you want? (see src/preamble.js)

@stale
Copy link

stale bot commented Sep 21, 2021

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

@stale stale bot added the wontfix label Sep 21, 2021
@curiousdannii
Copy link
Contributor Author

This is still worth considering

@stale stale bot removed the wontfix label Sep 22, 2021
@curiousdannii
Copy link
Contributor Author

curiousdannii commented Jun 2, 2022

I've found that this is actually pretty necessary.

So currently I'm using ENVIRONMENT=web because I don't want any of the require() calls to be included, so that it will work with EXPORT_ES6. I can still run it in Node though as long as I manually load and set Module.wasmBinary. But if I then enable ASSERTSIONS, it complains that it's not running in the web. But what I actually want is just no web or node code to be included!

Edit: And setting ENVIRONMENT=node still doesn't work in ES6 modules in Node because require isn't present!

@sbc100
Copy link
Collaborator

sbc100 commented Jun 2, 2022

Would making ENVIRONMENT=node + -sEXPORT_ES6 work (by removing require statements) be enough to address your concerns?

Maybe we could have something like ENVIRONMENT=js_module .. would that make sense?

@curiousdannii
Copy link
Contributor Author

ENVIRONMENT=node + -sEXPORT_ES6

This still outputs require, __dirname etc.

I've previously been using ENVIRONMENT=web because it's the most platform agnostic option, however if you then turn on ASSERTIONS it has runtime errors if it can't find the browser. So the assertions are testing for things that it doesn't actually use. 😕

@sbc100
Copy link
Collaborator

sbc100 commented Jun 3, 2022

ENVIRONMENT=node + -sEXPORT_ES6

This still outputs require, __dirname etc.

Yes, but presumably it should not, since the user is asking for an ES6 module.. in which case require should not be in the output.

If we fixed that .. would that work for you?

@curiousdannii
Copy link
Contributor Author

That would be a much bigger improvement, though we've been waiting a long time for that haha.

A none option would still be nice for reducing file size a little though.

@sbc100
Copy link
Collaborator

sbc100 commented Jun 3, 2022

Should EXPORT_ES6 imply none? By that I mean, can an ES6 module access any node or browser-specific APIs ever?

@curiousdannii
Copy link
Contributor Author

curiousdannii commented Jun 3, 2022

Yes it should be able to do node and/or browser depending on what the environment is. They're orthogonal settings. The issue is that EXPORT_ES6 currently only changes the exporting code, not the importing code.

The main issue is #11792. Also relevant: #12203

Probably the big issue is that for multi-environment builds, it would require using the import() function, which is async, meaning the whole organisation of the output JS needs to be changed to be async compatible. So this isn't really something that a community member like me can address with a PR, it really needs direction from the core Emscripten team on what direction to take.

@sbc100
Copy link
Collaborator

sbc100 commented Jun 3, 2022

I see.. so fixing EXPORT_ES6 to be hermetic might be the best solution.

Do you think that making a new none environment would be the same amount of effort as that making EXPORT_ES6 correct/hermetic?

Can we think of EXPORT_ES6 as implicitly setting something likeENVIRONMENT=es6_module or ENVIRONMENT=none?

@curiousdannii
Copy link
Contributor Author

curiousdannii commented Jun 3, 2022

Something like #12203 would probably be a big improvement, and it seems that's the way @kripken wanted to go. If it's approached from the top down, then adding a none environment is basically a freebie, just don't include anything.

EXPORT_ES6 should do nothing except making the exported code a valid ES module, meaning both its exports and imports. In #11792 (comment) I suggested even replacing it with a MODULE_FORMAT setting. ES modules are useable in browsers, node, and web workers, all of which could have environment-dependent code that needs to be included or imported.

We have three main settings impacting output modes, and they're all orthogonal: ENVIRONMENT, EXPORT_ES6, MODULARIZE. Well and then MINIMAL_RUNTIME and everything else too!

@sbc100
Copy link
Collaborator

sbc100 commented Jun 3, 2022

I see, so it is possible to have an ES6 module that uses node-specific, or web-specific builtins? Its just happens that the require and __dirname builtins are not available, but other node-ish things are?

@curiousdannii
Copy link
Contributor Author

All the Node standard modules are still there, you just have to import or import() them rather than require() them. And there are workarounds for __dirname etc.

@nick-thompson
Copy link

I'll admit that I don't fully understand the details/implications of this conversation, but I think this is something I'd really like to see as well.

My use case builds wasm which is strictly for computational purposes– takes numbers and returns numbers. My wasm bundle doesn't need any Node/Web/Etc APIs but I'm still struggling to find the right way to ask Emscripten to output a bundle which reflects this (i.e. a bundle which includes no mention of import/require/__dirname, etc).

If there's an existing way to do that please let me know, but otherwise chiming in here for a +1 on a 'none' environment. Thanks!

@konsumer
Copy link

konsumer commented Jul 16, 2022

I would love to help with this. (I was commenting over on that linked issue #11792 , and it should have more context.)

For me, it's more of a reduction of current es6-type, rather than an addition. Currently, I edit the generated code by hand to inject wasmBytes and ignore the code that does "get the bytes" (in node/web) and also export anything that is needed to use it (like {writeAsciiToMemory, UTF8ToString }, etc) so I don't have to rely on globals that may or may not be set (depending on where the js is running.) It would be cool to not have to make this change by hand.

EDIT: @curiousdannii mentioned EXPORTED_RUNTIME_METHODS which would help with the exports part, but I still need a way to just get an exported function that takes wasm-bytes and doesn't try to do __dirname and require. I can create this by hand, but it's a major pain, and seems kind flakey.

@curiousdannii
Copy link
Contributor Author

EDIT: @curiousdannii mentioned EXPORTED_RUNTIME_METHODS which would help with the exports part, but I still need a way to just get an exported function that takes wasm-bytes and doesn't try to do __dirname and require. I can create this by hand, but it's a major pain, and seems kind flakey.

Use ENVIRONMENT=web as it is closest to a 'none' environment right now. It will still include the browser loading code, but it won't use it.

Two little issues:

  1. This won't work outside the browser if ASSERTIONS is enabled. ASSERTIONS checks that the environment is what it's meant to be. But that's mainly a testing setting, so it shouldn't matter in production.
  2. If EXPORT_ES6 is enabled but you later try to use a bundler like Esbuild and output to CommonJS, then it may complain that import.meta.url isn't set. This can be worked around by passing locateFile() {} into the Module constructor, but it would be good if it could be fixed too.

@konsumer
Copy link

Good call. On point 2: the whole problem I have is that I want to skip all the code that does the byte-loading (so, no need for __dirname, import.meta.url, etc, if it's not trying to resolve the path of anything.)

@curiousdannii
Copy link
Contributor Author

Well I want that too, that's what this issue is for. 😉 But with the right settings you can get it to work currently, it just outputs a little bit of unnecessary code. Compared to the size of the rest of the build I don't care about that though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants