Skip to content

Support blob URLs on ES6 with USE_ES6_IMPORT_META enabled #23804

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Support blob URLs on ES6 with USE_ES6_IMPORT_META enabled #23804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions src/lib/libpthread.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,12 +424,16 @@ var LibraryPThread = {
worker = new Worker(p.createScriptURL('ignored'), {{{ pthreadWorkerOptions }}});
} else
#endif
// We need to generate the URL with import.meta.url as the base URL of the JS file
// instead of just using new URL(import.meta.url) because bundler's only recognize
// the first case in their bundling step. The latter ends up producing an invalid
// URL to import from the server (e.g., for webpack the file:// path).
// See https://github.com/webpack/webpack/issues/12638
worker = new Worker(new URL('{{{ TARGET_JS_NAME }}}', import.meta.url), {{{ pthreadWorkerOptions }}});
#if expectToReceiveOnModule('mainScriptUrlOrBlob')
if (Module['mainScriptUrlOrBlob']) {
var pthreadMainJs = Module['mainScriptUrlOrBlob'];
if (typeof pthreadMainJs != 'string') {
pthreadMainJs = URL.createObjectURL(pthreadMainJs);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like pthreadMainJs it not being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I had a typo on the next line. Should have read new Worker(pThreadMainJS). I’ll submit a fix soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve corrected it but I’m also ok to throw an error in case mainScriptUrlOrBlob is set under ES6_MODULES given -sASSERTIONS.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we split this up and into a PR just for mainScriptUrlOrBlob support? And include a test for that?

I think the change to use new Worker(import.meta.url..) can / should be separate .. and might take a longer to land since we want to confirm that it works in all the different bundlers.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, can you remove the USE_ES6_IMPORT_META from the title (that setting was removed).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I can do this.

}
worker = new Worker(new URL(pthreadMainJs, import.meta.url), {{{ pthreadWorkerOptions }}});
} else
#endif
worker = new Worker(import.meta.url, {{{ pthreadWorkerOptions }}});
#else // EXPORT_ES6
var pthreadMainJs = _scriptName;
#if expectToReceiveOnModule('mainScriptUrlOrBlob')
Expand Down
2 changes: 1 addition & 1 deletion test/third_party/googletest
Open in desktop
Submodule googletest updated 134 files