Skip to content

Commit 3ad9c5f

Browse files
committed
Simplify ReactFiberHydrationWarning in react-reconciler
Stopped printing non-hydratable nodes in the diff, simplified the ReactFiberHydrationWarning and ReactDOMHostConfig code. Removed ReactFiberHostConfig functions: ``` getAllChildHostInstances, getHostInstanceDisplayStringForHydrationWarning, getHostInstanceClosingDisplayStringForHydrationWarning, isHydratableInstance, ``` Added ReactFiberHostConfig functions: ``` getHostInstanceDisplayName, getHostInstanceProps, ``` Removed type HydratableInstance, it everywhere was aliased to Instance | TextInstance, and was used as such alias, but in the custom renderer it used an opaque type, and this failed the typechecks because the aliasing did not happen. Alternative solution would be to alias HydratableInstance in the custom renderer's ReactFiberHostConfig.custom to align with other renderers. I don't see any potential for HydratableInstance to be something else than Instance | TextInstance, so I chose to inline the alias. Removed type HostInstance, not needed anymore.
1 parent 81834b6 commit 3ad9c5f

File tree

9 files changed

+205
-235
lines changed

9 files changed

+205
-235
lines changed

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,6 @@ describe('ReactMount', () => {
313313
'Warning: Expected server HTML to contain a matching <h2> in <div>.\n\n' +
314314
' <div>\n' +
315315
" {'nested'}\n" +
316-
' <!-- -->\n' +
317316
" {' '}\n" +
318317
'- <h1>children <b>text</b></h1>\n' +
319318
'+ <h2>children <b>text</b></h2>\n' +
@@ -385,7 +384,6 @@ describe('ReactMount', () => {
385384
'Warning: Expected server HTML to contain a matching <h2> in <div>.\n\n' +
386385
' <div>\n' +
387386
" {'nested'}\n" +
388-
' <!-- -->\n' +
389387
" {' '}\n" +
390388
' <h1>children <b>text</b></h1>\n' +
391389
' <div data-ssr-mismatch-padding="1" />\n' +
@@ -455,7 +453,6 @@ describe('ReactMount', () => {
455453
"Warning: Expected server HTML to contain a matching text node for {'extra text node'} in <div>.\n\n" +
456454
' <div>\n' +
457455
" {'nested'}\n" +
458-
' <!-- -->\n' +
459456
" {' '}\n" +
460457
' <h1>children <b>text</b></h1>\n' +
461458
' <div data-ssr-mismatch-padding="1" />\n' +
@@ -999,7 +996,6 @@ describe('ReactMount', () => {
999996
).toWarnDev(
1000997
'Warning: Expected server HTML to contain a matching <span> in <div>.\n\n' +
1001998
' <div>\n' +
1002-
' <!-- a comment -->\n' +
1003999
'+ <span />\n' +
10041000
' </div>\n\n' +
10051001
' in span (at **)\n' +
@@ -1022,7 +1018,6 @@ describe('ReactMount', () => {
10221018
).toWarnDev(
10231019
'Warning: Expected server HTML to contain a matching <span> in <div>.\n\n' +
10241020
' <div>\n' +
1025-
' <!-- a comment <-- --&gt; > -->\n' +
10261021
'- <div />\n' +
10271022
'+ <span />\n' +
10281023
' </div>\n\n' +
@@ -1045,7 +1040,6 @@ describe('ReactMount', () => {
10451040
expect(() => ReactDOM.hydrate(<div />, xml)).toWarnDev(
10461041
'Warning: Expected server HTML to contain a matching <div> in <xml>.\n\n' +
10471042
' <xml>\n' +
1048-
' <?dom-processing-instruction content &gt; ?>\n' +
10491043
'+ <div />\n' +
10501044
' </xml>\n\n' +
10511045
' in div (at **)',

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

Lines changed: 12 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import {
3131
DOCUMENT_NODE,
3232
DOCUMENT_FRAGMENT_NODE,
3333
} from '../shared/HTMLNodeType';
34-
import omittedCloseTags from '../shared/omittedCloseTags';
3534

3635
export type Type = string;
3736
export type Props = {
@@ -44,8 +43,6 @@ export type Props = {
4443
export type Container = Element | Document;
4544
export type Instance = Element;
4645
export type TextInstance = Text;
47-
export type HydratableInstance = Instance | TextInstance;
48-
export type HostInstance = HydratableInstance | Container | Node;
4946
export type PublicInstance = Element | Text;
5047
type HostContextDev = {
5148
namespace: string,
@@ -403,31 +400,16 @@ export function removeChildFromContainer(
403400

404401
export const supportsHydration = true;
405402

406-
export function getAllChildHostInstances(
407-
parentInstance: HostInstance,
408-
): HostInstance[] {
409-
return [].slice.call(parentInstance.childNodes);
410-
}
411-
412-
export function getHostInstanceDisplayName(instance: HostInstance): string {
403+
export function getHostInstanceDisplayName(
404+
instance: Container | Instance,
405+
): string {
413406
return instance.nodeName.toLowerCase();
414407
}
415408

416-
export function getHostInstanceDisplayStringForHydrationWarning(
417-
instance: HostInstance,
418-
printElementOrText: (
419-
instanceDisplayName: string | null,
420-
instanceProps: Object | null,
421-
instanceTextOrHTML: string | {__html: any} | null,
422-
) => string,
423-
): string {
424-
const nodeType = instance.nodeType;
425-
if (nodeType === COMMENT_NODE) {
426-
return '<!--' + instance.textContent.replace(/-->/g, '--&gt;') + '-->';
427-
} else if (nodeType === TEXT_NODE) {
428-
// TODO: `instance.textContent` or `instance.nodeValue`?
429-
return printElementOrText(null, null, instance.textContent);
430-
} else if (nodeType === ELEMENT_NODE) {
409+
export function getHostInstanceProps(
410+
instance: Container | Instance,
411+
): Object | null {
412+
if (instance.nodeType === ELEMENT_NODE) {
431413
let instanceProps: Object | null = null;
432414
if (instance instanceof Element) {
433415
instanceProps = {};
@@ -437,62 +419,14 @@ export function getHostInstanceDisplayStringForHydrationWarning(
437419
instanceProps[attrs[i].name] = attrs[i].value;
438420
}
439421
}
440-
let canHaveChildNodes = !omittedCloseTags.hasOwnProperty(
441-
instance.nodeName.toLowerCase(),
442-
);
443-
let hasElementOrTextChildNodes = false;
444-
let hasElementChildNodes = false;
445-
if (canHaveChildNodes) {
446-
const ic = instance.childNodes.length;
447-
for (let i = 0; i < ic; ++i) {
448-
if (instance.childNodes[i].nodeType === ELEMENT_NODE) {
449-
hasElementOrTextChildNodes = true;
450-
hasElementChildNodes = true;
451-
break;
452-
} else if (instance.childNodes[i].nodeType === TEXT_NODE) {
453-
hasElementOrTextChildNodes = true;
454-
}
455-
}
456-
}
457-
const instanceTextOrHTML =
458-
canHaveChildNodes && hasElementOrTextChildNodes
459-
? instance instanceof Element && hasElementChildNodes
460-
? {__html: instance.innerHTML}
461-
: instance.textContent
462-
: null;
463-
return printElementOrText(
464-
getHostInstanceDisplayName(instance),
465-
instanceProps,
466-
instanceTextOrHTML,
467-
);
468-
} else {
469-
// In normal circumstances, we should not reach here.
470-
// But while looping over all host nodes in getHydrationDiff, an unexpected nodeType can show up.
471-
// Below is a safety net for when we have to display a Node that's not an element, text, or comment.
472-
// For example, a PROCESSING_INSTRUCTION_NODE.
473-
return (
474-
'<?' +
475-
instance.nodeName.replace(/>/g, '&gt;') +
476-
' ' +
477-
instance.textContent.replace(/>/g, '&gt;') +
478-
'?>'
479-
);
422+
return instanceProps;
480423
}
424+
return null;
481425
}
482426

483-
export function getHostInstanceClosingDisplayStringForHydrationWarning(
484-
instance: HostInstance,
485-
printElementClosingTag: (instanceDisplayName: string) => string,
486-
): string {
487-
return printElementClosingTag(getHostInstanceDisplayName(instance));
488-
}
489-
490-
export function isHydratableInstance(instance: HostInstance): boolean {
491-
// Same logic in getNextHydratableSibling, getFirstHydratableChild.
492-
return instance.nodeType === ELEMENT_NODE || instance.nodeType === TEXT_NODE;
493-
}
494-
495-
export function isTextInstance(instance: HostInstance): boolean {
427+
export function isTextInstance(
428+
instance: Container | Instance | TextInstance,
429+
): boolean {
496430
return instance.nodeType === TEXT_NODE;
497431
}
498432

packages/react-native-renderer/src/ReactFabricHostConfig.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,6 @@ export type Instance = {
6161
export type TextInstance = {
6262
node: Node,
6363
};
64-
export type HydratableInstance = Instance | TextInstance;
65-
export type HostInstance = HydratableInstance | Container;
6664
export type PublicInstance = ReactFabricHostComponent;
6765
export type ChildSet = Object;
6866
export type HostContext = $ReadOnly<{|

packages/react-native-renderer/src/ReactNativeHostConfig.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,6 @@ export type Instance = {
3434
viewConfig: ReactNativeBaseComponentViewConfig<>,
3535
};
3636
export type TextInstance = number;
37-
export type HydratableInstance = Instance | TextInstance;
38-
export type HostInstance = HydratableInstance | Container;
3937
export type PublicInstance = Instance;
4038
export type HostContext = $ReadOnly<{|
4139
isInAParentText: boolean,

packages/react-reconciler/src/ReactFiberHydrationContext.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import type {Fiber} from './ReactFiber';
1111
import type {
1212
Instance,
1313
TextInstance,
14-
HydratableInstance,
1514
Container,
1615
HostContext,
1716
} from './ReactFiberHostConfig';
@@ -46,7 +45,7 @@ import {
4645
// The deepest Fiber on the stack involved in a hydration context.
4746
// This may have been an insertion or a hydration.
4847
let hydrationParentFiber: null | Fiber = null;
49-
let nextHydratableInstance: null | HydratableInstance = null;
48+
let nextHydratableInstance: null | Instance | TextInstance = null;
5049
let isHydrating: boolean = false;
5150

5251
function enterHydrationState(fiber: Fiber): boolean {
@@ -63,7 +62,7 @@ function enterHydrationState(fiber: Fiber): boolean {
6362

6463
function deleteHydratableInstance(
6564
returnFiber: Fiber,
66-
instance: HydratableInstance,
65+
instance: Instance | TextInstance,
6766
) {
6867
if (__DEV__) {
6968
switch (returnFiber.tag) {

0 commit comments

Comments
 (0)