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

fix: force refresh on chunk preload error #7600

Merged
merged 3 commits into from Jun 11, 2022
Merged

Conversation

yangshun
Copy link
Contributor

@yangshun yangshun commented Jun 11, 2022

Pre-flight checklist

Motivation

Fixes #7599

Test Plan

  1. Build the site
  2. Do yarn run serve
  3. Load https://localhost:3000
  4. Kill the server
  5. Try to navigate, the page will immediately refresh and show the browser's error page. Previously it won't because we're using client-side navigation and just fail silently with an infinite loading bar

Test links

Deploy preview: https://deploy-preview-_____--docusaurus-2.netlify.app/

Related issues/PRs

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jun 11, 2022
@yangshun yangshun changed the title fix: force refresh on preload error fix: force refresh on chunk preload error Jun 11, 2022
@Josh-Cena Josh-Cena added the pr: polish This PR adds a very minor behavior improvement that users will enjoy. label Jun 11, 2022
Copy link
Collaborator

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's nice!

Have you managed to test the fresh deployment case? I'm wondering if it will reload and show the new page, or reloads and show the current page again.

@yangshun
Copy link
Contributor Author

yangshun commented Jun 11, 2022

Have you managed to test the fresh deployment case? I'm wondering if it will reload and show the new page, or reloads and show the current page again.

It will show the new page because the URL has updated already. I'm waiting for the preview build to complete, then change something to trigger another preview build while having a tab open on the previous preview build. Hopefully I can test it this way.

@netlify
Copy link

netlify bot commented Jun 11, 2022

[V2]

Name Link
🔨 Latest commit 45d3bef
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/62a458ef340cfc0008683e1b
😎 Deploy Preview https://deploy-preview-7600--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions
Copy link

github-actions bot commented Jun 11, 2022

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 88 🟢 100 🟢 100 🟢 100 🟢 90 Report
/docs/installation 🟠 87 🟢 100 🟢 100 🟢 100 🟢 90 Report

@yangshun yangshun merged commit feb9cf0 into main Jun 11, 2022
@yangshun yangshun deleted the yangshun/fix-stale-nav branch June 11, 2022 09:03
@Josh-Cena
Copy link
Collaborator

Nice, I think I've been tricked by this in the past.

@github-actions
Copy link

Size Change: +27 B (0%)

Total Size: 801 kB

ℹ️ View Unchanged
Filename Size Change
website/.docusaurus/globalData.json 52.6 kB 0 B
website/build/assets/css/styles.********.css 106 kB 0 B
website/build/assets/js/main.********.js 603 kB +27 B (0%)
website/build/index.html 38.9 kB 0 B

compressed-size-action

@slorber
Copy link
Collaborator

slorber commented Jun 15, 2022

hmmm we can give a try but wonder if it will work fine under all conditions

For example what if the user has a temporary bad network?
Trying to reload may not be ideal: maybe in some cases we'll want to keep waiting and set-up some polling 🤷‍♂️

We'll see

@Josh-Cena
Copy link
Collaborator

We didn't retry before this anyway. Refreshing is already a form of retry, and if it fails, the user instantly knows something is wrong, instead of getting stuck in an infinite loading bar.

@yangshun
Copy link
Contributor Author

We didn't retry before this anyway. Refreshing is already a form of retry, and if it fails, the user instantly knows something is wrong, instead of getting stuck in an infinite loading bar.

Was about to say something like this but @Josh-Cena said it better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: polish This PR adds a very minor behavior improvement that users will enjoy.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stale sessions error out during navigate if a deployment occured after the page first loads
4 participants