Removed unregistering of service workers on reload

[prajapati-parth]

Description

Fixed issue #9770

[DSchau]

As per @KyleAMathews's comment:

Hmm yeah — we definitely shouldn't unregister all service workers, just ones from gatsby-plugin-offline. Or perhaps make it configurable.

Let's see if we can make this only remove gatsby-plugin-offline instances?

This currently is a little challenging, since it does't seem like we have a great way to identify the Gatsby-registered service worker.

So - I see two options here:

1. Provide a way to uniquely identify a Gatsby instance of a service worker (it looks like prefix is a supported option for Workbox--perhaps that's usable?) also cc @davidbailey00, any idea here?
2. Presume that sw.js refers to the Gatsby service worker, therefore we can do something like:

if (supportsServiceWorkers(location, navigator)) {	
  navigator.serviceWorker.getRegistrations().then(registrations => {	
    for (let registration of registrations) {
      if (registration.active.scriptURL.endsWith('sw.js')) {
        registration.unregister()	
      }
    }	
  })	
}

[vtenfys]

Let's see if we can make this only remove gatsby-plugin-offline instances?

I think in this case it does make sense to remove all service workers.

Firstly, SWs are port-specific, so for example if you're testing a production build on port 9000, the worker won't be removed when testing the development version on port 8000. Therefore it shouldn't interfere with SWs from other HTTP servers as long as they're on different ports.

Secondly, I think if anything, it's more important to remove non-Gatsby service workers, rather than vice versa. Otherwise there would be random things going on outside of our control if another SW is installed from a different non-Gatsby site on the same port.

[prajapati-parth]

Secondly, I think if anything, it's more important to remove non-Gatsby service workers, rather than vice versa.

Isn't the main purpose of this PR to not remove non-Gatsby service workers? I tried developing firebase cloud messaging on a gatsby site and every time I reload the page in development mode, it de-registers the service worker.

[vtenfys]

Isn't the main purpose of this PR to not remove non-Gatsby service workers? I tried developing firebase cloud messaging on a gatsby site and every time I reload the page in development mode, it de-registers the service worker.

Ah, I understand the reasoning now. In this case I think it does make sense to remove this code and possibly display a warning instead - we can't really tell which SWs a user wants to keep or not so it's best not to guess, but at least if we display a warning in the console then it'll save people time when they're confused about why a site isn't updating on refresh.

@DSchau thoughts on this?

[DSchau]

Isn't the main purpose of this PR to not remove non-Gatsby service workers? I tried developing firebase cloud messaging on a gatsby site and every time I reload the page in development mode, it de-registers the service worker.

This seems like a reasonable use case--to me. I don't think we should remove "user code" that we're not in control of.

Ah, I understand the reasoning now. In this case I think it does make sense to remove this code and possibly display a warning instead - we can't really tell which SWs a user wants to keep or not so it's best not to guess, but at least if we display a warning in the console then it'll save people time when they're confused about why a site isn't updating on refresh.

Warning/prompt is OK but some outstanding questions are helpful:

1. Is it reasonable to remove the Gatsby service worker in development?
   • I think the answer to this is (apparently and historically) yes.
2. Is it reasonable to remove all service workers?
   • I think the answer to this is probably no--especially if it's breaking/preventing a development workflow for a common tool like Firebase.
3. Is there a clean way to remove a Gatsby specific service worker?
   • e.g. filename is decent (sw.js) but some way to uniquely identify would be even better

That being said--the solution should not be to entirely prevent removal of any service workers here (as in this PR)--we should target the Gatsby service worker for removal and leave others alone.

[prajapati-parth]

Is there a clean way to remove a Gatsby specific service worker?
e.g. filename is decent (sw.js) but some way to uniquely identify would be even better

Correct. Because most of the services that ask users to create service workers with a specific name also allows them to configure the use of an existing service worker with different name. Eg. Firebase cloud messaging needs a SW with name firebase-messaging-sw.js but it can also be configured with name sw.js. So we'll definitely need other way for unique identification.

[vtenfys]

I've created a PR at #13716 in favour of this one, which warns about SWs detected but doesn't remove them. In development, we shouldn't have any Gatsby SWs since gatsby-plugin-offline only creates a SW in production mode, so I think detecting the "creator" of a SW might be unnecessary.

As a result I'll be closing this PR, but thanks anyway your contribution! Feel free to open an issue or another PR in the future if you feel anything else is wrong and we'll be sure to have a look :)