-
Notifications
You must be signed in to change notification settings - Fork 2
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
Force reload demeris when errors are thrown due to probable new build. #1715
Comments
Thanks for the initiative! Fix sounds great, let's do this 🙏 |
Maybe we can try some Vite configurations for the chunks: https://vitejs.dev/guide/build.html#chunking-strategy Same issue but with Webpack: https://stackoverflow.com/questions/54889720/handle-loading-chunk-failed-errors-with-lazy-loading-code-splitting |
This also explain the issue in detail and propose to do the versioning (not deleting the old version, so the user avoid this problem before refreshing): https://rollbar.com/blog/javascript-chunk-load-error/ |
Considering all the talk we went through regarding this topic, Wrote up something but realized it was pretty much what this issue was suggesting as the fix anyway 😅 |
Debugging a bit, I noticed that Vite uses the following approach to load new modules: CleanShot.2022-05-15.at.19.52.36.mp4TL;DR: Vite inserts a new So a possible solution would be to preload all the module files at start, I found this plugin that does exactly what I'm expecting: https://github.com/jarlef/vite-plugin-preload The output will result in this: |
@luciorubeens Exactly, that is how it should work for lazy loading the modules while needed, and for that we are doing dynamic import and not loading everything at once. If you modulepreload everything in one place (the first load) then the performance will be worse than it's now. We can try this change locally and measure the performance in both cases, to see if it's a number of ms or s we can allow, or it's too much. |
Ok, another option would be to not clear the build files on each deploy, then Here is the option: https://vitejs.dev/config/#build-emptyoutdir Will require a script, workflow or something like to remove old files. |
Just adding context for the downvote... I think there's a considerable difference in the core web vitals for the two cases and I would prefer not going for the worse case :) |
@luciorubeens sounds like a good option 👍 , let's do it? |
What about listening for the error event and requesting to the user to reload if it occurs? A message like: your page is outdated - click to reload before proceeding:
other ideas here: https://imrecsige.dev/garden/how-to-solve-missing-chunk-code-splitting-errors-after-deploy/ |
@caudurodev Yes, exactly what @fl-y is doing here #1802. Not a fan of this option because the user will lose state, like pending transactions. We can also persist this in localStorage and rehydrate on load to fix this edge case, but.. Thanks for sharing the link. So any option has its cost, so we need to choose the one that works best for us. |
I think we won't have a solution that will be good to preserve state as it is currently stored impermanently in memory. How will you preserve state if there have been changes to the store for example? It seems like a minefield of edge cases. If we really want to maintain state, the easiest option would be via a backend session and another, turning our site into a PWA and persisting data in localstorage/sessions. There are some packages that help with persisting Vuex like https://www.npmjs.com/package/vuex-persist But just the sheer amount of work to get this to work and all the edge cases... what if the user reloads mid-transaction? What if... |
@caudurodev fair concerns!
Agree with the minefield of edge cases - but on the other hand, my guesstimate is that with most of the changes - the asset actually didn't change at all (most often a deploy doesn't necessarily touch the files you're using). (and even IF the user is using a specific file, I think in most cases it would be harmless to update it) - however, those are my guesstimates which are not worth that much. |
Describe the bug
Sentry is flooded with the following error
Failed to fetch dynamically imported module
The error was effectively reproduced in this thread.
Since said error is an effective indicator that the user is using an outdated build of Emeris,
Adding client code s.t. Demeris is force-reloaded from the client side when said error occurs will greatly reduce those generic errors.
To Reproduce
Keep Emeris open while a PR is being merged to PROD,
give it 5~10 minutes and try navigating to different pages.
Expected behavior
Instead of a malfunctional website throwing errors - the client should force-refresh itself.
Additional action points
This is not a long term solution but effective duct-tape for the current state of things.
In the long run a new(breaking) build should trigger an alert / message to any active website sessions and trigger a refresh.
Implementation
Message:
A new version of the website was detected! The page will now refresh.
^^ Wdyt? @josietyleung
The text was updated successfully, but these errors were encountered: