Support for navigationPreload in workbox-build

[merrywhether]

R: @jeffposnick @philipwalton

Fixes #1915

Adds navigationPreload config property (defaulting to false) to workbox-build's generateSW and generateSWString modes, which would also expose it to the wrappers like workbox-cli and workbox-webpack-plugin.

gulp lint test passes locally. I've been added to my company's CCLA group.

[jeffposnick]

Thanks for the contribution! I'll give it a review soon.

[jeffposnick]

Thanks a ton for this work! All of the current code looks good.

After thinking about navigation preload a little more bit more, one thing that might make sense is to try to steer developers in the direction of actually using the preloaded response, instead of setting navigationPreload: true but then not doing anything with the resulting response. (I'll do this in a separate PR against the documentation regardless.)

If you're using GenerateSW, with navigationPreload: true, then my assumption is that you almost always should use runtimeCaching to set up a route that will use that preloaded response.

This doesn't hold 100% of the time, because maybe you're using importScripts and taking advantage of the preloaded response inside of that. But for that scenario, you don't have to set navigationPreload: true at all, and you could just call workbox.navigationPreload.enable() inside your imported script.

If you set navigationPreload to true but don't use runtimeCaching, that's something that we could prevent via joi validation rules. )Unfortunately, we can't actually validate that runtimeCaching ends up using the preloaded responses, just that it's being set to something.)

@merrywhether, since you're the one who's actually planning on using this, does making runtimeCaching required whenever navigationPreload is true match your use case?

[merrywhether]

@jeffposnick We're actually starting slow with service workers and testing them out only on images (for which we have defined runtimeCaching) to begin with while we verify minimal performance regressions from cold starts, etc. As we fully SSR our pages (and they update rather often), we were intending to use navigationPreload to allow the browser to fetch the html immediately and are (at least for now) implicitly falling back to the NetworkOnly strategy for handling these requests. It sounds like we're potentially wrong about the behavior and would need to explicitly list a NetworkOnly strategy for our non-image routes in order for navigation preload to work? Especially if that's the case, ensuring at least something exists in runtimeCaching seems like a great way to help people towards proper configuration.

[jeffposnick]

So if you enable navigation preload but don't actually make use of the response in a fetch handler, you'll end up seeing something like the following logged:

The service worker navigation preload request was cancelled before 'preloadResponse' settled. If you intend to use 'preloadResponse', use waitUntil() or respondWith() to wait for the promise to settle.

There's no implicit usage of the preloaded responses when you don't respond to the fetch event.

You need to do something like https://developers.google.com/web/tools/workbox/modules/workbox-navigation-preload#basic_usage, i.e. at the minimum, set up a NetworkOnly strategy that will match your navigations and end up using the responses.

In the GenerateSW use case, setting that up would require using runtimeCaching, along the lines of:

{
  runtimeCaching: [{
    // urlPattern supports matchCallback functions in addition to RegExps...
    urlPattern: ({event}) => event.request.mode === 'navigate',
    handler: 'NetworkOnly',
  }]
}

It sounds like using runtimeCaching along with navigatePreload: true is a worthwhile restriction. I'm happy to modify your PR to add in that additional validation rule, or you could if you feel comfortable with joi's syntax.

[merrywhether]

@jeffposnick Thanks for pointers, makes the PR effort even more useful to us!

I went ahead and updated the joi validation logic and updated the tests to match.

[coveralls]

Coverage decreased (-5.7%) to 63.87% when pulling c711e6e on merrywhether:master into f116425 on GoogleChrome:master.

[jeffposnick]

Thanks! Your contribution is really appreciated.