[Fiber] Disable comments as containers in OSS (#32250)
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.
Josh Story committed
Feb 4, 2025 at 12:39 UTC
0605cd9f38f8b9d0ca6f8bd9dd3409db8d6c5c81
5 files changed
+75
-87
packages/react-dom-bindings/src/client/ReactDOMContainer.js
-13
@@ -27,16 +27,3 @@ export function isValidContainer(node: any): boolean {
27
(node: any).nodeValue === ' react-mount-point-unstable '))
28
);
29
}
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
@@ -94,6 +94,7 @@ import {
94
enableTrustedTypesIntegration,
95
disableLegacyMode,
96
enableMoveBefore,
97
+ disableCommentsAsDOMContainers,
98
} from 'shared/ReactFeatureFlags';
99
import {
100
HostComponent,
@@ -258,7 +259,7 @@ export function getRootHostContext(
259
}
260
default: {
261
const container: any =
261
- nodeType === COMMENT_NODE
262
+ !disableCommentsAsDOMContainers && nodeType === COMMENT_NODE
263
? rootContainerInstance.parentNode
264
: rootContainerInstance;
265
type = container.tagName;
@@ -802,29 +803,25 @@ export function appendChildToContainer(
803
container: Container,
804
child: Instance | TextInstance,
805
): 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);
819
}
820
+ return;
821
+ } else if (container.nodeName === 'HTML') {
822
+ parentNode = (container.ownerDocument.body: any);
823
+ } else {
824
+ parentNode = (container: any);
825
}
826
if (supportsMoveBefore) {
827
// $FlowFixMe[prop-missing]: We've checked this with supportsMoveBefore.
@@ -869,24 +866,18 @@ export function insertInContainerBefore(
866
child: Instance | TextInstance,
867
beforeChild: Instance | TextInstance | SuspenseInstance,
868
): 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);
881
}
882
if (supportsMoveBefore) {
883
// $FlowFixMe[prop-missing]: We've checked this with supportsMoveBefore.
@@ -943,20 +934,18 @@ export function removeChildFromContainer(
934
container: Container,
935
child: Instance | TextInstance | SuspenseInstance,
936
): 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);
949
}
950
parentNode.removeChild(child);
951
}
@@ -1039,18 +1028,20 @@ export function clearSuspenseBoundaryFromContainer(
1028
container: Container,
1029
suspenseInstance: SuspenseInstance,
1030
): void {
1042
- if (container.nodeType === COMMENT_NODE) {
1043
- clearSuspenseBoundary((container.parentNode: any), suspenseInstance);
1044
- } else if (container.nodeType === DOCUMENT_NODE) {
1045
- clearSuspenseBoundary((container: any).body, suspenseInstance);
1031
+ let parentNode: DocumentFragment | Element;
1032
+ if (container.nodeType === DOCUMENT_NODE) {
1033
+ parentNode = (container: any).body;
1034
+ } else if (
1035
+ !disableCommentsAsDOMContainers &&
1036
+ container.nodeType === COMMENT_NODE
1037
+ ) {
1038
+ parentNode = (container.parentNode: any);
1039
} else if (container.nodeName === 'HTML') {
1047
- clearSuspenseBoundary(
1048
- (container.ownerDocument.body: any),
1049
- suspenseInstance,
1050
- );
1040
+ parentNode = (container.ownerDocument.body: any);
1041
} else {
1052
- clearSuspenseBoundary((container: any), suspenseInstance);
1042
+ parentNode = (container: any);
1043
}
1044
+ clearSuspenseBoundary(parentNode, suspenseInstance);
1045
// Retry if any event replaying was blocked on this.
1046
retryIfBlockedOn(container);
1047
}
@@ -1992,7 +1983,7 @@ export function getNextHydratableSiblingAfterSingleton(
1983
export function describeHydratableInstanceForDevWarnings(
1984
instance: HydratableInstance,
1985
): string | {type: string, props: $ReadOnly<Props>} {
1995
- // Reverse engineer a pseudo react-element from hydratable instnace
1986
+ // Reverse engineer a pseudo react-element from hydratable instance
1987
if (instance.nodeType === ELEMENT_NODE) {
1988
// Reverse engineer a set of props that can print for dev warnings
1989
return {
packages/react-dom-bindings/src/events/DOMPluginEventSystem.js
+3
-1
@@ -53,6 +53,7 @@ import {
53
enableCreateEventHandleAPI,
54
enableScopeAPI,
55
enableOwnerStacks,
56
+ disableCommentsAsDOMContainers,
57
} from 'shared/ReactFeatureFlags';
58
import {createEventListenerWrapperWithPriority} from './ReactDOMEventListener';
59
import {
@@ -558,7 +559,8 @@ function isMatchingRootContainer(
559
): boolean {
560
return (
561
grandContainer === targetContainer ||
561
- (grandContainer.nodeType === COMMENT_NODE &&
562
+ (!disableCommentsAsDOMContainers &&
563
+ grandContainer.nodeType === COMMENT_NODE &&
564
grandContainer.parentNode === targetContainer)
565
);
566
}
packages/react-dom/src/client/ReactDOMRoot.js
+2
-1
@@ -16,6 +16,7 @@ import type {
16
import {isValidContainer} from 'react-dom-bindings/src/client/ReactDOMContainer';
17
import {queueExplicitHydrationTarget} from 'react-dom-bindings/src/events/ReactDOMEventReplaying';
18
import {REACT_ELEMENT_TYPE} from 'shared/ReactSymbols';
19
+import {disableCommentsAsDOMContainers} from 'shared/ReactFeatureFlags';
20
21
export type RootType = {
22
render(children: ReactNodeList): void,
@@ -236,7 +237,7 @@ export function createRoot(
237
markContainerAsRoot(root.current, container);
238
239
const rootContainerElement: Document | Element | DocumentFragment =
239
- container.nodeType === COMMENT_NODE
240
+ !disableCommentsAsDOMContainers && container.nodeType === COMMENT_NODE
241
? (container.parentNode: any)
242
: container;
243
listenToAllSupportedEvents(rootContainerElement);
packages/react-dom/src/client/ReactDOMRootFB.js
+14
-7
@@ -27,7 +27,10 @@ import {
27
hydrateRoot as hydrateRootImpl,
28
} from './ReactDOMRoot';
29
30
-import {disableLegacyMode} from 'shared/ReactFeatureFlags';
30
+import {
31
+ disableLegacyMode,
32
+ disableCommentsAsDOMContainers,
33
+} from 'shared/ReactFeatureFlags';
34
import {clearContainer} from 'react-dom-bindings/src/client/ReactFiberConfigDOM';
35
import {
36
getInstanceFromNode,
@@ -36,7 +39,7 @@ import {
39
unmarkContainerAsRoot,
40
} from 'react-dom-bindings/src/client/ReactDOMComponentTree';
41
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';
43
import {
44
DOCUMENT_NODE,
45
ELEMENT_NODE,
@@ -244,7 +247,9 @@ function legacyCreateRootFromDOMContainer(
247
markContainerAsRoot(root.current, container);
248
249
const rootContainerElement =
247
- container.nodeType === COMMENT_NODE ? container.parentNode : container;
250
+ !disableCommentsAsDOMContainers && container.nodeType === COMMENT_NODE
251
+ ? container.parentNode
252
+ : container;
253
// $FlowFixMe[incompatible-call]
254
listenToAllSupportedEvents(rootContainerElement);
255
@@ -278,7 +283,9 @@ function legacyCreateRootFromDOMContainer(
283
markContainerAsRoot(root.current, container);
284
285
const rootContainerElement =
281
- container.nodeType === COMMENT_NODE ? container.parentNode : container;
286
+ !disableCommentsAsDOMContainers && container.nodeType === COMMENT_NODE
287
+ ? container.parentNode
288
+ : container;
289
// $FlowFixMe[incompatible-call]
290
listenToAllSupportedEvents(rootContainerElement);
291
@@ -394,7 +401,7 @@ export function render(
401
);
402
}
403
397
- if (!isValidContainerLegacy(container)) {
404
+ if (!isValidContainer(container)) {
405
throw new Error('Target container is not a DOM element.');
406
}
407
@@ -428,7 +435,7 @@ export function unmountComponentAtNode(container: Container): boolean {
435
}
436
throw new Error('ReactDOM: Unsupported Legacy Mode API.');
437
}
431
- if (!isValidContainerLegacy(container)) {
438
+ if (!isValidContainer(container)) {
439
throw new Error('Target container is not a DOM element.');
440
}
441
@@ -472,7 +479,7 @@ export function unmountComponentAtNode(container: Container): boolean {
479
// Check if the container itself is a React root node.
480
const isContainerReactRoot =
481
container.nodeType === ELEMENT_NODE &&
475
- isValidContainerLegacy(container.parentNode) &&
482
+ isValidContainer(container.parentNode) &&
483
// $FlowFixMe[prop-missing]
484
// $FlowFixMe[incompatible-use]
485
!!container.parentNode._reactRootContainer;