Skip to content

Commit

Permalink
Move validation of text nesting into ReactDOMComponent (facebook#26594)
Browse files Browse the repository at this point in the history
Extract validateTextNesting from validateDOMNesting. We only need the
parent tag when validating text nodes. Then validate it in setProp.
  • Loading branch information
sebmarkbage authored and AndyPengc12 committed Apr 15, 2024
1 parent 90cf94f commit 1b88586
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 64 deletions.
7 changes: 7 additions & 0 deletions packages/react-dom-bindings/src/client/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
updateTextarea,
restoreControlledTextareaState,
} from './ReactDOMTextarea';
import {validateTextNesting} from './validateDOMNesting';
import {track} from './inputValueTracking';
import setInnerHTML from './setInnerHTML';
import setTextContent from './setTextContent';
Expand Down Expand Up @@ -279,6 +280,9 @@ function setProp(
switch (key) {
case 'children': {
if (typeof value === 'string') {
if (__DEV__) {
validateTextNesting(value, tag);
}
// Avoid setting initial textContent when the text is empty. In IE11 setting
// textContent on a <textarea> will cause the placeholder to not
// show within the <textarea> until it has been focused and blurred again.
Expand All @@ -290,6 +294,9 @@ function setProp(
setTextContent(domElement, value);
}
} else if (typeof value === 'number') {
if (__DEV__) {
validateTextNesting('' + value, tag);
}
const canSetTextContent = !enableHostSingletons || tag !== 'body';
if (canSetTextContent) {
setTextContent(domElement, '' + value);
Expand Down
41 changes: 11 additions & 30 deletions packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@ import {
} from './ReactDOMComponent';
import {getSelectionInformation, restoreSelection} from './ReactInputSelection';
import setTextContent from './setTextContent';
import {validateDOMNesting, updatedAncestorInfoDev} from './validateDOMNesting';
import {
validateDOMNesting,
validateTextNesting,
updatedAncestorInfoDev,
} from './validateDOMNesting';
import {
isEnabled as ReactBrowserEventEmitterIsEnabled,
setEnabled as ReactBrowserEventEmitterSetEnabled,
Expand Down Expand Up @@ -328,18 +332,7 @@ export function createInstance(
if (__DEV__) {
// TODO: take namespace into account when validating.
const hostContextDev: HostContextDev = (hostContext: any);
validateDOMNesting(type, null, hostContextDev.ancestorInfo);
if (
typeof props.children === 'string' ||
typeof props.children === 'number'
) {
const string = '' + props.children;
const ownAncestorInfo = updatedAncestorInfoDev(
hostContextDev.ancestorInfo,
type,
);
validateDOMNesting(null, string, ownAncestorInfo);
}
validateDOMNesting(type, hostContextDev.ancestorInfo);
namespace = hostContextDev.namespace;
} else {
const hostContextProd: HostContextProd = (hostContext: any);
Expand Down Expand Up @@ -491,21 +484,6 @@ export function prepareUpdate(
// TODO: Figure out how to validateDOMNesting when children turn into a string.
return null;
}
if (__DEV__) {
const hostContextDev = ((hostContext: any): HostContextDev);
if (
typeof newProps.children !== typeof oldProps.children &&
(typeof newProps.children === 'string' ||
typeof newProps.children === 'number')
) {
const string = '' + newProps.children;
const ownAncestorInfo = updatedAncestorInfoDev(
hostContextDev.ancestorInfo,
type,
);
validateDOMNesting(null, string, ownAncestorInfo);
}
}
return diffProperties(domElement, type, oldProps, newProps);
}

Expand All @@ -529,7 +507,10 @@ export function createTextInstance(
): TextInstance {
if (__DEV__) {
const hostContextDev = ((hostContext: any): HostContextDev);
validateDOMNesting(null, text, hostContextDev.ancestorInfo);
const ancestor = hostContextDev.ancestorInfo.current;
if (ancestor != null) {
validateTextNesting(text, ancestor.tag);
}
}
const textNode: TextInstance = getOwnerDocumentFromRootContainer(
rootContainerInstance,
Expand Down Expand Up @@ -1756,7 +1737,7 @@ export function resolveSingletonInstance(
if (__DEV__) {
const hostContextDev = ((hostContext: any): HostContextDev);
if (validateDOMNestingDev) {
validateDOMNesting(type, null, hostContextDev.ancestorInfo);
validateDOMNesting(type, hostContextDev.ancestorInfo);
}
}
const ownerDocument = getOwnerDocumentFromRootContainer(
Expand Down
67 changes: 33 additions & 34 deletions packages/react-dom-bindings/src/client/validateDOMNesting.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,29 +434,14 @@ function findInvalidAncestorForTag(
const didWarn: {[string]: boolean} = {};

function validateDOMNesting(
childTag: ?string,
childText: ?string,
childTag: string,
ancestorInfo: AncestorInfoDev,
): void {
if (__DEV__) {
ancestorInfo = ancestorInfo || emptyAncestorInfoDev;
const parentInfo = ancestorInfo.current;
const parentTag = parentInfo && parentInfo.tag;

if (childText != null) {
if (childTag != null) {
console.error(
'validateDOMNesting: when childText is passed, childTag should be null',
);
}
childTag = '#text';
} else if (childTag == null) {
console.error(
'validateDOMNesting: when childText or childTag must be provided',
);
return;
}

const invalidParent = isTagValidWithParent(childTag, parentTag)
? null
: parentInfo;
Expand All @@ -478,21 +463,7 @@ function validateDOMNesting(
}
didWarn[warnKey] = true;

let tagDisplayName = childTag;
let whitespaceInfo = '';
if (childTag === '#text') {
if (childText != null && /\S/.test(childText)) {
tagDisplayName = 'Text nodes';
} else {
tagDisplayName = 'Whitespace text nodes';
whitespaceInfo =
" Make sure you don't have any extra whitespace between tags on " +
'each line of your source code.';
}
} else {
tagDisplayName = '<' + childTag + '>';
}

const tagDisplayName = '<' + childTag + '>';
if (invalidParent) {
let info = '';
if (ancestorTag === 'table' && childTag === 'tr') {
Expand All @@ -501,10 +472,9 @@ function validateDOMNesting(
'the browser.';
}
console.error(
'validateDOMNesting(...): %s cannot appear as a child of <%s>.%s%s',
'validateDOMNesting(...): %s cannot appear as a child of <%s>.%s',
tagDisplayName,
ancestorTag,
whitespaceInfo,
info,
);
} else {
Expand All @@ -518,4 +488,33 @@ function validateDOMNesting(
}
}

export {updatedAncestorInfoDev, validateDOMNesting};
function validateTextNesting(childText: string, parentTag: string): void {
if (__DEV__) {
if (isTagValidWithParent('#text', parentTag)) {
return;
}

// eslint-disable-next-line react-internal/safe-string-coercion
const warnKey = '#text|' + parentTag;
if (didWarn[warnKey]) {
return;
}
didWarn[warnKey] = true;

if (/\S/.test(childText)) {
console.error(
'validateDOMNesting(...): Text nodes cannot appear as a child of <%s>.',
parentTag,
);
} else {
console.error(
'validateDOMNesting(...): Whitespace text nodes cannot appear as a child of <%s>. ' +
"Make sure you don't have any extra whitespace between tags on " +
'each line of your source code.',
parentTag,
);
}
}
}

export {updatedAncestorInfoDev, validateDOMNesting, validateTextNesting};
51 changes: 51 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1855,6 +1855,57 @@ describe('ReactDOMComponent', () => {
]);
});

it('warns nicely for updating table rows to use text', () => {
const container = document.createElement('div');

function Row({children}) {
return <tr>{children}</tr>;
}

function Foo({children}) {
return <table>{children}</table>;
}

// First is fine.
ReactDOM.render(<Foo />, container);

expect(() => ReactDOM.render(<Foo> </Foo>, container)).toErrorDev([
'Warning: validateDOMNesting(...): Whitespace text nodes cannot ' +
"appear as a child of <table>. Make sure you don't have any extra " +
'whitespace between tags on each line of your source code.' +
'\n in table (at **)' +
'\n in Foo (at **)',
]);

ReactDOM.render(
<Foo>
<tbody>
<Row />
</tbody>
</Foo>,
container,
);

expect(() =>
ReactDOM.render(
<Foo>
<tbody>
<Row>text</Row>
</tbody>
</Foo>,
container,
),
).toErrorDev([
'Warning: validateDOMNesting(...): Text nodes cannot appear as a ' +
'child of <tr>.' +
'\n in tr (at **)' +
'\n in Row (at **)' +
'\n in tbody (at **)' +
'\n in table (at **)' +
'\n in Foo (at **)',
]);
});

it('gives useful context in warnings', () => {
function Row() {
return <tr />;
Expand Down

0 comments on commit 1b88586

Please sign in to comment.