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

Switch to Workbox's InjectManifest plugin #9141

Closed
jeffposnick opened this issue Jun 11, 2020 · 8 comments · Fixed by #9205
Closed

Switch to Workbox's InjectManifest plugin #9141

jeffposnick opened this issue Jun 11, 2020 · 8 comments · Fixed by #9205

Comments

@jeffposnick
Copy link
Contributor

jeffposnick commented Jun 11, 2020

Is your proposal related to a problem?

Many want more control over their service worker. The current c-r-a setup uses Workbox's GenerateSW mode, which uses a declarative webpack configuration to generate the final service worker. Changing the behavior of the service worker therefore requires updating the webpack config, which is not allowed in c-r-a.

Describe the solution you'd like

In Workbox v5, the InjectManifest mode will take an "source" service worker file (either in JavaScript or TypeScript) and run it through a webpack child compilation, which allows you to write your service worker source file just like any other modern JavaScript code, including using ES module imports for the Workbox runtime.

The InjectManifest plugin also takes care of reading the list of assets generated by the webpack compilation and injecting a precache manifest into the service worker file that will precache those assets.

I'd like to switch from GenerateSW to InjectManifest in the c-r-a webpack configuration, and create a new packages/cra-template/template/service-worker.js (and TypeScript equivalent) that roughly contained:

import {clientsClaim} from 'workbox-core';
import {precacheAndRoute, createHandlerBoundToURL} from 'workbox-precaching';
import {registerRoute, NavigationRoute} from 'workbox-routing';

clientsClaim();

precacheAndRoute(self.__WB_MANIFEST);

const handler = createHandlerBoundToURL(process.env.PUBLIC_URL + '/index.html');
const navigationRoute = new NavigationRoute(handler, {
  denylist: [
    // Exclude URLs starting with /_, as they're likely an API call
    new RegExp('^/_'),
    // Exclude any URLs whose last part seems to be a file extension
    // as they're likely a resource and not a SPA route.
    // URLs containing a "?" character won't be blacklisted as they're likely
    // a route with query params (e.g. auth callbacks).
    new RegExp('/[^/?]+\\.[^/]+$'),
  ],
});
registerRoute(navigationRoute);

self.addEventListener('message', (event) => {
  if (event.data && event.data.type === 'SKIP_WAITING') {
    self.skipWaiting();
  }
});

This would lead to a service worker that, by default, behaved equivalently to what the previous GenerateSW config created. The benefits are that any part of that service-worker.js could be edited by the end developer, leading to, e.g., custom service worker runtime caching.

I would also like to rename the current packages/cra-template/template/serviceWorker.js file to packages/cra-template/template/serviceWorkerRegistration.js to make it clearer that the file is used in the window context to perform registration, and that it does not include the code for the service worker itself.

In general, giving developers control over their source service worker in this manner should be relatively decoupled from the underlying webpack config. Even if developers made significant changes to their source service worker, by adding in new Workbox modules or other third-party code, the same webpack configuration should still be compatible. The one requirement is that the developer would need to keep the string self.__WB_MANIFEST somewhere in their source service worker file, as InjectManifest checks for this and will fail if it's not present. (It could be inside of a comment like, e.g. /* self.__WB_MANIFEST */ as a workaround for developers who don't want to use precaching.)

Describe alternatives you've considered

#8822 is an alternative that will just update to Workbox v5.1.2 and keep using the GenerateSW mode, which is okay, but I think developers would be happier if they had the ability to control their service worker. Workbox v5 allows that to be done cleanly in a way that wasn't possible with prior releases.

Additional context

See #7966 for a similar proposal.

The difference between the two mode is described in more detail at https://developers.google.com/web/tools/workbox/modules/workbox-webpack-plugin#which_plugin_to_use

@mrmckeb
Copy link
Contributor

mrmckeb commented Jun 21, 2020

I like this approach, and particularly agree with renaming the file.

From what I understand, you say that it should be compatible with existing projects - or did I misunderstand, and existing CRA users may need to make changes? If so, can we automate that (easily)?

@jeffposnick
Copy link
Contributor Author

From what I understand, you say that it should be compatible with existing projects - or did I misunderstand, and existing CRA users may need to make changes? If so, can we automate that (easily)?

Well, in the proposal, packages/cra-template/template/serviceWorker.js is renamed, so that part isn't compatible. Depending on how much of a deal-breaker that is for the c-r-a maintainers, packages/cra-template/template/serviceWorker.js could keep the same name, but I do worry that folks would end up being confused by the presence of both packages/cra-template/template/serviceWorker.js and a separate service worker source file.

My point about compatibility was that I think moving forward, this setup would mean that local customizations to the service worker source file would be decoupled from the webpack config, since a fairly generic InjectManifest config should work regardless of what changes you make to your service worker source file—the one requirement is that you keep self.__WB_MANIFEST somewhere in the file if you want to use the InjectManifest plugin.

@mrmckeb
Copy link
Contributor

mrmckeb commented Jun 23, 2020

Yes, I understand - I think we could automate that rename as a part as an upgrade, in which case I'd be on board.

@jeffposnick
Copy link
Contributor Author

Yes, I understand - I think we could automate that rename as a part as an upgrade, in which case I'd be on board.

Great! I could put together a PR for this if there's buy-in.

Is there an upcoming major semver release of c-r-a planned?

@mrmckeb
Copy link
Contributor

mrmckeb commented Jun 23, 2020

Actually, it's due very soon I think - @ianschmitz was working on a few last pieces. CC @iansu.

@ianschmitz
Copy link
Contributor

The existing service worker file is already managed by the dev. I'm wondering if this could just be part of release notes to copy from the template if they want to use? Would be much simpler I imagine.

@jeffposnick
Copy link
Contributor Author

Okay—I've filed a PR with the changes discussed above. Based on past experience, I do want to make sure that this isn't something "rushed" out there to meet to a major release deadline, and I hope the community gets to try out this approach in some prereleases. I obviously trust the judgment of the maintainers 😄

@garyng2000
Copy link

I think this is the only san way to bring full service worker functionality into CRA, since SW is basically a separate 'daemon' running side by side with the front end part, and it needs webpack support for complicated logic. Currently, I am ejecting the project then change the webpack config via ImportScripts. This kind of work for simple logic but if I need to pull in other supporting libraries(say I need encryption from aes-js), it becomes complicated as my hand coded script is not part of the build process of webpack.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants