Move validation of text nesting into ReactDOMComponent (#26594)
Extract validateTextNesting from validateDOMNesting. We only need the parent tag when validating text nodes. Then validate it in setProp.
Sebastian Markbåge committed
Apr 10, 2023 at 21:41 UTC
ac43bf6870a15566507477a4504f22160835c8d3
4 files changed
+102
-64
packages/react-dom-bindings/src/client/ReactDOMComponent.js
+7
@@ -45,6 +45,7 @@ import {
45
updateTextarea,
46
restoreControlledTextareaState,
47
} from './ReactDOMTextarea';
48
+import {validateTextNesting} from './validateDOMNesting';
49
import {track} from './inputValueTracking';
50
import setInnerHTML from './setInnerHTML';
51
import setTextContent from './setTextContent';
@@ -279,6 +280,9 @@ function setProp(
280
switch (key) {
281
case 'children': {
282
if (typeof value === 'string') {
283
+ if (__DEV__) {
284
+ validateTextNesting(value, tag);
285
+ }
286
// Avoid setting initial textContent when the text is empty. In IE11 setting
287
// textContent on a <textarea> will cause the placeholder to not
288
// show within the <textarea> until it has been focused and blurred again.
@@ -290,6 +294,9 @@ function setProp(
294
setTextContent(domElement, value);
295
}
296
} else if (typeof value === 'number') {
297
+ if (__DEV__) {
298
+ validateTextNesting('' + value, tag);
299
+ }
300
const canSetTextContent = !enableHostSingletons || tag !== 'body';
301
if (canSetTextContent) {
302
setTextContent(domElement, '' + value);
packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
+11
-30
@@ -59,7 +59,11 @@ import {
59
} from './ReactDOMComponent';
60
import {getSelectionInformation, restoreSelection} from './ReactInputSelection';
61
import setTextContent from './setTextContent';
62
-import {validateDOMNesting, updatedAncestorInfoDev} from './validateDOMNesting';
62
+import {
63
+ validateDOMNesting,
64
+ validateTextNesting,
65
+ updatedAncestorInfoDev,
66
+} from './validateDOMNesting';
67
import {
68
isEnabled as ReactBrowserEventEmitterIsEnabled,
69
setEnabled as ReactBrowserEventEmitterSetEnabled,
@@ -328,18 +332,7 @@ export function createInstance(
332
if (__DEV__) {
333
// TODO: take namespace into account when validating.
334
const hostContextDev: HostContextDev = (hostContext: any);
331
- validateDOMNesting(type, null, hostContextDev.ancestorInfo);
332
- if (
333
- typeof props.children === 'string' ||
334
- typeof props.children === 'number'
335
- ) {
336
- const string = '' + props.children;
337
- const ownAncestorInfo = updatedAncestorInfoDev(
338
- hostContextDev.ancestorInfo,
339
- type,
340
- );
341
- validateDOMNesting(null, string, ownAncestorInfo);
342
- }
335
+ validateDOMNesting(type, hostContextDev.ancestorInfo);
336
namespace = hostContextDev.namespace;
337
} else {
338
const hostContextProd: HostContextProd = (hostContext: any);
@@ -491,21 +484,6 @@ export function prepareUpdate(
484
// TODO: Figure out how to validateDOMNesting when children turn into a string.
485
return null;
486
}
494
- if (__DEV__) {
495
- const hostContextDev = ((hostContext: any): HostContextDev);
496
- if (
497
- typeof newProps.children !== typeof oldProps.children &&
498
- (typeof newProps.children === 'string' ||
499
- typeof newProps.children === 'number')
500
- ) {
501
- const string = '' + newProps.children;
502
- const ownAncestorInfo = updatedAncestorInfoDev(
503
- hostContextDev.ancestorInfo,
504
- type,
505
- );
506
- validateDOMNesting(null, string, ownAncestorInfo);
507
- }
508
- }
487
return diffProperties(domElement, type, oldProps, newProps);
488
}
489
@@ -529,7 +507,10 @@ export function createTextInstance(
507
): TextInstance {
508
if (__DEV__) {
509
const hostContextDev = ((hostContext: any): HostContextDev);
532
- validateDOMNesting(null, text, hostContextDev.ancestorInfo);
510
+ const ancestor = hostContextDev.ancestorInfo.current;
511
+ if (ancestor != null) {
512
+ validateTextNesting(text, ancestor.tag);
513
+ }
514
}
515
const textNode: TextInstance = getOwnerDocumentFromRootContainer(
516
rootContainerInstance,
@@ -1756,7 +1737,7 @@ export function resolveSingletonInstance(
1737
if (__DEV__) {
1738
const hostContextDev = ((hostContext: any): HostContextDev);
1739
if (validateDOMNestingDev) {
1759
- validateDOMNesting(type, null, hostContextDev.ancestorInfo);
1740
+ validateDOMNesting(type, hostContextDev.ancestorInfo);
1741
}
1742
}
1743
const ownerDocument = getOwnerDocumentFromRootContainer(
packages/react-dom-bindings/src/client/validateDOMNesting.js
+33
-34
@@ -434,8 +434,7 @@ function findInvalidAncestorForTag(
434
const didWarn: {[string]: boolean} = {};
435
436
function validateDOMNesting(
437
- childTag: ?string,
438
- childText: ?string,
437
+ childTag: string,
438
ancestorInfo: AncestorInfoDev,
439
): void {
440
if (__DEV__) {
@@ -443,20 +442,6 @@ function validateDOMNesting(
442
const parentInfo = ancestorInfo.current;
443
const parentTag = parentInfo && parentInfo.tag;
444
446
- if (childText != null) {
447
- if (childTag != null) {
448
- console.error(
449
- 'validateDOMNesting: when childText is passed, childTag should be null',
450
- );
451
- }
452
- childTag = '#text';
453
- } else if (childTag == null) {
454
- console.error(
455
- 'validateDOMNesting: when childText or childTag must be provided',
456
- );
457
- return;
458
- }
459
-
445
const invalidParent = isTagValidWithParent(childTag, parentTag)
446
? null
447
: parentInfo;
@@ -478,21 +463,7 @@ function validateDOMNesting(
463
}
464
didWarn[warnKey] = true;
465
481
- let tagDisplayName = childTag;
482
- let whitespaceInfo = '';
483
- if (childTag === '#text') {
484
- if (childText != null && /\S/.test(childText)) {
485
- tagDisplayName = 'Text nodes';
486
- } else {
487
- tagDisplayName = 'Whitespace text nodes';
488
- whitespaceInfo =
489
- " Make sure you don't have any extra whitespace between tags on " +
490
- 'each line of your source code.';
491
- }
492
- } else {
493
- tagDisplayName = '<' + childTag + '>';
494
- }
495
-
466
+ const tagDisplayName = '<' + childTag + '>';
467
if (invalidParent) {
468
let info = '';
469
if (ancestorTag === 'table' && childTag === 'tr') {
@@ -501,10 +472,9 @@ function validateDOMNesting(
472
'the browser.';
473
}
474
console.error(
504
- 'validateDOMNesting(...): %s cannot appear as a child of <%s>.%s%s',
475
+ 'validateDOMNesting(...): %s cannot appear as a child of <%s>.%s',
476
tagDisplayName,
477
ancestorTag,
507
- whitespaceInfo,
478
info,
479
);
480
} else {
@@ -518,4 +488,33 @@ function validateDOMNesting(
488
}
489
}
490
521
-export {updatedAncestorInfoDev, validateDOMNesting};
491
+function validateTextNesting(childText: string, parentTag: string): void {
492
+ if (__DEV__) {
493
+ if (isTagValidWithParent('#text', parentTag)) {
494
+ return;
495
+ }
496
+
497
+ // eslint-disable-next-line react-internal/safe-string-coercion
498
+ const warnKey = '#text|' + parentTag;
499
+ if (didWarn[warnKey]) {
500
+ return;
501
+ }
502
+ didWarn[warnKey] = true;
503
+
504
+ if (/\S/.test(childText)) {
505
+ console.error(
506
+ 'validateDOMNesting(...): Text nodes cannot appear as a child of <%s>.',
507
+ parentTag,
508
+ );
509
+ } else {
510
+ console.error(
511
+ 'validateDOMNesting(...): Whitespace text nodes cannot appear as a child of <%s>. ' +
512
+ "Make sure you don't have any extra whitespace between tags on " +
513
+ 'each line of your source code.',
514
+ parentTag,
515
+ );
516
+ }
517
+ }
518
+}
519
+
520
+export {updatedAncestorInfoDev, validateDOMNesting, validateTextNesting};
packages/react-dom/src/__tests__/ReactDOMComponent-test.js
+51
@@ -1855,6 +1855,57 @@ describe('ReactDOMComponent', () => {
1855
]);
1856
});
1857
1858
+ it('warns nicely for updating table rows to use text', () => {
1859
+ const container = document.createElement('div');
1860
+
1861
+ function Row({children}) {
1862
+ return <tr>{children}</tr>;
1863
+ }
1864
+
1865
+ function Foo({children}) {
1866
+ return <table>{children}</table>;
1867
+ }
1868
+
1869
+ // First is fine.
1870
+ ReactDOM.render(<Foo />, container);
1871
+
1872
+ expect(() => ReactDOM.render(<Foo> </Foo>, container)).toErrorDev([
1873
+ 'Warning: validateDOMNesting(...): Whitespace text nodes cannot ' +
1874
+ "appear as a child of <table>. Make sure you don't have any extra " +
1875
+ 'whitespace between tags on each line of your source code.' +
1876
+ '\n in table (at **)' +
1877
+ '\n in Foo (at **)',
1878
+ ]);
1879
+
1880
+ ReactDOM.render(
1881
+ <Foo>
1882
+ <tbody>
1883
+ <Row />
1884
+ </tbody>
1885
+ </Foo>,
1886
+ container,
1887
+ );
1888
+
1889
+ expect(() =>
1890
+ ReactDOM.render(
1891
+ <Foo>
1892
+ <tbody>
1893
+ <Row>text</Row>
1894
+ </tbody>
1895
+ </Foo>,
1896
+ container,
1897
+ ),
1898
+ ).toErrorDev([
1899
+ 'Warning: validateDOMNesting(...): Text nodes cannot appear as a ' +
1900
+ 'child of <tr>.' +
1901
+ '\n in tr (at **)' +
1902
+ '\n in Row (at **)' +
1903
+ '\n in tbody (at **)' +
1904
+ '\n in table (at **)' +
1905
+ '\n in Foo (at **)',
1906
+ ]);
1907
+ });
1908
+
1909
it('gives useful context in warnings', () => {
1910
function Row() {
1911
return <tr />;