Uh oh!
There was an error while loading. Please reload this page.
Add simplified service worker invalidation - #2551
Conversation
d605035 to
d2e48e8Comparero-savage
commented
Jun 17, 2017
@gaearon@tizmagik @jeffposnick you might want to double check, its based on your feedback. CI failing is unrelated to change. It is some issue with windows build and the latest version of appveyor. |
jeffposnick
left a comment
There was a problem hiding this comment.
With the caveat that it's unfortunate that this sort of logic needs to be implemented by each framework/starter kit, rather than being baked in to the browser, I understand why you'd want to move ahead with this brute-force approach in the interim.
I've just got the one comment about potentially only running this additional logic on localhost instead of in production as well.
There was a problem hiding this comment.
Throwing this out there: the overhead of this extra request would be reduced if there were an additional check that only caused it to execute when you're on localhost or the equivalent. This PR would then solve the "zombie" SW on localhost problem immediately, but would not attempt to unregister a service worker on a production server.
// Inside the window.onload handler:constisLocalhost=Boolean(window.location.hostname==='localhost'||// [::1] is the IPv6 localhost address.window.location.hostname==='[::1]'||// 127.0.0.1/8 is considered localhost for IPv4.window.location.hostname.match(/^127(?:\.(?:25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)){3}$/));if(isLocalhost){fetch(swUrl)// ...the rest of your logic here...}else{registerValidSW(swUrl);// Avoid the extra fetch() outside of localhost.}It's unclear how w3c/ServiceWorker#204 will get resolved, but there has been some opposition to automatically unregistering production servers on an HTTP 404 response. My hope is that the code that comes out of this PR will evolve (post-merge) to match whatever's decided upon in that issue, providing equivalent behavior prior to the actual native browser implementation.
gaearon
commented
Jun 26, 2017
Do you mind scoping this for |
d2e48e8 to
aea0feeComparero-savage
commented
Jun 27, 2017
Added the check so it only applies on local host as per @jeffposnick code.* Personally, I liked the check happening on live servers because it means that if you deployed an app with CRA and then later deployed an app with some other boilerplate, to the same domain. The user (and developer) doesn't get stuck. It also shouldn't cause a slow down, because we are instantly displaying the app regardless. Its just if there is no service worker found. We reload it right away with the SW removed. (Alt. would be to removed the SW and show the toast from #2426). However, this is a good start and would be useful to be merged in with only localhost. |
jeffposnick
commented
Jun 27, 2017
👍 |
gaearon
commented
Jun 27, 2017
I haven't tested but I trust this works. |
gaearon
commented
Jun 28, 2017
I'd appreciate if you could test the latest version and ensure it works as intended. |
jeffposnick
commented
Jun 28, 2017
Will do. I'll report back. |
gaearon
commented
Jun 28, 2017
Thanks! |
jeffposnick
commented
Jun 28, 2017
LGTM—I tried out both the local service worker invalidation scenario, and the |
gaearon
commented
Jun 28, 2017
Thanks again for checking. |
* Add service worker invalidation * Update valid service worker check only on local host
* Add service worker invalidation * Update valid service worker check only on local host
* Add service worker invalidation * Update valid service worker check only on local host
| window.location.hostname === 'localhost' || | ||
| // [::1] is the IPv6 localhost address. | ||
| window.location.hostname === '[::1]' || | ||
| // 127.0.0.1/8 is considered localhost for IPv4. |
There was a problem hiding this comment.

Adds invalidation of service workers based on by attempting to
fetchservice-worker.jsand if its receives a 404, or if its the wrong content type (e.g. all not found urls are being redirected to index.html or serving a 404 page without a 404 code) it unregistered the current service worker and reloads the page.This issues the issue where sites hosted on the same
url:port(e.g.localhost:5000) are incorrectly displayingcreate-react-appinstead of the hosted page.This was addressed in a more complex manner in #2438. However, this is a simplified version based on the feedback of @jeffposnick.
Note: I created a new PR/Branch because I believe #2438 addresses more cases, however it also adds complexity. This PR should fix most use cases when combined with #2426.