[docs-infra] Stale service-worker cache issue

[oliviertassinari]

Context

The service worker was added in the past to make the docs work offline. We would load everything and cache it.

At some point, we gave up on the approach to cache everything upfront as it was leading to a LOT of bandwidth use. So instead, we would only lazy load pages. This also had the benefit to make the refresh time faster.

First request without any cache:

Follow-up request with HTTP cache and without service worker (see the 304 status, and the bandwidth use, it's only the HTTP headers, no body is transferred):

Follow-up request with service worker cache (it doesn't even hit Netlify CDN):

Problem

But this seems to introduce a cache invalidation issue. We constantly get strange load pages like:

https://user-images.githubusercontent.com/3165635/230731268-906d11a2-fa88-41b4-9de6-7e0af6ae5a07.mov

https://github.com/mui/material-ui/issues?q=is%3Aissue+service+worker+is%3Aclosed gives a sense of how frequently this problem happens.

Options

A. Remove sw.js

At first sight, we could remove the server worker logic. Is the partial offline support worth the current bug? No. Is making the page load 50ms faster worth the cache invalidation? Probably no.

If we decide to remove it in a way that doesn't break everybody, getting stuck on an outdated version for everyone.

B. Change strategy

We could use https://developer.chrome.com/docs/workbox/modules/workbox-strategies/#network-first-network-falling-back-to-cache instead of:

https://github.com/mui/material-ui/blob/fe5c2c9f2d27426d2551bce213c45e4c17f6442e/docs/src/sw.js#L13

We would lose the performance benefit but retain the partial offline support.

Proposal

Option A. Keep things simple, nor Next.js/Vercal/React,Messenger,Facebook, are using a sw. If we don't go with Option B. I think that we should be able to answer: Who is successfully using sw to make a difference? https://www.flipkart.com/ has one, but still loads the .html from the network.

Note: I have unsubscribed from this GitHub issue. My goal is to give visibility of the issue, and share what I know.

[Aribaskar-jb]

hi @oliviertassinari i can work on this Bug you can Assign me this Issue and i am a bigener you can help me to kick start on open source contribution's

[cherniavskii]

I thought about this issue a bit, and the most common scenario is:

1. Go to page A - you'll see cached outdated content (the new content is fetched in the background).
2. Refresh the page - the latest content is served from the cache that was just updated.
3. Click on the link to page B. You won't see outdated content, because of client-side Next.js navigation that fetches the latest data for the page.
4. Refresh page B. Effectively, you are at point 1 again - you see the cached outdated page (the new content is fetched in the background)
5. Refresh page B again - the latest content is served from the cache that was just updated.

To sum up, the issue is that we use a "page-based cache-first caching strategy", so users have to invalidate each page's cache individually.

Removing the service worker is a good start, we can always introduce a caching strategy later if necessary. I will submit a PR with a no-op service worker (following https://developer.chrome.com/docs/workbox/remove-buggy-service-workers/)

[cherniavskii]

Another example of the issue caused by stale cache: https://github.com/mui/mui-x/issues/8832#issuecomment-1533119554

We have added a no-op SW in MUI X docs (https://github.com/mui/mui-x/pull/8598). I thought it might solve the issue for MUI X, but it turns out that MUI X has been using SW from https://mui.com/sw.js all the way 😅: https://github.com/mui/mui-x/blob/d7d517930b6f544f7e7ae5c72d041d6202c3a155/docs/pages/_app.js#L150

I can't even find a URL to MUI X's sw.js - https://mui.com/x/sw.js returns 404.

What we can do now is:

1. Replace SW on mui.com with a no-op SW (same as https://github.com/mui/mui-x/pull/8598, but in the core repo) @mui/docs-infra would you accept such PR?
2. Remove sw.js from the MUI X repo and make it clear that SW from the core is being used.

Also tagging @mui/toolpad as you might be impacted by this as well.

[cherniavskii]

As an attempt to better encapsulate MUI X docs, we could explore using basePath.

[Janpot]

I know I've surfaced this before and we have valid reasons not to, but I still wonder whether this could be caused by a less than ideal isolation of assets of the different docs sites due to not using next.js basePath

edit 😄 your comment came up right as I posted mine, great minds think alike

[oliviertassinari]

As an attempt to better encapsulate MUI X docs, we could explore using basePath.

@cherniavskii I looked that this in the past (linked by Jan), but it seems to be a dead end. From what I understood, we wouldn't be able to write links like /x/react-data-grid/row-pinning/ but it would need to be /react-data-grid/row-pinning/ 🙈.

I can't even find a URL to MUI X's sw.js - https://mui.com/x/sw.js returns 404.

It looks expected. MUI X path is the same as MUI Core: https://docs-v6--material-ui-x.netlify.app/sw.js. The should probably have done this in the beginning (It's not Next.js related):

diff --git a/docs/pages/_app.js b/docs/pages/_app.js
index ccc8583ce..2c3b6f921 100644
--- a/docs/pages/_app.js
+++ b/docs/pages/_app.js
@@ -147,7 +147,7 @@ async function registerServiceWorker() {
     window.location.host.indexOf('mui.com') !== -1
   ) {
     // register() automatically attempts to refresh the sw.js.
-    const registration = await navigator.serviceWorker.register('/sw.js');
+    const registration = await navigator.serviceWorker.register('/x/sw.js');
     // Force the page reload for users.
     forcePageReload(registration);
   }
diff --git a/docs/scripts/buildServiceWorker.js b/docs/scripts/buildServiceWorker.js
index aefa001fb..d01ada194 100644
--- a/docs/scripts/buildServiceWorker.js
+++ b/docs/scripts/buildServiceWorker.js
@@ -8,7 +8,7 @@ async function prepend(file, string) {
 }

 async function run() {
-  const swDest = path.join(__dirname, '../export/sw.js');
+  const swDest = path.join(__dirname, '../export/x/sw.js');
   const swSrc = path.join(__dirname, '../src/sw.js');

   await fse.copy(swSrc, swDest);

What we can do now is:

👍

[Janpot]

From what I understood, we wouldn't be able to write links like /x/react-data-grid/row-pinning/ but it would need to be /react-data-grid/row-pinning/ 🙈.

We could create a wrapper for the Next.js Link component that strips the basePath.

[oliviertassinari]

@cherniavskii A follow-up idea on this. Today, developers have 3 sw installed:

What if we were to go to MUI X and MUI Toolpad to do:

-  registration.addEventListener('updatefound', listenInstalledStateChange);
-}
-
 async function registerServiceWorker() {
   if (
     'serviceWorker' in navigator &&
     process.env.NODE_ENV === 'production' &&
     window.location.host.indexOf('mui.com') !== -1
   ) {
-    // register() automatically attempts to refresh the sw.js.
-    const registration = await navigator.serviceWorker.register('/x/sw.js');
-    // Force the page reload for users.
-    forcePageReload(registration);
+    const registrations = await navigator.serviceWorker.getRegistrations();
+
+    // eslint-disable-next-line no-restricted-syntax
+    for (const registration of registrations) {
+      if (registration.scope === 'https://mui.com/x/') {
+        registration.unregister();
+      }
+    }
   }
 }

as well as removing the sw.js code? As far as I understand it, the /x/sw.js worker was never compromised and is a no-op. So we could progressively unregister it for everyone.

[cherniavskii]

Yeah, I think we can unregister Service Workers in X and Toolpad 👍 But I would rather update the Service Worker to unregister itself and keep it for a while - see https://medium.com/@nekrtemplar/self-destroying-serviceworker-73d62921d717 for more details

[oliviertassinari]

@cherniavskii The problem https://github.com/NekR/self-destroying-sw solves seems to be:

The problem here is that ServiceWorker, unregistered this way, isn’t going away immediately. It continues controlling already opened pages and won’t release them until they all are reloaded or closed. Only then, finally, ServiceWorker is being physically removed the website.

which I don't think is one we will face. We are cleaning this in case, in 2 years, we need to add one back. But let's say we want to fully clean it. https://love2dev.com/blog/how-to-uninstall-a-service-worker/ expands on the problem:

The next issue I see with the proffered code is executing this within the context of a new service worker, which must be registered first. I feel this sort of defeats the purpose of removing a service worker.

He proposes to do this instead (client side):

navigator.serviceWorker.getRegistrations().then(function (registrations) {
  for (let registration of registrations) {
    if (registration.scope === "https://mui.com/x/") {
      registration
        .unregister()
        .then(function () {
          return self.clients.matchAll();
        })
        .then(function (clients) {
          clients.forEach((client) => {
            if (client.url && "navigate" in client) {
              client.navigate(client.url);
            }
          });
        });
    }
  }
});