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

Regression bug: Relative import of ES6 WASM modules from ES6 Modules does not work #9233

Closed
adevress opened this issue Aug 14, 2019 · 5 comments

Comments

@adevress
Copy link
Contributor

adevress commented Aug 14, 2019

Regression bug introduced by the PR #8940 related to the bug #8729.

Problem :
The PR is based on the fact that 'document.currentScript.src' exists in ES6 modules.
This is not the case for VanillaJS ( no magic bundling ) for Firefox>=68 and Chrome>=76.

This causes any import of ES6 module which is not in the same directory that the source html file to fail.

Reproduce :

Generate a ES6 WASM + js hello_world in a subdirectory
Create a parent main.html page + main.js vanillaJS ES6 module in a parent directory
Import the hello_world from the main.js.
It will fail: The hello_world.js will not succeed to fetch the WASM file.

Temporary Fix :

Override the "locateFile" in the Module itself with a callback that enforce the WASM module path

Definitive Fix:
Enforce the usage of PTHREAD and WebWorker is not a good solution considering that this support is disable in almost all the main WebBrowser for now.

I would be for adding a flag to disable "import.meta.url" on demand. And enable it by default for ANY ES6_MODULE.

adevress added a commit to adevress/emscripten that referenced this issue Aug 14, 2019
    This fixes the problem reported by emscripten-core#9233 and by emscripten-core#8729 while fixing the
    regression bug causes by emscripten-core#8940
@kripken
Copy link
Member

kripken commented Aug 14, 2019

cc @VirtualTim

@adevress
Copy link
Contributor Author

Just added a pull request #9234 with a proposed Fix.

Please do not hesitate to tell me if you do not like the solution.

@VirtualTim
Copy link
Collaborator

So if I'm understanding your use case right, your example uses document.currentScript.src, but you want it to use import.meta.url? Before this PR #8306 document.currentScript.src was always used. So this only worked between PR #8306 and PR #8940 (May-July). So while it is a regression it's not that far back.

I guess adding a new flag might be the only solution that keeps everyone happy. I don't really like it, there's already about 200 flags and I was hoping that the toolchain could determine what's best.

This issue is also related to #5104.

@adevress
Copy link
Contributor Author

Hi Tim,
I was using a version between this two PR yes.
A new flag sounds an appropriate solution to me that can be deprecated in few years when the support for import.meta is universal.

kripken pushed a commit that referenced this issue Aug 21, 2019
…9234)

This fixes the problem reported by #9233 and by #8729 while fixing the
    regression bug causes by #8940
@adevress
Copy link
Contributor Author

Problem solved. Thanks guys

belraquib pushed a commit to belraquib/emscripten that referenced this issue Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants