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

Fix suppressHydrationWarning not working in production #24271

Merged
merged 1 commit into from
Apr 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2811,4 +2811,96 @@ describe('ReactDOMFizzServer', () => {
</ul>,
);
});

// @gate experimental
it('suppresses and fixes text mismatches with suppressHydrationWarning', async () => {
function App({isClient}) {
return (
<div>
<span
suppressHydrationWarning={true}
data-attr={isClient ? 'client-attr' : 'server-attr'}>
{isClient ? 'Client Text' : 'Server Text'}
</span>
<span suppressHydrationWarning={true}>{isClient ? 2 : 1}</span>
<span suppressHydrationWarning={true}>
hello,{isClient ? 'client' : 'server'}
</span>
</div>
);
}
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<App isClient={false} />,
);
pipe(writable);
});
expect(getVisibleChildren(container)).toEqual(
<div>
<span data-attr="server-attr">Server Text</span>
<span>1</span>
<span>
{'hello,'}
{'server'}
</span>
</div>,
);
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
// Don't miss a hydration error. There should be none.
Scheduler.unstable_yieldValue(error.message);
},
});
expect(Scheduler).toFlushAndYield([]);
// The text mismatch should be *silently* fixed. Even in production.
// The attribute mismatch should be ignored and not fixed.
expect(getVisibleChildren(container)).toEqual(
<div>
<span data-attr="server-attr">Client Text</span>
<span>2</span>
<span>
{'hello,'}
{'client'}
</span>
</div>,
);
});

// @gate experimental
it('suppresses and does not fix html mismatches with suppressHydrationWarning', async () => {
function App({isClient}) {
return (
<div>
<p
suppressHydrationWarning={true}
dangerouslySetInnerHTML={{
__html: isClient ? 'Client HTML' : 'Server HTML',
}}
/>
</div>
);
}
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<App isClient={false} />,
);
pipe(writable);
});
expect(getVisibleChildren(container)).toEqual(
<div>
<p>Server HTML</p>
</div>,
);
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(error.message);
},
});
expect(Scheduler).toFlushAndYield([]);
expect(getVisibleChildren(container)).toEqual(
<div>
<p>Server HTML</p>
</div>,
);
});
});
15 changes: 8 additions & 7 deletions packages/react-dom/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ const STYLE = 'style';
const HTML = '__html';

let warnedUnknownTags;
let suppressHydrationWarning;

let validatePropertiesInDevelopment;
let warnForPropDifference;
Expand Down Expand Up @@ -875,7 +874,6 @@ export function diffHydratedProperties(
let extraAttributeNames: Set<string>;

if (__DEV__) {
suppressHydrationWarning = rawProps[SUPPRESS_HYDRATION_WARNING] === true;
Copy link
Collaborator Author

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.

isCustomComponentTag = isCustomComponent(tag, rawProps);
validatePropertiesInDevelopment(tag, rawProps);
}
Expand Down Expand Up @@ -984,7 +982,7 @@ export function diffHydratedProperties(
// TODO: Should we use domElement.firstChild.nodeValue to compare?
if (typeof nextProp === 'string') {
if (domElement.textContent !== nextProp) {
if (!suppressHydrationWarning) {
if (rawProps[SUPPRESS_HYDRATION_WARNING] !== true) {
checkForUnmatchedText(
domElement.textContent,
nextProp,
Expand All @@ -996,7 +994,7 @@ export function diffHydratedProperties(
}
} else if (typeof nextProp === 'number') {
if (domElement.textContent !== '' + nextProp) {
if (!suppressHydrationWarning) {
if (rawProps[SUPPRESS_HYDRATION_WARNING] !== true) {
checkForUnmatchedText(
domElement.textContent,
nextProp,
Expand Down Expand Up @@ -1028,7 +1026,7 @@ export function diffHydratedProperties(
isCustomComponentTag && enableCustomElementPropertySupport
? null
: getPropertyInfo(propKey);
if (suppressHydrationWarning) {
if (rawProps[SUPPRESS_HYDRATION_WARNING] === true) {
Copy link
Collaborator Author

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.

// Don't bother comparing. We're ignoring all these warnings.
} else if (
propKey === SUPPRESS_CONTENT_EDITABLE_WARNING ||
Expand Down Expand Up @@ -1150,8 +1148,11 @@ export function diffHydratedProperties(

if (__DEV__) {
if (shouldWarnDev) {
// $FlowFixMe - Should be inferred as not undefined.
if (extraAttributeNames.size > 0 && !suppressHydrationWarning) {
if (
// $FlowFixMe - Should be inferred as not undefined.
extraAttributeNames.size > 0 &&
rawProps[SUPPRESS_HYDRATION_WARNING] !== true
) {
// $FlowFixMe - Should be inferred as not undefined.
warnForExtraAttributes(extraAttributeNames);
}
Expand Down
5 changes: 1 addition & 4 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,7 @@ type SelectionInformation = {|
selectionRange: mixed,
|};

let SUPPRESS_HYDRATION_WARNING;
if (__DEV__) {
SUPPRESS_HYDRATION_WARNING = 'suppressHydrationWarning';
Copy link
Collaborator Author

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.

}
const SUPPRESS_HYDRATION_WARNING = 'suppressHydrationWarning';

const SUSPENSE_START_DATA = '$';
const SUSPENSE_END_DATA = '/$';
Expand Down