Allow injecting manifests into existing webpack-compiled assets

[pimterry]

R: @jeffposnick @philipwalton

Fixes #1513

Description of what's changed/fixed:

Before this PR, the only way to use InjectManifest was to inject into files totally outside the rest of your webpack compilation process. As in #1513, this causes problems (in my case, it means it's impossible to use InjectManifest in TypeScript).

With this PR, there's now two ways to configure InjectManifest:

• As before: setting swSrc to the path of an input file, independent of webpack, and optionally using swDest to override the output file.
• New: setting only swDest, to specify an existing output file that you'd like to inject into.

This means if my build process already builds a service-worker.js file somehow (e.g. via TypeScript), I can now set up workbox by adding the plugin with just:

new InjectManifest({
    swDest: 'update-workers.js'
});

I considered creating a third parameter instead (swExisting?) so you could inject and use swDest as well. I don't think there's any case where you'll want to inject into an existing file and then have both files end up in your output though. I'm fairly sure you always want to totally overwrite an output file, and that's simpler this way. Let me know if you disagree.

I've written this such that all existing configs should keep working fine, and updated the validation in line with these changes to catch any issues. The new behaviour uses ConcatSource instead of bare strings, so it doesn't break source maps etc too. I wasn't sure how to update the docs corresponding to this, so I've left those for now.

[pimterry]

Build is failing here due to incorrect hashes in the produced manifest filenames. I'm not sure why though, and I can't seem to reproduce it at all locally with gulp test. Any idea what I'm missing?

It'd be easy to just update the test hashes so that travis will pass, but presumably that'd then break locally instead, and it doesn't seem sensible to do that without knowing why this is failing.

[pimterry]

In the meantime I need this for my build, so I've published this forked version to npm as @httptoolkit/workbox-webpack-plugin (version 3.6.4-pr1765.1). Might be useful for testing or for anybody else with the same issue.

[jeffposnick]

Thanks for the contribution, @pimterry!

We've roped @developit into working on a larger rethinking of the workbox-webpack-plugin interface. I'd like to get his thoughts about this approach, though the fact that it's backwards-compatible does make me more inclined to give the go-ahead.

A few logistical issues: we are (really!) going to launch v4.0.0 from the next branch at some point in the near future. If this PR were to land before we launched, we'd want to get it rebased against next. That should also work around the Travis CI failures, as the Travis config has been adjusted on next to lock down the expected version of Node, which is the underlying issue. Don't worry about that for now—I can rebase for you if needed.

[zetlen]

Thanks, @pimterry, for your fork; it helped me prototype a smarter integration of our ServiceWorker plugin.

The above code produces working ServiceWorkers and enables Webpack optimizations, but it still produces a large Webpack browser runtime, because some of our planned optimizations rely on the browser chunks and worker chunks sharing a Compilation instance, which Webpack currently can't do.

That issue and webpack/webpack#6908 seem relevant here. And I may be out of my depth here, but it seems like a ServiceWorker could also evade the Webpack target problem by using TemplatePlugins to write ServiceWorker code, the way the HtmlWebpackPlugin does for its generated assets.

[jeffposnick]

Hey @pimterry—with Workbox v5's changes to the InjectManifest plugin, I wanted to revisit this PR.

Could you give the current alpha a try and see if it handles your use case?

In particular, swSrc is now compiled by webpack, and you could pass in plugins to customize the compilation, which I think should allow for your TypeScript use case.

If it doesn't look like the approach in v5 will work for you, we could revisit making a change to support swDest-without-swSrc (or the equivalent).

[pimterry]

Yes, I've given 5.0.0-alpha1 a very quick test and it does seem to now compile nicely for me, when setting swSrc to point to my typescript SW source. I'll close this PR, thanks for all the work to get this sorted. Looking forward to the full v5 release!