Skip to content

Commit 3caa786

Browse files
gnoffgnoff
committed
[Fiber] Disable comments as containers in OSS
3 years ago we partially disabled comment nodes as valid containers. Some unflagged support was left in due to legacy APIs like `unmountComponentAtNode` and `unstable_renderSubtreeIntoContainer` but these were since removed in React 19. This update flags the remaining uses of comments as containers.
1 parent 7d0adf7 commit 3caa786

File tree

5 files changed

+75
-87
lines changed

5 files changed

+75
-87
lines changed

‎packages/react-dom-bindings/src/client/ReactDOMContainer.js

-13
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,3 @@ export function isValidContainer(node: any): boolean {
2727
(node: any).nodeValue === ' react-mount-point-unstable '))
2828
);
2929
}
30-
31-
// TODO: Remove this function which also includes comment nodes.
32-
// We only use it in places that are currently more relaxed.
33-
export function isValidContainerLegacy(node: any): boolean {
34-
return !!(
35-
node &&
36-
(node.nodeType === ELEMENT_NODE ||
37-
node.nodeType === DOCUMENT_NODE ||
38-
node.nodeType === DOCUMENT_FRAGMENT_NODE ||
39-
(node.nodeType === COMMENT_NODE &&
40-
(node: any).nodeValue === ' react-mount-point-unstable '))
41-
);
42-
}

‎packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

+56-65
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ import {
9494
enableTrustedTypesIntegration,
9595
disableLegacyMode,
9696
enableMoveBefore,
97+
disableCommentsAsDOMContainers,
9798
} from 'shared/ReactFeatureFlags';
9899
import {
99100
HostComponent,
@@ -258,7 +259,7 @@ export function getRootHostContext(
258259
}
259260
default: {
260261
const container: any =
261-
nodeType === COMMENT_NODE
262+
!disableCommentsAsDOMContainers && nodeType === COMMENT_NODE
262263
? rootContainerInstance.parentNode
263264
: rootContainerInstance;
264265
type = container.tagName;
@@ -802,29 +803,25 @@ export function appendChildToContainer(
802803
container: Container,
803804
child: Instance | TextInstance,
804805
): void {
805-
let parentNode: Document | Element;
806-
switch (container.nodeType) {
807-
case COMMENT_NODE: {
808-
parentNode = (container.parentNode: any);
809-
if (supportsMoveBefore) {
810-
// $FlowFixMe[prop-missing]: We've checked this with supportsMoveBefore.
811-
parentNode.moveBefore(child, container);
812-
} else {
813-
parentNode.insertBefore(child, container);
814-
}
815-
return;
816-
}
817-
case DOCUMENT_NODE: {
818-
parentNode = (container: any).body;
819-
break;
820-
}
821-
default: {
822-
if (container.nodeName === 'HTML') {
823-
parentNode = (container.ownerDocument.body: any);
824-
} else {
825-
parentNode = (container: any);
826-
}
806+
let parentNode: DocumentFragment | Element;
807+
if (container.nodeType === DOCUMENT_NODE) {
808+
parentNode = (container: any).body;
809+
} else if (
810+
!disableCommentsAsDOMContainers &&
811+
container.nodeType === COMMENT_NODE
812+
) {
813+
parentNode = (container.parentNode: any);
814+
if (supportsMoveBefore) {
815+
// $FlowFixMe[prop-missing]: We've checked this with supportsMoveBefore.
816+
parentNode.moveBefore(child, container);
817+
} else {
818+
parentNode.insertBefore(child, container);
827819
}
820+
return;
821+
} else if (container.nodeName === 'HTML') {
822+
parentNode = (container.ownerDocument.body: any);
823+
} else {
824+
parentNode = (container: any);
828825
}
829826
if (supportsMoveBefore) {
830827
// $FlowFixMe[prop-missing]: We've checked this with supportsMoveBefore.
@@ -869,24 +866,18 @@ export function insertInContainerBefore(
869866
child: Instance | TextInstance,
870867
beforeChild: Instance | TextInstance | SuspenseInstance,
871868
): void {
872-
let parentNode: Document | Element;
873-
switch (container.nodeType) {
874-
case COMMENT_NODE: {
875-
parentNode = (container.parentNode: any);
876-
break;
877-
}
878-
case DOCUMENT_NODE: {
879-
const ownerDocument: Document = (container: any);
880-
parentNode = (ownerDocument.body: any);
881-
break;
882-
}
883-
default: {
884-
if (container.nodeName === 'HTML') {
885-
parentNode = (container.ownerDocument.body: any);
886-
} else {
887-
parentNode = (container: any);
888-
}
889-
}
869+
let parentNode: DocumentFragment | Element;
870+
if (container.nodeType === DOCUMENT_NODE) {
871+
parentNode = (container: any).body;
872+
} else if (
873+
!disableCommentsAsDOMContainers &&
874+
container.nodeType === COMMENT_NODE
875+
) {
876+
parentNode = (container.parentNode: any);
877+
} else if (container.nodeName === 'HTML') {
878+
parentNode = (container.ownerDocument.body: any);
879+
} else {
880+
parentNode = (container: any);
890881
}
891882
if (supportsMoveBefore) {
892883
// $FlowFixMe[prop-missing]: We've checked this with supportsMoveBefore.
@@ -943,20 +934,18 @@ export function removeChildFromContainer(
943934
container: Container,
944935
child: Instance | TextInstance | SuspenseInstance,
945936
): void {
946-
let parentNode: Document | Element;
947-
switch (container.nodeType) {
948-
case COMMENT_NODE:
949-
parentNode = (container.parentNode: any);
950-
break;
951-
case DOCUMENT_NODE:
952-
parentNode = (container: any).body;
953-
break;
954-
default:
955-
if (container.nodeName === 'HTML') {
956-
parentNode = (container.ownerDocument.body: any);
957-
} else {
958-
parentNode = (container: any);
959-
}
937+
let parentNode: DocumentFragment | Element;
938+
if (container.nodeType === DOCUMENT_NODE) {
939+
parentNode = (container: any).body;
940+
} else if (
941+
!disableCommentsAsDOMContainers &&
942+
container.nodeType === COMMENT_NODE
943+
) {
944+
parentNode = (container.parentNode: any);
945+
} else if (container.nodeName === 'HTML') {
946+
parentNode = (container.ownerDocument.body: any);
947+
} else {
948+
parentNode = (container: any);
960949
}
961950
parentNode.removeChild(child);
962951
}
@@ -1032,18 +1021,20 @@ export function clearSuspenseBoundaryFromContainer(
10321021
container: Container,
10331022
suspenseInstance: SuspenseInstance,
10341023
): void {
1035-
if (container.nodeType === COMMENT_NODE) {
1036-
clearSuspenseBoundary((container.parentNode: any), suspenseInstance);
1037-
} else if (container.nodeType === DOCUMENT_NODE) {
1038-
clearSuspenseBoundary((container: any).body, suspenseInstance);
1024+
let parentNode: DocumentFragment | Element;
1025+
if (container.nodeType === DOCUMENT_NODE) {
1026+
parentNode = (container: any).body;
1027+
} else if (
1028+
!disableCommentsAsDOMContainers &&
1029+
container.nodeType === COMMENT_NODE
1030+
) {
1031+
parentNode = (container.parentNode: any);
10391032
} else if (container.nodeName === 'HTML') {
1040-
clearSuspenseBoundary(
1041-
(container.ownerDocument.body: any),
1042-
suspenseInstance,
1043-
);
1033+
parentNode = (container.ownerDocument.body: any);
10441034
} else {
1045-
clearSuspenseBoundary((container: any), suspenseInstance);
1035+
parentNode = (container: any);
10461036
}
1037+
clearSuspenseBoundary(parentNode, suspenseInstance);
10471038
// Retry if any event replaying was blocked on this.
10481039
retryIfBlockedOn(container);
10491040
}
@@ -1985,7 +1976,7 @@ export function getNextHydratableSiblingAfterSingleton(
19851976
export function describeHydratableInstanceForDevWarnings(
19861977
instance: HydratableInstance,
19871978
): string | {type: string, props: $ReadOnly<Props>} {
1988-
// Reverse engineer a pseudo react-element from hydratable instnace
1979+
// Reverse engineer a pseudo react-element from hydratable instance
19891980
if (instance.nodeType === ELEMENT_NODE) {
19901981
// Reverse engineer a set of props that can print for dev warnings
19911982
return {

‎packages/react-dom-bindings/src/events/DOMPluginEventSystem.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import {
5353
enableCreateEventHandleAPI,
5454
enableScopeAPI,
5555
enableOwnerStacks,
56+
disableCommentsAsDOMContainers,
5657
} from 'shared/ReactFeatureFlags';
5758
import {createEventListenerWrapperWithPriority} from './ReactDOMEventListener';
5859
import {
@@ -558,7 +559,8 @@ function isMatchingRootContainer(
558559
): boolean {
559560
return (
560561
grandContainer === targetContainer ||
561-
(grandContainer.nodeType === COMMENT_NODE &&
562+
(!disableCommentsAsDOMContainers &&
563+
grandContainer.nodeType === COMMENT_NODE &&
562564
grandContainer.parentNode === targetContainer)
563565
);
564566
}

‎packages/react-dom/src/client/ReactDOMRoot.js

+2-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import type {
1616
import {isValidContainer} from 'react-dom-bindings/src/client/ReactDOMContainer';
1717
import {queueExplicitHydrationTarget} from 'react-dom-bindings/src/events/ReactDOMEventReplaying';
1818
import {REACT_ELEMENT_TYPE} from 'shared/ReactSymbols';
19+
import {disableCommentsAsDOMContainers} from 'shared/ReactFeatureFlags';
1920

2021
export type RootType = {
2122
render(children: ReactNodeList): void,
@@ -236,7 +237,7 @@ export function createRoot(
236237
markContainerAsRoot(root.current, container);
237238

238239
const rootContainerElement: Document | Element | DocumentFragment =
239-
container.nodeType === COMMENT_NODE
240+
!disableCommentsAsDOMContainers && container.nodeType === COMMENT_NODE
240241
? (container.parentNode: any)
241242
: container;
242243
listenToAllSupportedEvents(rootContainerElement);

‎packages/react-dom/src/client/ReactDOMRootFB.js

+14-7
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,10 @@ import {
2727
hydrateRoot as hydrateRootImpl,
2828
} from './ReactDOMRoot';
2929

30-
import {disableLegacyMode} from 'shared/ReactFeatureFlags';
30+
import {
31+
disableLegacyMode,
32+
disableCommentsAsDOMContainers,
33+
} from 'shared/ReactFeatureFlags';
3134
import {clearContainer} from 'react-dom-bindings/src/client/ReactFiberConfigDOM';
3235
import {
3336
getInstanceFromNode,
@@ -36,7 +39,7 @@ import {
3639
unmarkContainerAsRoot,
3740
} from 'react-dom-bindings/src/client/ReactDOMComponentTree';
3841
import {listenToAllSupportedEvents} from 'react-dom-bindings/src/events/DOMPluginEventSystem';
39-
import {isValidContainerLegacy} from 'react-dom-bindings/src/client/ReactDOMContainer';
42+
import {isValidContainer} from 'react-dom-bindings/src/client/ReactDOMContainer';
4043
import {
4144
DOCUMENT_NODE,
4245
ELEMENT_NODE,
@@ -244,7 +247,9 @@ function legacyCreateRootFromDOMContainer(
244247
markContainerAsRoot(root.current, container);
245248

246249
const rootContainerElement =
247-
container.nodeType === COMMENT_NODE ? container.parentNode : container;
250+
!disableCommentsAsDOMContainers && container.nodeType === COMMENT_NODE
251+
? container.parentNode
252+
: container;
248253
// $FlowFixMe[incompatible-call]
249254
listenToAllSupportedEvents(rootContainerElement);
250255

@@ -278,7 +283,9 @@ function legacyCreateRootFromDOMContainer(
278283
markContainerAsRoot(root.current, container);
279284

280285
const rootContainerElement =
281-
container.nodeType === COMMENT_NODE ? container.parentNode : container;
286+
!disableCommentsAsDOMContainers && container.nodeType === COMMENT_NODE
287+
? container.parentNode
288+
: container;
282289
// $FlowFixMe[incompatible-call]
283290
listenToAllSupportedEvents(rootContainerElement);
284291

@@ -394,7 +401,7 @@ export function render(
394401
);
395402
}
396403

397-
if (!isValidContainerLegacy(container)) {
404+
if (!isValidContainer(container)) {
398405
throw new Error('Target container is not a DOM element.');
399406
}
400407

@@ -428,7 +435,7 @@ export function unmountComponentAtNode(container: Container): boolean {
428435
}
429436
throw new Error('ReactDOM: Unsupported Legacy Mode API.');
430437
}
431-
if (!isValidContainerLegacy(container)) {
438+
if (!isValidContainer(container)) {
432439
throw new Error('Target container is not a DOM element.');
433440
}
434441

@@ -472,7 +479,7 @@ export function unmountComponentAtNode(container: Container): boolean {
472479
// Check if the container itself is a React root node.
473480
const isContainerReactRoot =
474481
container.nodeType === ELEMENT_NODE &&
475-
isValidContainerLegacy(container.parentNode) &&
482+
isValidContainer(container.parentNode) &&
476483
// $FlowFixMe[prop-missing]
477484
// $FlowFixMe[incompatible-use]
478485
!!container.parentNode._reactRootContainer;

0 commit comments

Comments
 (0)