Workbox service worker #4169

davejm · 2018-03-17T03:33:56Z

This PR relates to #2340 and depends on #4159.

Switches the sw-precache-webpack-plugin for the Workbox webpack plugin
Generates service worker with identical functionality to that of the current implementation by default
Allows the user to customise the configuration of the service worker

Edit: I have removed the config/customisation behaviour as requested. The current behaviour is to use the GenerateSW method to precache app shell and assets.

Defaults (i.e. no config)

Generates a service worker that pre-caches app shell and assets, does navigateFallback and handles the Firebase whitelist scenario.

Customisation

You can do everything that you can normally do with the workbox webpack plugin by creating the file cra.config.js in the project route, and adding similar code to this:

module.exports = {
  workbox: {
    method: 'inject',
    config: {
      swSrc: 'src/my-custom-service-worker.js'
    }
  }
}

The method is 'generate' or 'inject' and corresponds to the two different usages of the workbox plugin. The config property is used for the actual plugin config.

See here for full configuration options.

Default configs (if not specified)

const defaultGenerateConfig = {
  exclude: [/\.map$/, /^(?:asset-)manifest.*\.js(?:on)?$/],
  navigateFallback: '/index.html',
  navigateFallbackWhitelist: [/^(?!\/__).*/], // fix for Firebase
};

const defaultInjectConfig = {
  exclude: defaultGenerateConfig.exclude,
  swSrc: path.join(paths.appSrc, 'sw.js'),
};

I haven't updated the documentation but I could help with that if necessary.

facebook-github-bot · 2018-03-17T03:34:12Z

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 up 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 the corporate CLA signed.

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

facebook-github-bot · 2018-03-17T03:39:22Z

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

davejm · 2018-03-17T12:27:13Z

In the meantime I published a package that can do something similar but doesn't require direct support from c-r-a or ejecting/forking https://github.com/davejm/react-app-rewire-workbox

wtgtybhertgeghgtwtg · 2018-03-17T22:07:45Z

I'm all for moving to workbox-webpack-plugin, but why does that necessitate configuration?

davejm · 2018-03-17T22:51:14Z

@wtgtybhertgeghgtwtg the configuration is optional. The default generation kicks in without having to do any config but a lot of people want to be able to customise their service workers so the configuration option is there if users want more control.

wtgtybhertgeghgtwtg · 2018-03-17T22:57:00Z

I think configuration would be better suited for another PR. That's a whole other can of worms.

davejm · 2018-03-18T10:11:13Z

Fair enough, I just wanted to make a prototype but I get that c-r-a config is a big deal to sort in and of itself.
I could remove the configuration from this PR and have a go implementing config in a separate PR. Something like https://github.com/vuejs/vue-cli/blob/dev/docs/README.md#configuration as suggested by @jeffposnick
cc @gaearon

davejm · 2018-03-18T10:28:04Z

This is a possible solution https://github.com/vuejs/vue-cli/tree/dev/packages/%40vue/cli-plugin-pwa
It's a bit easier than directly using their configureWebpack field and handles some other things as well. I quite like the idea of using webpack chain as well.
Also https://www.npmjs.com/package/register-service-worker could handle the registration as it's pretty standard and the package provides the hooks.

iansu · 2018-03-18T20:47:22Z

We don't have any plans to add a config file to Create React App. Please remove those changes from your PR so we can evaluate the switch from sw-precache-webpack-plugin to workbox.

davejm · 2018-03-18T22:29:48Z

@iansu not even an optional one? c-r-a is a life saver but I think there's a valid use-case for wanting to customise little things here and there without having to fully eject. At least there is react-app-rewired for that now.
Anyway, no problem, I'd still like to contribute so I can remove the code related to the optional external config file

davejm · 2018-03-20T15:48:24Z

@iansu I've made the changes

andriijas · 2018-03-20T16:32:39Z

packages/babel-preset-react-app/index.js

@@ -76,6 +76,9 @@ module.exports = function(api, opts) {
          // Adds component stack to warning messages
          // Adds __self attribute to JSX which React will use for some warnings
          development: isEnvDevelopment || isEnvTest,
+          // Will use the native built-in instead of trying to polyfill


The changes in the babel config, how do they relate to the workerbox?

Should it be done in seperate PR?

Sorry, at the time, next was broken and I had to merge #4159 as it was at the time. I'll rebase

andriijas · 2018-03-20T16:33:53Z

packages/react-scripts/config/webpack.config.prod.js

    }),
    // Moment.js is an extremely popular library that bundles large locale files
    // by default due to how Webpack interprets its code. This is a practical
    // solution that requires the user to opt into importing specific locales.
    // https://github.com/jmblog/how-to-optimize-momentjs-with-webpack
    // You can remove this if you don't use Moment.js:
    new webpack.IgnorePlugin(/^\.\/locale$/, /moment$/),
+    // Generate a service worker script that will precache, and keep up to date,
+    // the HTML & assets that are part of the Webpack build.
+    workboxPlugin,


All other plugins are just passing the config in the constructor.
Not saying its a bad idea to have it in a seperate file but for now Id say its easier

andriijas · 2018-03-20T16:34:40Z

packages/react-scripts/package.json

-    "@babel/core": "7.0.0-beta.38",
-    "@babel/runtime": "7.0.0-beta.38",
+    "@babel/core": "7.0.0-beta.42",
+    "@babel/runtime": "7.0.0-beta.42",


can you git rebase your changes on top of the next branch? babel is already upgraded to 42 in next and it will make the PR easier to review.

…rting optional configuration

davejm · 2018-03-20T17:02:26Z

@andriijas should be done

andriijas · 2018-03-20T20:37:43Z

I cloned your repo and checkout the branch

the content of the service-worker.js file is now

/**
 * Welcome to your Workbox-powered service worker!
 *
 * You'll need to register this file in your web app and you should
 * disable HTTP caching for this file too.
 * See https://goo.gl/nhQhGp
 *
 * The rest of the code is auto-generated. Please don't update this file
 * directly; instead, make changes to your Workbox build configuration
 * and re-run your build process.
 * See https://goo.gl/2aRDsh
 */

importScripts("https://storage.googleapis.com/workbox-cdn/releases/3.0.0/workbox-sw.js");

importScripts(
  "/precache-manifest.a2fe7ed56e9d3788775d424ccb33b17a.js"
);

/**
 * The workboxSW.precacheAndRoute() method efficiently caches and responds to
 * requests for URLs in the manifest.
 * See https://goo.gl/S9QRab
 */
self.__precacheManifest = [].concat(self.__precacheManifest || []);
workbox.precaching.suppressWarnings();
workbox.precaching.precacheAndRoute(self.__precacheManifest, {});

workbox.routing.registerNavigationRoute("/index.html", {
  whitelist: [/^(?!\/__).*/],

});

with the current plugin the asset list is built in to service-worker.js, is it possible to have this behaviour with workerbox?

davejm · 2018-03-20T21:00:48Z

I don’t think that’s possible. Is there a particular reason to not wanting the extra file which gets imported?

andriijas · 2018-03-20T21:07:22Z

I find it confusing with 2 manifests being generated by 2 different plugins, for different needs though.

And also the reference is hardcoded into the service-worker.js so the client needs to download 2 files (2 requests) to update the cache. Or?

davejm · 2018-03-20T21:19:11Z

Ah fair enough. I don’t think the web pack plugin supports embedding the list directly

jeffposnick

Thanks for working on this!

In addition to the line-level comments, my major feedback is that there is an in-flight PR that will update the legacy sw-precache-webpack-plugin config as well as some of the documentation: #3924

The changes made to the sw-precache-webpack-plugin config are superseded by this PR, but the documentation changes (to reflect that the service worker is going to be opt-in) still need to be merged. There are probably a few places in those docs that need to be updated, because they link to sw-precache-specific explanations, and linking to the equivalent Workbox explanations would make more sense.

So... I'm not the one merging this PRs, but what I think would make sense is to merge mine first and get in all the docs changes, and then your PR could include a clean break from sw-precache-webpack-plugin to Workbox. That might mean modifying those links in packages/react-scripts/template/README.md in this PR as well.

jeffposnick · 2018-03-21T17:20:24Z

packages/react-scripts/config/webpack.config.prod.js

+    new WorkboxWebpackPlugin.GenerateSW({
+      exclude: [/\.map$/, /^(?:asset-)manifest.*\.js(?:on)?$/],
+      navigateFallback: '/index.html',
+      navigateFallbackWhitelist: [/^(?!\/__).*/], // fix for Firebase


navigateFallbackBlacklist is newly available in workbox-webpack-plugin, to address the awkwardness of needing to put in a negative pattern here.

We could also potentially get a little more nuanced, and work around some of the issues raised in #3008 (comment) (which led to navigateFallback being turned off by default) with the blacklist. One approach would be to blacklist any URLs whose last part of the path contains a . character, since that's likely to be a file extension, and those have historically been files that should not be included in the navigateFallback logic.

And example config could be:

{ // ...other options... navigateFallback: '/index.html', navigateFallbackBlacklist: [ new RegExp('^/_'), new RegExp('/[^/]+\.[^/]+$'), ], }

See https://regex101.com/r/7f207Y/1

Got it. Should the . char RegExp maybe be /[^/]+\.[^/]+/?$ to include a potential trailing slash?

I think if there's a trailing slash, we don't want that to be blacklisted. I.e. we do want the navigateFallback behavior to take effect.

Also, in the current implementation, the firebase url is a double __ rather than single. Is that correct? If so should the single underscore prefixed urls also be blacklisted?

I think it's safe to go with blacklisting anything that starts with /_, i.e. a single underscore. I don't know how much of a convention it is to use a single vs. a double for "special" URLs outside of Firebase, but the false-positive rate of checking for a single underscores seems likely to be acceptable.

jeffposnick · 2018-03-21T17:21:20Z

packages/react-scripts/config/webpack.config.prod.js

+    // Generate a service worker script that will precache, and keep up to date,
+    // the HTML & assets that are part of the Webpack build.
+    new WorkboxWebpackPlugin.GenerateSW({
+      exclude: [/\.map$/, /^(?:asset-)manifest.*\.js(?:on)?$/],


I'd add in clientsClaim: true here, which would match the default behavior in sw-precache-webpack-plugin.

packages/react-scripts/config/webpack.config.prod.js

davejm · 2018-03-21T19:01:39Z

@jeffposnick thanks for the feedback. I'll make those changes.

jeffposnick · 2018-09-27T15:31:08Z

Sorry, wait, that's not that RegExp I was talking about 😄

I suggested using new RegExp('/[^/]+\.[^/]+$') as part of a blacklist in #4169 (comment). That's the RegExp that should hopefully catch URLs which consist of something, followed by ., followed by something else, in the last part of the URL path.

Timer · 2018-09-27T15:36:18Z

Whoa, where did I even get that Regex from then? I didn't write it ... 🤔

Timer · 2018-09-27T15:37:29Z

Ah, that's not valid regex and was formatted (per ESLint):

Are we going for this: new RegExp('/[^/]+[.][^/]+$')?

jeffposnick · 2018-09-27T15:52:32Z

My mistake—you need to escape the \ when using the RegExp constructor and passing in a string, so it's new RegExp('/[^/]+\\.[^/]+$')

jeffposnick · 2018-09-27T15:53:47Z

See https://regex101.com/r/7f207Y/1 for the RegExp in practice. (That link was lost in the noise of previous comments.)

facebook#4169 (comment)

Timer · 2018-09-27T15:56:57Z

Changed in ce5a2e4

gaearon · 2018-09-30T00:34:13Z

Is it normal that I get this in the build folder after doing the build? Do all these files need to be deployed?

I'm sure our users will start asking what Google Analytics is doing in their application output. Even if it doesn't send anything it'll look shady.

@jeffposnick

* Add workbox service worker functionality * Remove debug * Set workboxConfig for when there isn't a cra config file * Remove workbox configuration options as c-r-a isn't planning on supporting optional configuration * Remove c-r-a config path from paths * Add workbox service worker functionality * Remove c-r-a config path from paths * Inline the webpack workbox config * Use settings reccommended by @jeffposnick facebook#4169 * Fallback to public url index.html, not root * Add one comment * Update comment * Correct regex

facebook#4169 (comment)

levrik · 2018-10-01T10:30:10Z

Do I understand this correctly that it's still not possible to write your own service worker?
I'm currently adding push notifications to a CRA 1.x app and wanted to upgrade when I saw this PR but it looks like configuration has been removed.
What's the current procedure to use your own service worker? Currently I'm just placing it inside the public directory but there I'm loosing transpilation and, more important, the ability to import packages.

jeffposnick · 2018-10-01T17:33:14Z

Is it normal that I get this in the build folder after doing the build? Do all these files need to be deployed?

It's normal with the importWorkboxFrom: 'local' config to have those files generated, yes.

An alternative configuration that will produce fewer local artifacts would be to switch to importWorkboxFrom: 'cdn'... which it looks like was done by @Timer in 70b3110#diff-d2bddbd3bfb7051f9381474c844674fa, so I guess that's moot.

It's just a tradeoff between having a dependency on the "official" CDN for loading the Workbox files, or having more control over locally hosting them. Both are valid approaches, and switching over to the CDN is 👍 by me.

Timer · 2018-10-01T17:35:59Z

It'd be awesome if we could use local but specify what to bundle via the webpack plugin (so we can say routing and core only). It'd also be cool to specify prod only, using CDN as fallback for everything else.

jeffposnick · 2018-10-01T17:42:49Z

@philipwalton's been investigating the ergonomics of bundling the Workbox ES module source into a custom runtime.

The degree of bundle customization you're envisioning here is beyond what's currently supported, and also (I think) implies that users will end up with control over writing the code for their underlying service worker file. That's not currently something that c-r-a assumes—it assumes that your service worker will be generated as part of your build process.

gaearon · 2018-10-01T17:56:04Z

To me, the surprising part was that presumably you built a "React app" but only 10% of emitted files are your app. I know it's not a technical argument but I feel it's confusing. If workbox output was a single file that wouldn't have been an issue. Anyway, CDN works for us.

philipwalton · 2018-10-01T18:10:47Z

Workbox provides its own loader so that folks who aren't as familiar with bundlers (or the tooling in the JS ecosystem) can get started right away. But if you prefer to use a bundler to create your SW file, you absolutely can do that with Workbox (in fact, it's what I do on my blog with a custom build task using Rollup).

If you want to use Rollup or Webpack to bundle your SW, here's a basic example of what your entry script might look like (note, you don't use the workbox.* namespace because everything gets imported as a module):

import precaching from 'workbox-precaching';

precaching.precacheAndRoute(REPLACE_WITH_PRECACHE_ASSETS);

Then you can use something like rollup-plugin-replace or webpack's DefinePlugin to inject your precache manifest into your bundle (e.g. replace the REPLACE_WITH_PRECACHE_ASSETS above constant with an array of assets).

In other words, Workbox provides its own tooling to build a SW, but if you prefer the existing tooling in the ecosystem, you can totally just use that instead (or in addition).

longsleep · 2018-10-10T14:16:46Z

importWorkboxFrom: 'cdn',

is a bad default since it essentially means that the service worker is non functional if there is no internet access (for intranet only apps) or if there is a CSP

Refused to load the script '<URL>' because it violates the following Content Security Policy directive: "script-src 'self'"`).
service-worker.js:14 Uncaught

Similar to the other env vars i think this setting should be a configuration so it can be set to local. Imho local should also be the default.

levrik · 2018-10-10T15:59:31Z

@longsleep Also some people probably don't want to get it from a CDN for privacy reasons.

longsleep · 2018-10-10T16:05:59Z

@longsleep Also some people probably don't want to get it from a CDN for privacy reasons.

Yes, i created #5391 in the meanwhile in the hope that the issue can be quickly solved while we wait until the complete customziation of the workbox configuration is ready (#5369).

coderkevin · 2018-10-30T15:50:36Z

I've looked everywhere I can to find a way to add custom service worker code to CRA 2. From this PR it looks like it's simply not possible, and that's a shame. Especially so, since it would be so easy and unintrusive to do so.

karannagupta · 2018-12-12T22:04:13Z

@coderkevin While this is being worked upon in #5369 , you can try the workaround here to add a custom service worker (in CRA without ejecting) with the injectManifest mode using the workbox build workflow ...

facebook-github-bot added the CLA Signed label Mar 17, 2018

davejm changed the title ~~Workbox service worker config~~ Workbox service worker Mar 17, 2018

davejm mentioned this pull request Mar 20, 2018

Serviceworker - cache API response #4147

Closed

andriijas suggested changes Mar 20, 2018

View reviewed changes

davejm added 8 commits March 20, 2018 16:53

Add workbox service worker functionality

b792806

Remove debug

4059079

Set workboxConfig for when there isn't a cra config file

db75638

Remove workbox configuration options as c-r-a isn't planning on suppo…

7f17257

…rting optional configuration

Remove c-r-a config path from paths

b25b6e0

Add workbox service worker functionality

ee3c1ae

Remove c-r-a config path from paths

9a2024e

Inline the webpack workbox config

1a6f48d

jeffposnick suggested changes Mar 21, 2018

View reviewed changes

Update comment

285e663

Correct regex

7f6e7ef

Timer added tag: enhancement and removed issue: needs investigation labels Sep 27, 2018

Timer merged commit 3b71621 into facebook:master Sep 27, 2018

Timer added a commit to Timer/create-react-app that referenced this pull request Sep 27, 2018

Adjust regex fed to RegExp

ce5a2e4

facebook#4169 (comment)

frederikhors mentioned this pull request Sep 27, 2018

Last call for Create React App v2 #5103

Closed

gaearon mentioned this pull request Sep 30, 2018

react-scripts@2 final blockers #5141

Closed

25 tasks

zmitry pushed a commit to zmitry/create-react-app that referenced this pull request Sep 30, 2018

Adjust regex fed to RegExp

34ff052

facebook#4169 (comment)

HolyZheng mentioned this pull request Oct 29, 2018

基于react与typescript开发pwa HolyZheng/holyZheng-blog#24

Open

lock bot locked and limited conversation to collaborators Jan 8, 2019

Workbox service worker #4169

Workbox service worker #4169

Conversation

davejm commented Mar 17, 2018 • edited Loading

Defaults (i.e. no config)

Customisation

Default configs (if not specified)

facebook-github-bot commented Mar 17, 2018

facebook-github-bot commented Mar 17, 2018

davejm commented Mar 17, 2018

wtgtybhertgeghgtwtg commented Mar 17, 2018

davejm commented Mar 17, 2018

wtgtybhertgeghgtwtg commented Mar 17, 2018

davejm commented Mar 18, 2018

davejm commented Mar 18, 2018 • edited Loading

iansu commented Mar 18, 2018

davejm commented Mar 18, 2018

davejm commented Mar 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davejm commented Mar 20, 2018

andriijas commented Mar 20, 2018

davejm commented Mar 20, 2018

andriijas commented Mar 20, 2018

davejm commented Mar 20, 2018

jeffposnick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davejm commented Mar 21, 2018

jeffposnick commented Sep 27, 2018

Timer commented Sep 27, 2018

Timer commented Sep 27, 2018 • edited Loading

jeffposnick commented Sep 27, 2018 • edited Loading

jeffposnick commented Sep 27, 2018

Timer commented Sep 27, 2018

gaearon commented Sep 30, 2018

levrik commented Oct 1, 2018

jeffposnick commented Oct 1, 2018

Timer commented Oct 1, 2018

jeffposnick commented Oct 1, 2018

gaearon commented Oct 1, 2018

philipwalton commented Oct 1, 2018

longsleep commented Oct 10, 2018

levrik commented Oct 10, 2018 • edited Loading

longsleep commented Oct 10, 2018

coderkevin commented Oct 30, 2018

karannagupta commented Dec 12, 2018

davejm commented Mar 17, 2018 •

edited

Loading

davejm commented Mar 18, 2018 •

edited

Loading

Timer commented Sep 27, 2018 •

edited

Loading

jeffposnick commented Sep 27, 2018 •

edited

Loading

levrik commented Oct 10, 2018 •

edited

Loading