preactjs / preact-cli

😺 Your next Preact PWA starts in 30 seconds.
MIT License
4.69k stars 375 forks source link

index.html should be excluded from service worker cache #474

Closed kimamula closed 5 years ago

kimamula commented 6 years ago

Do you want to request a feature or report a bug?

bug

What is the current behaviour?

bundle.{hash}.js tries to load the old version of route-{page}.chunk.{hash}.js when JS code is modified, which results in an error.

If the current behaviour is a bug, please provide the steps to reproduce.

  1. Create new project from template (I used material)
  2. Run the project on localhost ($ npm run serve)
    • The following JS files are created
      • bundle.2eb65.js
      • route-home.chunk.f678b.js
      • route-profile.chunk.c26a1.js
  3. Open https://localhost:8080 in browser
    • index.html, the JS files listed above, and other resources are cached by service worker
  4. Edit src/routes/profile/index.js (I added console.log('something has changed'); at render() method)
  5. Rerun $ npm run serve
    • The JS files are updated as follows
      • bundle.2eb65.js -> bundle.a5db3.js
      • route-home.chunk.f678b.js (unchanged)
      • route-profile.chunk.c26a1.js -> route-profile.chunk.27ce5.js
  6. Reload https://localhost:8080 in browser
    • index.html cached at the step 3 is used as the response for https://localhost:8080
    • Cached index.html requests bundle.2eb65.js (the old version), which is also restored from the service worker cache
    • In the background, service worker is updated and activated immediately due to skipWaiting(), which triggers deletion of the old caches (such as route-profile.chunk.c26a1.js)
  7. Click link to the Profile page
    • bundle.2eb65.js (the old version) requests route-profile.chunk.c26a1.js (the old version), which exists neither on the server nor in the service worker cache
      • At this point an error occurs.

What is the expected behaviour?

All the client side code should be updated to the latest version when JS code is modified.

Excluding index.html from the service worker cache (i.e., adding /index\.html$/ here), which prevents the old index.html from requesting the old bundle at the step 6-ii, should fix the problem.

sdbondi commented 6 years ago

How to do the above in preact.config:

  const swPlugin = config.plugins.find(
    p => p instanceof SWPrecacheWebpackPlugin,
  );

  if (swPlugin) {
    swPlugin.options.staticFileGlobsIgnorePatterns.push(/index\.html$/);
  }

This will essentially disable offline capabilities as you cannot load index.html offline. However, the current solution also breaks as the old bundle is loaded as described above. It would be nice to have a built-in solution to this in preact cli.

lukeed commented 6 years ago

I think what we need to do is force the bundle hash to change when ANY chunk changes. Because we are extracting page chunks, which incapsulate the components, it COULD be the case that route bundles change but the main bundle hash doesn't change.

Otherwise, the current setup is the way that service worker works. You will get a stale app on the first visit after the new assets have been cached. This includes index.html. Every visit thereafter will be serving the new app assets from memory. This is also why offline-plugin forces a hard reload on the SW "updated" hook.

Not caching the index does not solve this issue, as attempted in the linked PR, because an HTML file is needed in order for the offline app to boot. Disregarding that point, if the new HTML file still had an asset link to a "new" bundle whose hash did not change, then the same problem would occur.

osdevisnot commented 6 years ago

@lukeed @sdbondi @kimamula I spent little more time on this and from my research so far sw-precache-webpack-plugin should work correctly with existing setup (including offline capability).

I uploaded my repro into separate repo here - but voila I could not reproduce the issue as reported originally. As documented in this repo, it seems at step 5 above, sw.js in fact has new hash for index.html which is downloaded correctly when browser is refreshed.

Am I missing something?

prateekbh commented 6 years ago

I guess if a route directly included in root's index.html is changed only then the hash of index.js changes.

Simplest sollution to this is to extract webpack's asset manifest and add it in index.html. so any route changes -> manifest's hash changes(i assume) -> index's hash changes

kimamula commented 6 years ago

@lukeed

Not caching the index does not solve this issue, as attempted in the linked PR, because an HTML file is needed in order for the offline app to boot.

I agree. I was missing this point.

@osdevisnot I git cloned your repo and could reproduce the issue. Please click the "Profile" link in the menu after you reload browser in your "3rd run" and you will see Uncaught SyntaxError: Unexpected token < error in console.

As shown in your image, index.html (localhost) is "from ServiceWorker" in your "3rd run".

Image for 3rd run

Therefore, the older version of index.html that has been cached by ServiceWorker is used in your "3rd run" even if the hash for index.html in sw.js is updated, while the cache for route-profile is renewed immediately after the browser reload which causes the issue.

It seems to me that setting skipWaiting to false should be the correct solution.

When skipWaiting is true, the new service worker's activate handler will be called immediately, and any out of date cache entries from the previous service worker will be deleted. Please keep this in mind if you rely on older cached resources to be available throughout the page's lifetime, because, for example, you defer the loading of some resources until they're needed at runtime.

Buom01 commented 6 years ago

I haven't tried but It could be that somebody wants:

const SWPrecacheWebpackPlugin = helpers.getPluginsByName(config, 'SWPrecacheWebpackPlugin')[0]
if(SWPrecacheWebpackPlugin){
  const {plugin} = SWPrecacheWebpackPlugin;
  const unwanted = [
    /^[^\.]*(\.html)?$/
  ]
  plugin.options.staticFileGlobsIgnorePatterns = plugin.options.staticFileGlobsIgnorePatterns.concat(unwanted);
  plugin.options.runtimeCaching = plugin.options.runtimeCaching || []
  plugin.options.runtimeCaching.push({
    urlPattern: /^[^\.]*(\.html)?$/,
    handler: 'networkFirst'
  })
}

Then online versions are tried firstly, For me the index.html works as expected

prateekbh commented 6 years ago

something similar i had in my mind. does total offline works with this?

Buom01 commented 6 years ago

Even if I don't take a lot of time to test it, it's seem to work quite good

qkdreyer commented 6 years ago

https://github.com/vuejs-templates/pwa/issues/70#issuecomment-398835001

tyleralves commented 5 years ago

Why is this one closed? Doesn't this bug occur every time a new version of bundle.{hash}.js is produced?

Ganpatkakar commented 1 year ago

I am not sure how much the vanilla service worker is helpful but there is my 2 pinch of salt on this problem statement

self.addEventListener('fetch', (event) => {
    console.log('Service Worker: Fetching');
    event.respondWith(
        caches.match(event.request).then(
            (response) => {
                if (event.request === '/index.html') {
                    if (navigator.onLine) {
                        console.log('online mode and fetching index.html');
                        return fetch(event.request);
                      }
                }
                if (response) {
                    return response;
                }
                return fetch(event.request)
                .then(
                    (response) => {
                        if (!response || response.status !== 200 || response.type !== 'basic') {
                            return response;
                        }

                        const responseClone = response.clone();
                        caches.open(CacheName).then((cache) => {
                            return cache.put(event.request, responseClone);
                        });
                        return responseClone;
                    }
                )
                .catch((err) => {
                    console.log("Error with fetching", event.request);
                });
            }
        )
    )
});

key is navigator.online condition check and if user is online then always fetch from network and register it in cache for latest html, and if its offline then go with cache response.

hope it may help someone.

dominikduda commented 11 months ago

@Ganpatkakar What file do you register serviceworker at?

I register mine in the index.html and this condition if (event.request === '/index.html') { is never true. As if this request never goes through the service worker (even though I can see that it does in dev tools network tab).