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.

Sign up for GitHub

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

Jump to bottom

Fix enableClientRenderFallbackOnTextMismatch flag #26457

Merged
merged 6 commits into from
Mar 22, 2023
Merged

Fix enableClientRenderFallbackOnTextMismatch flag #26457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2926d4f
Fix enableClientRenderFallbackOnTextMismatch flag
sebmarkbage Mar 22, 2023
3271076
Fix ReactDOMFizzSuppressHydrationWarning-test.js
rickhanlonii Mar 22, 2023
e708588
Actually tho
rickhanlonii Mar 22, 2023
9dde0e7
ReactDOMFloat-test.js
rickhanlonii Mar 22, 2023
56acc51
Also add check to HostComponent
rickhanlonii Mar 22, 2023
bfecd08
fix more tests
rickhanlonii Mar 22, 2023
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
2 changes: 1 addition & 1 deletion packages/react-dom-bindings/src/client/ReactDOMComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,7 @@ export function diffHydratedProperties(
shouldWarnDev,
);
}
if (!isConcurrentMode) {
if (!isConcurrentMode || !enableClientRenderFallbackOnTextMismatch) {
updatePayload = ['children', children];
}
}
Expand Down
120 changes: 120 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
: children;
}

// @gate enableClientRenderFallbackOnTextMismatch
it('suppresses but does not fix text mismatches with suppressHydrationWarning', async () => {
function App({isClient}) {
return (
Expand Down Expand Up @@ -169,6 +170,47 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
);
});

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

// @gate enableClientRenderFallbackOnTextMismatch
it('suppresses but does not fix multiple text node mismatches with suppressHydrationWarning', async () => {
function App({isClient}) {
return (
Expand Down Expand Up @@ -210,6 +252,48 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
);
});

// @gate !enableClientRenderFallbackOnTextMismatch
it('suppresses and fixes multiple text node mismatches with suppressHydrationWarning', async () => {
function App({isClient}) {
return (
<div>
<span suppressHydrationWarning={true}>
{isClient ? 'Client1' : 'Server1'}
{isClient ? 'Client2' : 'Server2'}
</span>
</div>
);
}
await act(() => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<App isClient={false} />,
);
pipe(writable);
});
expect(getVisibleChildren(container)).toEqual(
<div>
<span>
{'Server1'}
{'Server2'}
</span>
</div>,
);
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
Scheduler.log(error.message);
},
});
await waitForAll([]);
expect(getVisibleChildren(container)).toEqual(
<div>
<span>
{'Client1'}
{'Client2'}
</span>
</div>,
);
});

it('errors on text-to-element mismatches with suppressHydrationWarning', async () => {
function App({isClient}) {
return (
Expand Down Expand Up @@ -261,6 +345,7 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
);
});

// @gate enableClientRenderFallbackOnTextMismatch
it('suppresses but does not fix client-only single text node mismatches with suppressHydrationWarning', async () => {
function App({text}) {
return (
Expand Down Expand Up @@ -301,6 +386,41 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
);
});

// @gate !enableClientRenderFallbackOnTextMismatch
it('suppresses and fixes client-only single text node mismatches with suppressHydrationWarning', async () => {
function App({isClient}) {
return (
<div>
<span suppressHydrationWarning={true}>
{isClient ? 'Client' : null}
</span>
</div>
);
}
await act(() => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<App isClient={false} />,
);
pipe(writable);
});
expect(getVisibleChildren(container)).toEqual(
<div>
<span />
</div>,
);
ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
Scheduler.log(error.message);
},
});
await waitForAll([]);
expect(getVisibleChildren(container)).toEqual(
<div>
<span>{'Client'}</span>
</div>,
);
});

// TODO: This behavior is not consistent with client-only single text node.

it('errors on server-only single text node mismatches with suppressHydrationWarning', async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-dom/src/__tests__/ReactDOMFloat-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5617,7 +5617,7 @@ background-color: green;
]);
});

// @gate enableFloat && enableHostSingletons && enableClientRenderFallbackOnTextMismatch
// @gate enableFloat && enableHostSingletons && (enableClientRenderFallbackOnTextMismatch || !__DEV__)
it('can render a title before a singleton even if that singleton clears its contents', async () => {
await actIntoEmptyDocument(() => {
const {pipe} = renderToPipeableStream(
Expand Down
10 changes: 7 additions & 3 deletions packages/react-reconciler/src/ReactFiberHydrationContext.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ import {
NoFlags,
DidCapture,
} from './ReactFiberFlags';
import {enableHostSingletons, enableFloat} from 'shared/ReactFeatureFlags';
import {
enableHostSingletons,
enableFloat,
enableClientRenderFallbackOnTextMismatch,
} from 'shared/ReactFeatureFlags';

import {
createFiberFromHostInstanceForDeletion,
Expand Down Expand Up @@ -728,7 +732,7 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean {
isConcurrentMode,
shouldWarnIfMismatchDev,
);
if (isConcurrentMode) {
if (isConcurrentMode && enableClientRenderFallbackOnTextMismatch) {
// In concurrent mode we never update the mismatched text,
// even if the error was ignored.
return false;
Expand All @@ -752,7 +756,7 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean {
isConcurrentMode,
shouldWarnIfMismatchDev,
);
if (isConcurrentMode) {
if (isConcurrentMode && enableClientRenderFallbackOnTextMismatch) {
Copy link
Member

Choose a reason for hiding this comment

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

This cased was missed so I just added it

// In concurrent mode we never update the mismatched text,
// even if the error was ignored.
return false;
Expand Down