Skip to content

Commit

Permalink
Move Hydration Mismatch Errors to Throw or Log Once (Kind of) (#28502)
Browse files Browse the repository at this point in the history 
Stacked on #28476.

We used to `console.error` for every mismatch we found, up until the
error we threw for the hydration mismatch.

This changes it so that we build up a set of diffs up until we either
throw or complete hydrating the root/suspense boundary. If we throw, we
append the diff to the error message which gets passed to
onRecoverableError (which by default is also logged to console). If we
complete, we append it to a `console.error`.

Since we early abort when something throws, it effectively means that we
can only collect multiple diffs if there were preceding non-throwing
mismatches - i.e. only properties mismatched but tag name matched.

There can still be multiple logs if multiple siblings Suspense
boundaries all error hydrating but then they're separate errors
entirely.

We still log an extra line about something erroring but I think the goal
should be that it leads to a single recoverable or console.error.

This doesn't yet actually print the diff as part of this message. That's
in a follow up PR.
  • Loading branch information
@sebmarkbage
sebmarkbage committed Mar 27, 2024
1 parent 4b8dfd6 commit f7aa5e0
Show file tree
Hide file tree
Showing 16 changed files with 1,207 additions and 1,271 deletions.
10 changes: 8 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFizzForm-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ describe('ReactDOMFizzForm', () => {
ReactDOMClient.hydrateRoot(container, <App isClient={true} />);
});
}).toErrorDev(
'Prop `action` did not match. Server: "function" Client: "action"',
"A tree hydrated but some attributes of the server rendered HTML didn't match the client properties.",
{withoutStack: true},
);
});

Expand Down Expand Up @@ -344,7 +345,12 @@ describe('ReactDOMFizzForm', () => {
await act(async () => {
root = ReactDOMClient.hydrateRoot(container, <App />);
});
}).toErrorDev(['Prop `formTarget` did not match.']);
}).toErrorDev(
[
"A tree hydrated but some attributes of the server rendered HTML didn't match the client properties.",
],
{withoutStack: true},
);
await act(async () => {
root.render(<App isUpdate={true} />);
});
Expand Down
140 changes: 66 additions & 74 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ let waitForPaint;
let clientAct;
let streamingContainer;

function normalizeError(msg) {
// Take the first sentence to make it easier to assert on.
const idx = msg.indexOf('.');
if (idx > -1) {
return msg.slice(0, idx + 1);
}
return msg;
}

describe('ReactDOMFizzServer', () => {
beforeEach(() => {
jest.resetModules();
Expand Down Expand Up @@ -2391,26 +2400,22 @@ describe('ReactDOMFizzServer', () => {

ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.log('Log recoverable error: ' + error.message);
Scheduler.log(
'Log recoverable error: ' + normalizeError(error.message),
);
},
});

await expect(async () => {
// The first paint switches to client rendering due to mismatch
await waitForPaint([
'client',
'Log recoverable error: Hydration failed because the initial ' +
'UI does not match what was rendered on the server.',
'Log recoverable error: There was an error while hydrating. ' +
'Because the error happened outside of a Suspense boundary, the ' +
'entire root will switch to client rendering.',
"Log recoverable error: Hydration failed because the server rendered HTML didn't match the client.",
'Log recoverable error: There was an error while hydrating.',
]);
}).toErrorDev(
[
'Warning: An error occurred during hydration. The server HTML was replaced with client content.',
'Warning: Expected server HTML to contain a matching <div> in the root.\n' +
' in div (at **)\n' +
' in App (at **)',
],
{withoutStack: 1},
);
Expand Down Expand Up @@ -2474,7 +2479,9 @@ describe('ReactDOMFizzServer', () => {

ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.log('Log recoverable error: ' + error.message);
Scheduler.log(
'Log recoverable error: ' + normalizeError(error.message),
);
},
});

Expand All @@ -2483,18 +2490,12 @@ describe('ReactDOMFizzServer', () => {
// The first paint switches to client rendering due to mismatch
await waitForPaint([
'client',
'Log recoverable error: Hydration failed because the initial ' +
'UI does not match what was rendered on the server.',
'Log recoverable error: There was an error while hydrating. ' +
'Because the error happened outside of a Suspense boundary, the ' +
'entire root will switch to client rendering.',
"Log recoverable error: Hydration failed because the server rendered HTML didn't match the client.",
'Log recoverable error: There was an error while hydrating.',
]);
}).toErrorDev(
[
'Warning: An error occurred during hydration. The server HTML was replaced with client content',
'Warning: Expected server HTML to contain a matching <div> in the root.\n' +
' in div (at **)\n' +
' in App (at **)',
],
{withoutStack: 1},
);
Expand Down Expand Up @@ -2557,7 +2558,7 @@ describe('ReactDOMFizzServer', () => {
isClient = true;
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.log(error.message);
Scheduler.log(normalizeError(error.message));
},
});

Expand All @@ -2567,9 +2568,7 @@ describe('ReactDOMFizzServer', () => {
await waitForAll([
'Yay!',
'Hydration error',
'There was an error while hydrating. Because the error happened ' +
'outside of a Suspense boundary, the entire root will switch ' +
'to client rendering.',
'There was an error while hydrating.',
]);
}).toErrorDev(
'An error occurred during hydration. The server HTML was replaced',
Expand Down Expand Up @@ -2739,7 +2738,7 @@ describe('ReactDOMFizzServer', () => {
isClient = true;
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.log(error.message);
Scheduler.log(normalizeError(error.message));
},
});

Expand All @@ -2748,7 +2747,7 @@ describe('ReactDOMFizzServer', () => {
await waitForAll([
'Yay!',
'Hydration error',
'There was an error while hydrating this Suspense boundary. Switched to client rendering.',
'There was an error while hydrating this Suspense boundary.',
]);
expect(getVisibleChildren(container)).toEqual(
<div>
Expand Down Expand Up @@ -3194,16 +3193,15 @@ describe('ReactDOMFizzServer', () => {
isClient = true;
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.log(error.message);
Scheduler.log(normalizeError(error.message));
},
});

// An error logged but instead of surfacing it to the UI, we switched
// to client rendering.
await waitForAll([
'Hydration error',
'There was an error while hydrating this Suspense boundary. Switched ' +
'to client rendering.',
'There was an error while hydrating this Suspense boundary.',
]);
expect(getVisibleChildren(container)).toEqual(
<div>
Expand Down Expand Up @@ -3263,7 +3261,9 @@ describe('ReactDOMFizzServer', () => {

const root = ReactDOMClient.createRoot(container, {
onRecoverableError(error) {
Scheduler.log('Logged a recoverable error: ' + error.message);
Scheduler.log(
'Logged a recoverable error: ' + normalizeError(error.message),
);
},
});
React.startTransition(() => {
Expand Down Expand Up @@ -3339,7 +3339,9 @@ describe('ReactDOMFizzServer', () => {
isClient = true;
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.log('Logged recoverable error: ' + error.message);
Scheduler.log(
'Logged recoverable error: ' + normalizeError(error.message),
);
},
});

Expand All @@ -3349,11 +3351,11 @@ describe('ReactDOMFizzServer', () => {

'Logged recoverable error: Hydration error',
'Logged recoverable error: There was an error while hydrating this ' +
'Suspense boundary. Switched to client rendering.',
'Suspense boundary.',

'Logged recoverable error: Hydration error',
'Logged recoverable error: There was an error while hydrating this ' +
'Suspense boundary. Switched to client rendering.',
'Suspense boundary.',
]);
});

Expand Down Expand Up @@ -4395,7 +4397,9 @@ describe('ReactDOMFizzServer', () => {
const [ClientApp, clientResolve] = makeApp();
ReactDOMClient.hydrateRoot(container, <ClientApp />, {
onRecoverableError(error) {
Scheduler.log('Logged recoverable error: ' + error.message);
Scheduler.log(
'Logged recoverable error: ' + normalizeError(error.message),
);
},
});
await waitForAll([]);
Expand Down Expand Up @@ -4471,7 +4475,9 @@ describe('ReactDOMFizzServer', () => {
const [ClientApp, clientResolve] = makeApp();
ReactDOMClient.hydrateRoot(container, <ClientApp text="replaced" />, {
onRecoverableError(error) {
Scheduler.log('Logged recoverable error: ' + error.message);
Scheduler.log(
'Logged recoverable error: ' + normalizeError(error.message),
);
},
});
await waitForAll([]);
Expand All @@ -4486,14 +4492,10 @@ describe('ReactDOMFizzServer', () => {
// 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();
await expect(async () => {
await waitForAll([
'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: Text content did not match. Server: "initial" Client: "replaced',
);
await waitForAll([
"Logged recoverable error: Hydration failed because the server rendered HTML didn't match the client.",
'Logged recoverable error: There was an error while hydrating this Suspense boundary.',
]);
expect(getVisibleChildren(container)).toEqual(
<div>
<p>A</p>
Expand Down Expand Up @@ -4539,12 +4541,14 @@ describe('ReactDOMFizzServer', () => {

ReactDOMClient.hydrateRoot(container, <App text="replaced" />, {
onRecoverableError(error) {
Scheduler.log('Logged recoverable error: ' + error.message);
Scheduler.log(
'Logged recoverable error: ' + normalizeError(error.message),
);
},
});
await waitForAll([
'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.',
"Logged recoverable error: Hydration failed because the server rendered HTML didn't match the client.",
'Logged recoverable error: There was an error while hydrating this Suspense boundary.',
]);

expect(getVisibleChildren(container)).toEqual(
Expand All @@ -4556,21 +4560,7 @@ describe('ReactDOMFizzServer', () => {
);

await waitForAll([]);
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);
}
expect(mockError.mock.calls.length).toBe(0);
} finally {
console.error = originalConsoleError;
}
Expand Down Expand Up @@ -4626,12 +4616,14 @@ describe('ReactDOMFizzServer', () => {

ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.log('Logged recoverable error: ' + error.message);
Scheduler.log(
'Logged recoverable error: ' + normalizeError(error.message),
);
},
});
await waitForAll([
'Logged recoverable error: uh oh',
'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.',
'Logged recoverable error: There was an error while hydrating this Suspense boundary.',
]);

expect(getVisibleChildren(container)).toEqual(
Expand Down Expand Up @@ -4713,7 +4705,9 @@ describe('ReactDOMFizzServer', () => {

ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.log('Logged recoverable error: ' + error.message);
Scheduler.log(
'Logged recoverable error: ' + normalizeError(error.message),
);
},
});
await waitForAll([
Expand All @@ -4722,7 +4716,7 @@ describe('ReactDOMFizzServer', () => {
// onRecoverableError because the UI recovered without surfacing the
// error to the user.
'Logged recoverable error: first error',
'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.',
'Logged recoverable error: There was an error while hydrating this Suspense boundary.',
]);
expect(mockError.mock.calls).toEqual([]);
mockError.mockClear();
Expand Down Expand Up @@ -4830,7 +4824,9 @@ describe('ReactDOMFizzServer', () => {

ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.log('Logged recoverable error: ' + error.message);
Scheduler.log(
'Logged recoverable error: ' + normalizeError(error.message),
);
},
});
await waitForAll(['suspending']);
Expand All @@ -4847,7 +4843,7 @@ describe('ReactDOMFizzServer', () => {
await waitForAll([
'throwing: first error',
'Logged recoverable error: first error',
'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.',
'Logged recoverable error: There was an error while hydrating this Suspense boundary.',
]);
expect(getVisibleChildren(container)).toEqual(
<div>
Expand Down Expand Up @@ -4954,14 +4950,16 @@ describe('ReactDOMFizzServer', () => {

ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
Scheduler.log('Logged recoverable error: ' + error.message);
Scheduler.log(
'Logged recoverable error: ' + normalizeError(error.message),
);
},
});
await waitForAll([
'throwing: first error',
'suspending',
'Logged recoverable error: first error',
'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.',
'Logged recoverable error: There was an error while hydrating this Suspense boundary.',
]);
expect(mockError.mock.calls).toEqual([]);
mockError.mockClear();
Expand Down Expand Up @@ -6341,13 +6339,7 @@ describe('ReactDOMFizzServer', () => {
});
await expect(async () => {
await waitForAll([]);
}).toErrorDev(
[
'Expected server HTML to contain a matching <span> in the root',
'An error occurred during hydration',
],
{withoutStack: 1},
);
}).toErrorDev(['An error occurred during hydration'], {withoutStack: 1});
expect(errors.length).toEqual(2);
expect(getVisibleChildren(container)).toEqual(<span />);
});
Expand Down
Loading

0 comments on commit f7aa5e0

Please sign in to comment.