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

Revert to client render on text mismatch #23354

Merged
merged 2 commits into from
Feb 24, 2022

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Feb 24, 2022

Expands the behavior of enableClientRenderFallbackOnHydrationMismatch to check text content, too.

If the text is different from what was rendered on the server, we will recover the UI by falling back to client rendering, up to the nearest Suspense boundary.

The places where we revert to client rendering match the places where we used to only warn in development. That means we respect the suppressHydrationWarning prop — so the name is a bit weird now, since the behavior now affects production, too. We may rename the prop in a future release.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 24, 2022
@sizebot
Copy link

sizebot commented Feb 24, 2022

Comparing: 1ad8d81...e55142d

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.30% 130.87 kB 131.26 kB +0.30% 41.86 kB 41.99 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js +0.29% 135.72 kB 136.12 kB +0.35% 43.27 kB 43.42 kB
facebook-www/ReactDOM-prod.classic.js +0.72% 431.18 kB 434.31 kB +0.54% 78.99 kB 79.42 kB
facebook-www/ReactDOM-prod.modern.js +0.74% 420.97 kB 424.09 kB +0.56% 77.54 kB 77.97 kB
facebook-www/ReactDOMForked-prod.classic.js +0.72% 431.18 kB 434.31 kB +0.54% 79.00 kB 79.42 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
facebook-www/ReactDOMTesting-prod.modern.js +0.75% 409.28 kB 412.35 kB +0.56% 77.18 kB 77.62 kB
facebook-www/ReactDOM-prod.modern.js +0.74% 420.97 kB 424.09 kB +0.56% 77.54 kB 77.97 kB
facebook-www/ReactDOMForked-prod.modern.js +0.74% 420.97 kB 424.09 kB +0.56% 77.55 kB 77.98 kB
facebook-www/ReactDOM-profiling.modern.js +0.74% 451.41 kB 454.75 kB +0.56% 82.09 kB 82.55 kB
facebook-www/ReactDOMForked-profiling.modern.js +0.74% 451.41 kB 454.75 kB +0.56% 82.10 kB 82.56 kB
facebook-www/ReactDOMTesting-prod.classic.js +0.73% 422.86 kB 425.93 kB +0.48% 79.30 kB 79.68 kB
facebook-www/ReactDOM-prod.classic.js +0.72% 431.18 kB 434.31 kB +0.54% 78.99 kB 79.42 kB
facebook-www/ReactDOMForked-prod.classic.js +0.72% 431.18 kB 434.31 kB +0.54% 79.00 kB 79.42 kB
facebook-www/ReactDOM-profiling.classic.js +0.72% 461.68 kB 465.03 kB +0.49% 83.63 kB 84.04 kB
facebook-www/ReactDOMForked-profiling.classic.js +0.72% 461.68 kB 465.03 kB +0.49% 83.63 kB 84.04 kB
oss-stable-semver/react-dom/cjs/react-dom.production.min.js +0.30% 130.87 kB 131.26 kB +0.30% 41.86 kB 41.99 kB
oss-stable/react-dom/cjs/react-dom.production.min.js +0.30% 130.87 kB 131.26 kB +0.30% 41.86 kB 41.99 kB
oss-stable-semver/react-dom/umd/react-dom.production.min.js +0.30% 130.97 kB 131.36 kB +0.27% 42.60 kB 42.71 kB
oss-stable/react-dom/umd/react-dom.production.min.js +0.30% 130.97 kB 131.36 kB +0.27% 42.60 kB 42.71 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler.production.min.js +0.29% 92.89 kB 93.16 kB +0.22% 28.53 kB 28.59 kB
oss-stable/react-reconciler/cjs/react-reconciler.production.min.js +0.29% 92.89 kB 93.16 kB +0.22% 28.53 kB 28.59 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js +0.29% 135.72 kB 136.12 kB +0.35% 43.27 kB 43.42 kB
oss-experimental/react-dom/cjs/react-dom-unstable_testing.production.min.js +0.29% 135.91 kB 136.30 kB +0.29% 43.81 kB 43.94 kB
oss-experimental/react-dom/umd/react-dom.production.min.js +0.29% 135.75 kB 136.14 kB +0.30% 43.96 kB 44.09 kB
oss-stable-semver/react-dom/cjs/react-dom.profiling.min.js +0.28% 140.31 kB 140.70 kB +0.29% 44.39 kB 44.52 kB
oss-stable/react-dom/cjs/react-dom.profiling.min.js +0.28% 140.31 kB 140.70 kB +0.29% 44.39 kB 44.52 kB
oss-experimental/react-reconciler/cjs/react-reconciler.production.min.js +0.28% 97.15 kB 97.42 kB +0.28% 29.66 kB 29.75 kB
oss-stable-semver/react-dom/umd/react-dom.profiling.min.js +0.28% 139.72 kB 140.11 kB +0.29% 44.85 kB 44.98 kB
oss-stable/react-dom/umd/react-dom.profiling.min.js +0.28% 139.72 kB 140.11 kB +0.29% 44.85 kB 44.98 kB
oss-experimental/react-dom/cjs/react-dom.profiling.min.js +0.27% 145.16 kB 145.56 kB +0.31% 45.80 kB 45.94 kB
oss-experimental/react-dom/umd/react-dom.profiling.min.js +0.27% 144.48 kB 144.87 kB +0.26% 46.23 kB 46.35 kB
oss-stable-semver/react-reconciler/cjs/react-reconciler.profiling.min.js +0.27% 101.79 kB 102.06 kB +0.19% 30.73 kB 30.78 kB
oss-stable/react-reconciler/cjs/react-reconciler.profiling.min.js +0.27% 101.79 kB 102.06 kB +0.19% 30.73 kB 30.78 kB
oss-experimental/react-reconciler/cjs/react-reconciler.profiling.min.js +0.26% 106.04 kB 106.32 kB +0.28% 31.96 kB 32.06 kB

Generated by 🚫 dangerJS against e55142d

We're going to fork the behavior of this function between concurrent
roots and legacy roots.

The legacy behavior is to warn in dev when the text mismatches during
hydration. In concurrent roots, we'll log a recoverable error and revert
to client rendering. That means this is no longer a development-only
function — it affects the prod behavior, too.

I haven't changed any behavior in this commit. I only rearranged the
code slightly so that the dev environment check is inside the body
instead of around the function call. I also threaded through an
isConcurrentMode argument.
@acdlite acdlite force-pushed the revert-to-client-on-text-mismatch branch 2 times, most recently from c396018 to 721ccf6 Compare February 24, 2022 04:56
Expands the behavior of enableClientRenderFallbackOnHydrationMismatch to
check text content, too.

If the text is different from what was rendered on the server, we will
recover the UI by falling back to client rendering, up to the nearest
Suspense boundary.
@acdlite acdlite force-pushed the revert-to-client-on-text-mismatch branch from 721ccf6 to e55142d Compare February 24, 2022 05:04
@acdlite acdlite marked this pull request as ready for review February 24, 2022 05:09
@acdlite acdlite requested a review from sebmarkbage February 24, 2022 05:09
clientText: string | number,
isConcurrentMode: boolean,
) {
const normalizedClientText = normalizeMarkupForTextOrAttribute(clientText);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is not prod behavior it’s a bit more perf sensitive. We can first check if the original string is equal first, which it in most cases will be and if so bail early. Only if that is not true should we normalize.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh nvm. We only get here if they’re not already equal.

@@ -231,12 +231,19 @@ export function checkForUnmatchedText(
clientText: string | number,
isConcurrentMode: boolean,
) {
if (clientText === serverText) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don’t need this because we only call this function if there was a mismatch.

@acdlite acdlite force-pushed the revert-to-client-on-text-mismatch branch from acae21f to e55142d Compare February 24, 2022 05:22
@acdlite acdlite merged commit 52c393b into facebook:main Feb 24, 2022
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Mar 1, 2022
Summary:
This sync includes the following changes:
- **[17806594c](facebook/react@17806594c )**: Move createRoot/hydrateRoot to react-dom/client ([#23385](facebook/react#23385)) //<Sebastian Markbåge>//
- **[75662d6a7](facebook/react@75662d6a7 )**: Remove hacky stream.locked check, declare as byte stream instead ([#23387](facebook/react#23387)) //<Sebastian Markbåge>//
- **[a82ef6d40](facebook/react@a82ef6d40 )**: Add back skipUnmountedBoundaries flag only for www ([#23383](facebook/react#23383)) //<Andrew Clark>//
- **[f468816ef](facebook/react@f468816ef )**: Fix false positive hydration warnings ([#23364](facebook/react#23364)) //<Andrew Clark>//
- **[5d08a24c2](facebook/react@5d08a24c2 )**: useId: Use 'H' to separate main id from hook index ([#23363](facebook/react#23363)) //<Andrew Clark>//
- **[3a60844a0](facebook/react@3a60844a0 )**: Update error message for suspending at sync priority ([#23361](facebook/react#23361)) //<Andrew Clark>//
- **[efe4121ee](facebook/react@efe4121ee )**: Add : to beginning and end of every useId ([#23360](facebook/react#23360)) //<Andrew Clark>//
- **[42f15b324](facebook/react@42f15b324 )**: [DevTools][Transition Tracing] onTransitionComplete and onTransitionStart implmentation ([#23313](facebook/react#23313)) //<Luna Ruan>//
- **[a5b22155c](facebook/react@a5b22155c )**: Warn if renderSubtreeIntoContainer is called ([#23355](facebook/react#23355)) //<Andrew Clark>//
- **[52c393b5d](facebook/react@52c393b5d )**: Revert to client render on text mismatch ([#23354](facebook/react#23354)) //<Andrew Clark>//
- **[1ad8d8129](facebook/react@1ad8d8129 )**: Remove object-assign polyfill ([#23351](facebook/react#23351)) //<Sebastian Markbåge>//
- **[b3f3da205](facebook/react@b3f3da205 )**: Land warnOnSubscriptionInsideStartTransition flag ([#23353](facebook/react#23353)) //<Andrew Clark>//
- **[990098f88](facebook/react@990098f88 )**: Re-arrange main ReactFeatureFlags module ([#23350](facebook/react#23350)) //<Andrew Clark>//
- **[1f3f6db73](facebook/react@1f3f6db73 )**: Remove createMutableSource from stable exports ([#23352](facebook/react#23352)) //<Andrew Clark>//
- **[587e75930](facebook/react@587e75930 )**: Remove Numeric Fallback of Symbols ([#23348](facebook/react#23348)) //<Sebastian Markbåge>//
- **[40351575d](facebook/react@40351575d )**: Split writeChunk into void and return value ([#23343](facebook/react#23343)) //<Sebastian Markbåge>//
- **[2c693b2de](facebook/react@2c693b2de )**: Re-add reentrancy avoidance ([#23342](facebook/react#23342)) //<Sebastian Markbåge>//
- **[1760b27c0](facebook/react@1760b27c0 )**: Remove ./src/* export from public build ([#23262](facebook/react#23262)) //<Andrew Clark>//
- **[552c067bb](facebook/react@552c067bb )**: Remove public export for unstable-shared-subset.js ([#23261](facebook/react#23261)) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions 4de99b3...1780659

jest_e2e[run_all_tests]

Reviewed By: rickhanlonii

Differential Revision: D34552175

fbshipit-source-id: f1c831a45f96d335a20c3b4113196e0a42cefc03
@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 warnForTextDifference

We're going to fork the behavior of this function between concurrent
roots and legacy roots.

The legacy behavior is to warn in dev when the text mismatches during
hydration. In concurrent roots, we'll log a recoverable error and revert
to client rendering. That means this is no longer a development-only
function — it affects the prod behavior, too.

I haven't changed any behavior in this commit. I only rearranged the
code slightly so that the dev environment check is inside the body
instead of around the function call. I also threaded through an
isConcurrentMode argument.

* Revert to client render on text content mismatch

Expands the behavior of enableClientRenderFallbackOnHydrationMismatch to
check text content, too.

If the text is different from what was rendered on the server, we will
recover the UI by falling back to client rendering, up to the nearest
Suspense boundary.
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this pull request Jan 15, 2023
Summary:
This sync includes the following changes:
- **[17806594c](facebook/react@17806594c )**: Move createRoot/hydrateRoot to react-dom/client ([facebook#23385](facebook/react#23385)) //<Sebastian Markbåge>//
- **[75662d6a7](facebook/react@75662d6a7 )**: Remove hacky stream.locked check, declare as byte stream instead ([facebook#23387](facebook/react#23387)) //<Sebastian Markbåge>//
- **[a82ef6d40](facebook/react@a82ef6d40 )**: Add back skipUnmountedBoundaries flag only for www ([facebook#23383](facebook/react#23383)) //<Andrew Clark>//
- **[f468816ef](facebook/react@f468816ef )**: Fix false positive hydration warnings ([facebook#23364](facebook/react#23364)) //<Andrew Clark>//
- **[5d08a24c2](facebook/react@5d08a24c2 )**: useId: Use 'H' to separate main id from hook index ([facebook#23363](facebook/react#23363)) //<Andrew Clark>//
- **[3a60844a0](facebook/react@3a60844a0 )**: Update error message for suspending at sync priority ([facebook#23361](facebook/react#23361)) //<Andrew Clark>//
- **[efe4121ee](facebook/react@efe4121ee )**: Add : to beginning and end of every useId ([facebook#23360](facebook/react#23360)) //<Andrew Clark>//
- **[42f15b324](facebook/react@42f15b324 )**: [DevTools][Transition Tracing] onTransitionComplete and onTransitionStart implmentation ([facebook#23313](facebook/react#23313)) //<Luna Ruan>//
- **[a5b22155c](facebook/react@a5b22155c )**: Warn if renderSubtreeIntoContainer is called ([facebook#23355](facebook/react#23355)) //<Andrew Clark>//
- **[52c393b5d](facebook/react@52c393b5d )**: Revert to client render on text mismatch ([facebook#23354](facebook/react#23354)) //<Andrew Clark>//
- **[1ad8d8129](facebook/react@1ad8d8129 )**: Remove object-assign polyfill ([facebook#23351](facebook/react#23351)) //<Sebastian Markbåge>//
- **[b3f3da205](facebook/react@b3f3da205 )**: Land warnOnSubscriptionInsideStartTransition flag ([facebook#23353](facebook/react#23353)) //<Andrew Clark>//
- **[990098f88](facebook/react@990098f88 )**: Re-arrange main ReactFeatureFlags module ([facebook#23350](facebook/react#23350)) //<Andrew Clark>//
- **[1f3f6db73](facebook/react@1f3f6db73 )**: Remove createMutableSource from stable exports ([facebook#23352](facebook/react#23352)) //<Andrew Clark>//
- **[587e75930](facebook/react@587e75930 )**: Remove Numeric Fallback of Symbols ([facebook#23348](facebook/react#23348)) //<Sebastian Markbåge>//
- **[40351575d](facebook/react@40351575d )**: Split writeChunk into void and return value ([facebook#23343](facebook/react#23343)) //<Sebastian Markbåge>//
- **[2c693b2de](facebook/react@2c693b2de )**: Re-add reentrancy avoidance ([facebook#23342](facebook/react#23342)) //<Sebastian Markbåge>//
- **[1760b27c0](facebook/react@1760b27c0 )**: Remove ./src/* export from public build ([facebook#23262](facebook/react#23262)) //<Andrew Clark>//
- **[552c067bb](facebook/react@552c067bb )**: Remove public export for unstable-shared-subset.js ([facebook#23261](facebook/react#23261)) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions 4de99b3...1780659

jest_e2e[run_all_tests]

Reviewed By: rickhanlonii

Differential Revision: D34552175

fbshipit-source-id: f1c831a45f96d335a20c3b4113196e0a42cefc03
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