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

Service worker fails to load new dynamic/code-split chunk #3613

Closed
TheHolyWaffle opened this issue Dec 16, 2017 · 26 comments
Closed

Service worker fails to load new dynamic/code-split chunk #3613

TheHolyWaffle opened this issue Dec 16, 2017 · 26 comments

Comments

@TheHolyWaffle
Copy link

TheHolyWaffle commented Dec 16, 2017

Is this a bug report?

Yes

Can you also reproduce the problem with npm 4.x?

Yes

Which terms did you search for in User Guide?

progressive web app service workers

Environment

  1. node -v: 8.4.0
  2. npm -v: 5.6.0
  3. yarn --version (if you use Yarn):
  4. npm ls react-scripts (if you haven’t ejected): 1.0.17

Then, specify:

  1. Operating system: Windows 10
  2. Browser and version (if relevant): Chrome 63

Steps to Reproduce

  1. Set up a default CRA project
  2. Install react-loadable to code-split a certain React component on the page
  3. Build, serve and navigate to the page
  4. Modify the component earlier (to change the generated hash)
  5. Build, serve and reload the page

Expected Behavior

The new version of the code-splitted component should be loaded correctly.

Actual Behavior

The installed service worker tries to fetch the old component chunk instead of the new one, resulting in a failure to load the new chunk. This is because the hash has changed since the last time the service worker was installed. Because of this react-loadable interprets the error and refuses to display the new page, resulting in a blank page.

knipsel

This is less than ideal, and the only thing I could come up is to force a reload on the homepage. This will trigger the new service worker to be installed which results in the correct component/chunk being loaded.

@engmyahya
Copy link

engmyahya commented Dec 20, 2017

@TheHolyWaffle try use the following webpack.config.js


var path = require('path');
var webpack = require('webpack');

module.exports = {
  context: path.join(__dirname, 'app'),
  entry: ['./index.html', './index.js'],
  output: { 
    path: path.join(__dirname, 'dist'),
    filename: 'bundle.js'
  },
  module: {
    loaders: [
      {
        test: /.js?$/,
        loader: 'babel-loader',
        exclude: /node_modules/,
        query: {
          presets: ['es2015', 'react']
        }
      },
      {
        test: /\.html$/,
        loader: "file-loader?name=[name].[ext]",
      },
      {
        test: /\.css$/, 
        loader: "style-loader!css-loader" 
      },
      { 
        test: /\.png$/, 
        loader: "url-loader?limit=100000" 
      },
      { 
        test: /\.jpg$/, 
        loader: "file-loader" 
      }
    ]
  },
  devServer: {
    historyApiFallback: true
  }
};

@TheHolyWaffle
Copy link
Author

@engmyahya How would that help? I see nothing special sticking out from that webpack config that would fix this issue.

@TheHolyWaffle TheHolyWaffle changed the title Service worker failed to load new chunk in SPA Service worker fails to load new dynamic/code-split chunk Dec 21, 2017
@TheHolyWaffle
Copy link
Author

TheHolyWaffle commented Dec 21, 2017

@jeffposnick would you be able to tell what is going wrong here?

I have explicitly removed any cache headers from service-worker.js and index.html but to no avail.

@TarikHuber
Copy link

I have the exact same issue. "Solved" it also with reloading the page. But dosn't feel like a real solution.

@jeffposnick
Copy link
Contributor

I don't believe that this type of failure is limited to scenarios in which there's a service worker installed. For instance, you could imagine a scenario in which a long-lived single page app not controlled by a service worker attempts to dynamically load a hashed chunk as a result of a SPA-style navigation, but the URL for that chunk is no longer valid because the assets for the site have been updated and redeployed in the interim.

So, while there are ways to avoid getting into this state by modifying the service worker's behavior[*], I wonder if it would just make more sense to address it from the perspective of solving the non-service worker scenario as well.

One approach would be to detect when there's a failure to dynamically load one of the chunks that's needed to perform an SPA-style navigation and use that as a cue to trigger a "real" navigation request, which should recreate the DOM and get the client to the point where it's expecting the hashed chunk names that correspond to what's currently on the server.

I'm not familiar with the libraries you're using to dynamically load the chunks, but perhaps someone who knows more about that would have specific guidance as to the code changes that would be needed.

[*] Setting skipWaiting: false when generating the service worker should prevent the old caches from being purged until all open tabs that are controlled by the service worker have been unloaded, but the drawback of using that config is that developers who expect to see new content when they reload an open tab will have to wait until they close and reopen the tab instead.

@ugogo
Copy link

ugogo commented Dec 22, 2017

I went into the same scenario as TheHolyWaffle.
Calling unregister instead of register of registerServiceWorker fixed the thing, but it has broken our production for days, and recovering from this is really painful

@jeffposnick
Copy link
Contributor

@ugogo But again, isn't this a problem that you need to solve whenever you deploy a long-lived web app that relies on loading in chunks dynamically at runtime that might have been removed from the web server?

You mentioned that you've unregister()ed your service workers, which is fine, but what about the possibility of this occurring without service worker involvement?

@ugogo
Copy link

ugogo commented Dec 22, 2017

@jeffposnick Yeah I think you're right, chunks were removed from the server, which was leading to a 404.
unregister() was only used to keep our production safe before finding the correct solution 😄

@uditalias
Copy link

+1
I have the exact same issue

I have an "app shell" with split points for each container/page:
for example:

  • App Shell - contains app header and main router
    • Dashboard - split point
    • Report - split point
    • Account - split point
    • Settings - split point

Some times when i'm deploying my app im getting an error saying a chunk can't be loaded.
The error disappears after a refresh.

@jeffposnick
Copy link
Contributor

The change in https://github.com/facebook/create-react-app/pull/3924/files#diff-d2bddbd3bfb7051f9381474c844674faR446 should help avoid issues with lazy-loading for developers who use service workers via create-react-app.

But as mentioned previously, independent of the service worker, this is a problem you need to deal with any time you're lazy-loading versioned assets. How would you avoid it if you're just in a scenario in which a user has a tab open for an extended period of time (without a service worker registered), and then they attempt to load a versioned asset which has been removed from the server in the interim?

Add in logic into your lazy-loading code that handles failures by, e.g., automatically performing a full-refresh, or at least showing a meaningful error to your users so that they could manually refresh, seems like it would be necessary in general.

@zackify
Copy link

zackify commented Oct 19, 2018

With a service worker, you should be able to check when there is a new version, and then trigger a page load the next time someone navigates.

I'm using CRA without a service worker, is there really no way for me to catch the chunk loading error in my own code, and then i could trigger a page refresh so it feels normal when i deploy a new version while someone is on the old one?

@robertvansteen
Copy link
Contributor

There’s nothing stopping you from reloading the page when there is an error loading a chunk, you can just catch that error.

The only issue I could think of is that if for some reason that chunk could still not be loaded after a refresh you end up in a reload loop.

@zackify
Copy link

zackify commented Oct 19, 2018

how can i catch the error then? is there a code sample? and it would be easy to turn this on in production only, where it would be unlikely to happen for any other reason. It's the only solution i can think of. Since Netlify deletes all previous assets and doesn't acknowledge this problem.... haha random rant.

@zackify
Copy link

zackify commented Oct 19, 2018

Ahh, I had done this before but since I'm using react-loadable, you can check the error prop #4152 (comment) for anyone else interested

@robertvansteen
Copy link
Contributor

Import itself just returns a promise so you can catch any errors just like with any other promise. See: http://2ality.com/2017/01/import-operator.html

I wouldn’t just assume that there are no other reasons for chunks to fail to load on production. If you accidentally push a chunk that contains a runtime error to production all your users will end up in a reload loop which will probably crash their browser/tab if they are not protected against it.

@robertvansteen
Copy link
Contributor

Last time I checked there was no way (except perhaps with some regex) to find out if the error was because the chunk was missing or there was something wrong with the chunk. Because the import will catch any runtime errors in your chunk as well.

@zackify
Copy link

zackify commented Oct 19, 2018

I know it's a promise but I didn't know how to catch it when using the react-loadable abstraction :) thanks a lot. I have enough tests and linters in place where a chunk loading error shouldn't happen.

I get it's not perfect but I don't see another solution when not using a SW :/ i wish code splitting errors could cover more normal cases like this and be easier to catch!

@robertvansteen
Copy link
Contributor

I feel your pain. I've been thinking about it a lot but it's hard to come up with a unified solution as this is mostly an issue with the hosting providers. Using something like Cloudflare to cache all your bundles on their servers (up to a month) could help if you don't have full control over the hosting of your application.

@AustinGomez
Copy link

AustinGomez commented Oct 23, 2018

I'm still a bit fuzzy on what to do here. When there's a new chunk how can I set up the service worker to get the new one? Anyone have a code snippet?

How does calling unregister instead of register fix things?

@zackify
Copy link

zackify commented Oct 23, 2018

If you're using a service worker, heres some ideas @AustinGomez https://zach.codes/handling-client-side-app-updates-with-service-workers/

I am not trying to self-promote, but this was my thought process when I was setting up service workers for the first time and trying to auto update people on my apps. I think this can help

@AustinGomez
Copy link

@zackify thanks for that. That's a cool solution!

@robertvansteen
Copy link
Contributor

@zackify thanks for sharing! Really helpful as this is something a lot of people run in to. Perhaps we should add a link to the post in the documentation about service workers?

@zackify
Copy link

zackify commented Oct 24, 2018

Sure if you want!

@peterbe
Copy link

peterbe commented Oct 25, 2018

Ash @jeffposnick said, talking about service workers here is just making things more confusing. Same problem, just harder to see.

Another good solution is to "cake on the built assets". Running yarn run build will wipe out any old ./build/ directory but on your server, you can keep the old files from the old deployment.

If the Monday build was:

  1. build/static/js/main.aaa.js
  2. build/static/js/chunk.111.js

...and someone loads main.aaa.js on that Monday but don't click the button that causes chunk.111.js to load yet.

Tuesday's build was:

  1. build/static/js/main.bbb.js
  2. build/static/js/chunk.222.js

So if that Monday user eventually tries to load chunk.111.js it's going to be gone. It would be better if your server (e.g. Netlify, S3, Nginx, whatever) would be available to host all of...:

  1. build/static/js/main.aaa.js
  2. build/static/js/chunk.111.js
  3. build/static/js/main.bbb.js
  4. build/static/js/chunk.222.js

that you would avoid that 404 error.

Having to reload the page just because a chunk failed feels dangerous. What if the user is in the midst of typing in their shipping address and a reloaod causes all sorts of disruption to the shopping cart checkout process.

@stale
Copy link

stale bot commented Nov 24, 2018

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Nov 24, 2018
@stale
Copy link

stale bot commented Nov 29, 2018

This issue has been automatically closed because it has not had any recent activity. If you have a question or comment, please open a new issue.

@stale stale bot closed this as completed Nov 29, 2018
@lock lock bot locked and limited conversation to collaborators Jan 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests