Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cache invalidation based on service work url #2438

Closed

Conversation

ro-savage
Copy link
Contributor

@ro-savage ro-savage commented Jun 1, 2017

Addresses some of the issues raised in #2398

Specifically, that service workers are

  • Showing the wrong page when a non-CRA app is hosted at the same url + port, with manually deleting the SW the only way to fix it.
  • Showing an old CRA project if you are hosting a new on at the CRA on first visit. When refreshed the new project will show.

This adds the following checks when using service workers to fix these issues

  • Check if service worker already exists
  • If no service work, add service worker
  • If service worker is found, check the serviceworker url.
  • If its finds a 200 ok at the url. Then use the service worker as normal. (Same app)
  • If it doesn't doesn't get a 200 ok. Then check if the domain is working.
  • If domain responses with 200 ok. Then remove service worker and refresh the page (different app)
  • If anything else. Assume domain or user is offline and used service worker to provide cache.

For this to work properly, we'd need to give the service-worker.js a unique name such as service-worker-[projectname].js. This way each project as a unique service worker URL and we can tell if we are using the correct service worker.

For the webpack setup, we can just create a customer name for the generated the service worker using (service-worker-${packageName}].

For the registerServiceWorker.js we create an environment variable that contains the projects package.json name, and use this to know the right file to look for.

@gaearon @Timer @jeffposnick - Any ideas how we can name/rename the service-worker.js?

  • Make it work
  • How to correctly name service-worker.js
  • Think of edge cases and how to handle them - Currently handles offline and website down
  • How big is the slowdown in using fetch before we load the service worker? Especially if we are offline.
  • Is the a quicker way to tell if a device is offline? If offline we should catch immediately
  • Tests?
  • Are there other ways to deterministically tell what project a cached service worker is attributed to?

registerServiceWorker(swUrl);
} else {
// SW URL was invalid.
fetch(`${window.location.protocol}//${window.location.host}`)
Copy link
Contributor Author

@ro-savage ro-savage Jun 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if this is actually needed.

It might be fine to say that if we can't fetch swUrl, then just unregister and reload the page.

  • For disconnected internet or server down it will get caught in the catch and will cached service work.
  • If we get 200 ok from swUrl, we can register it as usual.
  • If we get anything else just unregister and and reload.

I am not sure the is actually a reason to check the host directly?

@gaearon
Copy link
Contributor

gaearon commented Jun 1, 2017

cc @jeffposnick

@jeffposnick
Copy link
Contributor

Sorry for not chiming in sooner.

Regarding this specific PR, there are two stated problems that this is trying to fix:

  1. Showing the wrong page when a non-CRA app is hosted at the same url + port, with manually deleting the SW the only way to fix it.
  2. Showing an old CRA project if you are hosting a new on at the CRA on first visit. When refreshed the new project will show.

Regarding problem 1, I've filed a feature request against the Chromium project to see what the appetite is for addressing this on the browser-level.

In the meantime, code along the lines of what you're doing in this PR could address problem 1, but let's also look at problem 2. An approach like what's been prototyped in #2426 would also address problem 2, since developers would see a Toast message in that scenario letting them know that they should update.

So if #2426 is viewed as a way forward on 2, the question is whether the code in this PR could be simplified to only address 1.

I think you could get away with a very stripped down version of your code, like

fetch(swUrl).then(response => {
  if (response.status === 404) {
    navigator.serviceWorker.ready.then(registration => {
      registration.unregister();
      location.reload();
    });
  }
}).catch(() => {});

to address problem 1, without having to deal with unique package names.

(This is definitely amounts to a hack, but it's a useful one that addresses a source of developer confusion.)

@ro-savage ro-savage force-pushed the service-worker-cache-invalidation branch from 92ce60c to 0edb081 Compare June 17, 2017 07:19
@ro-savage
Copy link
Contributor Author

Simplified version in PR #2551

@ro-savage ro-savage closed this Jun 17, 2017
@lock lock bot locked and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants