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

Enable es-module-shims usage in web workers #300

Merged
merged 5 commits into from
Jun 14, 2022
Merged

Enable es-module-shims usage in web workers #300

merged 5 commits into from
Jun 14, 2022

Conversation

costingeana
Copy link
Collaborator

The purpose of this commit is to enable the usage of es-module-shims in module web workers, which at the moment are supported in Chrome based browsers and Safari > 15. As you know, in Firefox the support is still missing.

However, web workers cannot access objects like document or window. Therefore, I had to do the following changes in es-modules-shims code:

  • add some simple pieces of code that check whether document/window exists before doing something with the document/window object; it's not elegant, but these checks are necessary;
  • add a function that computes the baseUrl. In fact, this function was already defined in es-module-shims version 0.4.7.
  • I also added a new function setImportMap which can be used to set the import map from outside (obviously). This function is useful when you start a web worker which uses es-module-shims.

It’s important to mention that in web workers, I use the es-module-shims.wasm.js output. Why? Because of the dynamicImportScript function; I’m not sure if there is a solution for the CSP version in web workers.
Also, I want to emphasize that I didn't add new tests because no new functionality was added. The existing tests cover the changes from this pull request. The usage of es-module-shims in web worker is a decision that belongs to the client code.

Below, I attach a piece of code that starts a web worker which uses es-module-shims:

// baseURL, esModuleShimsURL, aURL are provided

const workerScriptUrl = URL.createObjectURL(new Blob(
        [`importScripts('${new URL(esModuleShimsURL, baseURL).href}');importShim.setImportMap(${JSON.stringify(importShim.getImportMap())});importShim('${new URL(aURL, baseURL).href}').catch(e=>setTimeout(()=>{throw e}))`],
        { type: 'application/javascript' }))

const worker = new Worker(workerScriptUrl);

…in web workers.

However, web workers cannot access objects like document or window. Therefore, I had to do the following changes:

- add some simple pieces of code that check whether document/window exists before doing something with document/window object;
- add a function that computed the baseUrl. In fact, this function was already defined in es-module-shims version 0.4.7.
- I also added a new function setImportMap which can be used to set the import map from outside (obviously). This function is useful when you start a web worker which uses es-module-shims.

It’s important to mention that in web workers, I use the es-module-shims.wasm.js output. Why? Because of the dynamicImportScript function; I’m not sure if there is a solution for the CSP version in web workers.
Copy link
Owner

@guybedford guybedford left a comment

Choose a reason for hiding this comment

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

Thanks this looks good!

Could you add a small documentation section for workers, explaining how this can be used? This may well be useful to others if they know about it.

See note about unifying some of the guards.

Perhaps worker CSP may be possible using import scripts of a blob? Not sure, but may well make sense to keep current CSP separation just unsupported for now.

src/es-module-shims.js Outdated Show resolved Hide resolved
src/es-module-shims.js Outdated Show resolved Hide resolved
src/es-module-shims.js Outdated Show resolved Hide resolved
src/env.js Outdated Show resolved Hide resolved
@costingeana
Copy link
Collaborator Author

Hello @guybedford and sorry for the delay.

I applied the suggested changes and I added a small documentation section; however, I'm not sure if it's in the right place.
For now, CSP is not supported for workers. I will try your suggestion in the next days.

Thank you.

Copy link
Owner

@guybedford guybedford left a comment

Choose a reason for hiding this comment

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

Couple more doc and code comments, but I think we're there. Thanks I really like how simple this approach turned out.

I'll aim to merge in the next few days.

README.md Outdated Show resolved Hide resolved
src/es-module-shims.js Outdated Show resolved Hide resolved
src/es-module-shims.js Outdated Show resolved Hide resolved
src/es-module-shims.js Outdated Show resolved Hide resolved
src/es-module-shims.js Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@guybedford
Copy link
Owner

guybedford commented Jun 14, 2022

I added the addImportMap reimplementation and rename, please just check that still works out correctly for your use cases.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/es-module-shims.js Outdated Show resolved Hide resolved
@guybedford guybedford merged commit ffd4fd9 into main Jun 14, 2022
@guybedford guybedford deleted the extra-checks branch June 14, 2022 20:43
@guybedford guybedford mentioned this pull request Jun 14, 2022
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

Successfully merging this pull request may close these issues.

2 participants