GoogleChrome / workbox

📦 Workbox: JavaScript libraries for Progressive Web Apps
https://developers.google.com/web/tools/workbox/
MIT License
12.33k stars 815 forks source link

Proposal/RFC: workbox-webpack-plugin changes #1854

Closed developit closed 5 years ago

developit commented 5 years ago

đź‘‹ Hi all!

I put together a proposal for how we might modify workbox-webpack-plugin to allow for a more bundler-centric development approach, using Workbox as a modular library and Webpack to bundle the resulting ServiceWorker.

You can view the proposal here: https://docs.google.com/document/d/1qyzxo5MBZ5BHva2GaAPk_Hw-jkVk1-NXcqfAIlvHgkc

Comments welcome!

mcshaman commented 5 years ago

Cool. Questions:

  1. In your proposal are you suggesting doing away with the webpackbootstrap and just using the importScript to import chunks?
  2. You only seem to mention modules from workbox. What about other modules either relative or in the node_modules folder? Will they be bundled under this proposal and if so can they be bundled into a single service worker file or is the proposition to have them as a seperate chunk?
madmoizo commented 5 years ago

My wishes for v5:

This gives full control over precache-manifest assets to the developper. Breaking change: if you want to change __precacheManifest it has to be explicit

__precacheManifest = __precacheManifest.concat([{
   url: 'nonAssetScript.js',
   revision: 'abcdefg'
}])

The solution was to “teach” Webpack to detect Service Workers by introducing parser logic for navigator.serviceWorker.register()

Parser seems to match simple cases https://github.com/GoogleChromeLabs/squoosh/blob/master/config/auto-sw-plugin.js#L73 what about:

const n = navigator.serviceWorker
n.register()

Should this be a new plugin?

It can't be just a flag ?

developit commented 5 years ago

@mcshaman:

In your proposal are you suggesting doing away with the webpackbootstrap and just using the importScript to import chunks?

No - webpack's bootstrapping code uses importScripts() when the output target is set to webworker.

You only seem to mention modules from workbox. What about other modules either relative or in the node_modules folder? Will they be bundled under this proposal and if so can they be bundled into a single service worker file or is the proposition to have them as a seperate chunk?

This would support importing from node_modules using the same webpack semantics as main thread code. I believe @jeffposnick had some good arguments against the use of dynamically loaded code in Service Workers (blocking on importScripts, import error handling, code caching issues)

@frlinw A flag might be good, yeah.

wood1986 commented 5 years ago

My wish for v5,


I did some research on how the plugin can be implemented for bring your own chunk(BYOC). I think one webpack config, two entries, and one plugin is not feasible. Because at runtime, the bundle is using window instead of self as the global. it needs multiple target for different entries. And webpack does not support that Not only that, it is difficult for people to configure especially in optimization because you do not want the service worker depends on vendors.

image

Because of the previous limitation, I tried to return multiple configs with different targets. But before injecting the manifest to your own sw.js bundle, you need to wait the index entry compilation to complete. This means two configurations need to have some communication before the manifest injection. But this is way easier and cleaner than one webpack config, two entries, and one plugin. The downside is it needs two plugins.

wood1986 commented 5 years ago

I have just created a plugin for my wish. You can take it if you want. Check https://github.com/wood1986/workbox-poc/tree/multiple-configs

jeffposnick commented 5 years ago

That sounds like the best stop-gap approach in the meantime—I'm glad you're able to enable your use case while we work on the formal v5 release.

wood1986 commented 5 years ago

That sounds like the best stop-gap approach in the meantime—I'm glad you're able to enable your use case while we work on the formal v5 release.

Thanks for checking it out. I hope you guys can take my work. If not, can you guys share your concerns? I can think about it.

jeffposnick commented 5 years ago

This should be addressed by the current Workbox v5.0.0 alpha.

madmoizo commented 5 years ago

Because at runtime, the bundle is using window instead of self as the global. it needs multiple target for different entries. And webpack does not support that

@wood1986 I has a similar issue. Try globalObject: 'this' https://webpack.js.org/configuration/output/#outputglobalobject

{
  entry: {
    'app': `${rootPath}/src/app/main.js`,
    'sw': `${rootPath}/src/sw/main.js`
  },
  output: {
    publicPath: '/',
    globalObject: 'this' // fix window is not defined issue
  }
}

For the plugins part, importWorkboxFrom supports chunks when importScripts doesn't, so just use importWorkboxFrom to import your chunk.

{
  plugins: {
    new WorkboxPlugin.InjectManifest({
      swSrc: `${rootPath}/src/sw.js`,
      importWorkboxFrom: 'sw', // chunk name supported !
      include: [
        /\.html$/,
        /\.js$/,
        /\.css$/,
        /\.woff2$/,
        /\.jpg$/,
        /\.png$/
      ]
    })
  }
}