@samitouri / QOS-React / commits / 5268492536

Fix: Activity should hide portal contents (#35091)

This PR updates the behavior of Activity so that when it is hidden, it hides the contents of any portals contained within it. Previously we had intentionally chosen not to implement this behavior, because it was thought that this concern should be left to the userspace code that manages the portal, e.g. by adding or removing the portal container from the DOM. Depending on the use case for the portal, this is often desirable anyway because the portal container itself is not controlled by React. However, React does own the _contents_ of the portal, and we can hide those elements regardless of what the user chooses to do with the container. This makes the hiding/unhiding behavior of portals with Activity automatic in the majority of cases, and also benefits from aligning the DOM mutations with the rest of the React's commit phase lifecycle. The reason we have to special case this at all is because usually we only hide the direct DOM children of the Activity boundary. There's no reason to go deeper than that, because hiding a parent DOM element effectively hides everything inside of it. Portals are the exception, because they don't exist in the normal DOM hierarchy; we can't assume that just because a portal has a parent in the React tree that it will also have that parent in the actual DOM. So, whenever an Activity boundary is hidden, we must search for and hide _any_ portal that is contained within it, and recursively hide its direct children, too. To optimize this search, we use a new subtree flag, PortalStatic, that is set only on fiber paths that contain a HostPortal. This lets us skip over any subtree that does not contain a portal.

Andrew Clark committed Nov 10, 2025 at 07:42 UTC 52684925368a41a0c9fbfca9016cdcbb72fc9d1e
4 files changed +245 -57
packages/react-dom/src/__tests__/ReactDOMActivity-test.js new
+134
@@ -0,0 +1,134 @@
1 +/**
2 + * Copyright (c) Meta Platforms, Inc. and affiliates.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + *
7 + * @emails react-core
8 + */
9 +
10 +'use strict';
11 +
12 +let React;
13 +let Activity;
14 +let useState;
15 +let ReactDOM;
16 +let ReactDOMClient;
17 +let act;
18 +
19 +describe('ReactDOMActivity', () => {
20 + let container;
21 +
22 + beforeEach(() => {
23 + jest.resetModules();
24 + React = require('react');
25 + Activity = React.Activity;
26 + useState = React.useState;
27 + ReactDOM = require('react-dom');
28 + ReactDOMClient = require('react-dom/client');
29 + act = require('internal-test-utils').act;
30 + container = document.createElement('div');
31 + document.body.appendChild(container);
32 + });
33 +
34 + afterEach(() => {
35 + document.body.removeChild(container);
36 + });
37 +
38 + // @gate enableActivity
39 + it(
40 + 'hiding an Activity boundary also hides the direct children of any ' +
41 + 'portals it contains, regardless of how deeply nested they are',
42 + async () => {
43 + const portalContainer = document.createElement('div');
44 +
45 + let setShow;
46 + function Accordion({children}) {
47 + const [shouldShow, _setShow] = useState(true);
48 + setShow = _setShow;
49 + return (
50 + <Activity mode={shouldShow ? 'visible' : 'hidden'}>
51 + {children}
52 + </Activity>
53 + );
54 + }
55 +
56 + function App({portalContents}) {
57 + return (
58 + <Accordion>
59 + <div>
60 + {ReactDOM.createPortal(
61 + <div>Portal contents</div>,
62 + portalContainer,
63 + )}
64 + </div>
65 + </Accordion>
66 + );
67 + }
68 +
69 + const root = ReactDOMClient.createRoot(container);
70 + await act(() => root.render(<App />));
71 + expect(container.innerHTML).toBe('<div></div>');
72 + expect(portalContainer.innerHTML).toBe('<div>Portal contents</div>');
73 +
74 + // Hide the Activity boundary. Not only are the nearest DOM elements hidden,
75 + // but also the children of the nested portal contained within it.
76 + await act(() => setShow(false));
77 + expect(container.innerHTML).toBe('<div style="display: none;"></div>');
78 + expect(portalContainer.innerHTML).toBe(
79 + '<div style="display: none;">Portal contents</div>',
80 + );
81 + },
82 + );
83 +
84 + // @gate enableActivity
85 + it(
86 + 'revealing an Activity boundary inside a portal does not reveal the ' +
87 + 'portal contents if has a hidden Activity parent',
88 + async () => {
89 + const portalContainer = document.createElement('div');
90 +
91 + let setShow;
92 + function Accordion({children}) {
93 + const [shouldShow, _setShow] = useState(false);
94 + setShow = _setShow;
95 + return (
96 + <Activity mode={shouldShow ? 'visible' : 'hidden'}>
97 + {children}
98 + </Activity>
99 + );
100 + }
101 +
102 + function App({portalContents}) {
103 + return (
104 + <Activity mode="hidden">
105 + <div>
106 + {ReactDOM.createPortal(
107 + <Accordion>
108 + <div>Portal contents</div>
109 + </Accordion>,
110 + portalContainer,
111 + )}
112 + </div>
113 + </Activity>
114 + );
115 + }
116 +
117 + // Start with both boundaries hidden.
118 + const root = ReactDOMClient.createRoot(container);
119 + await act(() => root.render(<App />));
120 + expect(container.innerHTML).toBe('<div style="display: none;"></div>');
121 + expect(portalContainer.innerHTML).toBe(
122 + '<div style="display: none;">Portal contents</div>',
123 + );
124 +
125 + // Reveal the inner Activity boundary. It should not reveal its children,
126 + // because there's a parent Activity boundary that is still hidden.
127 + await act(() => setShow(true));
128 + expect(container.innerHTML).toBe('<div style="display: none;"></div>');
129 + expect(portalContainer.innerHTML).toBe(
130 + '<div style="display: none;">Portal contents</div>',
131 + );
132 + },
133 + );
134 +});
packages/react-reconciler/src/ReactFiberCommitWork.js
+102 -53
@@ -117,6 +117,7 @@ import {
117 DidCapture,
118 AffectedParentLayout,
119 ViewTransitionNamedStatic,
120 + PortalStatic,
121 } from './ReactFiberFlags';
122 import {
123 commitStartTime,
@@ -1182,66 +1183,104 @@ function commitTransitionProgress(offscreenFiber: Fiber) {
1183 }
1184 }
1185
1185 -function hideOrUnhideAllChildren(finishedWork: Fiber, isHidden: boolean) {
1186 - // Only hide or unhide the top-most host nodes.
1187 - let hostSubtreeRoot = null;
1186 +function hideOrUnhideAllChildren(parentFiber: Fiber, isHidden: boolean) {
1187 + if (!supportsMutation) {
1188 + return;
1189 + }
1190 + // Finds the nearest host component children and updates their visibility
1191 + // to either hidden or visible.
1192 + let child = parentFiber.child;
1193 + while (child !== null) {
1194 + hideOrUnhideAllChildrenOnFiber(child, isHidden);
1195 + child = child.sibling;
1196 + }
1197 +}
1198
1189 - if (supportsMutation) {
1190 - // We only have the top Fiber that was inserted but we need to recurse down its
1191 - // children to find all the terminal nodes.
1192 - let node: Fiber = finishedWork;
1193 - while (true) {
1194 - if (
1195 - node.tag === HostComponent ||
1196 - (supportsResources ? node.tag === HostHoistable : false)
1197 - ) {
1198 - if (hostSubtreeRoot === null) {
1199 - hostSubtreeRoot = node;
1200 - commitShowHideHostInstance(node, isHidden);
1201 - }
1202 - } else if (node.tag === HostText) {
1203 - if (hostSubtreeRoot === null) {
1204 - commitShowHideHostTextInstance(node, isHidden);
1205 - }
1206 - } else if (node.tag === DehydratedFragment) {
1207 - if (hostSubtreeRoot === null) {
1208 - commitShowHideSuspenseBoundary(node, isHidden);
1209 - }
1210 - } else if (
1211 - (node.tag === OffscreenComponent ||
1212 - node.tag === LegacyHiddenComponent) &&
1213 - (node.memoizedState: OffscreenState) !== null &&
1214 - node !== finishedWork
1215 - ) {
1199 +function hideOrUnhideAllChildrenOnFiber(fiber: Fiber, isHidden: boolean) {
1200 + if (!supportsMutation) {
1201 + return;
1202 + }
1203 + switch (fiber.tag) {
1204 + case HostComponent:
1205 + case HostHoistable: {
1206 + // Found the nearest host component. Hide it.
1207 + commitShowHideHostInstance(fiber, isHidden);
1208 + // Typically, only the nearest host nodes need to be hidden, since that
1209 + // has the effect of also hiding everything inside of them.
1210 + //
1211 + // However, there's a special case for portals, because portals do not
1212 + // exist in the regular host tree hierarchy; we can't assume that just
1213 + // because a portal's HostComponent parent in the React tree will also be
1214 + // a parent in the actual host tree.
1215 + //
1216 + // So, if any portals exist within the tree, regardless of how deeply
1217 + // nested they are, we need to repeat this algorithm for its children.
1218 + hideOrUnhideNearestPortals(fiber, isHidden);
1219 + return;
1220 + }
1221 + case HostText: {
1222 + commitShowHideHostTextInstance(fiber, isHidden);
1223 + return;
1224 + }
1225 + case DehydratedFragment: {
1226 + commitShowHideSuspenseBoundary(fiber, isHidden);
1227 + return;
1228 + }
1229 + case OffscreenComponent:
1230 + case LegacyHiddenComponent: {
1231 + const offscreenState: OffscreenState | null = fiber.memoizedState;
1232 + if (offscreenState !== null) {
1233 // Found a nested Offscreen component that is hidden.
1234 // Don't search any deeper. This tree should remain hidden.
1218 - } else if (node.child !== null) {
1219 - node.child.return = node;
1220 - node = node.child;
1221 - continue;
1222 - }
1223 -
1224 - if (node === finishedWork) {
1225 - return;
1235 + } else {
1236 + hideOrUnhideAllChildren(fiber, isHidden);
1237 }
1227 - while (node.sibling === null) {
1228 - if (node.return === null || node.return === finishedWork) {
1229 - return;
1230 - }
1231 -
1232 - if (hostSubtreeRoot === node) {
1233 - hostSubtreeRoot = null;
1234 - }
1238 + return;
1239 + }
1240 + default: {
1241 + hideOrUnhideAllChildren(fiber, isHidden);
1242 + return;
1243 + }
1244 + }
1245 +}
1246
1236 - node = node.return;
1237 - }
1247 +function hideOrUnhideNearestPortals(parentFiber: Fiber, isHidden: boolean) {
1248 + if (!supportsMutation) {
1249 + return;
1250 + }
1251 + if (parentFiber.subtreeFlags & PortalStatic) {
1252 + let child = parentFiber.child;
1253 + while (child !== null) {
1254 + hideOrUnhideNearestPortalsOnFiber(child, isHidden);
1255 + child = child.sibling;
1256 + }
1257 + }
1258 +}
1259
1239 - if (hostSubtreeRoot === node) {
1240 - hostSubtreeRoot = null;
1260 +function hideOrUnhideNearestPortalsOnFiber(fiber: Fiber, isHidden: boolean) {
1261 + if (!supportsMutation) {
1262 + return;
1263 + }
1264 + switch (fiber.tag) {
1265 + case HostPortal: {
1266 + // Found a portal. Switch back to the normal hide/unhide algorithm to
1267 + // toggle the visibility of its children.
1268 + hideOrUnhideAllChildrenOnFiber(fiber, isHidden);
1269 + return;
1270 + }
1271 + case OffscreenComponent: {
1272 + const offscreenState: OffscreenState | null = fiber.memoizedState;
1273 + if (offscreenState !== null) {
1274 + // Found a nested Offscreen component that is hidden. Don't search any
1275 + // deeper. This tree should remain hidden.
1276 + } else {
1277 + hideOrUnhideNearestPortals(fiber, isHidden);
1278 }
1242 -
1243 - node.sibling.return = node.return;
1244 - node = node.sibling;
1279 + return;
1280 + }
1281 + default: {
1282 + hideOrUnhideNearestPortals(fiber, isHidden);
1283 + return;
1284 }
1285 }
1286 }
@@ -2305,6 +2344,15 @@ function commitMutationEffectsOnFiber(
2344 break;
2345 }
2346 case HostPortal: {
2347 + // For the purposes of visibility toggling, the direct children of a
2348 + // portal are considered "children" of the nearest hidden
2349 + // OffscreenComponent, regardless of whether there are any host components
2350 + // in between them. This is because portals are not part of the regular
2351 + // host tree hierarchy; we can't assume that just because a portal's
2352 + // HostComponent parent in the React tree will also be a parent in the
2353 + // actual host tree. So we must hide all of them.
2354 + const prevOffscreenDirectParentIsHidden = offscreenDirectParentIsHidden;
2355 + offscreenDirectParentIsHidden = offscreenSubtreeIsHidden;
2356 const prevMutationContext = pushMutationContext();
2357 if (supportsResources) {
2358 const previousHoistableRoot = currentHoistableRoot;
@@ -2326,6 +2374,7 @@ function commitMutationEffectsOnFiber(
2374 rootViewTransitionAffected = true;
2375 }
2376 popMutationContext(prevMutationContext);
2377 + offscreenDirectParentIsHidden = prevOffscreenDirectParentIsHidden;
2378
2379 if (flags & Update) {
2380 if (supportsPersistence) {
packages/react-reconciler/src/ReactFiberCompleteWork.js
+2
@@ -99,6 +99,7 @@ import {
99 Cloned,
100 ViewTransitionStatic,
101 Hydrate,
102 + PortalStatic,
103 } from './ReactFiberFlags';
104
105 import {
@@ -1665,6 +1666,7 @@ function completeWork(
1666 if (current === null) {
1667 preparePortalMount(workInProgress.stateNode.containerInfo);
1668 }
1669 + workInProgress.flags |= PortalStatic;
1670 bubbleProperties(workInProgress);
1671 return null;
1672 case ContextProvider:
packages/react-reconciler/src/ReactFiberFlags.js
+7 -4
@@ -83,11 +83,13 @@ export const ViewTransitionNamedStatic =
83 // ViewTransitionStatic tracks whether there are an ViewTransition components from
84 // the nearest HostComponent down. It resets at every HostComponent level.
85 export const ViewTransitionStatic = /* */ 0b0000010000000000000000000000000;
86 +// Tracks whether a HostPortal is present in the tree.
87 +export const PortalStatic = /* */ 0b0000100000000000000000000000000;
88
89 // Flag used to identify newly inserted fibers. It isn't reset after commit unlike `Placement`.
88 -export const PlacementDEV = /* */ 0b0000100000000000000000000000000;
89 -export const MountLayoutDev = /* */ 0b0001000000000000000000000000000;
90 -export const MountPassiveDev = /* */ 0b0010000000000000000000000000000;
90 +export const PlacementDEV = /* */ 0b0001000000000000000000000000000;
91 +export const MountLayoutDev = /* */ 0b0010000000000000000000000000000;
92 +export const MountPassiveDev = /* */ 0b0100000000000000000000000000000;
93
94 // Groups of flags that are used in the commit phase to skip over trees that
95 // don't contain effects, by checking subtreeFlags.
@@ -139,4 +141,5 @@ export const StaticMask =
141 RefStatic |
142 MaySuspendCommit |
143 ViewTransitionStatic |
142 - ViewTransitionNamedStatic;
144 + ViewTransitionNamedStatic |
145 + PortalStatic;