Skip to content

Commit 4ad6e2d

Browse files
committed
Inline assertValidProps
This removes some runtime validation for the hydration path because we generally don't do this kind of validation against the props there. Only if the path ends up mismatching in the children somehow. This leads to some duplication but I plan on unifying later.
1 parent d5e5a65 commit 4ad6e2d

File tree

5 files changed

+187
-156
lines changed

5 files changed

+187
-156
lines changed

packages/react-dom-bindings/src/client/ReactDOMComponent.js

Lines changed: 185 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ import {
5959
} from './CSSPropertyOperations';
6060
import {HTML_NAMESPACE, getIntrinsicNamespace} from './DOMNamespaces';
6161
import {getPropertyInfo} from '../shared/DOMProperty';
62-
import assertValidProps from './assertValidProps';
6362
import {DOCUMENT_NODE} from './HTMLNodeType';
6463
import isCustomComponent from '../shared/isCustomComponent';
6564
import possibleStandardNames from '../shared/possibleStandardNames';
@@ -119,6 +118,18 @@ function validatePropertiesInDevelopment(type: string, props: any) {
119118
registrationNameDependencies,
120119
possibleRegistrationNames,
121120
});
121+
if (
122+
props.contentEditable &&
123+
!props.suppressContentEditableWarning &&
124+
props.children != null
125+
) {
126+
console.error(
127+
'A component is `contentEditable` and contains `children` managed by ' +
128+
'React. It is now your responsibility to guarantee that none of ' +
129+
'those nodes are unexpectedly modified or duplicated. This is ' +
130+
'probably not intentional.',
131+
);
132+
}
122133
}
123134
}
124135

@@ -289,6 +300,13 @@ function setInitialDOMProperties(
289300
const nextProp = nextProps[propKey];
290301
switch (propKey) {
291302
case 'style': {
303+
if (nextProp != null && typeof nextProp !== 'object') {
304+
throw new Error(
305+
'The `style` prop expects a mapping from style properties to values, ' +
306+
"not a string. For example, style={{marginRight: spacing + 'em'}} when " +
307+
'using JSX.',
308+
);
309+
}
292310
if (__DEV__) {
293311
if (nextProp) {
294312
// Freeze the next style object so that we can assume it won't be
@@ -301,12 +319,26 @@ function setInitialDOMProperties(
301319
break;
302320
}
303321
case 'dangerouslySetInnerHTML': {
304-
const nextHtml = nextProp ? nextProp.__html : undefined;
305-
if (nextHtml != null) {
306-
if (disableIEWorkarounds) {
307-
domElement.innerHTML = nextHtml;
308-
} else {
309-
setInnerHTML(domElement, nextHtml);
322+
if (nextProp != null) {
323+
if (typeof nextProp !== 'object' || !('__html' in nextProp)) {
324+
throw new Error(
325+
'`props.dangerouslySetInnerHTML` must be in the form `{__html: ...}`. ' +
326+
'Please visit https://reactjs.org/link/dangerously-set-inner-html ' +
327+
'for more information.',
328+
);
329+
}
330+
const nextHtml = nextProp.__html;
331+
if (nextHtml != null) {
332+
if (nextProps.children != null) {
333+
throw new Error(
334+
'Can only set one of `children` or `props.dangerouslySetInnerHTML`.',
335+
);
336+
}
337+
if (disableIEWorkarounds) {
338+
domElement.innerHTML = nextHtml;
339+
} else {
340+
setInnerHTML(domElement, nextHtml);
341+
}
310342
}
311343
}
312344
break;
@@ -566,6 +598,16 @@ export function setInitialProperties(
566598
case 'iframe':
567599
case 'object':
568600
case 'embed':
601+
if (
602+
rawProps.children != null ||
603+
rawProps.dangerouslySetInnerHTML != null
604+
) {
605+
// TODO: Can we make this a DEV warning to avoid this deny list?
606+
throw new Error(
607+
`${tag} is a void element tag and must neither have \`children\` nor ` +
608+
'use `dangerouslySetInnerHTML`.',
609+
);
610+
}
569611
// We listen to this event in case to ensure emulated bubble
570612
// listeners still fire for the load event.
571613
listenToNonDelegatedEvent('load', domElement);
@@ -581,14 +623,34 @@ export function setInitialProperties(
581623
props = rawProps;
582624
break;
583625
case 'source':
626+
if (
627+
rawProps.children != null ||
628+
rawProps.dangerouslySetInnerHTML != null
629+
) {
630+
// TODO: Can we make this a DEV warning to avoid this deny list?
631+
throw new Error(
632+
`${tag} is a void element tag and must neither have \`children\` nor ` +
633+
'use `dangerouslySetInnerHTML`.',
634+
);
635+
}
584636
// We listen to this event in case to ensure emulated bubble
585637
// listeners still fire for the error event.
586638
listenToNonDelegatedEvent('error', domElement);
587639
props = rawProps;
588640
break;
589641
case 'img':
590-
case 'image':
591642
case 'link':
643+
if (
644+
rawProps.children != null ||
645+
rawProps.dangerouslySetInnerHTML != null
646+
) {
647+
throw new Error(
648+
`${tag} is a void element tag and must neither have \`children\` nor ` +
649+
'use `dangerouslySetInnerHTML`.',
650+
);
651+
}
652+
// eslint-disable-next-line no-fallthrough
653+
case 'image':
592654
// We listen to these events in case to ensure emulated bubble
593655
// listeners still fire for error and load events.
594656
listenToNonDelegatedEvent('error', domElement);
@@ -602,6 +664,15 @@ export function setInitialProperties(
602664
props = rawProps;
603665
break;
604666
case 'input':
667+
if (
668+
rawProps.children != null ||
669+
rawProps.dangerouslySetInnerHTML != null
670+
) {
671+
throw new Error(
672+
`${tag} is a void element tag and must neither have \`children\` nor ` +
673+
'use `dangerouslySetInnerHTML`.',
674+
);
675+
}
605676
ReactDOMInputInitWrapperState(domElement, rawProps);
606677
props = ReactDOMInputGetHostProps(domElement, rawProps);
607678
// We listen to this event in case to ensure emulated bubble
@@ -626,12 +697,33 @@ export function setInitialProperties(
626697
// listeners still fire for the invalid event.
627698
listenToNonDelegatedEvent('invalid', domElement);
628699
break;
700+
case 'area':
701+
case 'base':
702+
case 'br':
703+
case 'col':
704+
case 'hr':
705+
case 'keygen':
706+
case 'meta':
707+
case 'param':
708+
case 'track':
709+
case 'wbr':
710+
case 'menuitem': {
711+
if (
712+
rawProps.children != null ||
713+
rawProps.dangerouslySetInnerHTML != null
714+
) {
715+
// TODO: Can we make this a DEV warning to avoid this deny list?
716+
throw new Error(
717+
`${tag} is a void element tag and must neither have \`children\` nor ` +
718+
'use `dangerouslySetInnerHTML`.',
719+
);
720+
}
721+
}
722+
// eslint-disable-next-line no-fallthrough
629723
default:
630724
props = rawProps;
631725
}
632726

633-
assertValidProps(tag, props);
634-
635727
setInitialDOMProperties(tag, domElement, props, isCustomComponentTag);
636728

637729
switch (tag) {
@@ -679,6 +771,15 @@ export function diffProperties(
679771
let nextProps: Object;
680772
switch (tag) {
681773
case 'input':
774+
if (
775+
nextRawProps.children != null ||
776+
nextRawProps.dangerouslySetInnerHTML != null
777+
) {
778+
throw new Error(
779+
`${tag} is a void element tag and must neither have \`children\` nor ` +
780+
'use `dangerouslySetInnerHTML`.',
781+
);
782+
}
682783
lastProps = ReactDOMInputGetHostProps(domElement, lastRawProps);
683784
nextProps = ReactDOMInputGetHostProps(domElement, nextRawProps);
684785
updatePayload = [];
@@ -693,6 +794,33 @@ export function diffProperties(
693794
nextProps = ReactDOMTextareaGetHostProps(domElement, nextRawProps);
694795
updatePayload = [];
695796
break;
797+
case 'img':
798+
case 'link':
799+
case 'area':
800+
case 'base':
801+
case 'br':
802+
case 'col':
803+
case 'embed':
804+
case 'hr':
805+
case 'keygen':
806+
case 'meta':
807+
case 'param':
808+
case 'source':
809+
case 'track':
810+
case 'wbr':
811+
case 'menuitem': {
812+
if (
813+
nextRawProps.children != null ||
814+
nextRawProps.dangerouslySetInnerHTML != null
815+
) {
816+
// TODO: Can we make this a DEV warning to avoid this deny list?
817+
throw new Error(
818+
`${tag} is a void element tag and must neither have \`children\` nor ` +
819+
'use `dangerouslySetInnerHTML`.',
820+
);
821+
}
822+
}
823+
// eslint-disable-next-line no-fallthrough
696824
default:
697825
lastProps = lastRawProps;
698826
nextProps = nextRawProps;
@@ -706,8 +834,6 @@ export function diffProperties(
706834
break;
707835
}
708836

709-
assertValidProps(tag, nextProps);
710-
711837
let propKey;
712838
let styleName;
713839
let styleUpdates = null;
@@ -783,6 +909,13 @@ export function diffProperties(
783909
}
784910
switch (propKey) {
785911
case 'style': {
912+
if (nextProp != null && typeof nextProp !== 'object') {
913+
throw new Error(
914+
'The `style` prop expects a mapping from style properties to values, ' +
915+
"not a string. For example, style={{marginRight: spacing + 'em'}} when " +
916+
'using JSX.',
917+
);
918+
}
786919
if (__DEV__) {
787920
if (nextProp) {
788921
// Freeze the next style object so that we can assume it won't be
@@ -828,11 +961,25 @@ export function diffProperties(
828961
break;
829962
}
830963
case 'dangerouslySetInnerHTML': {
831-
const nextHtml = nextProp ? nextProp.__html : undefined;
832-
const lastHtml = lastProp ? lastProp.__html : undefined;
833-
if (nextHtml != null) {
834-
if (lastHtml !== nextHtml) {
835-
(updatePayload = updatePayload || []).push(propKey, nextHtml);
964+
if (nextProp != null) {
965+
if (typeof nextProp !== 'object' || !('__html' in nextProp)) {
966+
throw new Error(
967+
'`props.dangerouslySetInnerHTML` must be in the form `{__html: ...}`. ' +
968+
'Please visit https://reactjs.org/link/dangerously-set-inner-html ' +
969+
'for more information.',
970+
);
971+
}
972+
const nextHtml = nextProp.__html;
973+
if (nextHtml != null) {
974+
if (nextProps.children != null) {
975+
throw new Error(
976+
'Can only set one of `children` or `props.dangerouslySetInnerHTML`.',
977+
);
978+
}
979+
const lastHtml = lastProp ? lastProp.__html : undefined;
980+
if (lastHtml !== nextHtml) {
981+
(updatePayload = updatePayload || []).push(propKey, nextHtml);
982+
}
836983
}
837984
} else {
838985
// TODO: It might be too late to clear this if we have children
@@ -971,6 +1118,23 @@ function getPossibleStandardName(propName: string): string | null {
9711118
return null;
9721119
}
9731120

1121+
function diffHydratedStyles(domElement: Element, value: mixed) {
1122+
if (value != null && typeof value !== 'object') {
1123+
throw new Error(
1124+
'The `style` prop expects a mapping from style properties to values, ' +
1125+
"not a string. For example, style={{marginRight: spacing + 'em'}} when " +
1126+
'using JSX.',
1127+
);
1128+
}
1129+
if (canDiffStyleForHydrationWarning) {
1130+
const expectedStyle = createDangerousStringForStyles(value);
1131+
const serverValue = domElement.getAttribute('style');
1132+
if (expectedStyle !== serverValue) {
1133+
warnForPropDifference('style', serverValue, expectedStyle);
1134+
}
1135+
}
1136+
}
1137+
9741138
function diffHydratedCustomComponent(
9751139
domElement: Element,
9761140
tag: string,
@@ -997,7 +1161,6 @@ function diffHydratedCustomComponent(
9971161
continue;
9981162
}
9991163
// Validate that the properties correspond to their expected values.
1000-
let serverValue;
10011164
switch (propKey) {
10021165
case 'children': // Checked above already
10031166
case 'suppressContentEditableWarning':
@@ -1020,14 +1183,7 @@ function diffHydratedCustomComponent(
10201183
case 'style':
10211184
// $FlowFixMe - Should be inferred as not undefined.
10221185
extraAttributeNames.delete(propKey);
1023-
1024-
if (canDiffStyleForHydrationWarning) {
1025-
const expectedStyle = createDangerousStringForStyles(nextProp);
1026-
serverValue = domElement.getAttribute('style');
1027-
if (expectedStyle !== serverValue) {
1028-
warnForPropDifference(propKey, serverValue, expectedStyle);
1029-
}
1030-
}
1186+
diffHydratedStyles(domElement, nextProp);
10311187
continue;
10321188
case 'offsetParent':
10331189
case 'offsetTop':
@@ -1061,7 +1217,7 @@ function diffHydratedCustomComponent(
10611217
// $FlowFixMe - Should be inferred as not undefined.
10621218
extraAttributeNames.delete(propKey);
10631219
}
1064-
serverValue = getValueForAttributeOnCustomComponent(
1220+
const serverValue = getValueForAttributeOnCustomComponent(
10651221
domElement,
10661222
propKey,
10671223
nextProp,
@@ -1100,7 +1256,6 @@ function diffHydratedGenericElement(
11001256
continue;
11011257
}
11021258
// Validate that the properties correspond to their expected values.
1103-
let serverValue;
11041259
switch (propKey) {
11051260
case 'children': // Checked above already
11061261
case 'suppressContentEditableWarning':
@@ -1126,14 +1281,7 @@ function diffHydratedGenericElement(
11261281
case 'style':
11271282
// $FlowFixMe - Should be inferred as not undefined.
11281283
extraAttributeNames.delete(propKey);
1129-
1130-
if (canDiffStyleForHydrationWarning) {
1131-
const expectedStyle = createDangerousStringForStyles(nextProp);
1132-
serverValue = domElement.getAttribute('style');
1133-
if (expectedStyle !== serverValue) {
1134-
warnForPropDifference(propKey, serverValue, expectedStyle);
1135-
}
1136-
}
1284+
diffHydratedStyles(domElement, nextProp);
11371285
continue;
11381286
// eslint-disable-next-line no-fallthrough
11391287
default:
@@ -1148,6 +1296,7 @@ function diffHydratedGenericElement(
11481296
}
11491297
const propertyInfo = getPropertyInfo(propKey);
11501298
let isMismatchDueToBadCasing = false;
1299+
let serverValue;
11511300
if (propertyInfo !== null) {
11521301
// $FlowFixMe - Should be inferred as not undefined.
11531302
extraAttributeNames.delete(propertyInfo.attributeName);
@@ -1268,8 +1417,6 @@ export function diffHydratedProperties(
12681417
listenToNonDelegatedEvent('scroll', domElement);
12691418
}
12701419

1271-
assertValidProps(tag, rawProps);
1272-
12731420
let updatePayload = null;
12741421

12751422
const children = rawProps.children;

0 commit comments

Comments
 (0)