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

Hydration errors should force a client render #22416

Merged
merged 2 commits into from
Sep 24, 2021

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Sep 24, 2021

If something errors during hydration, we should try rendering again without hydrating.

We'll find the nearest Suspense boundary and force it to client render, discarding the server-rendered content.

I suggest reviewing commit-by-commit because the first step refactors the control flow in throwException. The actual change comes after that.

@facebook-github-bot facebook-github-bot added the React Core Team Opened by a member of the React Core Team label Sep 24, 2021
@sizebot
Copy link

sizebot commented Sep 24, 2021

Comparing: 1c73cee...01af049

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js +0.17% 129.54 kB 129.76 kB +0.21% 41.29 kB 41.38 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js +0.16% 132.37 kB 132.58 kB +0.25% 42.23 kB 42.34 kB
facebook-www/ReactDOM-prod.classic.js +0.12% 411.19 kB 411.67 kB +0.20% 76.09 kB 76.24 kB
facebook-www/ReactDOM-prod.modern.js +0.12% 399.75 kB 400.24 kB +0.20% 74.38 kB 74.53 kB
facebook-www/ReactDOMForked-prod.classic.js +0.12% 411.19 kB 411.67 kB +0.20% 76.09 kB 76.24 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable-semver/react-reconciler/cjs/react-reconciler.development.js +0.31% 714.37 kB 716.59 kB +0.40% 151.34 kB 151.94 kB
oss-stable/react-reconciler/cjs/react-reconciler.development.js +0.31% 714.37 kB 716.59 kB +0.40% 151.34 kB 151.94 kB
oss-experimental/react-reconciler/cjs/react-reconciler.development.js +0.30% 730.45 kB 732.66 kB +0.40% 154.77 kB 155.38 kB
facebook-www/ReactDOMTesting-dev.modern.js +0.25% 956.72 kB 959.13 kB +0.31% 214.91 kB 215.58 kB
facebook-www/ReactART-prod.modern.js +0.25% 260.12 kB 260.77 kB +0.35% 46.52 kB 46.69 kB
facebook-www/ReactDOMTesting-dev.classic.js +0.24% 983.79 kB 986.20 kB +0.31% 220.53 kB 221.21 kB
facebook-www/ReactART-prod.classic.js +0.24% 267.79 kB 268.43 kB +0.33% 47.78 kB 47.94 kB
facebook-www/ReactDOMForked-dev.modern.js +0.23% 1,054.03 kB 1,056.48 kB +0.28% 233.37 kB 234.01 kB
facebook-www/ReactDOM-dev.modern.js +0.23% 1,054.03 kB 1,056.48 kB +0.28% 233.37 kB 234.01 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler.production.min.js +0.23% 91.53 kB 91.74 kB +0.03% 28.10 kB 28.10 kB
oss-stable/react-reconciler/cjs/react-reconciler.production.min.js +0.23% 91.53 kB 91.74 kB +0.03% 28.10 kB 28.10 kB
oss-stable-semver/react-dom/cjs/react-dom.development.js +0.23% 980.68 kB 982.93 kB +0.28% 220.09 kB 220.71 kB
oss-stable/react-dom/cjs/react-dom.development.js +0.23% 980.68 kB 982.93 kB +0.28% 220.09 kB 220.71 kB
oss-stable-semver/react-dom/umd/react-dom.development.js +0.23% 1,030.34 kB 1,032.70 kB +0.28% 222.70 kB 223.32 kB
oss-stable/react-dom/umd/react-dom.development.js +0.23% 1,030.34 kB 1,032.70 kB +0.28% 222.70 kB 223.32 kB
facebook-www/ReactART-dev.modern.js +0.23% 717.02 kB 718.65 kB +0.29% 152.54 kB 152.98 kB
facebook-www/ReactDOMForked-dev.classic.js +0.23% 1,078.48 kB 1,080.92 kB +0.27% 238.41 kB 239.06 kB
facebook-www/ReactDOM-dev.classic.js +0.23% 1,078.48 kB 1,080.93 kB +0.27% 238.41 kB 239.06 kB
oss-experimental/react-dom/cjs/react-dom.development.js +0.23% 998.52 kB 1,000.77 kB +0.27% 223.80 kB 224.41 kB
oss-experimental/react-dom/umd/react-dom.development.js +0.23% 1,049.14 kB 1,051.50 kB +0.27% 226.41 kB 227.03 kB
facebook-www/ReactART-dev.classic.js +0.22% 727.30 kB 728.93 kB +0.30% 154.64 kB 155.10 kB
oss-experimental/react-reconciler/cjs/react-reconciler.production.min.js +0.22% 93.99 kB 94.20 kB +0.24% 28.86 kB 28.93 kB
facebook-www/ReactDOM-profiling.modern.js +0.22% 426.88 kB 427.83 kB +0.23% 79.05 kB 79.23 kB
facebook-www/ReactDOMForked-profiling.modern.js +0.22% 426.88 kB 427.83 kB +0.23% 79.05 kB 79.23 kB
oss-stable-semver/react-art/cjs/react-art.development.js +0.22% 662.51 kB 663.98 kB +0.30% 143.03 kB 143.45 kB
oss-stable/react-art/cjs/react-art.development.js +0.22% 662.51 kB 663.98 kB +0.30% 143.03 kB 143.45 kB
facebook-www/ReactDOM-profiling.classic.js +0.22% 438.37 kB 439.32 kB +0.19% 80.76 kB 80.91 kB
facebook-www/ReactDOMForked-profiling.classic.js +0.22% 438.37 kB 439.32 kB +0.19% 80.76 kB 80.92 kB
oss-experimental/react-art/cjs/react-art.development.js +0.22% 678.39 kB 679.86 kB +0.29% 146.42 kB 146.84 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler.profiling.min.js +0.21% 99.58 kB 99.79 kB +0.37% 30.45 kB 30.56 kB
oss-stable/react-reconciler/cjs/react-reconciler.profiling.min.js +0.21% 99.58 kB 99.79 kB +0.37% 30.45 kB 30.56 kB
oss-experimental/react-reconciler/cjs/react-reconciler.profiling.min.js +0.21% 102.05 kB 102.26 kB +0.29% 31.22 kB 31.31 kB
oss-stable-semver/react-art/umd/react-art.development.js +0.20% 766.32 kB 767.88 kB +0.27% 161.35 kB 161.78 kB
oss-stable/react-art/umd/react-art.development.js +0.20% 766.32 kB 767.88 kB +0.27% 161.35 kB 161.78 kB

Generated by 🚫 dangerJS against 01af049

@acdlite acdlite force-pushed the hydration-error-force-client branch from 1899a9a to 0c212f5 Compare September 24, 2021 14:02
@acdlite acdlite force-pushed the hydration-error-force-client branch 2 times, most recently from 2a4b09f to c998cfe Compare September 24, 2021 14:36
Copy link
Contributor

@salazarm salazarm left a comment

Choose a reason for hiding this comment

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

Read this pr 4 times now, things are starting to click.
The tests make sense and all tests pass so go forth :)

//
// If we hit a "Delayed" case, such as when we'd switch from content back into
// a fallback, then we should always suspend/restart. Transitions apply
// to this case. If none is defined, JND is used instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

whats JND?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It stands for "Just Noticeable Difference". We mostly don't use it anymore, but the idea is that if we're missing some data, and we've already waited n milliseconds, the user won't notice if we wait just a bit longer before showing a fallback.

I'm about to add more branches to the Suspense-related logic in
`throwException`, so before I do, I split some of the steps into
separate functions so that later I can use them in multiple places.

This commit does not change any program behavior, only the control flow
surrounding existing code.
@acdlite acdlite force-pushed the hydration-error-force-client branch from c998cfe to 6a33c0d Compare September 24, 2021 17:07
If something errors during hydration, we should try rendering again
without hydrating.

We'll find the nearest Suspense boundary and force it to client render,
discarding the server-rendered content.
@acdlite acdlite merged commit 04ccc01 into facebook:main Sep 24, 2021
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Oct 12, 2021
Summary:
This sync includes the following changes:
- **[579c008a7](facebook/react@579c008a7 )**: [Fizz/Flight] pipeToNodeWritable(..., writable).startWriting() -> renderToPipeableStream(...).pipe(writable) ([#22450](facebook/react#22450)) //<Sebastian Markbåge>//
- **[f2c381131](facebook/react@f2c381131 )**: fix: useSyncExternalStoreExtra ([#22500](facebook/react#22500)) //<Daishi Kato>//
- **[0ecbbe142](facebook/react@0ecbbe142 )**: Sync hydrate discrete events in capture phase and dont replay discrete events ([#22448](facebook/react#22448)) //<salazarm>//
- **[a724a3b57](facebook/react@a724a3b57 )**: [RFC] Codemod invariant -> throw new Error ([#22435](facebook/react#22435)) //<Andrew Clark>//
- **[201af81b0](facebook/react@201af81b0 )**: Release pooled cache reference in complete/unwind ([#22464](facebook/react#22464)) //<Joseph Savona>//
- **[033efe731](facebook/react@033efe731 )**: Call get snapshot in useSyncExternalStore server shim ([#22453](facebook/react#22453)) //<salazarm>//
- **[7843b142a](facebook/react@7843b142a )**: [Fizz/Flight] Pass in Destination lazily to startFlowing instead of in createRequest ([#22449](facebook/react#22449)) //<Sebastian Markbåge>//
- **[d9fb383d6](facebook/react@d9fb383d6 )**: Extract queueing logic into shared functions ([#22452](facebook/react#22452)) //<Andrew Clark>//
- **[9175f4d15](facebook/react@9175f4d15 )**: Scheduling Profiler: Show Suspense resource .displayName ([#22451](facebook/react#22451)) //<Brian Vaughn>//
- **[eba248c39](facebook/react@eba248c39 )**: [Fizz/Flight] Remove reentrancy hack ([#22446](facebook/react#22446)) //<Sebastian Markbåge>//
- **[66388150e](facebook/react@66388150e )**: Remove usereducer eager bailout ([#22445](facebook/react#22445)) //<Joseph Savona>//
- **[d3e086932](facebook/react@d3e086932 )**: Make root.unmount() synchronous  ([#22444](facebook/react#22444)) //<Andrew Clark>//
- **[2cc6d79c9](facebook/react@2cc6d79c9 )**: Rename onReadyToStream to onCompleteShell ([#22443](facebook/react#22443)) //<Sebastian Markbåge>//
- **[c88fb49d3](facebook/react@c88fb49d3 )**: Improve DEV errors if string coercion throws (Temporal.*, Symbol, etc.) ([#22064](facebook/react#22064)) //<Justin Grant>//
- **[05726d72c](facebook/react@05726d72c )**: [Fix] Errors should not "unsuspend" a transition ([#22423](facebook/react#22423)) //<Andrew Clark>//
- **[3746eaf98](facebook/react@3746eaf98 )**: Packages/React/src/ReactLazy ---> changing -1 to unintialized ([#22421](facebook/react#22421)) //<BIKI DAS>//
- **[04ccc01d9](facebook/react@04ccc01d9 )**: Hydration errors should force a client render ([#22416](facebook/react#22416)) //<Andrew Clark>//
- **[029fdcebb](facebook/react@029fdcebb )**: root.hydrate -> root.isDehydrated ([#22420](facebook/react#22420)) //<Andrew Clark>//
- **[af87f5a83](facebook/react@af87f5a83 )**: Scheduling Profiler marks should include thrown Errors ([#22417](facebook/react#22417)) //<Brian Vaughn>//
- **[d47339ea3](facebook/react@d47339ea3 )**: [Fizz] Remove assignID mechanism ([#22410](facebook/react#22410)) //<Sebastian Markbåge>//
- **[3a50d9557](facebook/react@3a50d9557 )**: Never attach ping listeners in legacy Suspense ([#22407](facebook/react#22407)) //<Andrew Clark>//
- **[82c8fa90b](facebook/react@82c8fa90b )**: Add back useMutableSource temporarily ([#22396](facebook/react#22396)) //<Andrew Clark>//
- **[5b57bc6e3](facebook/react@5b57bc6e3 )**: [Draft] don't patch console during first render ([#22308](facebook/react#22308)) //<Luna Ruan>//
- **[cf07c3df1](facebook/react@cf07c3df1 )**: Delete all but one `build2` reference ([#22391](facebook/react#22391)) //<Andrew Clark>//
- **[bb0d06935](facebook/react@bb0d06935 )**: [build2 -> build] Local scripts //<Andrew Clark>//
- **[0c81d347b](facebook/react@0c81d347b )**: Write artifacts to `build` instead of `build2` //<Andrew Clark>//
- **[4da03c9fb](facebook/react@4da03c9fb )**: useSyncExternalStore React Native version ([#22367](facebook/react#22367)) //<salazarm>//
- **[48d475c9e](facebook/react@48d475c9e )**: correct typos ([#22294](facebook/react#22294)) //<Bowen>//
- **[cb6c619c0](facebook/react@cb6c619c0 )**: Remove Fiber fields that were used for hydrating useMutableSource ([#22368](facebook/react#22368)) //<Sebastian Silbermann>//
- **[64e70f82e](facebook/react@64e70f82e )**: [Fizz] add avoidThisFallback support ([#22318](facebook/react#22318)) //<salazarm>//
- **[3ee7a004e](facebook/react@3ee7a004e )**: devtools: Display actual ReactDOM API name in root type ([#22363](facebook/react#22363)) //<Sebastian Silbermann>//
- **[79b8fc667](facebook/react@79b8fc667 )**: Implement getServerSnapshot in userspace shim ([#22359](facebook/react#22359)) //<Andrew Clark>//
- **[86b3e2461](facebook/react@86b3e2461 )**: Implement useSyncExternalStore on server ([#22347](facebook/react#22347)) //<Andrew Clark>//
- **[8209de269](facebook/react@8209de269 )**: Delete useMutableSource implementation ([#22292](facebook/react#22292)) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions e8feb11...afcb9cd

jest_e2e[run_all_tests]

Reviewed By: yungsters

Differential Revision: D31541359

fbshipit-source-id: c35941bc303fdf55cb061e9996200dc868a6f2af
@gaearon gaearon mentioned this pull request Mar 29, 2022
zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022
* Refactor throwException control flow

I'm about to add more branches to the Suspense-related logic in
`throwException`, so before I do, I split some of the steps into
separate functions so that later I can use them in multiple places.

This commit does not change any program behavior, only the control flow
surrounding existing code.

* Hydration errors should force a client render

If something errors during hydration, we should try rendering again
without hydrating.

We'll find the nearest Suspense boundary and force it to client render,
discarding the server-rendered content.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants