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

Avoid accumulating hydration mismatch errors after the first hydration error #24427

Closed
wants to merge 13 commits into from
Closed

Avoid accumulating hydration mismatch errors after the first hydration error #24427

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
0f8775a
Avoid accumulating hydration mismatch errors after the first hydratio…
gnoff Apr 22, 2022
72fa9bb
Reduce uncaught errors caused by IGC and refactor mismatch errors to …
gnoff Apr 28, 2022
136a2ac
add test for matching tree hydration error warnings
gnoff Apr 28, 2022
4783e68
add tests to assert that invokeGuardedCallback is only called when de…
gnoff Apr 28, 2022
3773d00
catch up forks
gnoff Apr 28, 2022
f15c5f2
merge workloop fork
gnoff Apr 28, 2022
9648314
fix import
gnoff Apr 28, 2022
f531201
fix test gates
gnoff Apr 28, 2022
a84b1ad
more gates
gnoff Apr 28, 2022
e74488b
reset throw state on hydration entry and reset
gnoff Apr 28, 2022
87d8ef5
only queue first thrown hydration error
gnoff Apr 29, 2022
0489ee0
improve tests
gnoff Apr 29, 2022
d016d50
fix forks
gnoff Apr 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
446 changes: 441 additions & 5 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 1 addition & 3 deletions packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
View file
Open in desktop
Expand Up @@ -350,9 +350,7 @@ describe('ReactDOMServerPartialHydration', () => {
);

if (__DEV__) {
const secondToLastCall =
mockError.mock.calls[mockError.mock.calls.length - 2];
expect(secondToLastCall).toEqual([
expect(mockError.mock.calls[0]).toEqual([
'Warning: Expected server HTML to contain a matching <%s> in <%s>.%s',
'article',
'section',
Expand Down
15 changes: 14 additions & 1 deletion packages/react-reconciler/src/ReactFiberHydrationContext.new.js
View file
Open in desktop
Expand Up @@ -88,6 +88,10 @@ let didSuspendOrErrorDEV: boolean = false;
// Hydration errors that were thrown inside this boundary
let hydrationErrors: Array<mixed> | null = null;

export function hydrationDidSuspendOrErrorDEV() {
return didSuspendOrErrorDEV;
}

function warnIfHydrating() {
if (__DEV__) {
if (isHydrating) {
Expand Down Expand Up @@ -457,6 +461,7 @@ function prepareToHydrateHostInstance(
fiber,
shouldWarnIfMismatchDev,
);

// TODO: Type this specific to this type of component.
fiber.updateQueue = (updatePayload: any);
// If the update payload indicates that there is a change or if there
Expand Down Expand Up @@ -673,7 +678,13 @@ function getIsHydrating(): boolean {
return isHydrating;
}

export function queueHydrationError(error: mixed): void {
function queueIfFirstHydrationError(error: mixed): void {
if (hydrationErrors === null) {
hydrationErrors = [error];
}
}

function queueHydrationError(error: mixed): void {
if (hydrationErrors === null) {
hydrationErrors = [error];
} else {
Expand All @@ -694,4 +705,6 @@ export {
popHydrationState,
hasUnhydratedTailNodes,
warnIfUnhydratedTailNodes,
queueIfFirstHydrationError,
queueHydrationError,
};
15 changes: 14 additions & 1 deletion packages/react-reconciler/src/ReactFiberHydrationContext.old.js
View file
Open in desktop
Expand Up @@ -88,6 +88,10 @@ let didSuspendOrErrorDEV: boolean = false;
// Hydration errors that were thrown inside this boundary
let hydrationErrors: Array<mixed> | null = null;

export function hydrationDidSuspendOrErrorDEV() {
return didSuspendOrErrorDEV;
}

function warnIfHydrating() {
if (__DEV__) {
if (isHydrating) {
Expand Down Expand Up @@ -457,6 +461,7 @@ function prepareToHydrateHostInstance(
fiber,
shouldWarnIfMismatchDev,
);

// TODO: Type this specific to this type of component.
fiber.updateQueue = (updatePayload: any);
// If the update payload indicates that there is a change or if there
Expand Down Expand Up @@ -673,7 +678,13 @@ function getIsHydrating(): boolean {
return isHydrating;
}

export function queueHydrationError(error: mixed): void {
function queueIfFirstHydrationError(error: mixed): void {
if (hydrationErrors === null) {
hydrationErrors = [error];
}
}

function queueHydrationError(error: mixed): void {
if (hydrationErrors === null) {
hydrationErrors = [error];
} else {
Expand All @@ -694,4 +705,6 @@ export {
popHydrationState,
hasUnhydratedTailNodes,
warnIfUnhydratedTailNodes,
queueIfFirstHydrationError,
queueHydrationError,
};
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberThrow.new.js
View file
Open in desktop
Expand Up @@ -84,7 +84,7 @@ import {
import {
getIsHydrating,
markDidThrowWhileHydratingDEV,
queueHydrationError,
queueIfFirstHydrationError,
} from './ReactFiberHydrationContext.new';

const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;
Expand Down Expand Up @@ -542,7 +542,7 @@ function throwException(

// Even though the user may not be affected by this error, we should
// still log it so it can be fixed.
queueHydrationError(value);
queueIfFirstHydrationError(value);
return;
}
} else {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactFiberThrow.old.js
View file
Open in desktop
Expand Up @@ -84,7 +84,7 @@ import {
import {
getIsHydrating,
markDidThrowWhileHydratingDEV,
queueHydrationError,
queueIfFirstHydrationError,
} from './ReactFiberHydrationContext.old';

const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;
Expand Down Expand Up @@ -542,7 +542,7 @@ function throwException(

// Even though the user may not be affected by this error, we should
// still log it so it can be fixed.
queueHydrationError(value);
queueIfFirstHydrationError(value);
return;
}
} else {
Expand Down
11 changes: 7 additions & 4 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
View file
Open in desktop
Expand Up @@ -88,6 +88,7 @@ import {
assignFiberPropertiesInDEV,
} from './ReactFiber.new';
import {isRootDehydrated} from './ReactFiberShellHydration';
import {hydrationDidSuspendOrErrorDEV} from './ReactFiberHydrationContext.new';
import {NoMode, ProfileMode, ConcurrentMode} from './ReactTypeOfMode';
import {
HostRoot,
Expand Down Expand Up @@ -3001,11 +3002,13 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
return originalBeginWork(current, unitOfWork, lanes);
} catch (originalError) {
if (
originalError !== null &&
typeof originalError === 'object' &&
typeof originalError.then === 'function'
hydrationDidSuspendOrErrorDEV() ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

What’s the timing of this flag being set? We want to be able to still break on the first error so if this flag is already set it wouldn’t.

I think you might need to read it before beginWork to see if it was already set before.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Begin work catches first and then handleError delegates to throwException where the flag is set. Timing too fragile?

(originalError !== null &&
typeof originalError === 'object' &&
typeof originalError.then === 'function')
) {
// Don't replay promises. Treat everything else like an error.
// Don't replay promises.
// Don't replay when hydration has suspended or errored already
throw originalError;
}

Expand Down
11 changes: 7 additions & 4 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
View file
Open in desktop
Expand Up @@ -88,6 +88,7 @@ import {
assignFiberPropertiesInDEV,
} from './ReactFiber.old';
import {isRootDehydrated} from './ReactFiberShellHydration';
import {hydrationDidSuspendOrErrorDEV} from './ReactFiberHydrationContext.old';
import {NoMode, ProfileMode, ConcurrentMode} from './ReactTypeOfMode';
import {
HostRoot,
Expand Down Expand Up @@ -3001,11 +3002,13 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
return originalBeginWork(current, unitOfWork, lanes);
} catch (originalError) {
if (
originalError !== null &&
typeof originalError === 'object' &&
typeof originalError.then === 'function'
hydrationDidSuspendOrErrorDEV() ||
(originalError !== null &&
typeof originalError === 'object' &&
typeof originalError.then === 'function')
) {
// Don't replay promises. Treat everything else like an error.
// Don't replay promises.
// Don't replay when hydration has suspended or errored already
throw originalError;
}

Expand Down