-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Improve UX for service worker updates #6263
Improve UX for service worker updates #6263
Conversation
Deploy preview for using-drupal ready! Built with commit d6eda7b |
Deploy preview for gatsbygram ready! Built with commit d6eda7b |
packages/gatsby-link/src/index.js
Outdated
@@ -153,7 +155,11 @@ class GatsbyLink extends React.Component { | |||
// loaded before continuing. | |||
if (process.env.NODE_ENV === `production`) { | |||
e.preventDefault() | |||
window.___push(this.state.to) | |||
if (window.GATSBY_SW_UPDATED) { | |||
window.location = this.state.to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about checking this flag and doing location change in https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/cache-dir/production-app.js#L68 (TBD if before or after redirect handling)? So programatic navigation would get same behaviour (if someone manually call window.___push
/window.___replace
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh snap, that is much better! will move this over there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
@@ -17,7 +17,7 @@ if (`serviceWorker` in navigator) { | |||
// have been added to the cache. | |||
// We reload immediately so the user sees the new content. | |||
// This could/should be made configurable in the future. | |||
window.location.reload() | |||
window.GATSBY_SW_UPDATED = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not super important but we generally add 3 underscores before our globals.
@@ -64,6 +64,12 @@ apiRunnerAsync(`onClientEntry`).then(() => { | |||
if (redirect) { | |||
pathname = redirect.toPath | |||
} | |||
|
|||
// If we had a service worker update, no matter the path, reload window | |||
if (window.GATSBY_SW_UPDATED) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pieh can you check this out when you get a chance? is this essentially what you had in mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
Thanks! This has been bugging me forever!!! |
This PR adds support for waiting on page refreshes from service worker updates. How this is achieved:
This allows the new resources to be loaded without interrupting any current work or browsing being done by the site user.
Closes #2757