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

refactor(core): replace useDocusaurusContext().isClient by useIsBrowser() #5349

Merged
merged 6 commits into from
Aug 12, 2021

Conversation

slorber
Copy link
Collaborator

@slorber slorber commented Aug 12, 2021

Breaking change:

  • Replace removed useDocusaurusContext().isClient by new useIsBrowser()

Motivation

Perf reasons: isClient is toggled after React hydration, leading to a duplicate re-rendering of all components using useDocusaurusContext(), even if they don't even use the isClient value.
By extracting isClient in a separate context, only components reading that value will render twice on hydration, and the rest will just render once => less work for the browser and better perfs.
More explanations on the problem space here: https://www.joshwcomeau.com/react/the-perils-of-rehydration/

Naming reasons: we have <BrowserOnly> (and not <ClientOnly>) so we'd rather keep a consistent naming for things that happen after React hydration

Also, remove some unexpected usage of isClient as key: we'll see if it produce unintended side-effects.

Have you read the Contributing Guidelines on pull requests?

yes

Test Plan

tests + preview

@slorber slorber added pr: breaking change Existing sites may not build successfully in the new version. Description contains more details. pr: performance This PR does not add a new behavior, but existing behaviors will be more memory- / time-efficient. labels Aug 12, 2021
@slorber slorber requested a review from lex111 as a code owner August 12, 2021 14:57
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Aug 12, 2021
@netlify
Copy link

netlify bot commented Aug 12, 2021

✔️ [V2]

🔨 Explore the source changes: 630f019

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/6115471f1c33c30008353efa

😎 Browse the preview: https://deploy-preview-5349--docusaurus-2.netlify.app

@github-actions
Copy link

github-actions bot commented Aug 12, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟢 Performance 93
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-5349--docusaurus-2.netlify.app/

@github-actions
Copy link

github-actions bot commented Aug 12, 2021

Size Change: +403 B (0%)

Total Size: 794 kB

Filename Size Change
website/build/assets/js/main.********.js 404 kB +401 B (0%)
ℹ️ View Unchanged
Filename Size Change
website/.docusaurus/globalData.json 36.2 kB 0 B
website/build/assets/css/styles.********.css 93.1 kB +2 B (0%)
website/build/blog/2017/12/14/introducing-docusaurus/index.html 64.2 kB 0 B
website/build/blog/index.html 33.1 kB 0 B
website/build/docs/index.html 41.9 kB 0 B
website/build/docs/installation/index.html 49.6 kB 0 B
website/build/index.html 28.7 kB 0 B
website/build/tests/docs/index.html 22.5 kB 0 B
website/build/tests/docs/standalone/index.html 20.5 kB 0 B

compressed-size-action

@slorber slorber changed the title refactor(core): make useDocusaurusContext() return constant + extract new useIsClient() hook refactor(core): useDocusaurusContext() returns constant + add useIsBrowser() hook Aug 12, 2021
@slorber slorber changed the title refactor(core): useDocusaurusContext() returns constant + add useIsBrowser() hook refactor(core): replace useDocusaurusContext().isClient by useIsBrowser() hook Aug 12, 2021
@slorber slorber changed the title refactor(core): replace useDocusaurusContext().isClient by useIsBrowser() hook refactor(core): replace useDocusaurusContext().isClient by useIsBrowser() Aug 12, 2021
Copy link
Contributor

@lex111 lex111 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 awesome!

@slorber
Copy link
Collaborator Author

slorber commented Aug 12, 2021

thanks 😄

Comparing before/after on lighthouse for a very large page seems to have a significant impact on blocking time / TTI:

Before: https://deploy-preview-5345--docusaurus-2.netlify.app/community/changelog/

image

After: https://deploy-preview-5349--docusaurus-2.netlify.app/community/changelog/

image

Going to merge this as I didn't see any weird key related side effect, but let's keep our eyes open 😄

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: breaking change Existing sites may not build successfully in the new version. Description contains more details. pr: performance This PR does not add a new behavior, but existing behaviors will be more memory- / time-efficient.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants