@samitouri / QOS-React-2 / commits / 825c3021f0

Don't delete trailing mismatches during hydration at the root (#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

Sebastian Markbåge committed Mar 17, 2021 at 11:18 UTC 825c3021f0477bf8bd4b57693f3303078e7c77db
15 files changed +159 -63
packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
+13 -16
@@ -19,6 +19,7 @@ let Suspense;
19 let textCache;
20 let document;
21 let writable;
22 +let container;
23 let buffer = '';
24 let hasErrored = false;
25 let fatalError = undefined;
@@ -38,10 +39,14 @@ describe('ReactDOMFizzServer', () => {
39 textCache = new Map();
40
41 // 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 + );
48 document = jsdom.window.document;
49 + container = document.getElementById('container');
50
51 buffer = '';
52 hasErrored = false;
@@ -80,9 +85,9 @@ describe('ReactDOMFizzServer', () => {
85 const script = document.createElement('script');
86 script.textContent = node.textContent;
87 fakeBody.removeChild(node);
83 - document.body.appendChild(script);
88 + container.appendChild(script);
89 } else {
85 - document.body.appendChild(node);
90 + container.appendChild(node);
91 }
92 }
93 }
@@ -200,11 +205,11 @@ describe('ReactDOMFizzServer', () => {
205 writable,
206 );
207 });
203 - expect(getVisibleChildren(document.body)).toEqual(<div>Loading...</div>);
208 + expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);
209 await act(async () => {
210 resolveText('Hello World');
211 });
207 - expect(getVisibleChildren(document.body)).toEqual(<div>Hello World</div>);
212 + expect(getVisibleChildren(container)).toEqual(<div>Hello World</div>);
213 });
214
215 // @gate experimental
@@ -224,20 +229,12 @@ describe('ReactDOMFizzServer', () => {
229 }
230
231 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);
233 });
234
235 // We're still showing a fallback.
236
237 // Attempt to hydrate the content.
240 - const container = document.body.firstChild;
238 const root = ReactDOM.unstable_createRoot(container, {hydrate: true});
239 root.render(<App />);
240 Scheduler.unstable_flushAll();
packages/react-dom/src/__tests__/ReactDOMServerIntegrationFragment-test.js
+9 -1
@@ -103,7 +103,15 @@ describe('ReactDOMServerIntegration', () => {
103 });
104
105 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);
115 });
116 });
117 });
packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js
+18 -10
@@ -1654,9 +1654,13 @@ describe('ReactDOMServerHooks', () => {
1654 // This is the wrong HTML string
1655 container.innerHTML = '<span></span>';
1656 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 + );
1664 });
1665
1666 // @gate experimental
@@ -1740,9 +1744,13 @@ describe('ReactDOMServerHooks', () => {
1744 // This is the wrong HTML string
1745 container.innerHTML = '<span></span>';
1746 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 + );
1754 });
1755
1756 // @gate experimental
@@ -1764,7 +1772,7 @@ describe('ReactDOMServerHooks', () => {
1772 expect(() => Scheduler.unstable_flushAll()).toErrorDev(
1773 [
1774 '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>.',
1776 ],
1777 {withoutStack: 1},
1778 );
@@ -1789,7 +1797,7 @@ describe('ReactDOMServerHooks', () => {
1797 expect(() => Scheduler.unstable_flushAll()).toErrorDev(
1798 [
1799 '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>.',
1801 ],
1802 {withoutStack: 1},
1803 );
@@ -1813,7 +1821,7 @@ describe('ReactDOMServerHooks', () => {
1821 expect(() => Scheduler.unstable_flushAll()).toErrorDev(
1822 [
1823 '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>.',
1825 ],
1826 {withoutStack: 1},
1827 );
@@ -1834,7 +1842,7 @@ describe('ReactDOMServerHooks', () => {
1842 expect(() => Scheduler.unstable_flushAll()).toErrorDev(
1843 [
1844 '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>.',
1846 ],
1847 {withoutStack: 1},
1848 );
packages/react-dom/src/__tests__/ReactDOMServerIntegrationModes-test.js
+9 -1
@@ -153,7 +153,15 @@ describe('ReactDOMServerIntegration', () => {
153 });
154
155 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);
165 });
166 });
167 });
packages/react-dom/src/__tests__/ReactMount-test.js
+3 -7
@@ -123,16 +123,12 @@ describe('ReactMount', () => {
123 expect(instance1 === instance2).toBe(true);
124 });
125
126 - it('should warn if mounting into left padded rendered markup', () => {
126 + it('does not warn if mounting into left padded rendered markup', () => {
127 const container = document.createElement('container');
128 container.innerHTML = ReactDOMServer.renderToString(<div />) + ' ';
129
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);
132 });
133
134 it('should warn if mounting into right padded rendered markup', () => {
packages/react-dom/src/__tests__/ReactRenderDocument-test.js
+20 -1
@@ -368,12 +368,31 @@ describe('rendering React components at document', () => {
368 expect(testDocument.body.innerHTML).toBe('Hello world');
369 });
370
371 - it('renders over an existing text child without throwing', () => {
371 + it('cannot render over an existing text child at the root', () => {
372 const container = document.createElement('div');
373 container.textContent = 'potato';
374 expect(() => ReactDOM.hydrate(<div>parsnip</div>, container)).toErrorDev(
375 'Expected server HTML to contain a matching <div> in <div>.',
376 );
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 + );
396 expect(container.textContent).toBe('parsnip');
397 });
398
packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js
+28 -7
@@ -510,26 +510,47 @@ describe('ReactDOMServerHydration', () => {
510
511 it('Suspense + hydration in legacy mode', () => {
512 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;
515 const ref = React.createRef();
516 expect(() =>
517 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>,
523 element,
524 ),
525 ).toErrorDev(
526 'Warning: Did not expect server HTML to contain a <div> in <div>.',
525 - {withoutStack: true},
527 );
528
529 // The content should've been client rendered and replaced the
530 // existing div.
531 expect(ref.current).not.toBe(div);
532 // 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 + );
554 });
555
556 it('Suspense + hydration in legacy mode with no fallback', () => {
packages/react-dom/src/client/ReactDOMHostConfig.js
+15
@@ -894,6 +894,12 @@ export function commitHydratedSuspenseInstance(
894 retryIfBlockedOn(suspenseInstance);
895 }
896
897 +export function shouldDeleteUnhydratedTailInstances(
898 + parentType: string,
899 +): boolean {
900 + return parentType !== 'head' || parentType !== 'body';
901 +}
902 +
903 export function didNotMatchHydratedContainerTextInstance(
904 parentContainer: Container,
905 textInstance: TextInstance,
@@ -1008,6 +1014,15 @@ export function didNotFindHydratableSuspenseInstance(
1014 }
1015 }
1016
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 +
1026 export function getInstanceFromNode(node: HTMLElement): null | Object {
1027 return getClosestInstanceFromNode(node) || null;
1028 }
packages/react-reconciler/src/ReactFiberHostConfigWithNoHydration.js
+2
@@ -40,6 +40,7 @@ export const commitHydratedContainer = shim;
40 export const commitHydratedSuspenseInstance = shim;
41 export const clearSuspenseBoundary = shim;
42 export const clearSuspenseBoundaryFromContainer = shim;
43 +export const shouldDeleteUnhydratedTailInstances = shim;
44 export const didNotMatchHydratedContainerTextInstance = shim;
45 export const didNotMatchHydratedTextInstance = shim;
46 export const didNotHydrateContainerInstance = shim;
@@ -50,3 +51,4 @@ export const didNotFindHydratableContainerSuspenseInstance = shim;
51 export const didNotFindHydratableInstance = shim;
52 export const didNotFindHydratableTextInstance = shim;
53 export const didNotFindHydratableSuspenseInstance = shim;
54 +export const errorHydratingContainer = shim;
packages/react-reconciler/src/ReactFiberHydrationContext.new.js
+6 -8
@@ -43,6 +43,7 @@ import {
43 hydrateTextInstance,
44 hydrateSuspenseInstance,
45 getNextHydratableInstanceAfterSuspenseInstance,
46 + shouldDeleteUnhydratedTailInstances,
47 didNotMatchHydratedContainerTextInstance,
48 didNotMatchHydratedTextInstance,
49 didNotHydrateContainerInstance,
@@ -438,18 +439,15 @@ function popHydrationState(fiber: Fiber): boolean {
439 return false;
440 }
441
441 - const type = fiber.type;
442 -
442 // If we have any remaining hydratable nodes, we need to delete them now.
443 // We only do this deeper than head and body since they tend to have random
444 // 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.
446 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)))
451 ) {
452 let nextInstance = nextHydratableInstance;
453 while (nextInstance) {
packages/react-reconciler/src/ReactFiberHydrationContext.old.js
+6 -8
@@ -43,6 +43,7 @@ import {
43 hydrateTextInstance,
44 hydrateSuspenseInstance,
45 getNextHydratableInstanceAfterSuspenseInstance,
46 + shouldDeleteUnhydratedTailInstances,
47 didNotMatchHydratedContainerTextInstance,
48 didNotMatchHydratedTextInstance,
49 didNotHydrateContainerInstance,
@@ -438,18 +439,15 @@ function popHydrationState(fiber: Fiber): boolean {
439 return false;
440 }
441
441 - const type = fiber.type;
442 -
442 // If we have any remaining hydratable nodes, we need to delete them now.
443 // We only do this deeper than head and body since they tend to have random
444 // 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.
446 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)))
451 ) {
452 let nextInstance = nextHydratableInstance;
453 while (nextInstance) {
packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+7
@@ -88,6 +88,7 @@ import {
88 clearContainer,
89 getCurrentEventPriority,
90 supportsMicrotasks,
91 + errorHydratingContainer,
92 } from './ReactFiberHostConfig';
93
94 import {
@@ -782,6 +783,9 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
783 // discard server response and fall back to client side render.
784 if (root.hydrate) {
785 root.hydrate = false;
786 + if (__DEV__) {
787 + errorHydratingContainer(root.containerInfo);
788 + }
789 clearContainer(root.containerInfo);
790 }
791
@@ -992,6 +996,9 @@ function performSyncWorkOnRoot(root) {
996 // discard server response and fall back to client side render.
997 if (root.hydrate) {
998 root.hydrate = false;
999 + if (__DEV__) {
1000 + errorHydratingContainer(root.containerInfo);
1001 + }
1002 clearContainer(root.containerInfo);
1003 }
1004
packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+7
@@ -88,6 +88,7 @@ import {
88 clearContainer,
89 getCurrentEventPriority,
90 supportsMicrotasks,
91 + errorHydratingContainer,
92 } from './ReactFiberHostConfig';
93
94 import {
@@ -782,6 +783,9 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
783 // discard server response and fall back to client side render.
784 if (root.hydrate) {
785 root.hydrate = false;
786 + if (__DEV__) {
787 + errorHydratingContainer(root.containerInfo);
788 + }
789 clearContainer(root.containerInfo);
790 }
791
@@ -992,6 +996,9 @@ function performSyncWorkOnRoot(root) {
996 // discard server response and fall back to client side render.
997 if (root.hydrate) {
998 root.hydrate = false;
999 + if (__DEV__) {
1000 + errorHydratingContainer(root.containerInfo);
1001 + }
1002 clearContainer(root.containerInfo);
1003 }
1004
packages/react-reconciler/src/__tests__/useMutableSourceHydration-test.js
+13 -4
@@ -214,7 +214,8 @@ describe('useMutableSourceHydration', () => {
214 source.value = 'two';
215 });
216 }).toErrorDev(
217 - 'Warning: Did not expect server HTML to contain a <div> in <div>.',
217 + 'Warning: An error occurred during hydration. ' +
218 + 'The server HTML was replaced with client content in <div>.',
219 {withoutStack: true},
220 );
221 expect(Scheduler).toHaveYielded(['only:two']);
@@ -266,7 +267,8 @@ describe('useMutableSourceHydration', () => {
267 source.value = 'two';
268 });
269 }).toErrorDev(
269 - 'Warning: Did not expect server HTML to contain a <div> in <div>.',
270 + 'Warning: An error occurred during hydration. ' +
271 + 'The server HTML was replaced with client content in <div>.',
272 {withoutStack: true},
273 );
274 expect(Scheduler).toHaveYielded(['a:two', 'b:two']);
@@ -334,7 +336,8 @@ describe('useMutableSourceHydration', () => {
336 source.valueB = 'b:two';
337 });
338 }).toErrorDev(
337 - 'Warning: Did not expect server HTML to contain a <div> in <div>.',
339 + 'Warning: An error occurred during hydration. ' +
340 + 'The server HTML was replaced with client content in <div>.',
341 {withoutStack: true},
342 );
343 expect(Scheduler).toHaveYielded(['0:a:one', '1:b:two']);
@@ -401,7 +404,13 @@ describe('useMutableSourceHydration', () => {
404 source.value = 'two';
405 });
406 }).toErrorDev(
404 - 'Warning: Text content did not match. Server: "1" Client: "2"',
407 + [
408 + 'Warning: An error occurred during hydration. ' +
409 + 'The server HTML was replaced with client content in <div>.',
410 +
411 + 'Warning: Text content did not match. Server: "1" Client: "2"',
412 + ],
413 + {withoutStack: 1},
414 );
415 expect(Scheduler).toHaveYielded([2, 'a:two']);
416 expect(source.listenerCount).toBe(1);
packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js
+3
@@ -156,6 +156,8 @@ export const commitHydratedSuspenseInstance =
156 export const clearSuspenseBoundary = $$$hostConfig.clearSuspenseBoundary;
157 export const clearSuspenseBoundaryFromContainer =
158 $$$hostConfig.clearSuspenseBoundaryFromContainer;
159 +export const shouldDeleteUnhydratedTailInstances =
160 + $$$hostConfig.shouldDeleteUnhydratedTailInstances;
161 export const didNotMatchHydratedContainerTextInstance =
162 $$$hostConfig.didNotMatchHydratedContainerTextInstance;
163 export const didNotMatchHydratedTextInstance =
@@ -175,3 +177,4 @@ export const didNotFindHydratableTextInstance =
177 $$$hostConfig.didNotFindHydratableTextInstance;
178 export const didNotFindHydratableSuspenseInstance =
179 $$$hostConfig.didNotFindHydratableSuspenseInstance;
180 +export const errorHydratingContainer = $$$hostConfig.errorHydratingContainer;