Don't mute hydration errors forcing client render #24276
Merged
+99
−33
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
React Core Team
gaearon
commented
Apr 5, 2022
| @@ -248,9 +248,10 @@ describe('ReactDOMFizzServerHydrationWarning', () => { | |||
| ]); | |||
| }).toErrorDev( | |||
| [ | |||
| 'Expected server HTML to contain a matching <span> in <span>', | |||
There was a problem hiding this comment.
These were being covered up, making mismatches with prod behavior hard to debug and fix.
|
Comparing: 5f7f528...380dc5b Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
sebmarkbage
reviewed
Apr 5, 2022
| @@ -248,9 +248,10 @@ describe('ReactDOMFizzServerHydrationWarning', () => { | |||
| ]); | |||
| }).toErrorDev( | |||
| [ | |||
| 'Expected server HTML to contain a matching <span> in <span>', | |||
| 'An error occurred during hydration. The server HTML was replaced with client content in <div>.', | |||
There was a problem hiding this comment.
You still have these though. Isn't it better to have the context here for the ones we check in prod anyway? Is it worth having both? Is it confusing that there are two with subtly different timing?
Maybe it would make more sense to have the DEV only error if it was part of a larger diff across multiple mismatches which also included attributes.
There was a problem hiding this comment.
In other words, should we just move the insert/deletion error messages to the onRecoverableError? Perhaps batching them up if there are multiple. Oops, now you just got nerd sniped into making a larger diff view.
There was a problem hiding this comment.
I have another PR in the works for the diff. It's not coalesced though.
There was a problem hiding this comment.
But in either case I think we should fix up the inconsistency first. I.e. your concerns don't seem related to this PR per se, it just aligns the behavior for suppressHydration ones with the rest.
sebmarkbage
reviewed
Apr 5, 2022
packages/react-reconciler/src/ReactFiberHydrationContext.old.js
Outdated
Show resolved
Hide resolved
sebmarkbage
reviewed
Apr 5, 2022
sebmarkbage
approved these changes
Apr 5, 2022
facebook-github-bot
pushed a commit
to facebook/react-native
that referenced
this pull request
Apr 12, 2022
Summary: This sync includes the following changes: - **[e8f4a6653](facebook/react@e8f4a6653 )**: Fix import in example //<dan>// - **[bb49abea2](facebook/react@bb49abea2 )**: Update some READMEs ([#24290](facebook/react#24290)) //<dan>// - **[4bc465a16](facebook/react@4bc465a16 )**: Rename Controls to PipeableStream ([#24286](facebook/react#24286)) //<Sebastian Markbåge>// - **[ece5295e5](facebook/react@ece5295e5 )**: Remove unnecessary flag check ([#24284](facebook/react#24284)) //<zhoulixiang>// - **[9ededef94](facebook/react@9ededef94 )**: Don't mute hydration errors forcing client render ([#24276](facebook/react#24276)) //<dan>// - **[985272e26](facebook/react@985272e26 )**: Fix name mismatch in react-reconciler custom build. ([#24272](facebook/react#24272)) //<Hikari Hayashi>// - **[b8cfda15e](facebook/react@b8cfda15e )**: changed Transitions type to Array<Transition> ([#24249](facebook/react#24249)) //<Luna Ruan>// - **[c89a15c71](facebook/react@c89a15c71 )**: [ReactDebugTools] wrap uncaught error from rendering user's component ([#24216](facebook/react#24216)) //<Mengdi "Monday" Chen>// - **[ebd7ff65b](facebook/react@ebd7ff65b )**: Don't recreate the same fallback on the client if hydrating suspends ([#24236](facebook/react#24236)) //<dan>// - **[aa05e7315](facebook/react@aa05e7315 )**: Add 4.4.0 release to eslint rules CHANGELOG ([#24234](facebook/react#24234)) //<Brian Vaughn>// - **[7e3121e1c](facebook/react@7e3121e1c )**: Remove unstable_createMutableSource from experimental build ([#24209](facebook/react#24209)) //<Sebastian Silbermann>// - **[0415b18a1](facebook/react@0415b18a1 )**: [ReactDebugTools] add custom error type for future new hooks ([#24168](facebook/react#24168)) //<Mengdi "Monday" Chen>// Changelog: [General][Changed] - React Native sync for revisions 34aa5cf...e8f4a66 jest_e2e[run_all_tests] Reviewed By: kacieb Differential Revision: D35581059 fbshipit-source-id: ee031dfc49b1ef663b601f33f3f3f6c5a804971e
rickhanlonii
pushed a commit
that referenced
this pull request
Apr 13, 2022
* Don't mute hydration errors forcing client render * Nits
rickhanlonii
pushed a commit
that referenced
this pull request
Apr 14, 2022
* Don't mute hydration errors forcing client render * Nits
zhengjitf
pushed a commit
to zhengjitf/react
that referenced
this pull request
Apr 15, 2022
* Don't mute hydration errors forcing client render * Nits
Saadnajmi
pushed a commit
to Saadnajmi/react-native-macos
that referenced
this pull request
Jan 15, 2023
Summary: This sync includes the following changes: - **[e8f4a6653](facebook/react@e8f4a6653 )**: Fix import in example //<dan>// - **[bb49abea2](facebook/react@bb49abea2 )**: Update some READMEs ([facebook#24290](facebook/react#24290)) //<dan>// - **[4bc465a16](facebook/react@4bc465a16 )**: Rename Controls to PipeableStream ([facebook#24286](facebook/react#24286)) //<Sebastian Markbåge>// - **[ece5295e5](facebook/react@ece5295e5 )**: Remove unnecessary flag check ([facebook#24284](facebook/react#24284)) //<zhoulixiang>// - **[9ededef94](facebook/react@9ededef94 )**: Don't mute hydration errors forcing client render ([facebook#24276](facebook/react#24276)) //<dan>// - **[985272e26](facebook/react@985272e26 )**: Fix name mismatch in react-reconciler custom build. ([facebook#24272](facebook/react#24272)) //<Hikari Hayashi>// - **[b8cfda15e](facebook/react@b8cfda15e )**: changed Transitions type to Array<Transition> ([facebook#24249](facebook/react#24249)) //<Luna Ruan>// - **[c89a15c71](facebook/react@c89a15c71 )**: [ReactDebugTools] wrap uncaught error from rendering user's component ([facebook#24216](facebook/react#24216)) //<Mengdi "Monday" Chen>// - **[ebd7ff65b](facebook/react@ebd7ff65b )**: Don't recreate the same fallback on the client if hydrating suspends ([facebook#24236](facebook/react#24236)) //<dan>// - **[aa05e7315](facebook/react@aa05e7315 )**: Add 4.4.0 release to eslint rules CHANGELOG ([facebook#24234](facebook/react#24234)) //<Brian Vaughn>// - **[7e3121e1c](facebook/react@7e3121e1c )**: Remove unstable_createMutableSource from experimental build ([facebook#24209](facebook/react#24209)) //<Sebastian Silbermann>// - **[0415b18a1](facebook/react@0415b18a1 )**: [ReactDebugTools] add custom error type for future new hooks ([facebook#24168](facebook/react#24168)) //<Mengdi "Monday" Chen>// Changelog: [General][Changed] - React Native sync for revisions 34aa5cf...e8f4a66 jest_e2e[run_all_tests] Reviewed By: kacieb Differential Revision: D35581059 fbshipit-source-id: ee031dfc49b1ef663b601f33f3f3f6c5a804971e
This was referenced Mar 4, 2023
rickhanlonii
added a commit
that referenced
this pull request
Feb 1, 2024
gaearon
pushed a commit
that referenced
this pull request
Feb 3, 2024
This was referenced Feb 10, 2024
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
## Overview Branched off facebook#28130 ## Why In facebook#24276 we changed the new root behavior to error when a client-render is forced for certain cases, so these now expect a mismatch even though they're using `suppressHydrationWarning`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
tldr:
suppressHydrationWarningshouldn't mute warnings that force a client render.Before 18 +
createRoot, all hydration mismatches except prop mismatches were patched up. Prop mismatches were warned about but not patched up. AndsuppressHydrationWarningon the parent was a universal way to suppress messages — about all kinds of mismatches.In 18 +
createRoot, mismatches always revert to a clean render up to the closest<Suspense>above except for one case. The exception issuppressHydrationWarning: it enables patching for text mismatches.However, this means that you can't debug other kinds of mismatches inside
suppressHydrationWarning. Insertions and deletions now revert to client render but we still mute the warning because in 17 it was harmless to mute. We've introduced limited production behavior for the prop, but we haven't adjusted the warning messages themselves to show for the case where mismatches are not patched up.This fixes the issue.