Remove registerNavigationRoute, add createHandlerForURL #2143
Conversation
| } | ||
|
|
||
| // This might still fail if the browser is offline... | ||
| return fetch(cacheKey); |
There was a problem hiding this comment.
Do you want to fetch the cacheKey or the url in the fallback case?
There was a problem hiding this comment.
cacheKey makes the most sense, I think. Whenever workbox-precaching makes a network request, it's for the cache key (which includes the __WB_REVISION query parameter, as needed). Using the cacheKey here is consistent.
There was a problem hiding this comment.
Interesting. Doesn't that potentially bypass the HTTP cache in a way that may be unnecessary (or is that the point)?
There was a problem hiding this comment.
It's more likely to hit the HTTP cache, since /something.html?__WB_REVISION=1234 will likely have been requested by the service worker previously (in order to populate the precache), while /something.html hasn't necessarily been requested.
Getting to this point in the code is, by definition, a failure scenario, so I'm not sure what the expectation is for a developer. We're just making a best-case effort to (temporarily) return something when this happens.
There was a problem hiding this comment.
I see, my question was based on the assumption that this was different from the initial precache fetch, but it sounds like my assumption was incorrect. As long as the same URL is used in both cases, caching shouldn't be an issue.
I will say that I'm surprised the cache key is used in the initial precaching fetch (hadn't noticed that before because the _addURLToCache() method always references the url variable, which is not the same as the url variable used in addToCacheList()).
Anyway, if this is what's been happening for a while and no one has complained, I'm sure it's fine. My concern would have been that adding a query param to the URL that the developer may not be expecting might have unintended side effects -- depending on how their server is set up.
There was a problem hiding this comment.
Right, I think there's a decent amount of refactoring and renaming that could be done to the workbox-precaching codebase that would clarify some of those points.
You could visit a site that uses workbox-precaching in an Incognito window to confirm that __WB_REVISION=... is included in the outgoing request URLs.
|
PTAL. |
PR-Bot Size PluginChanged File Sizes
New FilesNo new files have been added. All File SizesView Table
Workbox Aggregate Size Plugin3.5KB gzip'ed (23% of limit) |
R: @philipwalton
Fixes #2095
This includes the removal of
registerNavigationRoute(), the addition ofcreateHandlerForURL(), and changes to thegenerateSWtemplate to preserve the equivalent behavior whennavigateFallbackis set.