v4 Background Sync API updates #1710
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Oct 15, 2018
|
What about some means of mapping over the queue? |
7 tasks
jeffposnick
suggested changes
Oct 16, 2018
philipwalton
force-pushed
the
bg-sync-updates
branch
from
98415b7 to
a226808
Compare
PR-Bot Size PluginChanged File Sizes
New Files
All File SizesView Table
Workbox Aggregate Size Plugin9.71KB gzip'ed (65% of limit) |
@bennypowers, Jeff and I discussed something like this: for await (const entry of queue.entries()) {
try {
await fetch(entry.request);
} catch (error) {
await this.unshiftRequest(entry);
break;
}
}But unfortunately that would require async iterators, which aren't supported in Edge. We could add a queue.forEachRequest(async (entryPromise) => {
// But calling `return` here won't exit the loop...
})Is that what you were envisioning by your suggestion? I think given the limitations of both options a |
|
It's clunky but if it's all we have...
|
jeffposnick
approved these changes
Oct 18, 2018
This was referenced Oct 24, 2018
38 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
R: @jeffposnick
Addresses #1424, #1502, #1503, #1637, and #1695.
Based on lots of feedback in the above issues/PRs, its clear that the callbacks provided in
workbox.backgroundSync.Queueare not sufficient to handle many real-world background sync needs.This PR attempts to solve this issue by exposing more low-level features of the queue, which should give developers all the flexibility they need; the downside is it may make very simple replay modifications a bit more complicated. Hopefully (due to the complex nature of background sync in general), this downside will affect very few people.
Current API:
In Workbox v3, the
workbox.backgroundSync.Queueclass had the following public methods:addRequest()replayRequests()And the following replay callbacks that allowed you modify requests as the queue was replaying:
requestWillEnqueuerequestWillReplayqueueDidReplayBut if you wanted to do something other than simply replay every single request, it was very difficult if not impossible. For example, you couldn't easily:
Proposed changes:
To support the above use cases, the changes in this PR remove the
callbacksobject and replace it with a singleonSynccallback. TheonSynccallback will be invoked whenever the queue receives asyncevent (or at SW startup in non-sync browsers), and if you don't specify anonSynccallback, the default behavior is to runreplayRequests()like in v3 (but without invoking the v3 callbacks).The
onSynccallback is invoked with thequeueinstance as its only argument, allowing you to handle the queue however you want in response to asyncevent.In addition, the
workbox.backgroundSync.Queueclass has removed theaddRequest()method, and replaced it with four array-like methods to make it easier to manage queued requests manually.pushRequest()popRequests()shiftRequest()unshiftRequests()The
pushandunshiftmethods take an object in the form{request, [metadata], [timestamp]}(onlyrequestis required), and theshiftandpopmethods return an object in exactly the same form.The optional
metadataproperty allows you to store data associated with the request in the queue that may be relevant when re-fetching the request later.The optional
timestampproperty defaults to the current time, and it affects when requests expire. Normally you don't need to pass this, but it can be useful if you want to explicitly reset the expiry time for a particular request in order to keep it in the queue longer.Example
onSynccallbackTo replay all requests in the queue after a sync event, you could do the following:
Note: when using an
asyncfunction for theonSynccallback (recommended) thesyncevent automatically waits for the function to complete, so you don't have to manually callevent.waitUntil().Using the
PluginclassSince the plugin class mostly just shadows the
Queueclass, you can pass theonSynccallback to plugin instances in exactly the same way.Note that when using the plugin, the
onSynccallback is still invoked with aQueueinstance, so you don't have to create a reference to the queue yourself.Open questions
By removing the
callbackobject in favor of a singleonSynccallback, one thing that is harder to accomplish is notifying of a successful queue replay in situations where you don't want to modify the replay logic.With the changes in this PR, it's still possible, it's just perhaps not as intuative as before. Here's what you'd have to do:
If you want to notify of success and failure, you can use a try/catch block:
Since the above examples are both pretty simple, I'm leaning toward not having onsuccess or onfailure callbacks like before, but if enough people want them, I could be persuaded otherwise.