Update workbox plugin to 5.1.2

[jflayhart]

Closes #8821

I would like to propose this change to take advantage of some great defaults Google gives us out of the box (pun intended) with their new V5 workbox plugin, however, I mainly want to remedy the issue Google fixes here.

This wonderful CRA library would be remiss if it did not stay up-to-date with Google's PWA best practices.

⚠️ In Workbox V5, Google recommends we move from CDN to local hosting of workbox (section: A shift towards local Workbox bundles & away from the CDN).

Verification

✔️ Run tests
✔️ Create an app with CRA
✔️ Build CRA (service-worker output below)

if(!self.define){const e=e=>{"require"!==e&&(e+=".js");let s=Promise.resolve();return r[e]||(s=new Promise(async s=>{if("document"in self){const r=document.createElement("script");r.src=e,document.head.appendChild(r),r.onload=s}else importScripts(e),s()})),s.then(()=>{if(!r[e])throw new Error(`Module ${e} didn’t register its module`);return r[e]})},s=(s,r)=>{Promise.all(s.map(e)).then(e=>r(1===e.length?e[0]:e))},r={require:Promise.resolve(s)};self.define=(s,i,t)=>{r[s]||(r[s]=Promise.resolve().then(()=>{let r={};const n={uri:location.origin+s.slice(1)};return Promise.all(i.map(s=>{switch(s){case"exports":return r;case"module":return n;default:return e(s)}})).then(e=>{const s=t(...e);return r.default||(r.default=s),r})}))}}define("./service-worker.js",["./workbox-c53a4ca0"],(function(e){"use strict";self.addEventListener("message",e=>{e.data&&"SKIP_WAITING"===e.data.type&&self.skipWaiting()}),e.clientsClaim(),e.precacheAndRoute([{url:"/index.html",revision:"eb8bfb6bb64b366dba06a3f93435cbfa"},{url:"/static/css/main.5f361e03.chunk.css",revision:"1472653e7cd5f55fc8eabda93770382e"},{url:"/static/js/2.07a05dc4.chunk.js",revision:"68929f3a533ff0586c32b25eb95307aa"},{url:"/static/js/2.07a05dc4.chunk.js.LICENSE.txt",revision:"e88a3e95b5364d46e95b35ae8c0dc27d"},{url:"/static/js/main.4a6b0373.chunk.js",revision:"8362d8add8803304da898f2b30a532e9"},{url:"/static/js/runtime-main.549be430.js",revision:"79926b8e343973b253d98be461e9b89a"},{url:"/static/media/logo.5d5d9eef.svg",revision:"5d5d9eefa31e5e13a6610d9fa7a283bb"}],{}),e.registerRoute(new e.NavigationRoute(e.createHandlerBoundToURL("/index.html"),{denylist:[/^\/_/,/\/[^\/?]+\.[^\/]+$/]}))}));
//# sourceMappingURL=service-worker.js.map

[facebook-github-bot]

Hi @jflayhart!

Thank you for your pull request and welcome to our community.We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[jflayhart]

deprecated as part of V5 breaking changes

[jflayhart]

[bebraw]

Would it make sense to join the efforts at #8485? That PR is doing the update and adding support for InjectManifest as well.

[jflayhart]

@bebraw Sure, but that PR involves much more change while this PR simply upgrades Workbox so CRA can immediately take advantage of the new workbox V5 features. Seems like this could go in prior to #8485 as an incremental step in the right direction rather than waiting on #8485 with all its improvements to be worked out.

I'd love to help where I can, though. The hope was that this would go in much more quickly, not create more work.

[bebraw]

@jflayhart Yeah, agreed. It would be cool to see this one land first as #8485 builds on top of this. I added a couple of notes there based on my initial experiences.

Let's hope a maintainer finds time to look at these. 👍

[iansu]

@jeffposnick Any default config changes you think we should make with this upgrade?

[jeffposnick]

(For reference, here's our release notes and migration guide.)

Two new options to consider adding:

• inlineWorkboxRuntime: true will produce a single service-worker.js output instead of both service-worker.js and workbox-[hash].js outputs. We default to false because it's technically more efficient to break out the Workbox runtime code from the top-level service-worker.js (e.g. code splitting, but for the service worker), but at the same time, I'm anticipating folks getting confused by this extra workbox-[hash].js file that they need to deploy. So, it might be best to go with inlineWorkboxRuntime: true.

• dontCacheBustURLsMatching: /\.[0-9a-f]{8}\./ can be used to optimize the precaching requests for resources that include 8 character hashes, which seems to be what c-r-a is using. It tells Workbox that it's safe to fulfill precaching requests from the HTTP cache for those URLs, saving a trip to the network.

I would love to see more flexibility in the long term around SW generation, via #8485 or something similar. But for now, that's what I'd suggest for a GenerateSW upgrade.

[jflayhart]

What about defaulting skipWaiting to true? Any reason we would not want to do that? Would fix #5316 as well, but I understand it's opt-in and can easily be done today by posting a "skip waiting" message to the service worker (that's how we're doing it at Shipt anyway).

Honestly, my main concern is simply purgeOnError with this PR 👍

[jeffposnick]

Please don't set skipWaiting to true. It can cause the set of currently cached URLs to get out of sync with the actual URLs that the current web app expects, breaking lazy-loading.

C.f. https://stackoverflow.com/questions/49482680/workbox-the-danger-of-self-skipwaiting/49494141#49494141 and https://pawll.glitch.me/

The various recipes for showing a "Reload for updated content" toast are the UX that works best here, where clicking on the button will both trigger skipWaiting() via postMessage(), and then immediately do a page reload.

[jeffposnick]

...actually, taking a step back for a second, how would the c-r-a maintainers feel if we switched from using GenerateSW to InjectManifest, along with an equivalent base service worker file that would, by default, behave the same way that the one produced by GenerateSW behaves?

I think that would actually make a lot of folks who want more control over their service worker's behavior much happier, as folks could edit the base service worker file as they see fit to customize the behavior.

The difference is explained at https://developers.google.com/web/tools/workbox/modules/workbox-webpack-plugin#which_plugin_to_use and this was previously a bit more complex to do in a controlled environment like c-r-a before Workbox v5.

[jeffposnick]

I wrote this up in more detail at #9141

[mrmckeb]

@jeffposnick I think more control over the SW would be great - and we know the community want that - so I'm all for this approach.

[jflayhart]

love it! thanks @jeffposnick

Feel free to close this as needed.

[iansu]

I believe this has all been addressed and superseded by #9205. Thank you for the PR though!