vite-pwa / vite-plugin-pwa

Zero-config PWA for Vite
https://vite-pwa-org.netlify.app/
MIT License
3.19k stars 207 forks source link

navigateFallbackDenylist is not working with generateSW Strategy #546

Closed ad1992 closed 2 months ago

ad1992 commented 1 year ago

Hi I want to exclude contents of locales folder completely from getting pre-cached however thats not working. And similarly the runtime cache for locales folder also doesn't work.

The locales folder is inside src/locales/*.json and I want to avoid precaching locales folder, however even after adding to navigateFallbackDenylist it still gets pre-cached under assets.

Screenshot 2023-07-11 at 1 49 48 PM

This is how I do it 👇🏻

navigateFallbackDenylist: [new RegExp("locales/[^/]+.json")]

Is this because locales are not inside public folder but src ? However since I am using it in code to import so I cannot move it outside src folder.

Please let me know how can I fix this If I want to use generateSW strategy

userquin commented 1 year ago

@ad1992 the regex is applied to the build/dist folder not to the src folder. Try adding a custom regex for locale assets, something like this /^assets\/[a-z]{2}-[A-Z]{2}\./

userquin commented 1 year ago

If you want to exclude them also from sw precache you need to add similar glob pattern to ignore entry.

EDIT: you can also add a manifestTransform to filter out those entries: https://github.com/vite-pwa/vite-plugin-pwa/blob/main/examples/sveltekit-pwa/pwa-configuration.js#L61-L79

ad1992 commented 1 year ago

If you want to exclude them also from sw precache you need to add similar glob pattern to ignore entry.

EDIT: you can also add a manifestTransform to filter out those entries: https://github.com/vite-pwa/vite-plugin-pwa/blob/main/examples/sveltekit-pwa/pwa-configuration.js#L61-L79

@userquin thank you so much for the suggestion! Adding manifestTransform actually works for me in local but not in production, any idea why that would happen ? and here is the preview for reference https://excalidraw-39qbvraqa-excalidraw.vercel.app/

userquin commented 1 year ago

@ad1992

The json files are converted to js files when you build the repo, and so you need to exclude the js files, not the json files. workbox-build will run against the dist folder, change the manifest transform with this one:

  manifestTransforms: [
      (entries) => {
        const manifest = entries.filter(({ url }) => !/assets\/[a-z]{2}-[A-Z]{2}-[a-f0-9]{8}\.js$/.test(url))
        return { manifest };
      },
    ],

You can add a similar regex to the runtime config for the locales.

Wow, great PR.

userquin commented 1 year ago

imagen

userquin commented 1 year ago

imagen

ad1992 commented 1 year ago

@userquin filtering the assets works but I am not sure if this would be the best way to do it as its not necessary to follow the specific format eg the below ones will still be pre-cached since its a different format and similarly more could in future

Screenshot 2023-07-12 at 11 13 22 AM

Hence if there is some way so we can say anything inside locales folder shouldn't be pre-cached instead of checking the content of locales and matching them with regex, this way it would be easier for longer term as lang format could change.

Also I am trying similar regex with assets for runtime cache but it doesn't work and doesn't create locales folder, so anything additional I need to do to create locales runtime cache ?

And one last question for some reason fonts.css doesn't get cached under runtime cache fonts

Screenshot 2023-07-12 at 11 20 42 AM

Thanks again for helping out!

userquin commented 1 year ago

@ad1992 maybe you need to modify assets names for any locale and move it to a custom folder (locales/ for example), then you can use workbox: { globIgnores: ['locales/**'] }.

I'll check your vite config file in the PR

userquin commented 1 year ago

And one last question for some reason fonts.css doesn't get cached under runtime cache fonts

Runtime caching will not include any resource in first navigation, it will be included in the cache in next navigation:

imagen

imagen

ad1992 commented 1 year ago

And one last question for some reason fonts.css doesn't get cached under runtime cache fonts

Runtime caching will not include any resource in first navigation, it will be included in the cache in next navigation:

imagen

imagen

Hey yep thats right but you notice fonts.css is not in the fonts cache, its in workbox pre-cache instead, thats what I was referring to, I am trying to push it in fonts cache instead via the config.

userquin commented 1 year ago

@ad1992 you can exclude it using workbox.globIgnore option

About moving your locale files: check this https://rollupjs.org/configuration-options/#output-manualchunks, there is a similar example.


// vite.config.ts
build: {
  rollupOptions: {
    output: {
      manualChunks(/* options */) {

      }
    }
  }
}
ad1992 commented 1 year ago

@userquin I was able to resolve the remaining issues with what you suggested ✨

  1. Locales folder is now nested inside assets with rollup build
  2. Locales cache is also created separately - however in chrome I don't see it getting updated when more langs are added but in other browser eg firefox it does and it works offline too in both so I think some caching issue with my chrome but offline behaviour is as expected so thats great
  3. fonts.css is also now cached inside fonts cache after glob ignore.

Thanks a ton ❤️

userquin commented 1 year ago

@ad1992 I've notice you're using auto register without any virtual (registerSW.js), the app will not update until next navigation when a new version found (open app in a new browser session or hard refresh), the current sw will not claim all controlled clients.

I have no idea how do you store client state, if you're using local storage or IndexedDB, you can add a new module importing it in the entry point or use dynamic import (you will need to add vite-plugin-pwa/vanillajs to tsconfig or use triple slash reference):

import('virtual:pwa-register').then(({ registerSW }) => registerSW({ immediate: true }))

Check the warning in the docs: https://vite-pwa-org.netlify.app/guide/auto-update.html#automatic-reload

ad1992 commented 1 year ago

@userquin thats right I am using auto register right now without virtual module. We store the data in local storage. One concern when using automatic reload is if this could lead to some inconsistent user experience / race condition due to which LS data could get overridden resulting in loss of data ? I might be completely wrong and I am not sure if this would ever happen but since the browser tabs will be automatically reloaded when user is using the app so just little worried about the overall user experience. Would like to hear your thoughts on the same as well

userquin commented 1 year ago

thats right I am using auto register right now without virtual module. We store the data in local storage. One concern when using automatic reload is if this could lead to some inconsistent user experience / race condition due to which LS data could get overridden resulting in loss of data ?

Yes, that's the main problem (lose data and page reload); you can also switch to prompt for update, but it will need some additional work to add the dialog to reload the app.

I might be completely wrong and I am not sure if this would ever happen but since the browser tabs will be automatically reloaded when user is using the app so just little worried about the overall user experience.

That's why I'm asking you, using the virtual will automatically reload the app once the new sw installed and activated, I use it only in docs websites (with the exception of PWA docs).

userquin commented 1 year ago

A few months/years ago I added the pwa with automatic reload to Layout It: https://github.com/Leniolabs/layoutit-grid/blob/main/src/App.vue

ad1992 commented 1 year ago

Yes, that's the main problem (lose data and page reload); you can also switch to prompt for update, but it will need some additional work to add the dialog to reload the app.

Thats the reason I didn't introduce automatic reload yet as I am little skeptical about data loss during page reload. Switching to prompt we would like to avoid since that would introduce an extra step for users to decide.

So I am thinking is launch with current state (auto register) and see how it works and then introduce automatic reload as needed.

ad1992 commented 1 year ago

Hi @userquin there is one issue I am facing and wanted to know in case you have any insights to share

Separating out the locales to manualChunks using rollup config is preventing the site from pre-caching the necessary assets for first load, is this expected behaviour ?

To reproduce

  1. You can visit https://excalidraw-50736k678-excalidraw.vercel.app/
  2. Switch to offline
  3. Refresh - the site won't load

if you try the same in https://excalidraw-aaakct09b-excalidraw.vercel.app/ it will work fine - this is before separating out locales to manual chunks

userquin commented 1 year ago

@ad1992 I Will check it later...

userquin commented 1 year ago

It seems it is working because the browser is caching the locale for en, check the first visit (using es), then switch to offline and refresh page will switch to en locale (check the network errors):

imagen

userquin commented 1 year ago

I'll try to fix it with a simple repro test.

userquin commented 1 year ago

@ad1992 can you update to latest vite-plugin-pwa? you're using 0.14.7 and we've 0.16.4, also remove all workbox-** from dependencies (maybe you need to add workbox-build ^7.0.0 in devDependencies).

ad1992 commented 1 year ago

Sure @userquin let me try upgrading In prev url 2 languages were getting cached (due to wrong regex I think) and thats how I misunderstood it to be locales getting pre-cached

Screenshot 2023-07-25 at 3 01 30 PM
userquin commented 1 year ago

@ad1992 I guess we also need to remove locales from sw precache, I mean, since it has a custom cache we should include the pattern in the deny list.

Here my changes to locales (or just change it to use the navigateFallbackDenylist Regex):

// workbox runtimeCaching entry
      runtimeCaching: [{
          urlPattern: ({ sameOrigin, url }) => sameOrigin && url.pathname.startsWith('/assets/locales/'),
          // urlPattern: /^\/assets\/locales\//,
          handler: 'CacheFirst',
          options: {
            cacheName: 'locales',
            expiration: {
              maxEntries: 50,
              maxAgeSeconds: 60 * 60 * 24 * 30, // <== 30 days
            },
            // add this entry, we don't want to cache failing responses
            cacheableResponse: {
              statuses: [200],
            },
          },
        }],

Then add this entry to workbox:

navigateFallbackDenylist: [/^\/assets\/locales\//],
userquin commented 1 year ago

If working we should apply the same for fonts and fonts.css entries:

        navigateFallbackDenylist: [/\.(ttf|woff2|otf)$/, /^\/fonts\.css$/, /^\/assets\/locales\//],
        runtimeCaching: [{
          // urlPattern: ({ sameOrigin, url }) => sameOrigin && /\.(ttf|woff2|otf)$/.test(url.pathname),
          urlPattern: /\.(ttf|woff2|otf)$/,
          handler: 'CacheFirst',
          options: {
            cacheName: 'fonts',
            expiration: {
              maxEntries: 50,
              maxAgeSeconds: 60 * 60 * 24 * 90, // <== 90 days
            },
            cacheableResponse: {
              statuses: [200],
            },
          },
        }, {
          // urlPattern: ({ sameOrigin, url }) => sameOrigin && url.pathname === '/fonts.css',
          urlPattern: /^\/fonts\.css$/,
          handler: 'StaleWhileRevalidate',
          options: {
            cacheName: 'fonts',
            expiration: {
              maxEntries: 50,
            },
            cacheableResponse: {
              statuses: [200],
            },
          },
        }, {
          // urlPattern: ({ sameOrigin, url }) => sameOrigin && url.pathname.startsWith('/assets/locales/'),
          urlPattern: /^\/assets\/locales\//,
          handler: 'CacheFirst',
          options: {
            cacheName: 'locales',
            expiration: {
              maxEntries: 50,
              maxAgeSeconds: 60 * 60 * 24 * 30, // <== 30 days
            },
            cacheableResponse: {
              statuses: [200],
            },
          },
        }],
userquin commented 1 year ago

@ad1992 we've the same cacheName for fonts and fonts.css file with distinct expiration options, we should store them in separated caches or include both in the regex

ad1992 commented 1 year ago

@userquin was playing with it after upgrading and changes you suggested. So the issue with necessary assets not being pre cached still persists after upgrading

And as per navigateFallbackDenylist, I have the globIgnores which is taking care of not adding it in workbox precache so any special purpose of adding navigateFallbackDenylist ?

userquin commented 1 year ago

@ad1992 globIgnores is for build time, navigateFallbackDenylist is for runtime: globIgnores will prevent adding local assets to the sw precache manifest (self.__WB_MANIFEST), while navigateFallbackDenylist will prevent the sw to intercept those requests at runtime.

Workbox will add this route:

registerRoute(new NavigationRoute(createHandlerBoundToURL('index.html')))

navigateFallbackDenylist will prevent that route to intercept those assets, previous route will use this new one:

registerRoute(new NavigationRoute(createHandlerBoundToURL('index.html'),  { denylist: navigateFallbackDenylist }))
ad1992 commented 1 year ago

There is some weird behaviour which I am experiencing with different browsers (I am currently testing in local with preview build)

When changing only the url pattern regex from new RegExp("locales/[^/]+.js") -> /^\/assets\/locales\//

With this change in firefox - the first load starts pre- caching the en.json so on refresh with network disabled it works image

however in chrome it doesn't since en.json isn't precached.

image

userquin commented 1 year ago

@ad1992 try adding the navigateFallbackDenylist

userquin commented 1 year ago

FF is caching the en json file, not the sw, the Tranferred column should be service worker and it is cached.

Maybe we can add to the sw precache manifest the default lang and required locales entries (percentages.json for example, maybe we can also add some more).

ad1992 commented 1 year ago

There is some weird behaviour which I am experiencing with different browsers (I am currently testing in local with preview build)

When changing only the url pattern regex from new RegExp("locales/[^/]+.js") -> /^\/assets\/locales\//

With this change in firefox - the first load starts pre- caching the en.json so on refresh with network disabled it works image

however in chrome it doesn't since en.json isn't precached.

image

Also one more thing locales folder is no more created after regex updated in both browsers. Yes for ff browser is caching en.json but for chrome browser isn't with navigateFallbackDenylist as well the first reload issue isn't solved in chrome.

I will be afk for sometime. will post more updates once I am back.

userquin commented 1 year ago

@ad1992 I'll prepare an small repro...

userquin commented 1 year ago

@ad1992 one last thing, you should remove all the workbox-** from public/workbox folder

userquin commented 1 year ago

@ad1992 here a small repro https://github.com/userquin/pwa-locales-custom-cache, I've enabled mode: 'development' and so we can check workbox messages:

The problem is the cache for runtime caching, it not being created on first visit, you need refresh the page. I've added also png and ico files to the sw precache (to include pwa icons and screenshots).

You should also update the PWA theme color entry to match the theme color in the entry point.

Using or removing the navigateFallbackDenylist option has the same result, will not work if go offline on first visit and refresh the page.

userquin commented 1 year ago

It seems you're using Vercel, maybe we can add the immutable or cache header for these assets (fonts, font.css and assets/locales/*), this way the browser can use the cache.

Here an example using Netlify: https://github.com/userquin/vuetify-nuxt-module/blob/main/docs/public/_headers#L26-L28 For fonts.css and the fonts you can use the cache headers you've included in the corresponding runtime caching.

ad1992 commented 1 year ago

@ad1992 here a small repro https://github.com/userquin/pwa-locales-custom-cache, I've enabled mode: 'development' and so we can check workbox messages:

  • pnpm install (or yarn from root)
  • pnpm build && pnpm preview

The problem is the cache for runtime caching, it not being created on first visit, you need refresh the page. I've added also png and ico files to the sw precache (to include pwa icons and screenshots).

You should also update the PWA theme color entry to match the theme color in the entry point.

Using or removing the navigateFallbackDenylist option has the same result, will not work if go offline on first visit and refresh the page.

Yep thats whats happening right now so the only issue left is first visit and the going offline isn't working because en.json is not pre-cached. So earlier we were using CRA and in that en.json wasn't pre-cached separately I just checked in production https://excalidraw.com/ and I don't see it being cached in workbox pre-cache as well so looks like its bundled with the code as there is no separate call for en.json. Screenshot 2023-07-25 at 8 18 28 PM

Screenshot 2023-07-25 at 8 18 40 PM

So even if you were at some language "x" on first visit and you go offline, its going to fallback to en.json since runtime cache isn't created until you refresh again and that I think is how service worker works ?

So we cannot do anything about runtime cache but if we can bundle en.json so there is no separate call for it then the use case will be resolved.

So what I can try is when creating manual chunks - consider all files except en.json and percentages.json then it will be considered part of main cache and that might help in overcoming the first load+offline issue

ad1992 commented 1 year ago

looks like the above strategy is working and first load + offline issue seems to be resolved https://excalidraw-9xup84w1r-excalidraw.vercel.app/

ad1992 commented 1 year ago

@ad1992 I've notice you're using auto register without any virtual (registerSW.js), the app will not update until next navigation when a new version found (open app in a new browser session or hard refresh), the current sw will not claim all controlled clients.

I have no idea how do you store client state, if you're using local storage or IndexedDB, you can add a new module importing it in the entry point or use dynamic import (you will need to add vite-plugin-pwa/vanillajs to tsconfig or use triple slash reference):

import('virtual:pwa-register').then(({ registerSW }) => registerSW({ immediate: true }))

Check the warning in the docs: https://vite-pwa-org.netlify.app/guide/auto-update.html#automatic-reload

@userquin since I have disabled automatic reload so page won't auto reload which is fine and it won'r reload even after refresh unless "Hard refresh" is done so is there a way to reload the page with regular refresh ?

If I just call registerSW without any options then what would be the behaviour ?

registerSW()
userquin commented 1 year ago

@userquin since I have disabled automatic reload so page won't auto reload which is fine and it won'r reload even after refresh unless "Hard refresh" is done so is there a way to reload the page with regular refresh ?

No, only hard refresh or reopening the browser: check sw in dev tools, new sw will be awaiting (yellow flag)

If I just call registerSW without any options then what would be the behaviour ?

Using any virtual pwa module will auto reload all client pages once new sw detected and activated (the sw will claim all clients controlled by previous sw, this event will fire the reload in all clients)

EDIT: I suggest you to provide {immediate: true} in the call to the virtual, you can import the virtual in the entry point (tsx) or use dynamic import moving the logic to pwa.ts module (import('./pwa.ts') in the entry point, this new module will have the static import and the registerSW call)

ad1992 commented 1 year ago

@userquin can you confirm if my understanding is correct ? This is what I am seeing when testing 👇🏻

  1. with registerType: autoUpdate - The sw isn't updated unless until page refreshed. Once page refreshed it checks for sw and post that once page refreshed, the new changes are reflected
  2. with calling registerSW - The sw isn't updated unless until page refreshed. Once sw is updated, the page is automatically reloaded.
  3. registerSW({immediate: true}) - Same as above (I am not seeing any difference)

Here is the URL - https://excalidraw-git-aakansha-fix-sw-reload-excalidraw.vercel.app/ and PR for reference - https://github.com/excalidraw/excalidraw/pull/6824

userquin commented 1 year ago

Auto update and page auto refresh is about the sw behaviour, if you want to auto check if there is a new version, you'll need to add periodic sync for updates: https://vite-pwa-org.netlify.app/guide/periodic-sw-updates.html (use the code in the example here https://vite-pwa-org.netlify.app/guide/periodic-sw-updates.html#handling-edge-cases)

ad1992 commented 1 year ago

@userquin we shipped the sw with automatic reloading. But I have a query, so in chrome and firefox I had excalidraw.com opened with previous version (without automatic reload) and on refresh it it got updated with new one but on firefox it didn't and I had to hard refresh. What can we do to make sure it works the same for everyone browser and on refresh sw is actually updated ? I can see sw is not being cached so it should have gotten updated on all browsers when refreshing.

userquin commented 1 year ago

It should work on next browser session or hard refresh on any browser (IIRC chrome fix it allowing to refresh the sw with just f5, check closed issue 33 in pwa plugin repo). We cannot do anything.

ad1992 commented 1 year ago

@userquin unfortunately there is one weird issue (probably related to SW / Vite I am not sure), in a browser which I haven't used for long time (lets say few months), in that If I open excalidraw.com, it goes to some older commit way before vite was pushed. So latest sw isn't getting fetched hence its older version.

So the change now is earlier we had our custom sw code and now vite is handling it, but looks like for browsers which were hardly used the changes are not getting updated as well.

Any suggestion on what we can do to migrate everyone to vite ? Also let me know If I should open a different issue for this as its deviating from the topic of issue

userquin commented 1 year ago

uhmm, the service worker name was changed? I mean, what was the old service worker asset name? the current is sw.js

ad1992 commented 1 year ago

yes it was service-worker.js earlier and now its sw.js

userquin commented 1 year ago

ok, I'll check the old sw behavior, open a new draft PR adding service-worker.js to public folder with this content, I'll check how do you register the old service-worker, the new app has been published?:

self.addEventListener('install', function(e) {
  self.skipWaiting();
});
self.addEventListener('activate', function(e) {
  self.registration.unregister()
    .then(function() {
      return self.clients.matchAll();
    })
    .then(function(clients) {
      clients.forEach(client => client.navigate(client.url))
    });
});
userquin commented 1 year ago

I guess previous code should work, the browser should update the service worker and once updated should remove it clearing all caches and reloading any client being still controlled by its previous version

ad1992 commented 1 year ago

ok, I'll check the old sw behavior, open a new draft PR adding service-worker.js to public folder with this content, I'll check how do you register the old service-worker, the new app has been published?:

self.addEventListener('install', function(e) {
  self.skipWaiting();
});
self.addEventListener('activate', function(e) {
  self.registration.unregister()
    .then(function() {
      return self.clients.matchAll();
    })
    .then(function(clients) {
      clients.forEach(client => client.navigate(client.url))
    });
});

yes Vite is live in https://excalidraw.com/. Let me create a PR with this. The thing is we cannot test it out unless we actually deploy it now as its not testable on preview :(