Expose the event object to plugin methods #1640
Conversation
|
It's too bad that destructured params increase the file size so much... |
There was a problem hiding this comment.
There are also some changes that will need to made to https://developers.google.com/web/tools/workbox/guides/using-plugins#custom_plugins in a separate PR to reflect the new plugin parameters.
| * @param {Request} options.request | ||
| * @param {Response} options.response | ||
| * @param {Event} [options.event] | ||
| * @param {Array} [options.plugins=[]] |
There was a problem hiding this comment.
{Array<Object>} for consistency.
| request, | ||
| event, | ||
| fetchOptions, | ||
| preloadResponse, |
There was a problem hiding this comment.
preloadResponse should be removed here.
I guess that could be why Matt wanted us to avoid them in the core libraries 😄 This might be the case of something that gets transpiled into bloated code when we need to support older browsers, and once we change our |
Yes, but I'm assuming that should wait until the next release, right? |
PR-Bot Size PluginChanged File Sizes
New FilesNo new files have been added. All File SizesView Table
Workbox Aggregate Size Plugin9.97KB gzip'ed (66% of limit) |
Right; we don't have a great way of keeping track of that, so I tend to try to file a PR (or at least create a local commit with the change) against WebFundamentals at around the same time that the Workbox change goes in, and time the merge for when we cut the new Workbox release. |
|
I'm curious to see whether the size differential goes down depending on the |
|
I don't think
AFAICT, the main cause of the size increase is previously we could minify code like this: cacheWrapper.match(s.e,r,s.s,s.t)But now we're forced to minify it like this (from an actual diff): cacheWrapper.match({cacheName:r.e,request:s,event:e,matchOptions:r.r,plugins:r.t});And enough of these occurrences adds up I guess... |
|
Ah, that makes sense regarding the parameter name mangling. Thanks for checking! I'm cool with the extra bytes, especially since we have a plan for reducing bundle sizes once we hit v4 and can cut out some of the other transpilation overhead. |
Fixes #1639.
This PR implements the proposal in #1639 by updating the fetch and catch wrappers as well as the strategies (and other code) that calls them. It also removes the
preloadResponseoption passed to the fetch wrapper, since that can now be found on the fetch event (@jeffposnick can you confirm that's okay?).In addition, I noticed we're being inconsistent in how we refer to restructured parameters in our JSDoc comments (
optionsvsinput), so, since I was already updating some, I decided to change all instances ofinputtooptions.