Warn for using a React owned node as a Container if it also has text content (#32774)

The problem with setting both `children` or `dangerouslySetInnerHTML`
and also using a ref on a DOM node to either manually append children or
using it as a Container for `createRoot` or `createPortal` is that it's
ambiguous which children should win. Ideally you use one of the four
options to control children. Meaning that ideally you always use a leaf
container for refs like this.

Unfortunately it's very common to use a React owned thing with children
as a Container of a Portal. For example `document.body` can have both
regular React children and be used as a Portal container. This isn't
really fully supported and has some undefined behavior like relative
order isn't guaranteed but still very common.

It is extra bad if the children are a `string`/`number` or if
`dangerouslySetInnerHTML` is set. Because then when ever that reactively
updates it'll clear out any manually added DOM nodes. When this happens
isn't guaranteed. It's always happening as far as the reactivity is
concerned. See https://github.com/facebook/react/issues/31600

Therefore, we should warn for this specific pattern. This still allows
non-text children as a compromise even though that behavior is also
somewhat undefined.
This commit is contained in:
Sebastian Markbåge
2025-04-01 12:11:29 -04:00
committed by GitHub
parent 731ae3e0ad
commit e5dd82a79d
2 changed files with 48 additions and 1 deletions
@@ -211,7 +211,7 @@ export function getNodeFromInstance(inst: Fiber): Instance | TextInstance {
}
export function getFiberCurrentPropsFromNode(
node: Instance | TextInstance | SuspenseInstance,
node: Container | Instance | TextInstance | SuspenseInstance,
): Props {
return (node: any)[internalPropsKey] || null;
}
@@ -30,6 +30,7 @@ import type {TransitionTypes} from 'react/src/ReactTransitionType';
import {NotPending} from '../shared/ReactDOMFormActions';
import {getCurrentRootHostContainer} from 'react-reconciler/src/ReactFiberHostContext';
import {runWithFiberInDEV} from 'react-reconciler/src/ReactCurrentFiber';
import hasOwnProperty from 'shared/hasOwnProperty';
import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
@@ -43,6 +44,8 @@ export {
import {
precacheFiberNode,
updateFiberProps,
getFiberCurrentPropsFromNode,
getInstanceFromNode,
getClosestInstanceFromNode,
getFiberFromScopeInstance,
getInstanceFromNode as getInstanceFromNodeDOMTree,
@@ -821,10 +824,51 @@ export function appendChild(
}
}
function warnForReactChildrenConflict(container: Container): void {
if (__DEV__) {
if ((container: any).__reactWarnedAboutChildrenConflict) {
return;
}
const props = getFiberCurrentPropsFromNode(container);
if (props !== null) {
const fiber = getInstanceFromNode(container);
if (fiber !== null) {
if (
typeof props.children === 'string' ||
typeof props.children === 'number'
) {
(container: any).__reactWarnedAboutChildrenConflict = true;
// Run the warning with the Fiber of the container for context of where the children are specified.
// We could also maybe use the Portal. The current execution context is the child being added.
runWithFiberInDEV(fiber, () => {
console.error(
'Cannot use a ref on a React element as a container to `createRoot` or `createPortal` ' +
'if that element also sets "children" text content using React. It should be a leaf with no children. ' +
"Otherwise it's ambiguous which children should be used.",
);
});
} else if (props.dangerouslySetInnerHTML != null) {
(container: any).__reactWarnedAboutChildrenConflict = true;
runWithFiberInDEV(fiber, () => {
console.error(
'Cannot use a ref on a React element as a container to `createRoot` or `createPortal` ' +
'if that element also sets "dangerouslySetInnerHTML" using React. It should be a leaf with no children. ' +
"Otherwise it's ambiguous which children should be used.",
);
});
}
}
}
}
}
export function appendChildToContainer(
container: Container,
child: Instance | TextInstance,
): void {
if (__DEV__) {
warnForReactChildrenConflict(container);
}
let parentNode: DocumentFragment | Element;
if (container.nodeType === DOCUMENT_NODE) {
parentNode = (container: any).body;
@@ -888,6 +932,9 @@ export function insertInContainerBefore(
child: Instance | TextInstance,
beforeChild: Instance | TextInstance | SuspenseInstance,
): void {
if (__DEV__) {
warnForReactChildrenConflict(container);
}
let parentNode: DocumentFragment | Element;
if (container.nodeType === DOCUMENT_NODE) {
parentNode = (container: any).body;