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

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

Closed
kimamula opened this issue Jan 29, 2018 · 12 comments
Closed

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

kimamula opened this issue Jan 29, 2018 · 12 comments

Comments

@kimamula
Copy link

kimamula commented Jan 29, 2018

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
Copy link

sdbondi commented Feb 26, 2018

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
Copy link
Member

lukeed commented Apr 9, 2018

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
Copy link
Contributor

@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
Copy link
Member

prateekbh commented Apr 11, 2018

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
Copy link
Author

kimamula commented May 5, 2018

@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
Copy link

Buom01 commented Jun 13, 2018

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
Copy link
Member

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

@Buom01
Copy link

Buom01 commented Jun 14, 2018

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

@qkdreyer
Copy link

qkdreyer commented Jul 3, 2018

@tyleralves
Copy link

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

@Ganpatkakar
Copy link

Ganpatkakar commented Oct 22, 2023

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
Copy link

@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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.