-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Update Workbox "Using Plugins" documentation #9017
Conversation
…ox Using Plugins page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this!
As with the other content that targets Workbox v6, we're going to hold off on merging until we've released v6.0.0.
plugin did (e.g. compute the time delta between running `requestWillFetch()` and `fetchDidSucceed()` | ||
or `fetchDidFail()`). | ||
|
||
## Lifecycle Callbacks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there's a meaningful distinction between these "lifecycle callbacks" and the other group of existing callbacks. All of the callbacks in this section are new for v6 and are tied to the handler*
lifecycle, but conceptually, they're very similar to the existing callbacks and should probably be listed alongside them.
Would you mind rearranging things so that these aren't broken out into their own section? You can add in something like "The lifecycle callbacks starting with handler
are all new in Workbox v6." to make it clear what the support story is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, and added a note mentioning how handler
callbacks are new in v6 :)
Whoops! There were 4 warnings that will prevent this PR from being merged. Please take a look, and either fix, or provide a justification for why they can't be fixed. WARNINGS |
@jeffposnick - can you take a peek and LGTM/merge? |
What's changed, or what was fixed?
Target Live Date: TBD
npm test
locally and all tests pass.type-something
label.CC: @jeffposnick @philipwalton