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

Issue with SW async install (& importScripts)? #470

Open
jeffposnick opened this issue May 29, 2019 · 2 comments
Open

Issue with SW async install (& importScripts)? #470

jeffposnick opened this issue May 29, 2019 · 2 comments
Assignees

Comments

@jeffposnick
Copy link

Debug info
Screen Shot 2019-05-29 at 10 01 22 AM

Describe the bug
(I'm not sure how much of an issue this actually is, and I'd be interested in hearing from @jakearchibald in particular here.)

I was planning on using rollup-plugin-loadz0r in a project of my own, and was curious as to how Proxx addressed an issue I first reported at surma/rollup-plugin-loadz0r#3. Here's the summary:

As per the SW spec, and summarized at w3c/ServiceWorker#1319 (comment), calls to importScripts() are allowed either during the initial, synchronous, top-level SW execution or during the install event handler.

From what I can tell by looking at the final, bundled Proxx service worker, the install event listener doesn't get registered until after importScripts() is called, and both importScripts() and then the install listener registration happens asynchronously, following a promise resolution.

My understanding was that both of those things were problematic to do asynchronously, and instead had to be done during the initial, synchronous execution of the service worker script.

To Reproduce
There's inherently a race condition here, and I haven't been able to reproduce a failure, but in theory, the following could happen:

  1. Visit Proxx without an existing service worker registration.
  2. The importScripts() and install event listener happen following a promise resolution, which seems like it could cause issues in certain browsers, as per the service worker specification.

Expected behavior
The safest thing (to my mind, at least, but correct me if I'm wrong) would be if rollup-plugin-loadz0r had a mode in which it detected that you were in the ServiceWorkerGlobalScope and instead of implementing an asynchronous define, instead just implemented a synchronous defined that was effectively an alias for importScripts().

@jakearchibald
Copy link
Contributor

There isn't a race condition here, as there's a microtask checkpoint before the has ever been evaluated flag is set.

When spec land calls JavaScript, microtasks will always happen before the next line in the spec, due to the clean up after running script steps.

Regardless, I can't decide if loadz0r should be sync in workers, or if it should be 'async' for consistency. My gut reaction is that it should be sync, but I don't have any good reasoning.

@surma
Copy link
Contributor

surma commented May 29, 2019

Yeah, it should probably be sync. I need to update it for some breaking changes from rollup anyway.

Until then: We ship our own loadz0r template anyways, so we can fix it there. I’ll tackle that in the next sprint.

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