Skip to content

Commit fc05420

Browse files
sebmarkbagekoto
sebmarkbage
authored and
koto
committed
Don't delete trailing mismatches during hydration at the root (facebook#21021)
* Don't delete any trailing nodes in the container during hydration error * Warn when an error during hydration causes us to clear the container * Encode unfortunate case in test * Wrap the root for tests that are applicable to nested cases * Now we can pipe Fizz into a container * Grammatical fix
1 parent a1ae068 commit fc05420

15 files changed

+159
-63
lines changed

‎packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ let Suspense;
1919
let textCache;
2020
let document;
2121
let writable;
22+
let container;
2223
let buffer = '';
2324
let hasErrored = false;
2425
let fatalError = undefined;
@@ -38,10 +39,14 @@ describe('ReactDOMFizzServer', () => {
3839
textCache = new Map();
3940

4041
// Test Environment
41-
const jsdom = new JSDOM('<!DOCTYPE html><html><head></head><body>', {
42-
runScripts: 'dangerously',
43-
});
42+
const jsdom = new JSDOM(
43+
'<!DOCTYPE html><html><head></head><body><div id="container">',
44+
{
45+
runScripts: 'dangerously',
46+
},
47+
);
4448
document = jsdom.window.document;
49+
container = document.getElementById('container');
4550

4651
buffer = '';
4752
hasErrored = false;
@@ -80,9 +85,9 @@ describe('ReactDOMFizzServer', () => {
8085
const script = document.createElement('script');
8186
script.textContent = node.textContent;
8287
fakeBody.removeChild(node);
83-
document.body.appendChild(script);
88+
container.appendChild(script);
8489
} else {
85-
document.body.appendChild(node);
90+
container.appendChild(node);
8691
}
8792
}
8893
}
@@ -200,11 +205,11 @@ describe('ReactDOMFizzServer', () => {
200205
writable,
201206
);
202207
});
203-
expect(getVisibleChildren(document.body)).toEqual(<div>Loading...</div>);
208+
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);
204209
await act(async () => {
205210
resolveText('Hello World');
206211
});
207-
expect(getVisibleChildren(document.body)).toEqual(<div>Hello World</div>);
212+
expect(getVisibleChildren(container)).toEqual(<div>Hello World</div>);
208213
});
209214

210215
// @gate experimental
@@ -224,20 +229,12 @@ describe('ReactDOMFizzServer', () => {
224229
}
225230

226231
await act(async () => {
227-
ReactDOMFizzServer.pipeToNodeWritable(
228-
// We currently have to wrap the server node in a container because
229-
// otherwise the Fizz nodes get deleted during hydration.
230-
<div id="container">
231-
<App />
232-
</div>,
233-
writable,
234-
);
232+
ReactDOMFizzServer.pipeToNodeWritable(<App />, writable);
235233
});
236234

237235
// We're still showing a fallback.
238236

239237
// Attempt to hydrate the content.
240-
const container = document.body.firstChild;
241238
const root = ReactDOM.unstable_createRoot(container, {hydrate: true});
242239
root.render(<App />);
243240
Scheduler.unstable_flushAll();

‎packages/react-dom/src/__tests__/ReactDOMServerIntegrationFragment-test.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,15 @@ describe('ReactDOMServerIntegration', () => {
103103
});
104104

105105
itRenders('an empty fragment', async render => {
106-
expect(await render(<React.Fragment />)).toBe(null);
106+
expect(
107+
(
108+
await render(
109+
<div>
110+
<React.Fragment />
111+
</div>,
112+
)
113+
).firstChild,
114+
).toBe(null);
107115
});
108116
});
109117
});

‎packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1654,9 +1654,13 @@ describe('ReactDOMServerHooks', () => {
16541654
// This is the wrong HTML string
16551655
container.innerHTML = '<span></span>';
16561656
ReactDOM.unstable_createRoot(container, {hydrate: true}).render(<App />);
1657-
expect(() => Scheduler.unstable_flushAll()).toErrorDev([
1658-
'Warning: Expected server HTML to contain a matching <div> in <div>.',
1659-
]);
1657+
expect(() => Scheduler.unstable_flushAll()).toErrorDev(
1658+
[
1659+
'Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>.',
1660+
'Warning: Expected server HTML to contain a matching <div> in <div>.',
1661+
],
1662+
{withoutStack: 1},
1663+
);
16601664
});
16611665

16621666
// @gate experimental
@@ -1740,9 +1744,13 @@ describe('ReactDOMServerHooks', () => {
17401744
// This is the wrong HTML string
17411745
container.innerHTML = '<span></span>';
17421746
ReactDOM.unstable_createRoot(container, {hydrate: true}).render(<App />);
1743-
expect(() => Scheduler.unstable_flushAll()).toErrorDev([
1744-
'Warning: Expected server HTML to contain a matching <div> in <div>.',
1745-
]);
1747+
expect(() => Scheduler.unstable_flushAll()).toErrorDev(
1748+
[
1749+
'Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>.',
1750+
'Warning: Expected server HTML to contain a matching <div> in <div>.',
1751+
],
1752+
{withoutStack: 1},
1753+
);
17461754
});
17471755

17481756
// @gate experimental
@@ -1764,7 +1772,7 @@ describe('ReactDOMServerHooks', () => {
17641772
expect(() => Scheduler.unstable_flushAll()).toErrorDev(
17651773
[
17661774
'Warning: The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. Do not read the value directly.',
1767-
'Warning: Did not expect server HTML to contain a <span> in <div>.',
1775+
'Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>.',
17681776
],
17691777
{withoutStack: 1},
17701778
);
@@ -1789,7 +1797,7 @@ describe('ReactDOMServerHooks', () => {
17891797
expect(() => Scheduler.unstable_flushAll()).toErrorDev(
17901798
[
17911799
'Warning: The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. Do not read the value directly.',
1792-
'Warning: Did not expect server HTML to contain a <span> in <div>.',
1800+
'Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>.',
17931801
],
17941802
{withoutStack: 1},
17951803
);
@@ -1813,7 +1821,7 @@ describe('ReactDOMServerHooks', () => {
18131821
expect(() => Scheduler.unstable_flushAll()).toErrorDev(
18141822
[
18151823
'Warning: The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. Do not read the value directly.',
1816-
'Warning: Did not expect server HTML to contain a <div> in <div>.',
1824+
'Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>.',
18171825
],
18181826
{withoutStack: 1},
18191827
);
@@ -1834,7 +1842,7 @@ describe('ReactDOMServerHooks', () => {
18341842
expect(() => Scheduler.unstable_flushAll()).toErrorDev(
18351843
[
18361844
'Warning: The object passed back from useOpaqueIdentifier is meant to be passed through to attributes only. Do not read the value directly.',
1837-
'Warning: Did not expect server HTML to contain a <div> in <div>.',
1845+
'Warning: An error occurred during hydration. The server HTML was replaced with client content in <div>.',
18381846
],
18391847
{withoutStack: 1},
18401848
);

‎packages/react-dom/src/__tests__/ReactDOMServerIntegrationModes-test.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,15 @@ describe('ReactDOMServerIntegration', () => {
153153
});
154154

155155
itRenders('an empty strict mode', async render => {
156-
expect(await render(<React.StrictMode />)).toBe(null);
156+
expect(
157+
(
158+
await render(
159+
<div>
160+
<React.StrictMode />
161+
</div>,
162+
)
163+
).firstChild,
164+
).toBe(null);
157165
});
158166
});
159167
});

‎packages/react-dom/src/__tests__/ReactMount-test.js

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,16 +123,12 @@ describe('ReactMount', () => {
123123
expect(instance1 === instance2).toBe(true);
124124
});
125125

126-
it('should warn if mounting into left padded rendered markup', () => {
126+
it('does not warn if mounting into left padded rendered markup', () => {
127127
const container = document.createElement('container');
128128
container.innerHTML = ReactDOMServer.renderToString(<div />) + ' ';
129129

130-
expect(() =>
131-
ReactDOM.hydrate(<div />, container),
132-
).toErrorDev(
133-
'Did not expect server HTML to contain the text node " " in <container>.',
134-
{withoutStack: true},
135-
);
130+
// This should probably ideally warn but we ignore extra markup at the root.
131+
ReactDOM.hydrate(<div />, container);
136132
});
137133

138134
it('should warn if mounting into right padded rendered markup', () => {

‎packages/react-dom/src/__tests__/ReactRenderDocument-test.js

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,12 +368,31 @@ describe('rendering React components at document', () => {
368368
expect(testDocument.body.innerHTML).toBe('Hello world');
369369
});
370370

371-
it('renders over an existing text child without throwing', () => {
371+
it('cannot render over an existing text child at the root', () => {
372372
const container = document.createElement('div');
373373
container.textContent = 'potato';
374374
expect(() => ReactDOM.hydrate(<div>parsnip</div>, container)).toErrorDev(
375375
'Expected server HTML to contain a matching <div> in <div>.',
376376
);
377+
// This creates an unfortunate double text case.
378+
expect(container.textContent).toBe('potatoparsnip');
379+
});
380+
381+
it('renders over an existing nested text child without throwing', () => {
382+
const container = document.createElement('div');
383+
const wrapper = document.createElement('div');
384+
wrapper.textContent = 'potato';
385+
container.appendChild(wrapper);
386+
expect(() =>
387+
ReactDOM.hydrate(
388+
<div>
389+
<div>parsnip</div>
390+
</div>,
391+
container,
392+
),
393+
).toErrorDev(
394+
'Expected server HTML to contain a matching <div> in <div>.',
395+
);
377396
expect(container.textContent).toBe('parsnip');
378397
});
379398

‎packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -510,26 +510,47 @@ describe('ReactDOMServerHydration', () => {
510510

511511
it('Suspense + hydration in legacy mode', () => {
512512
const element = document.createElement('div');
513-
element.innerHTML = '<div>Hello World</div>';
514-
const div = element.firstChild;
513+
element.innerHTML = '<div><div>Hello World</div></div>';
514+
const div = element.firstChild.firstChild;
515515
const ref = React.createRef();
516516
expect(() =>
517517
ReactDOM.hydrate(
518-
<React.Suspense fallback={null}>
519-
<div ref={ref}>Hello World</div>
520-
</React.Suspense>,
518+
<div>
519+
<React.Suspense fallback={null}>
520+
<div ref={ref}>Hello World</div>
521+
</React.Suspense>
522+
</div>,
521523
element,
522524
),
523525
).toErrorDev(
524526
'Warning: Did not expect server HTML to contain a <div> in <div>.',
525-
{withoutStack: true},
526527
);
527528

528529
// The content should've been client rendered and replaced the
529530
// existing div.
530531
expect(ref.current).not.toBe(div);
531532
// The HTML should be the same though.
532-
expect(element.innerHTML).toBe('<div>Hello World</div>');
533+
expect(element.innerHTML).toBe('<div><div>Hello World</div></div>');
534+
});
535+
536+
it('Suspense + hydration in legacy mode (at root)', () => {
537+
const element = document.createElement('div');
538+
element.innerHTML = '<div>Hello World</div>';
539+
const div = element.firstChild;
540+
const ref = React.createRef();
541+
ReactDOM.hydrate(
542+
<React.Suspense fallback={null}>
543+
<div ref={ref}>Hello World</div>
544+
</React.Suspense>,
545+
element,
546+
);
547+
548+
// The content should've been client rendered.
549+
expect(ref.current).not.toBe(div);
550+
// Unfortunately, since we don't delete the tail at the root, a duplicate will remain.
551+
expect(element.innerHTML).toBe(
552+
'<div>Hello World</div><div>Hello World</div>',
553+
);
533554
});
534555

535556
it('Suspense + hydration in legacy mode with no fallback', () => {

‎packages/react-dom/src/client/ReactDOMHostConfig.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -894,6 +894,12 @@ export function commitHydratedSuspenseInstance(
894894
retryIfBlockedOn(suspenseInstance);
895895
}
896896

897+
export function shouldDeleteUnhydratedTailInstances(
898+
parentType: string,
899+
): boolean {
900+
return parentType !== 'head' || parentType !== 'body';
901+
}
902+
897903
export function didNotMatchHydratedContainerTextInstance(
898904
parentContainer: Container,
899905
textInstance: TextInstance,
@@ -1008,6 +1014,15 @@ export function didNotFindHydratableSuspenseInstance(
10081014
}
10091015
}
10101016

1017+
export function errorHydratingContainer(parentContainer: Container): void {
1018+
if (__DEV__) {
1019+
console.error(
1020+
'An error occurred during hydration. The server HTML was replaced with client content in <%s>.',
1021+
parentContainer.nodeName.toLowerCase(),
1022+
);
1023+
}
1024+
}
1025+
10111026
export function getInstanceFromNode(node: HTMLElement): null | Object {
10121027
return getClosestInstanceFromNode(node) || null;
10131028
}

‎packages/react-reconciler/src/ReactFiberHostConfigWithNoHydration.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export const commitHydratedContainer = shim;
4040
export const commitHydratedSuspenseInstance = shim;
4141
export const clearSuspenseBoundary = shim;
4242
export const clearSuspenseBoundaryFromContainer = shim;
43+
export const shouldDeleteUnhydratedTailInstances = shim;
4344
export const didNotMatchHydratedContainerTextInstance = shim;
4445
export const didNotMatchHydratedTextInstance = shim;
4546
export const didNotHydrateContainerInstance = shim;
@@ -50,3 +51,4 @@ export const didNotFindHydratableContainerSuspenseInstance = shim;
5051
export const didNotFindHydratableInstance = shim;
5152
export const didNotFindHydratableTextInstance = shim;
5253
export const didNotFindHydratableSuspenseInstance = shim;
54+
export const errorHydratingContainer = shim;

‎packages/react-reconciler/src/ReactFiberHydrationContext.new.js

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
hydrateTextInstance,
4444
hydrateSuspenseInstance,
4545
getNextHydratableInstanceAfterSuspenseInstance,
46+
shouldDeleteUnhydratedTailInstances,
4647
didNotMatchHydratedContainerTextInstance,
4748
didNotMatchHydratedTextInstance,
4849
didNotHydrateContainerInstance,
@@ -438,18 +439,15 @@ function popHydrationState(fiber: Fiber): boolean {
438439
return false;
439440
}
440441

441-
const type = fiber.type;
442-
443442
// If we have any remaining hydratable nodes, we need to delete them now.
444443
// We only do this deeper than head and body since they tend to have random
445444
// other nodes in them. We also ignore components with pure text content in
446-
// side of them.
447-
// TODO: Better heuristic.
445+
// side of them. We also don't delete anything inside the root container.
448446
if (
449-
fiber.tag !== HostComponent ||
450-
(type !== 'head' &&
451-
type !== 'body' &&
452-
!shouldSetTextContent(type, fiber.memoizedProps))
447+
fiber.tag !== HostRoot &&
448+
(fiber.tag !== HostComponent ||
449+
(shouldDeleteUnhydratedTailInstances(fiber.type) &&
450+
!shouldSetTextContent(fiber.type, fiber.memoizedProps)))
453451
) {
454452
let nextInstance = nextHydratableInstance;
455453
while (nextInstance) {

‎packages/react-reconciler/src/ReactFiberHydrationContext.old.js

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
hydrateTextInstance,
4444
hydrateSuspenseInstance,
4545
getNextHydratableInstanceAfterSuspenseInstance,
46+
shouldDeleteUnhydratedTailInstances,
4647
didNotMatchHydratedContainerTextInstance,
4748
didNotMatchHydratedTextInstance,
4849
didNotHydrateContainerInstance,
@@ -438,18 +439,15 @@ function popHydrationState(fiber: Fiber): boolean {
438439
return false;
439440
}
440441

441-
const type = fiber.type;
442-
443442
// If we have any remaining hydratable nodes, we need to delete them now.
444443
// We only do this deeper than head and body since they tend to have random
445444
// other nodes in them. We also ignore components with pure text content in
446-
// side of them.
447-
// TODO: Better heuristic.
445+
// side of them. We also don't delete anything inside the root container.
448446
if (
449-
fiber.tag !== HostComponent ||
450-
(type !== 'head' &&
451-
type !== 'body' &&
452-
!shouldSetTextContent(type, fiber.memoizedProps))
447+
fiber.tag !== HostRoot &&
448+
(fiber.tag !== HostComponent ||
449+
(shouldDeleteUnhydratedTailInstances(fiber.type) &&
450+
!shouldSetTextContent(fiber.type, fiber.memoizedProps)))
453451
) {
454452
let nextInstance = nextHydratableInstance;
455453
while (nextInstance) {

0 commit comments

Comments
 (0)