@samitouri / QOS-React-2 / commits / 8fa41ffa27

Don't "fix up" mismatched text content with suppressedHydrationWarning (#26391)

In concurrent mode we error if child nodes mismatches which triggers a recreation of the whole hydration boundary. This ensures that we don't replay the wrong thing, transform state or other security issues. For text content, we respect `suppressedHydrationWarning` to allow for things like `<div suppressedHydrationWarning>{timestamp}</div>` to ignore the timestamp. This mode actually still patches up the text content to be the client rendered content. In principle we shouldn't have to do that because either value should be ok, and arguably it's better not to trigger layout thrash after the fact. We do have a lot of code still to deal with patching up the tree because that's what legacy mode does which is still in the code base. When we delete legacy mode we would still be stuck with a lot of it just to deal with this case. Therefore I propose that we change the semantics to not patch up hydration errors for text nodes. We already don't for attributes.

Sebastian Markbåge committed Mar 16, 2023 at 22:39 UTC 8fa41ffa275cae4895b650b0c3b5e8acdbb5055d
5 files changed +205 -198
packages/react-dom-bindings/src/client/ReactDOMComponent.js
+172 -181
@@ -893,11 +893,9 @@ export function diffHydratedProperties(
893 shouldWarnDev: boolean,
894 parentNamespaceDev: string,
895 ): null | Array<mixed> {
896 - let isCustomComponentTag;
896 let extraAttributeNames: Set<string>;
897
898 if (__DEV__) {
900 - isCustomComponentTag = isCustomComponent(tag, rawProps);
899 validatePropertiesInDevelopment(tag, rawProps);
900 }
901
@@ -963,6 +961,10 @@ export function diffHydratedProperties(
961 break;
962 }
963
964 + if (rawProps.hasOwnProperty('onScroll')) {
965 + listenToNonDelegatedEvent('scroll', domElement);
966 + }
967 +
968 assertValidProps(tag, rawProps);
969
970 if (__DEV__) {
@@ -988,207 +990,196 @@ export function diffHydratedProperties(
990 }
991
992 let updatePayload = null;
991 - for (const propKey in rawProps) {
992 - if (!rawProps.hasOwnProperty(propKey)) {
993 - continue;
994 - }
995 - const nextProp = rawProps[propKey];
996 - if (propKey === CHILDREN) {
997 - // For text content children we compare against textContent. This
998 - // might match additional HTML that is hidden when we read it using
999 - // textContent. E.g. "foo" will match "f<span>oo</span>" but that still
1000 - // satisfies our requirement. Our requirement is not to produce perfect
1001 - // HTML and attributes. Ideally we should preserve structure but it's
1002 - // ok not to if the visible content is still enough to indicate what
1003 - // even listeners these nodes might be wired up to.
1004 - // TODO: Warn if there is more than a single textNode as a child.
1005 - // TODO: Should we use domElement.firstChild.nodeValue to compare?
1006 - if (typeof nextProp === 'string') {
1007 - if (domElement.textContent !== nextProp) {
1008 - if (rawProps[SUPPRESS_HYDRATION_WARNING] !== true) {
1009 - checkForUnmatchedText(
1010 - domElement.textContent,
1011 - nextProp,
1012 - isConcurrentMode,
1013 - shouldWarnDev,
1014 - );
1015 - }
1016 - updatePayload = [CHILDREN, nextProp];
1017 - }
1018 - } else if (typeof nextProp === 'number') {
1019 - if (domElement.textContent !== '' + nextProp) {
1020 - if (rawProps[SUPPRESS_HYDRATION_WARNING] !== true) {
1021 - checkForUnmatchedText(
1022 - domElement.textContent,
1023 - nextProp,
1024 - isConcurrentMode,
1025 - shouldWarnDev,
1026 - );
1027 - }
1028 - updatePayload = [CHILDREN, '' + nextProp];
1029 - }
993 +
994 + const children = rawProps.children;
995 + // For text content children we compare against textContent. This
996 + // might match additional HTML that is hidden when we read it using
997 + // textContent. E.g. "foo" will match "f<span>oo</span>" but that still
998 + // satisfies our requirement. Our requirement is not to produce perfect
999 + // HTML and attributes. Ideally we should preserve structure but it's
1000 + // ok not to if the visible content is still enough to indicate what
1001 + // even listeners these nodes might be wired up to.
1002 + // TODO: Warn if there is more than a single textNode as a child.
1003 + // TODO: Should we use domElement.firstChild.nodeValue to compare?
1004 + if (typeof children === 'string' || typeof children === 'number') {
1005 + if (domElement.textContent !== '' + children) {
1006 + if (rawProps[SUPPRESS_HYDRATION_WARNING] !== true) {
1007 + checkForUnmatchedText(
1008 + domElement.textContent,
1009 + children,
1010 + isConcurrentMode,
1011 + shouldWarnDev,
1012 + );
1013 }
1031 - } else if (registrationNameDependencies.hasOwnProperty(propKey)) {
1032 - if (nextProp != null) {
1033 - if (__DEV__ && typeof nextProp !== 'function') {
1034 - warnForInvalidEventListener(propKey, nextProp);
1035 - }
1036 - if (propKey === 'onScroll') {
1037 - listenToNonDelegatedEvent('scroll', domElement);
1038 - }
1014 + if (!isConcurrentMode) {
1015 + updatePayload = [CHILDREN, children];
1016 }
1040 - } else if (
1041 - shouldWarnDev &&
1042 - __DEV__ &&
1043 - // Convince Flow we've calculated it (it's DEV-only in this method.)
1044 - typeof isCustomComponentTag === 'boolean'
1045 - ) {
1046 - // Validate that the properties correspond to their expected values.
1047 - let serverValue;
1048 - const propertyInfo =
1049 - isCustomComponentTag && enableCustomElementPropertySupport
1050 - ? null
1051 - : getPropertyInfo(propKey);
1052 - if (rawProps[SUPPRESS_HYDRATION_WARNING] === true) {
1053 - // Don't bother comparing. We're ignoring all these warnings.
1054 - } else if (
1055 - propKey === SUPPRESS_CONTENT_EDITABLE_WARNING ||
1056 - propKey === SUPPRESS_HYDRATION_WARNING ||
1057 - // Controlled attributes are not validated
1058 - // TODO: Only ignore them on controlled tags.
1059 - propKey === 'value' ||
1060 - propKey === 'checked' ||
1061 - propKey === 'selected'
1062 - ) {
1063 - // Noop
1064 - } else if (propKey === DANGEROUSLY_SET_INNER_HTML) {
1065 - const serverHTML = domElement.innerHTML;
1066 - const nextHtml = nextProp ? nextProp[HTML] : undefined;
1067 - if (nextHtml != null) {
1068 - const expectedHTML = normalizeHTML(domElement, nextHtml);
1069 - if (expectedHTML !== serverHTML) {
1070 - warnForPropDifference(propKey, serverHTML, expectedHTML);
1017 + }
1018 + }
1019 +
1020 + if (__DEV__ && shouldWarnDev) {
1021 + const isCustomComponentTag = isCustomComponent(tag, rawProps);
1022 +
1023 + for (const propKey in rawProps) {
1024 + if (!rawProps.hasOwnProperty(propKey)) {
1025 + continue;
1026 + }
1027 + const nextProp = rawProps[propKey];
1028 + if (propKey === CHILDREN) {
1029 + // Checked above already
1030 + } else if (registrationNameDependencies.hasOwnProperty(propKey)) {
1031 + if (nextProp != null) {
1032 + if (typeof nextProp !== 'function') {
1033 + warnForInvalidEventListener(propKey, nextProp);
1034 }
1035 }
1073 - } else if (propKey === STYLE) {
1074 - // $FlowFixMe - Should be inferred as not undefined.
1075 - extraAttributeNames.delete(propKey);
1076 -
1077 - if (canDiffStyleForHydrationWarning) {
1078 - const expectedStyle = createDangerousStringForStyles(nextProp);
1079 - serverValue = domElement.getAttribute('style');
1080 - if (expectedStyle !== serverValue) {
1081 - warnForPropDifference(propKey, serverValue, expectedStyle);
1036 + } else {
1037 + // Validate that the properties correspond to their expected values.
1038 + let serverValue;
1039 + const propertyInfo =
1040 + isCustomComponentTag && enableCustomElementPropertySupport
1041 + ? null
1042 + : getPropertyInfo(propKey);
1043 + if (rawProps[SUPPRESS_HYDRATION_WARNING] === true) {
1044 + // Don't bother comparing. We're ignoring all these warnings.
1045 + } else if (
1046 + propKey === SUPPRESS_CONTENT_EDITABLE_WARNING ||
1047 + propKey === SUPPRESS_HYDRATION_WARNING ||
1048 + // Controlled attributes are not validated
1049 + // TODO: Only ignore them on controlled tags.
1050 + propKey === 'value' ||
1051 + propKey === 'checked' ||
1052 + propKey === 'selected'
1053 + ) {
1054 + // Noop
1055 + } else if (propKey === DANGEROUSLY_SET_INNER_HTML) {
1056 + const serverHTML = domElement.innerHTML;
1057 + const nextHtml = nextProp ? nextProp[HTML] : undefined;
1058 + if (nextHtml != null) {
1059 + const expectedHTML = normalizeHTML(domElement, nextHtml);
1060 + if (expectedHTML !== serverHTML) {
1061 + warnForPropDifference(propKey, serverHTML, expectedHTML);
1062 + }
1063 }
1083 - }
1084 - } else if (
1085 - enableCustomElementPropertySupport &&
1086 - isCustomComponentTag &&
1087 - (propKey === 'offsetParent' ||
1088 - propKey === 'offsetTop' ||
1089 - propKey === 'offsetLeft' ||
1090 - propKey === 'offsetWidth' ||
1091 - propKey === 'offsetHeight' ||
1092 - propKey === 'isContentEditable' ||
1093 - propKey === 'outerText' ||
1094 - propKey === 'outerHTML')
1095 - ) {
1096 - // $FlowFixMe - Should be inferred as not undefined.
1097 - extraAttributeNames.delete(propKey.toLowerCase());
1098 - if (__DEV__) {
1099 - console.error(
1100 - 'Assignment to read-only property will result in a no-op: `%s`',
1101 - propKey,
1102 - );
1103 - }
1104 - } else if (isCustomComponentTag && !enableCustomElementPropertySupport) {
1105 - // $FlowFixMe - Should be inferred as not undefined.
1106 - extraAttributeNames.delete(propKey.toLowerCase());
1107 - serverValue = getValueForAttribute(
1108 - domElement,
1109 - propKey,
1110 - nextProp,
1111 - isCustomComponentTag,
1112 - );
1064 + } else if (propKey === STYLE) {
1065 + // $FlowFixMe - Should be inferred as not undefined.
1066 + extraAttributeNames.delete(propKey);
1067
1114 - if (nextProp !== serverValue) {
1115 - warnForPropDifference(propKey, serverValue, nextProp);
1116 - }
1117 - } else if (
1118 - !shouldIgnoreAttribute(propKey, propertyInfo, isCustomComponentTag) &&
1119 - !shouldRemoveAttribute(
1120 - propKey,
1121 - nextProp,
1122 - propertyInfo,
1123 - isCustomComponentTag,
1124 - )
1125 - ) {
1126 - let isMismatchDueToBadCasing = false;
1127 - if (propertyInfo !== null) {
1068 + if (canDiffStyleForHydrationWarning) {
1069 + const expectedStyle = createDangerousStringForStyles(nextProp);
1070 + serverValue = domElement.getAttribute('style');
1071 + if (expectedStyle !== serverValue) {
1072 + warnForPropDifference(propKey, serverValue, expectedStyle);
1073 + }
1074 + }
1075 + } else if (
1076 + enableCustomElementPropertySupport &&
1077 + isCustomComponentTag &&
1078 + (propKey === 'offsetParent' ||
1079 + propKey === 'offsetTop' ||
1080 + propKey === 'offsetLeft' ||
1081 + propKey === 'offsetWidth' ||
1082 + propKey === 'offsetHeight' ||
1083 + propKey === 'isContentEditable' ||
1084 + propKey === 'outerText' ||
1085 + propKey === 'outerHTML')
1086 + ) {
1087 + // $FlowFixMe - Should be inferred as not undefined.
1088 + extraAttributeNames.delete(propKey.toLowerCase());
1089 + if (__DEV__) {
1090 + console.error(
1091 + 'Assignment to read-only property will result in a no-op: `%s`',
1092 + propKey,
1093 + );
1094 + }
1095 + } else if (
1096 + isCustomComponentTag &&
1097 + !enableCustomElementPropertySupport
1098 + ) {
1099 // $FlowFixMe - Should be inferred as not undefined.
1129 - extraAttributeNames.delete(propertyInfo.attributeName);
1130 - serverValue = getValueForProperty(
1100 + extraAttributeNames.delete(propKey.toLowerCase());
1101 + serverValue = getValueForAttribute(
1102 domElement,
1103 propKey,
1104 nextProp,
1134 - propertyInfo,
1105 + isCustomComponentTag,
1106 );
1136 - } else {
1137 - let ownNamespaceDev = parentNamespaceDev;
1138 - if (ownNamespaceDev === HTML_NAMESPACE) {
1139 - ownNamespaceDev = getIntrinsicNamespace(tag);
1107 +
1108 + if (nextProp !== serverValue) {
1109 + warnForPropDifference(propKey, serverValue, nextProp);
1110 }
1141 - if (ownNamespaceDev === HTML_NAMESPACE) {
1111 + } else if (
1112 + !shouldIgnoreAttribute(propKey, propertyInfo, isCustomComponentTag) &&
1113 + !shouldRemoveAttribute(
1114 + propKey,
1115 + nextProp,
1116 + propertyInfo,
1117 + isCustomComponentTag,
1118 + )
1119 + ) {
1120 + let isMismatchDueToBadCasing = false;
1121 + if (propertyInfo !== null) {
1122 // $FlowFixMe - Should be inferred as not undefined.
1143 - extraAttributeNames.delete(propKey.toLowerCase());
1123 + extraAttributeNames.delete(propertyInfo.attributeName);
1124 + serverValue = getValueForProperty(
1125 + domElement,
1126 + propKey,
1127 + nextProp,
1128 + propertyInfo,
1129 + );
1130 } else {
1145 - const standardName = getPossibleStandardName(propKey);
1146 - if (standardName !== null && standardName !== propKey) {
1147 - // If an SVG prop is supplied with bad casing, it will
1148 - // be successfully parsed from HTML, but will produce a mismatch
1149 - // (and would be incorrectly rendered on the client).
1150 - // However, we already warn about bad casing elsewhere.
1151 - // So we'll skip the misleading extra mismatch warning in this case.
1152 - isMismatchDueToBadCasing = true;
1131 + let ownNamespaceDev = parentNamespaceDev;
1132 + if (ownNamespaceDev === HTML_NAMESPACE) {
1133 + ownNamespaceDev = getIntrinsicNamespace(tag);
1134 + }
1135 + if (ownNamespaceDev === HTML_NAMESPACE) {
1136 + // $FlowFixMe - Should be inferred as not undefined.
1137 + extraAttributeNames.delete(propKey.toLowerCase());
1138 + } else {
1139 + const standardName = getPossibleStandardName(propKey);
1140 + if (standardName !== null && standardName !== propKey) {
1141 + // If an SVG prop is supplied with bad casing, it will
1142 + // be successfully parsed from HTML, but will produce a mismatch
1143 + // (and would be incorrectly rendered on the client).
1144 + // However, we already warn about bad casing elsewhere.
1145 + // So we'll skip the misleading extra mismatch warning in this case.
1146 + isMismatchDueToBadCasing = true;
1147 + // $FlowFixMe - Should be inferred as not undefined.
1148 + extraAttributeNames.delete(standardName);
1149 + }
1150 // $FlowFixMe - Should be inferred as not undefined.
1154 - extraAttributeNames.delete(standardName);
1151 + extraAttributeNames.delete(propKey);
1152 }
1156 - // $FlowFixMe - Should be inferred as not undefined.
1157 - extraAttributeNames.delete(propKey);
1153 + serverValue = getValueForAttribute(
1154 + domElement,
1155 + propKey,
1156 + nextProp,
1157 + isCustomComponentTag,
1158 + );
1159 }
1159 - serverValue = getValueForAttribute(
1160 - domElement,
1161 - propKey,
1162 - nextProp,
1163 - isCustomComponentTag,
1164 - );
1165 - }
1160
1167 - const dontWarnCustomElement =
1168 - enableCustomElementPropertySupport &&
1169 - isCustomComponentTag &&
1170 - (typeof nextProp === 'function' || typeof nextProp === 'object');
1171 - if (
1172 - !dontWarnCustomElement &&
1173 - nextProp !== serverValue &&
1174 - !isMismatchDueToBadCasing
1175 - ) {
1176 - warnForPropDifference(propKey, serverValue, nextProp);
1161 + const dontWarnCustomElement =
1162 + enableCustomElementPropertySupport &&
1163 + isCustomComponentTag &&
1164 + (typeof nextProp === 'function' || typeof nextProp === 'object');
1165 + if (
1166 + !dontWarnCustomElement &&
1167 + nextProp !== serverValue &&
1168 + !isMismatchDueToBadCasing
1169 + ) {
1170 + warnForPropDifference(propKey, serverValue, nextProp);
1171 + }
1172 }
1173 }
1174 }
1180 - }
1175
1182 - if (__DEV__) {
1183 - if (shouldWarnDev) {
1184 - if (
1185 - // $FlowFixMe - Should be inferred as not undefined.
1186 - extraAttributeNames.size > 0 &&
1187 - rawProps[SUPPRESS_HYDRATION_WARNING] !== true
1188 - ) {
1189 - // $FlowFixMe - Should be inferred as not undefined.
1190 - warnForExtraAttributes(extraAttributeNames);
1191 - }
1176 + if (
1177 + // $FlowFixMe - Should be inferred as not undefined.
1178 + extraAttributeNames.size > 0 &&
1179 + rawProps[SUPPRESS_HYDRATION_WARNING] !== true
1180 + ) {
1181 + // $FlowFixMe - Should be inferred as not undefined.
1182 + warnForExtraAttributes(extraAttributeNames);
1183 }
1184 }
1185
packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
+1 -1
@@ -3813,7 +3813,7 @@ describe('ReactDOMFizzServer', () => {
3813 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.',
3814 ]);
3815 }).toErrorDev(
3816 - 'Warning: Prop `name` did not match. Server: "initial" Client: "replaced"',
3816 + 'Warning: Text content did not match. Server: "initial" Client: "replaced',
3817 );
3818 expect(getVisibleChildren(container)).toEqual(
3819 <div>
packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js
+21 -15
@@ -130,7 +130,7 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
130 : children;
131 }
132
133 - it('suppresses and fixes text mismatches with suppressHydrationWarning', async () => {
133 + it('suppresses but does not fix text mismatches with suppressHydrationWarning', async () => {
134 function App({isClient}) {
135 return (
136 <div>
@@ -163,13 +163,13 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
163 // The text mismatch should be *silently* fixed. Even in production.
164 expect(getVisibleChildren(container)).toEqual(
165 <div>
166 - <span>Client Text</span>
167 - <span>2</span>
166 + <span>Server Text</span>
167 + <span>1</span>
168 </div>,
169 );
170 });
171
172 - it('suppresses and fixes multiple text node mismatches with suppressHydrationWarning', async () => {
172 + it('suppresses but does not fix multiple text node mismatches with suppressHydrationWarning', async () => {
173 function App({isClient}) {
174 return (
175 <div>
@@ -203,8 +203,8 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
203 expect(getVisibleChildren(container)).toEqual(
204 <div>
205 <span>
206 - {'Client1'}
207 - {'Client2'}
206 + {'Server1'}
207 + {'Server2'}
208 </span>
209 </div>,
210 );
@@ -261,19 +261,17 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
261 );
262 });
263
264 - it('suppresses and fixes client-only single text node mismatches with suppressHydrationWarning', async () => {
265 - function App({isClient}) {
264 + it('suppresses but does not fix client-only single text node mismatches with suppressHydrationWarning', async () => {
265 + function App({text}) {
266 return (
267 <div>
268 - <span suppressHydrationWarning={true}>
269 - {isClient ? 'Client' : null}
270 - </span>
268 + <span suppressHydrationWarning={true}>{text}</span>
269 </div>
270 );
271 }
272 await act(() => {
273 const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
276 - <App isClient={false} />,
274 + <App text={null} />,
275 );
276 pipe(writable);
277 });
@@ -282,7 +280,7 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
280 <span />
281 </div>,
282 );
285 - ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
283 + const root = ReactDOMClient.hydrateRoot(container, <App text="Client" />, {
284 onRecoverableError(error) {
285 Scheduler.log(error.message);
286 },
@@ -290,7 +288,15 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
288 await waitForAll([]);
289 expect(getVisibleChildren(container)).toEqual(
290 <div>
293 - <span>{'Client'}</span>
291 + <span />
292 + </div>,
293 + );
294 + // An update fixes it though.
295 + root.render(<App text="Client 2" />);
296 + await waitForAll([]);
297 + expect(getVisibleChildren(container)).toEqual(
298 + <div>
299 + <span>Client 2</span>
300 </div>,
301 );
302 });
@@ -495,7 +501,7 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
501 );
502 });
503
498 - it('suppresses and does not fix attribute mismatches with suppressHydrationWarning', async () => {
504 + it('suppresses but does not fix attribute mismatches with suppressHydrationWarning', async () => {
505 function App({isClient}) {
506 return (
507 <div>
packages/react-dom/src/__tests__/ReactDOMFloat-test.js
+1 -1
@@ -5617,7 +5617,7 @@ background-color: green;
5617 ]);
5618 });
5619
5620 - // @gate enableFloat && enableHostSingletons && (enableClientRenderFallbackOnTextMismatch || !__DEV__)
5620 + // @gate enableFloat && enableHostSingletons && enableClientRenderFallbackOnTextMismatch
5621 it('can render a title before a singleton even if that singleton clears its contents', async () => {
5622 await actIntoEmptyDocument(() => {
5623 const {pipe} = renderToPipeableStream(
packages/react-reconciler/src/ReactFiberHydrationContext.js
+10
@@ -728,6 +728,11 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean {
728 isConcurrentMode,
729 shouldWarnIfMismatchDev,
730 );
731 + if (isConcurrentMode) {
732 + // In concurrent mode we never update the mismatched text,
733 + // even if the error was ignored.
734 + return false;
735 + }
736 break;
737 }
738 case HostSingleton:
@@ -747,6 +752,11 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean {
752 isConcurrentMode,
753 shouldWarnIfMismatchDev,
754 );
755 + if (isConcurrentMode) {
756 + // In concurrent mode we never update the mismatched text,
757 + // even if the error was ignored.
758 + return false;
759 + }
760 break;
761 }
762 }