Skip to content

Commit

Permalink
Fix false positive hydration warnings (facebook#23364)
Browse files Browse the repository at this point in the history 
* Failing test for react#23331

* Don't warn on hydration mismatch if suspended

When something suspends during hydration, we continue rendering the
siblings to warm up the cache and fire off any lazy network requests.
However, if there are any mismatches while rendering the siblings, it's
likely a false positive caused by the earlier suspended component. So
we should suppress any hydration warnings until the tree no
longer suspends.

Fixes facebook#23332

Co-authored-by: Marcel Laverdet <[email protected]>
  • Loading branch information
@acdlite @laverdet @zhengjitf
2 people authored and zhengjitf committed Apr 15, 2022
1 parent 3468e3c commit 877efb9
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 15 deletions.
66 changes: 66 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,72 @@ describe('ReactDOMFizzServer', () => {
);
});

// @gate experimental
it('#23331: does not warn about hydration mismatches if something suspended in an earlier sibling', async () => {
const makeApp = () => {
let resolve;
const imports = new Promise(r => {
resolve = () => r({default: () => <span id="async">async</span>});
});
const Lazy = React.lazy(() => imports);

const App = () => (
<div>
<Suspense fallback={<span>Loading...</span>}>
<Lazy />
<span id="after">after</span>
</Suspense>
</div>
);

return [App, resolve];
};

// Server-side
const [App, resolve] = makeApp();
await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
pipe(writable);
});
expect(getVisibleChildren(container)).toEqual(
<div>
<span>Loading...</span>
</div>,
);
await act(async () => {
resolve();
});
expect(getVisibleChildren(container)).toEqual(
<div>
<span id="async">async</span>
<span id="after">after</span>
</div>,
);

// Client-side
const [HydrateApp, hydrateResolve] = makeApp();
await act(async () => {
ReactDOM.hydrateRoot(container, <HydrateApp />);
});

expect(getVisibleChildren(container)).toEqual(
<div>
<span id="async">async</span>
<span id="after">after</span>
</div>,
);

await act(async () => {
hydrateResolve();
});
expect(getVisibleChildren(container)).toEqual(
<div>
<span id="async">async</span>
<span id="after">after</span>
</div>,
);
});

// @gate experimental
it('should support nonce scripts', async () => {
CSPnonce = 'R4nd0m';
Expand Down
31 changes: 20 additions & 11 deletions packages/react-dom/src/client/ReactDOMComponent.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,21 +230,24 @@ export function checkForUnmatchedText(
serverText: string,
clientText: string | number,
isConcurrentMode: boolean,
shouldWarnDev: boolean,
) {
const normalizedClientText = normalizeMarkupForTextOrAttribute(clientText);
const normalizedServerText = normalizeMarkupForTextOrAttribute(serverText);
if (normalizedServerText === normalizedClientText) {
return;
}

if (__DEV__) {
if (!didWarnInvalidHydration) {
didWarnInvalidHydration = true;
console.error(
'Text content did not match. Server: "%s" Client: "%s"',
normalizedServerText,
normalizedClientText,
);
if (shouldWarnDev) {
if (__DEV__) {
if (!didWarnInvalidHydration) {
didWarnInvalidHydration = true;
console.error(
'Text content did not match. Server: "%s" Client: "%s"',
normalizedServerText,
normalizedClientText,
);
}
}
}

Expand Down Expand Up @@ -866,6 +869,7 @@ export function diffHydratedProperties(
parentNamespace: string,
rootContainerElement: Element | Document,
isConcurrentMode: boolean,
shouldWarnDev: boolean,
): null | Array<mixed> {
let isCustomComponentTag;
let extraAttributeNames: Set<string>;
Expand Down Expand Up @@ -985,6 +989,7 @@ export function diffHydratedProperties(
domElement.textContent,
nextProp,
isConcurrentMode,
shouldWarnDev,
);
}
updatePayload = [CHILDREN, nextProp];
Expand All @@ -996,6 +1001,7 @@ export function diffHydratedProperties(
domElement.textContent,
nextProp,
isConcurrentMode,
shouldWarnDev,
);
}
updatePayload = [CHILDREN, '' + nextProp];
Expand All @@ -1011,6 +1017,7 @@ export function diffHydratedProperties(
}
}
} else if (
shouldWarnDev &&
__DEV__ &&
// Convince Flow we've calculated it (it's DEV-only in this method.)
typeof isCustomComponentTag === 'boolean'
Expand Down Expand Up @@ -1142,10 +1149,12 @@ export function diffHydratedProperties(
}

if (__DEV__) {
// $FlowFixMe - Should be inferred as not undefined.
if (extraAttributeNames.size > 0 && !suppressHydrationWarning) {
if (shouldWarnDev) {
// $FlowFixMe - Should be inferred as not undefined.
warnForExtraAttributes(extraAttributeNames);
if (extraAttributeNames.size > 0 && !suppressHydrationWarning) {
// $FlowFixMe - Should be inferred as not undefined.
warnForExtraAttributes(extraAttributeNames);
}
}
}

Expand Down
19 changes: 17 additions & 2 deletions packages/react-dom/src/client/ReactDOMHostConfig.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,7 @@ export function hydrateInstance(
rootContainerInstance: Container,
hostContext: HostContext,
internalInstanceHandle: Object,
shouldWarnDev: boolean,
): null | Array<mixed> {
precacheFiberNode(internalInstanceHandle, instance);
// TODO: Possibly defer this until the commit phase where all the events
Expand All @@ -811,13 +812,15 @@ export function hydrateInstance(
parentNamespace,
rootContainerInstance,
isConcurrentMode,
shouldWarnDev,
);
}

export function hydrateTextInstance(
textInstance: TextInstance,
text: string,
internalInstanceHandle: Object,
shouldWarnDev: boolean,
): boolean {
precacheFiberNode(internalInstanceHandle, textInstance);

Expand Down Expand Up @@ -924,7 +927,13 @@ export function didNotMatchHydratedContainerTextInstance(
text: string,
isConcurrentMode: boolean,
) {
checkForUnmatchedText(textInstance.nodeValue, text, isConcurrentMode);
const shouldWarnDev = true;
checkForUnmatchedText(
textInstance.nodeValue,
text,
isConcurrentMode,
shouldWarnDev,
);
}

export function didNotMatchHydratedTextInstance(
Expand All @@ -936,7 +945,13 @@ export function didNotMatchHydratedTextInstance(
isConcurrentMode: boolean,
) {
if (parentProps[SUPPRESS_HYDRATION_WARNING] !== true) {
checkForUnmatchedText(textInstance.nodeValue, text, isConcurrentMode);
const shouldWarnDev = true;
checkForUnmatchedText(
textInstance.nodeValue,
text,
isConcurrentMode,
shouldWarnDev,
);
}
}

Expand Down
27 changes: 26 additions & 1 deletion packages/react-reconciler/src/ReactFiberHydrationContext.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ import {queueRecoverableErrors} from './ReactFiberWorkLoop.new';
let hydrationParentFiber: null | Fiber = null;
let nextHydratableInstance: null | HydratableInstance = null;
let isHydrating: boolean = false;
let didSuspend: boolean = false;

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

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

function enterHydrationState(fiber: Fiber): boolean {
if (!supportsHydration) {
return false;
Expand All @@ -110,6 +117,7 @@ function enterHydrationState(fiber: Fiber): boolean {
hydrationParentFiber = fiber;
isHydrating = true;
hydrationErrors = null;
didSuspend = false;
return true;
}

Expand All @@ -127,6 +135,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance(
hydrationParentFiber = fiber;
isHydrating = true;
hydrationErrors = null;
didSuspend = false;
if (treeContext !== null) {
restoreSuspendedTreeContext(fiber, treeContext);
}
Expand Down Expand Up @@ -185,6 +194,13 @@ function deleteHydratableInstance(

function warnNonhydratedInstance(returnFiber: Fiber, fiber: Fiber) {
if (__DEV__) {
if (didSuspend) {
// 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.
return;
}

switch (returnFiber.tag) {
case HostRoot: {
const parentContainer = returnFiber.stateNode.containerInfo;
Expand Down Expand Up @@ -418,13 +434,15 @@ function prepareToHydrateHostInstance(
}

const instance: Instance = fiber.stateNode;
const shouldWarnIfMismatchDev = !didSuspend;
const updatePayload = hydrateInstance(
instance,
fiber.type,
fiber.memoizedProps,
rootContainerInstance,
hostContext,
fiber,
shouldWarnIfMismatchDev,
);
// TODO: Type this specific to this type of component.
fiber.updateQueue = (updatePayload: any);
Expand All @@ -446,7 +464,13 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean {

const textInstance: TextInstance = fiber.stateNode;
const textContent: string = fiber.memoizedProps;
const shouldUpdate = hydrateTextInstance(textInstance, textContent, fiber);
const shouldWarnIfMismatchDev = !didSuspend;
const shouldUpdate = hydrateTextInstance(
textInstance,
textContent,
fiber,
shouldWarnIfMismatchDev,
);
if (shouldUpdate) {
// We assume that prepareToHydrateHostTextInstance is called in a context where the
// hydration parent is the parent host component of this host text.
Expand Down Expand Up @@ -616,6 +640,7 @@ function resetHydrationState(): void {
hydrationParentFiber = null;
nextHydratableInstance = null;
isHydrating = false;
didSuspend = false;
}

export function upgradeHydrationErrorsToRecoverable(): void {
Expand Down
27 changes: 26 additions & 1 deletion packages/react-reconciler/src/ReactFiberHydrationContext.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ import {queueRecoverableErrors} from './ReactFiberWorkLoop.old';
let hydrationParentFiber: null | Fiber = null;
let nextHydratableInstance: null | HydratableInstance = null;
let isHydrating: boolean = false;
let didSuspend: boolean = false;

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

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

function enterHydrationState(fiber: Fiber): boolean {
if (!supportsHydration) {
return false;
Expand All @@ -110,6 +117,7 @@ function enterHydrationState(fiber: Fiber): boolean {
hydrationParentFiber = fiber;
isHydrating = true;
hydrationErrors = null;
didSuspend = false;
return true;
}

Expand All @@ -127,6 +135,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance(
hydrationParentFiber = fiber;
isHydrating = true;
hydrationErrors = null;
didSuspend = false;
if (treeContext !== null) {
restoreSuspendedTreeContext(fiber, treeContext);
}
Expand Down Expand Up @@ -185,6 +194,13 @@ function deleteHydratableInstance(

function warnNonhydratedInstance(returnFiber: Fiber, fiber: Fiber) {
if (__DEV__) {
if (didSuspend) {
// 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.
return;
}

switch (returnFiber.tag) {
case HostRoot: {
const parentContainer = returnFiber.stateNode.containerInfo;
Expand Down Expand Up @@ -418,13 +434,15 @@ function prepareToHydrateHostInstance(
}

const instance: Instance = fiber.stateNode;
const shouldWarnIfMismatchDev = !didSuspend;
const updatePayload = hydrateInstance(
instance,
fiber.type,
fiber.memoizedProps,
rootContainerInstance,
hostContext,
fiber,
shouldWarnIfMismatchDev,
);
// TODO: Type this specific to this type of component.
fiber.updateQueue = (updatePayload: any);
Expand All @@ -446,7 +464,13 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean {

const textInstance: TextInstance = fiber.stateNode;
const textContent: string = fiber.memoizedProps;
const shouldUpdate = hydrateTextInstance(textInstance, textContent, fiber);
const shouldWarnIfMismatchDev = !didSuspend;
const shouldUpdate = hydrateTextInstance(
textInstance,
textContent,
fiber,
shouldWarnIfMismatchDev,
);
if (shouldUpdate) {
// We assume that prepareToHydrateHostTextInstance is called in a context where the
// hydration parent is the parent host component of this host text.
Expand Down Expand Up @@ -616,6 +640,7 @@ function resetHydrationState(): void {
hydrationParentFiber = null;
nextHydratableInstance = null;
isHydrating = false;
didSuspend = false;
}

export function upgradeHydrationErrorsToRecoverable(): void {
Expand Down
Loading

0 comments on commit 877efb9

Please sign in to comment.