-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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
Fix suppressHydrationWarning not working in production #24271
Conversation
@@ -875,7 +874,6 @@ export function diffHydratedProperties( | |||
let extraAttributeNames: Set<string>; | |||
|
|||
if (__DEV__) { | |||
suppressHydrationWarning = rawProps[SUPPRESS_HYDRATION_WARNING] === true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved inline since we don't want to hit the property checks for every props object.
@@ -1028,7 +1026,7 @@ export function diffHydratedProperties( | |||
isCustomComponentTag && enableCustomElementPropertySupport | |||
? null | |||
: getPropertyInfo(propKey); | |||
if (suppressHydrationWarning) { | |||
if (rawProps[SUPPRESS_HYDRATION_WARNING] === true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This runs repeatedly but this part is DEV-only.
@@ -132,10 +132,7 @@ type SelectionInformation = {| | |||
selectionRange: mixed, | |||
|}; | |||
|
|||
let SUPPRESS_HYDRATION_WARNING; | |||
if (__DEV__) { | |||
SUPPRESS_HYDRATION_WARNING = 'suppressHydrationWarning'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a regression test for this one as well.
Comparing: c89a15c...98de7f6 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) |
1786a32
to
98de7f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me though I'm not super familiar with this code path so not sure if there are other implications
Fixes #24270.
We intended
suppressHydrationWarning
to be checked in production as an escape hatch in 18 and prevent mismatch errors. However, the logic mistakingly only read it in development mode. I've added regression tests for the lines I've changed.I've noticed we don't patch up
dangerouslySetInnerHTML
like we do text content. (Although we do ignore its mismatch in production.) I left that as is, but added a test verifying the current behavior.