workbox icon indicating copy to clipboard operation
workbox copied to clipboard

Support for opt-in compilation in injectManifest mode

Open jeffposnick opened this issue 3 years ago • 10 comments

(Moving a conversation from https://twitter.com/jeffposnick/status/1286392399036588037)

To summarize, workbox-build's generateSW and workbox-webpack-plugin's InjectManifest both perform a compilation, using Rollup in the case of workbox-build's generateSW, and a child compiler in the case of workbox-webpack-plugin's InjectManifest.

workbox-build's injectManifest, however, does not perform a compilation. It just generates a precache manifest and does string replacement to swap that manifest in for self.__WB_MANIFEST in the swSrc file.

The reasons for this (some good, some bad) are:

  1. Before v5, workbox-webpack-plugin's InjectManifest didn't perform a compilation either. So there wasn't an inconsistency historically.

  2. The assumption is that developers who are comfortable using JavaScript modules are already using a bundler like Rollup or Parcel in other parts of their web app. Asking them to bundle their service worker using their existing bundling process isn't a burden. (This is likely to be less true over time, as <script type="module"> support makes it easier to consume JavaScript modules in other parts of web apps without bundling.)

  3. Not every developer has migrated to using Workbox via JavaScript modules. Many of them still use workbox-sw to load in Workbox's runtime via importScripts(), especially if they've migrated code from v4 or earlier. Bundling of their service worker isn't required in that case.

  4. It arguably makes more sense for the recommended steps to be "use Rollup with a plugin that handles injectManifest" as opposed to "use injectManifest with a configuring that runs Rollup automatically".

I'm not sure if any of those reasons are great given how developers are using JavaScript in 2020, and I'm personally open to exploring a mode in workbox-build that does both automatic compilation and manifest injection—probably not for v6, though.

jeffposnick avatar Jul 23 '20 20:07 jeffposnick

Personally, I prefer to keep these things detached:

  • Construct service worker by manifest injection https://github.com/webmaxru/angular-pwa/blob/workbox-v4-bundle/workbox-build-inject.js
  • Compile it by Rollup with all the customizations I need https://github.com/webmaxru/angular-pwa/blob/workbox-v4-bundle/rollup.config.js

And I combine these two in the general build script https://github.com/webmaxru/angular-pwa/blob/workbox-v4-bundle/package.json#L13

Not too much hassle while keeping a control over every single detail

webmaxru avatar Jul 23 '20 21:07 webmaxru

Personally, I prefer to keep these things detached:

If injectManifest would support the mode property you'd still be able to keep the way youre doing it, and have 'finegrain' control over your service worker. (Although I imagine the mode property would probably achieve pretty much the same goal your rollup config is doing, theres not that much finegrain control needed there, which means with mode being supported you have one less rollup config/script in your project to deal with 🙂)

I've also at first-hand seen people (myself included) be confused about their service worker code including process.env variables, especially in an ecosystem thats moving more and more to ESM.

And frankly, the discrepancy between generateSW handling mode just fine, but injectManifest not, is... odd. If a user uses generateSW, they're good to go. But if they at some point during their project switch to injectManifest because they want to add custom sw logic, they now also have to set up a buildstep for their service worker as well. (And again, are required to add extra configuration to their project)

Edit: For anybody using Rollup, rollup-plugin-workbox now supports the mode property. read more

thepassle avatar Jul 23 '20 21:07 thepassle

As user of Create-React-App + TypeScript + injectManifest (we try to build offline first app), that not able to configure webpack directly (without ejecting), I would rather have a compile mode. And many users of CRA as I see, too. Because now I must use backdoor with node script for compilation of my SW.

TheForsakenSpirit avatar Aug 13 '20 09:08 TheForsakenSpirit

I'm making PWA for some times now, and i'm using typescript / babel / webpack without CRA to bundle my javascript. I love to keep controls over things so i'm using InjectManifest.

With this configuration, i can't have one step to build my service worker and i need to :

  • Make a script to run InjectManifest after the app build to inject manifest in the service-worker bundled (like this way)
  • Or make another webpack configuration and use the workbox-webpack-plugin to inject manifest in the service-worker bundled

That's a bit obvious for me, and i would like to have service-worker compilation (the same way as my app) and manifest injection in the same build process.

So, a compilation would be great :)

blephy avatar Oct 03 '20 17:10 blephy

Hello folks! After working more with CLI, workbox-build, external bundlers I now feel that having opt-in compilation in injectManifest is a very good idea from DX perspective. Maybe, in v7? :)

webmaxru avatar Dec 01 '20 15:12 webmaxru

Long-term, https://bugs.chromium.org/p/chromium/issues/detail?id=824647 will hopefully be a thing, and you'll be able to import the Workbox runtime from ES modules without bundling. But that's a ways off.

I'm still not sold on adding this in to workbox-build directly, for the same reasons mentioned above.

If folks on this thread feel strongly about it, wrapping a call to a pre-configured Rollup/ESBuild/Parcel/webpack execution + workbox-build's injectManifest inside of a Node CLI should not be that tough, and if you're interested in publishing it, we could link to your CLI from https://developers.google.com/web/tools/workbox/guides/using-bundlers.

jeffposnick avatar Dec 01 '20 15:12 jeffposnick

I'm for at least adding the option to bundle the serviceworker.

Since it's generated on build it doesn't belong in src , but it doesn't belong in build either since it only works as a source file for a bundler.

For me, this would be the dream:

npx workbox injectManifest --inlineImports

It could be solved by using esbuild as an optional dependency.

jakobrosenberg avatar Feb 05 '21 14:02 jakobrosenberg

If someone finds this thread and wants a one-liner that made use of npx executions of esbuild and workbox-cli (chained together), the following works:

npx esbuild src/service-worker.ts --bundle --define:process.env.NODE_ENV=\"production\" --outfile='/tmp/service-worker.js' && npx workbox-cli injectManifest

(assuming you have a local workbox-config.js file configured with swSrc: '/tmp/service-worker.js'). If you'd prefer not to write to /tmp/, you could output from esbuild directly to your build directory, and then use that path for both the swSrc and swDest options, and workbox-cli injectManifest will overwrite the file with the version that contains the injected manifest.

I really like esbuild, and the fact that it's a) fast and b) can handle TypeScript compilation and bundling and process.env.NODE_ENV at the same time, without plugins, is great. That being said, if we had implemented the original feature request back when folks asked for it, chances are it would have been done using Rollup (which we still use for generateSW mode). The fact that the bundler tooling ecosystem is improving so quickly is, I think, an argument for not baking that functionality into injectManifest, and instead letting developers take advantage of all the improvements being made in the toolchain of their choice.

jeffposnick avatar Feb 05 '21 18:02 jeffposnick

If you'd prefer not to write to /tmp/, you could output from esbuild directly to your build directory, and then use that path for both the swSrc and swDest options

I had done the opposite (workbox -> esbuild) and was getting Refusing to overwrite input file: public/build/serviceworker.js. Never even considered that workbox didn't have to run first. 😅

Thanks @jeffposnick

jakobrosenberg avatar Feb 06 '21 07:02 jakobrosenberg

Yup! We added support for the same swSrc and swDest specifically to allow for this use case: https://github.com/GoogleChrome/workbox/pull/2231

jeffposnick avatar Feb 06 '21 16:02 jeffposnick