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

Suppress hydration warnings when a preceding sibling suspends #24404

Merged
merged 14 commits into from Apr 20, 2022
243 changes: 243 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Expand Up @@ -2841,4 +2841,247 @@ describe('ReactDOMFizzServer', () => {
expect(window.__test_outlet).toBe(1);
});
});

// @gate experimental && enableClientRenderFallbackOnTextMismatch
it('#24384: Suspending should halt hydration warnings while still allowing siblings to warm up', async () => {
Copy link

@xiel xiel Apr 21, 2022

Choose a reason for hiding this comment

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

Can we add a test, where there is no mismatch at all between client and server (and no emitted warnings)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, thanks: #24436

const makeApp = () => {
let resolve, resolved;
const promise = new Promise(r => {
resolve = () => {
resolved = true;
return r();
};
});
function ComponentThatSuspends() {
if (!resolved) {
throw promise;
}
return <p>A</p>;
}

const App = ({text}) => {
return (
<div>
<Suspense fallback={<h1>Loading...</h1>}>
<ComponentThatSuspends />
<h2 name={text}>{text}</h2>
</Suspense>
</div>
);
};

return [App, resolve];
};

const [ServerApp, serverResolve] = makeApp();
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<ServerApp text="initial" />,
);
pipe(writable);
});
await act(() => {
serverResolve();
});

expect(getVisibleChildren(container)).toEqual(
<div>
<p>A</p>
<h2 name="initial">initial</h2>
</div>,
);

// The client app is rendered with an intentionally incorrect text. The still Suspended component causes
// hydration to fail silently (allowing for cache warming but otherwise skipping this boundary) until it
// resolves.
const [ClientApp, clientResolve] = makeApp();
ReactDOMClient.hydrateRoot(container, <ClientApp text="replaced" />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(
'Logged recoverable error: ' + error.message,
);
},
});
Scheduler.unstable_flushAll();

expect(getVisibleChildren(container)).toEqual(
<div>
<p>A</p>
<h2 name="initial">initial</h2>
</div>,
);

// Now that the boundary resolves to it's children the hydration completes and discovers that there is a mismatch requiring
// client-side rendering.
await clientResolve();
expect(() => {
expect(Scheduler).toFlushAndYield([
'Logged recoverable error: Text content does not match server-rendered HTML.',
'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.',
]);
}).toErrorDev(
'Warning: Prop `name` did not match. Server: "initial" Client: "replaced"',
);
expect(getVisibleChildren(container)).toEqual(
<div>
<p>A</p>
<h2 name="replaced">replaced</h2>
</div>,
);

expect(Scheduler).toFlushAndYield([]);
});

// @gate experimental && enableClientRenderFallbackOnTextMismatch
it('only warns once on hydration mismatch while within a suspense boundary', async () => {
const originalConsoleError = console.error;
const mockError = jest.fn();
console.error = (...args) => {
mockError(...args.map(normalizeCodeLocInfo));
};

const App = ({text}) => {
return (
<div>
<Suspense fallback={<h1>Loading...</h1>}>
<h2>{text}</h2>
<h2>{text}</h2>
<h2>{text}</h2>
</Suspense>
</div>
);
};

try {
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
<App text="initial" />,
);
pipe(writable);
});

expect(getVisibleChildren(container)).toEqual(
<div>
<h2>initial</h2>
<h2>initial</h2>
<h2>initial</h2>
</div>,
);

ReactDOMClient.hydrateRoot(container, <App text="replaced" />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(
'Logged recoverable error: ' + error.message,
);
},
});
expect(Scheduler).toFlushAndYield([
'Logged recoverable error: Text content does not match server-rendered HTML.',
'Logged recoverable error: Text content does not match server-rendered HTML.',
'Logged recoverable error: Text content does not match server-rendered HTML.',
'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.',
]);

expect(getVisibleChildren(container)).toEqual(
<div>
<h2>replaced</h2>
<h2>replaced</h2>
<h2>replaced</h2>
</div>,
);

expect(Scheduler).toFlushAndYield([]);
if (__DEV__) {
expect(mockError.mock.calls.length).toBe(1);
expect(mockError.mock.calls[0]).toEqual([
'Warning: Text content did not match. Server: "%s" Client: "%s"%s',
'initial',
'replaced',
'\n' +
' in h2 (at **)\n' +
' in Suspense (at **)\n' +
' in div (at **)\n' +
' in App (at **)',
]);
} else {
expect(mockError.mock.calls.length).toBe(0);
}
} finally {
console.error = originalConsoleError;
}
});

// @gate experimental
it('supresses hydration warnings when an error occurs within a Suspense boundary', async () => {
let isClient = false;
let shouldThrow = true;

function ThrowUntilOnClient({children}) {
if (isClient && shouldThrow) {
throw new Error('uh oh');
}
return children;
}

function StopThrowingOnClient() {
if (isClient) {
shouldThrow = false;
}
return null;
}

const App = () => {
return (
<div>
<Suspense fallback={<h1>Loading...</h1>}>
<ThrowUntilOnClient>
<h1>one</h1>
</ThrowUntilOnClient>
<h2>two</h2>
<h3>{isClient ? 'five' : 'three'}</h3>
<StopThrowingOnClient />
</Suspense>
</div>
);
};

await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
pipe(writable);
});

expect(getVisibleChildren(container)).toEqual(
<div>
<h1>one</h1>
<h2>two</h2>
<h3>three</h3>
</div>,
);

isClient = true;

ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.unstable_yieldValue(
'Logged recoverable error: ' + error.message,
);
},
});
expect(Scheduler).toFlushAndYield([
'Logged recoverable error: uh oh',
'Logged recoverable error: Hydration failed because the initial UI does not match what was rendered on the server.',
'Logged recoverable error: Hydration failed because the initial UI does not match what was rendered on the server.',
'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.',
]);

expect(getVisibleChildren(container)).toEqual(
<div>
<h1>one</h1>
<h2>two</h2>
<h3>five</h3>
</div>,
);

expect(Scheduler).toFlushAndYield([]);
});
});
Expand Up @@ -285,7 +285,7 @@ describe('ReactDOMServerPartialHydration', () => {
}
try {
const finalHTML = ReactDOMServer.renderToString(<App />);
const container = document.createElement('div');
const container = document.createElement('section');
container.innerHTML = finalHTML;
expect(Scheduler).toHaveYielded([
'Hello',
Expand Down Expand Up @@ -350,12 +350,14 @@ describe('ReactDOMServerPartialHydration', () => {
);

if (__DEV__) {
expect(mockError.mock.calls[0]).toEqual([
const secondToLastCall =
mockError.mock.calls[mockError.mock.calls.length - 2];
expect(secondToLastCall).toEqual([
'Warning: Expected server HTML to contain a matching <%s> in <%s>.%s',
'div',
'div',
'article',
'section',
'\n' +
' in div (at **)\n' +
' in article (at **)\n' +
Comment on lines +353 to +360
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 warning used to be for trying to match the first Component div against the Text node. It was an expression of sorts of the issue brought up in #24384

This warning now is the intentional mismatch between article and div that the test introduces between client/server renders.

' in Component (at **)\n' +
' in Suspense (at **)\n' +
' in App (at **)',
Expand Down
21 changes: 12 additions & 9 deletions packages/react-reconciler/src/ReactFiberHydrationContext.new.js
Expand Up @@ -80,7 +80,10 @@ import {queueRecoverableErrors} from './ReactFiberWorkLoop.new';
let hydrationParentFiber: null | Fiber = null;
let nextHydratableInstance: null | HydratableInstance = null;
let isHydrating: boolean = false;
let didSuspend: boolean = false;

// This flag allows for warning supression when we expect there to be mismatches
// due to earlier mismatches or a suspended fiber.
let didSuspendOrErrorDEV: boolean = false;

// Hydration errors that were thrown inside this boundary
let hydrationErrors: Array<mixed> | null = null;
Expand All @@ -95,9 +98,9 @@ function warnIfHydrating() {
}
}

export function markDidSuspendWhileHydratingDEV() {
export function markDidThrowWhileHydratingDEV() {
if (__DEV__) {
didSuspend = true;
didSuspendOrErrorDEV = true;
}
}

Expand All @@ -113,7 +116,7 @@ function enterHydrationState(fiber: Fiber): boolean {
hydrationParentFiber = fiber;
isHydrating = true;
hydrationErrors = null;
didSuspend = false;
didSuspendOrErrorDEV = false;
return true;
}

Expand All @@ -131,7 +134,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance(
hydrationParentFiber = fiber;
isHydrating = true;
hydrationErrors = null;
didSuspend = false;
didSuspendOrErrorDEV = false;
if (treeContext !== null) {
restoreSuspendedTreeContext(fiber, treeContext);
}
Expand Down Expand Up @@ -196,7 +199,7 @@ function deleteHydratableInstance(

function warnNonhydratedInstance(returnFiber: Fiber, fiber: Fiber) {
if (__DEV__) {
if (didSuspend) {
if (didSuspendOrErrorDEV) {
// Inside a boundary that already suspended. We're currently rendering the
// siblings of a suspended node. The mismatch may be due to the missing
// data, so it's probably a false positive.
Expand Down Expand Up @@ -444,7 +447,7 @@ function prepareToHydrateHostInstance(
}

const instance: Instance = fiber.stateNode;
const shouldWarnIfMismatchDev = !didSuspend;
const shouldWarnIfMismatchDev = !didSuspendOrErrorDEV;
const updatePayload = hydrateInstance(
instance,
fiber.type,
Expand Down Expand Up @@ -474,7 +477,7 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean {

const textInstance: TextInstance = fiber.stateNode;
const textContent: string = fiber.memoizedProps;
const shouldWarnIfMismatchDev = !didSuspend;
const shouldWarnIfMismatchDev = !didSuspendOrErrorDEV;
const shouldUpdate = hydrateTextInstance(
textInstance,
textContent,
Expand Down Expand Up @@ -653,7 +656,7 @@ function resetHydrationState(): void {
hydrationParentFiber = null;
nextHydratableInstance = null;
isHydrating = false;
didSuspend = false;
didSuspendOrErrorDEV = false;
}

export function upgradeHydrationErrorsToRecoverable(): void {
Expand Down