Fix: Activity should hide portal contents
Project: facebook/react | Language: JavaScript | Concepts: data-structures, refactoring | Difficulty: senior
Link: https://github.com/react/react/pull/35091
The problem
When an Activity boundary in React is hidden, its direct DOM children are hidden, but any portal contents inside it remain visible. This is because portals are rendered outside the normal DOM hierarchy, so hiding the parent Activity does not affect the portal's contents. This leads to inconsistent UI where parts of the app that should be hidden remain visible.
Why it's non-trivial
A naive fix would only hide the direct DOM children of the Activity component, missing any portals nested within. Portals break the usual parent-child DOM structure, so you can't rely on hiding a parent to hide everything inside it. You need to explicitly find and hide all portal contents, no matter how deeply nested, without incurring unnecessary tree traversal costs for subtrees that don't contain portals.
The approach
1. Add a new subtree flag to efficiently track portal presence
A new flag, PortalStatic, is introduced to mark subtrees that contain a portal. This allows the reconciler to skip traversing subtrees that do not contain portals when hiding/unhiding.
+// Tracks whether a HostPortal is present in the tree.
+export const PortalStatic = /* */ 0b0000100000000000000000000000000;
...
-export const StaticMask =
- RefStatic |
- MaySuspendCommit |
- ViewTransitionStatic |
- ViewTransitionNamedStatic;
+export const StaticMask =
+ RefStatic |
+ MaySuspendCommit |
+ ViewTransitionStatic |
+ ViewTransitionNamedStatic |
+ PortalStatic;
This flag is used to mark and quickly identify subtrees with portals, optimizing traversal.
2. Set the PortalStatic flag when completing a portal
When React finishes processing a HostPortal fiber, it sets the PortalStatic flag.
+ workInProgress.flags |= PortalStatic;
bubbleProperties(workInProgress);
return null;
This ensures that any subtree containing a portal is marked for later traversal.
3. Refactor the hide/unhide logic to handle portals recursively
The core logic for hiding/unhiding is restructured to:
- Hide direct host children as before.
- Recursively search for and hide any portals within the subtree, using the new flag for efficiency.
-function hideOrUnhideAllChildren(finishedWork: Fiber, isHidden: boolean) {
- // Only hide or unhide the top-most host nodes.
- let hostSubtreeRoot = null;
-
- if (supportsMutation) {
- // We only have the top Fiber that was inserted but we need to recurse down its
- // children to find all the terminal nodes.
- let node: Fiber = finishedWork;
- while (true) {
- if (
- node.tag === HostComponent ||
- (supportsResources ? node.tag === HostHoistable : false)
- ) {
- if (hostSubtreeRoot === null) {
- hostSubtreeRoot = node;
- commitShowHideHostInstance(node, isHidden);
- }
- } else if (node.tag === HostText) {
- if (hostSubtreeRoot === null) {
- commitShowHideHostTextInstance(node, isHidden);
- }
- } else if (node.tag === DehydratedFragment) {
- if (hostSubtreeRoot === null) {
- commitShowHideSuspenseBoundary(node, isHidden);
- }
- } else if (
- (node.tag === OffscreenComponent ||
- node.tag === LegacyHiddenComponent) &&
- (node.memoizedState: OffscreenState) !== null &&
- node !== finishedWork
- ) {
- // Found a nested Offscreen component that is hidden.
- // Don't search any deeper. This tree should remain hidden.
- } else if (node.child !== null) {
- node.child.return = node;
- node = node.child;
- continue;
- }
-
- if (node === finishedWork) {
- return;
- }
- while (node.sibling === null) {
- if (node.return === null || node.return === finishedWork) {
- return;
- }
-
- if (hostSubtreeRoot === node) {
- hostSubtreeRoot = null;
- }
-
- node = node.return;
- }
-
- if (hostSubtreeRoot === node) {
- hostSubtreeRoot = null;
- }
-
- node.sibling.return = node.return;
- node = node.sibling;
- }
- }
-}
+function hideOrUnhideAllChildren(parentFiber: Fiber, isHidden: boolean) {
+ if (!supportsMutation) {
+ return;
+ }
+ // Finds the nearest host component children and updates their visibility
+ // to either hidden or visible.
+ let child = parentFiber.child;
+ while (child !== null) {
+ hideOrUnhideAllChildrenOnFiber(child, isHidden);
+ child = child.sibling;
+ }
+}
This replaces the previous iterative approach with recursion, which is easier to reason about and extend.
4. Add logic to recursively hide/unhide portal contents
+function hideOrUnhideAllChildrenOnFiber(fiber: Fiber, isHidden: boolean) {
+ if (!supportsMutation) {
+ return;
+ }
+ switch (fiber.tag) {
+ case HostComponent:
+ case HostHoistable: {
+ commitShowHideHostInstance(fiber, isHidden);
+ hideOrUnhideNearestPortals(fiber, isHidden);
+ return;
+ }
+ case HostText: {
+ commitShowHideHostTextInstance(fiber, isHidden);
+ return;
+ }
+ case DehydratedFragment: {
+ commitShowHideSuspenseBoundary(fiber, isHidden);
+ return;
+ }
+ case OffscreenComponent:
+ case LegacyHiddenComponent: {
+ const offscreenState: OffscreenState | null = fiber.memoizedState;
+ if (offscreenState !== null) {
+ // Found a nested Offscreen component that is hidden.
+ // Don't search any deeper. This tree should remain hidden.
+ } else {
+ hideOrUnhideAllChildren(fiber, isHidden);
+ }
+ return;
+ }
+ default: {
+ hideOrUnhideAllChildren(fiber, isHidden);
+ return;
+ }
+ }
+}
+
+function hideOrUnhideNearestPortals(parentFiber: Fiber, isHidden: boolean) {
+ if (!supportsMutation) {
+ return;
+ }
+ if (parentFiber.subtreeFlags & PortalStatic) {
+ let child = parentFiber.child;
+ while (child !== null) {
+ hideOrUnhideNearestPortalsOnFiber(child, isHidden);
+ child = child.sibling;
+ }
+ }
+}
+
+function hideOrUnhideNearestPortalsOnFiber(fiber: Fiber, isHidden: boolean) {
+ if (!supportsMutation) {
+ return;
+ }
+ switch (fiber.tag) {
+ case HostPortal: {
+ hideOrUnhideAllChildrenOnFiber(fiber, isHidden);
+ return;
+ }
+ case OffscreenComponent: {
+ const offscreenState: OffscreenState | null = fiber.memoizedState;
+ if (offscreenState !== null) {
+ // Found a nested Offscreen component that is hidden. Don't search any
+ // deeper. This tree should remain hidden.
+ } else {
+ hideOrUnhideNearestPortals(fiber, isHidden);
+ }
+ return;
+ }
+ default: {
+ hideOrUnhideNearestPortals(fiber, isHidden);
+ return;
+ }
+ }
+}
This ensures that all portal contents within a hidden Activity are also hidden, no matter how deeply nested.
5. Update mutation effects to track hidden state through portals
+ // For the purposes of visibility toggling, the direct children of a
+ // portal are considered "children" of the nearest hidden
+ // OffscreenComponent, regardless of whether there are any host components
+ // in between them. This is because portals are not part of the regular
+ // host tree hierarchy; we can't assume that just because a portal's
+ // HostComponent parent in the React tree will also be a parent in the
+ // actual host tree. So we must hide all of them.
+ const prevOffscreenDirectParentIsHidden = offscreenDirectParentIsHidden;
+ offscreenDirectParentIsHidden = offscreenSubtreeIsHidden;
...
+ offscreenDirectParentIsHidden = prevOffscreenDirectParentIsHidden;
This ensures the correct hidden state is applied to portal children.
6. Add tests to verify the new behavior
New tests ensure that hiding an Activity boundary also hides portal contents, and that revealing an Activity inside a portal does not reveal its contents if the parent Activity is still hidden.
+ it(
+ 'hiding an Activity boundary also hides the direct children of any ' +
+ 'portals it contains, regardless of how deeply nested they are',
+ async () => {
+ // ...test setup...
+ expect(container.innerHTML).toBe('<div></div>');
+ expect(portalContainer.innerHTML).toBe('<div>Portal contents</div>');
+
+ // Hide the Activity boundary. Not only are the nearest DOM elements hidden,
+ // but also the children of the nested portal contained within it.
+ await act(() => setShow(false));
+ expect(container.innerHTML).toBe('<div style="display: none;"></div>');
+ expect(portalContainer.innerHTML).toBe(
+ '<div style="display: none;">Portal contents</div>',
+ );
+ },
+ );
This test ensures the fix works as intended.
From the review thread
rickhanlonii: Do you know why the old code tracked the
hostSubtreeRoot? tbh the old code doesn't make sense to me, and the new code is very clean but it's hard to know if there are subtle bugs.
acdlite: It was to prevent the traversal from going deeper than the nearest host components, which is now solved via recursion instead
rickhanlonii: This had a
node !== finishedWorkcheck before? Not sure why?
acdlite: It was to prevent the traversal from going deeper than the nearest host components, which is now solved via recursion instead
Takeaways
- Special data structures (like portals) often require custom traversal logic: When UI elements break the normal parent-child hierarchy, you can't rely on generic tree operations—explicit handling is needed.
- Use subtree flags to optimize tree traversal: Marking subtrees with flags (like
PortalStatic) enables efficient, targeted operations and avoids unnecessary work. - Refactoring from iterative to recursive traversal can improve clarity: Recursive approaches can make complex tree operations easier to reason about and extend, especially when handling special cases.