Skip to content

fix(core): do not recreate ReactDOM Root, fix React warning on hot reload#10103

Merged
slorber merged 2 commits into
mainfrom
slorber/fix-react18.3-hotreload-warning
May 3, 2024
Merged

fix(core): do not recreate ReactDOM Root, fix React warning on hot reload#10103
slorber merged 2 commits into
mainfrom
slorber/fix-react18.3-hotreload-warning

Conversation

@slorber

@slorber slorber commented May 3, 2024

Copy link
Copy Markdown
Collaborator

Motivation

Fix React 18 hot reloading warning:

Initially reported here: #10099 (comment)

Warning: You are calling ReactDOMClient.createRoot() on a container that has already been passed to createRoot() before. Instead, call root.render() on the existing root instead if you want to update it.

image

Note: this is not a React v18.3 warning, we can also have it in v18.2.

Note: Docusaurus does not have React Fast Refresh support, but we probably should have that. It doesn't look super easy to support 😅

Test Plan

CI + Tests

Local dev, hot reload works for both MD and JS files

Test links

https://deploy-preview-10103--docusaurus-2.netlify.app/

@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label May 3, 2024
@slorber slorber requested a review from Josh-Cena as a code owner May 3, 2024 12:40
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label May 3, 2024
prefetch: (url: string) => false | Promise<void[]>;
preload: (url: string) => false | Promise<void[]>;
};
docusaurusRoot?: import('react-dom/client').Root;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Note to self

I decided to add a new attribute here because it's a mutable variable, unlike window.docusaurus (which gets reassigned on hot reload and is also frozen, so it was complicated to add the mutable root here in practice)

@slorber slorber mentioned this pull request May 3, 2024
7 tasks
@netlify

netlify Bot commented May 3, 2024

Copy link
Copy Markdown

[V2]

Name Link
🔨 Latest commit 9f6789d
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/6634dc5a4eb65f0008c062db
😎 Deploy Preview https://deploy-preview-10103--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 configuration.

@github-actions

github-actions Bot commented May 3, 2024

Copy link
Copy Markdown

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 59 🟢 98 🟢 96 🟢 100 🟠 88 Report
/docs/installation 🔴 49 🟢 96 🟢 100 🟢 100 🟠 88 Report
/docs/category/getting-started 🟠 76 🟢 100 🟢 100 🟢 90 🟠 88 Report
/blog 🟠 69 🟢 100 🟢 100 🟢 90 🟠 88 Report
/blog/preparing-your-site-for-docusaurus-v3 🟠 63 🟢 96 🟢 100 🟢 100 🟠 88 Report
/blog/tags/release 🟠 68 🟢 100 🟢 100 🟠 80 🟠 88 Report
/blog/tags 🟠 75 🟢 100 🟢 100 🟢 90 🟠 88 Report

@github-actions

github-actions Bot commented May 3, 2024

Copy link
Copy Markdown

Size Change: +86 B (+0.01%)

Total Size: 1.59 MB

Filename Size Change
website/build/assets/js/main.********.js 810 kB +86 B (+0.01%)
ℹ️ View Unchanged
Filename Size
website/.docusaurus/codeTranslations.json 2 B
website/.docusaurus/docusaurus.config.mjs 26.7 kB
website/.docusaurus/globalData.json 91.2 kB
website/.docusaurus/i18n.json 930 B
website/.docusaurus/registry.js 247 kB
website/.docusaurus/routes.js 156 kB
website/.docusaurus/routesChunkNames.json 109 kB
website/.docusaurus/site-metadata.json 2.17 kB
website/build/assets/css/styles.********.css 112 kB
website/build/index.html 38 kB

compressed-size-action

@slorber slorber merged commit 2d8281f into main May 3, 2024
@slorber slorber deleted the slorber/fix-react18.3-hotreload-warning branch May 3, 2024 13:54
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: bug fix This PR fixes a bug in a past release.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants