This repository has been archived by the owner on Aug 31, 2021. It is now read-only.
networkFirst timeout #34
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
| @@ -186,7 +186,7 @@ Apache License | |||
| same "printed page" as the copyright notice for easier | |||
| identification within third-party archives. | |||
|
|
|||
| Copyright ${copyright_attribution} | |||
| Copyright 2015 Google, Inc. | |||
There was a problem hiding this comment.
I "fixed" this too, early on, but if you read the text this is NOT the copyright statement for the project, but rather the template for how to apply the license to other projects.
| Most methods that involve a cache (`toolbox.cache`, `toolbox.uncache`, `toolbox.fastest`, `toolbox.cacheFirst`, `toolbox.cacheOnly`, `toolbox.networkFirst`) accept an option called `cache`, which is the **name** of the [Cache](https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache) that should be used. If not specifed Service Worker Toolbox will use a default cache. | ||
| Most methods that involve a cache (`toolbox.cache`, `toolbox.uncache`, `toolbox.fastest`, `toolbox.cacheFirst`, `toolbox.cacheOnly`, `toolbox.networkFirst`) accept an option called `cacheName`, which is the **name** of the [Cache](https://slightlyoff.github.io/ServiceWorker/spec/service_worker/#cache) that should be used. If not specified, Service Worker Toolbox will use a default cache. | ||
|
|
||
| The `networkFirst` strategy supports a timeout, specified in seconds via the `networkTimeoutSeconds` |
There was a problem hiding this comment.
I know that I would have just called it timeout, but I also know that I'm lazy and fond of shortcuts. I leave it to your judgement :)
There was a problem hiding this comment.
I don't think anyone will mind the verbosity. If there ends up being some other sort of configurable timeout in the future (not outside the realm of possibility), using a more descriptive identifier here will end up paying off.
|
LGTM, resolve the remaining two nits or not at your discretion :) |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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: @wibblymat
Closes #12, although it doesn't enforce the network timeout by default.
I put together a super-simple page in
demo/index.htmlthat registers a service worker, mainly to play around with in DevTools. It can be fleshed out a bit more in the future, which is tracked in #9.None of the existing tests that exercise
networkFirstbreak, but I didn't add any new ones—if you have any ideas for simulating a slow network for just one test case, I'm all ears.There are also miscellaneous changes to the docs—new docs for
networkTimeoutSeconds, updates to a few copyright years, and correcting the documentation for thecacheNameoption.