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

Avoid a race between reading from and writing to the importScripts array #352

Conversation

errendir
Copy link
Contributor

When WorkboxPlugin.GenerateSW is called it reads from the importScripts array. Webpack instance called from build-custom-worker.js appends to that array via the success callback. There is no guarantee that webpack compilation will finish before GenerateSW is called. This results in a bug where sometimes no scripts will be imported in the Service Worker code.

From my experiments to replicate this issue you need to ensure the webpack compilation is delayed enough. Then GenerateSW will not pick up any importScripts. For an example project triggering this bug see: https://github.com/errendir/next-pwa/tree/import-scripts-race-demo/examples/custom-worker

After running yarn build, please open the public/sw.js and format it with Prettier. You will find that no scripts have been imported into the ServiceWorker generated by workbox:

define(["./workbox-6316bd60"], function (e) {
  "use strict";
  importScripts(),
    self.skipWaiting(),
    e.clientsClaim(),

The above example works by feeding a very large function into Webpack during custom worker compilation. This slows it down enough so that the race condition can be highlighted.

This PR fixes this issue by appending the name of the custom worker script to the importScripts synchronously before GenerateSW is called. Note that this may be before the file is generated, but AFAIK GenerateSW doesn't verify if the script exists so the problem is solved

@shadowwalker shadowwalker merged commit 86a4c18 into shadowwalker:master Jun 6, 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