From 52684925368a41a0c9fbfca9016cdcbb72fc9d1e Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 10 Nov 2025 07:42:26 -0800 Subject: [PATCH 1/2] 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. --- .../src/__tests__/ReactDOMActivity-test.js | 134 +++++++++++++++ .../src/ReactFiberCommitWork.js | 155 ++++++++++++------ .../src/ReactFiberCompleteWork.js | 2 + .../react-reconciler/src/ReactFiberFlags.js | 11 +- 4 files changed, 245 insertions(+), 57 deletions(-) create mode 100644 packages/react-dom/src/__tests__/ReactDOMActivity-test.js diff --git a/packages/react-dom/src/__tests__/ReactDOMActivity-test.js b/packages/react-dom/src/__tests__/ReactDOMActivity-test.js new file mode 100644 index 0000000000000..e849ddc501d7e --- /dev/null +++ b/packages/react-dom/src/__tests__/ReactDOMActivity-test.js @@ -0,0 +1,134 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +let React; +let Activity; +let useState; +let ReactDOM; +let ReactDOMClient; +let act; + +describe('ReactDOMActivity', () => { + let container; + + beforeEach(() => { + jest.resetModules(); + React = require('react'); + Activity = React.Activity; + useState = React.useState; + ReactDOM = require('react-dom'); + ReactDOMClient = require('react-dom/client'); + act = require('internal-test-utils').act; + container = document.createElement('div'); + document.body.appendChild(container); + }); + + afterEach(() => { + document.body.removeChild(container); + }); + + // @gate enableActivity + it( + 'hiding an Activity boundary also hides the direct children of any ' + + 'portals it contains, regardless of how deeply nested they are', + async () => { + const portalContainer = document.createElement('div'); + + let setShow; + function Accordion({children}) { + const [shouldShow, _setShow] = useState(true); + setShow = _setShow; + return ( + + {children} + + ); + } + + function App({portalContents}) { + return ( + +
+ {ReactDOM.createPortal( +
Portal contents
, + portalContainer, + )} +
+ + ); + } + + const root = ReactDOMClient.createRoot(container); + await act(() => root.render()); + expect(container.innerHTML).toBe('
'); + expect(portalContainer.innerHTML).toBe('
Portal contents
'); + + // 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('
'); + expect(portalContainer.innerHTML).toBe( + '
Portal contents
', + ); + }, + ); + + // @gate enableActivity + it( + 'revealing an Activity boundary inside a portal does not reveal the ' + + 'portal contents if has a hidden Activity parent', + async () => { + const portalContainer = document.createElement('div'); + + let setShow; + function Accordion({children}) { + const [shouldShow, _setShow] = useState(false); + setShow = _setShow; + return ( + + {children} + + ); + } + + function App({portalContents}) { + return ( + +
+ {ReactDOM.createPortal( + +
Portal contents
+ , + portalContainer, + )} +
+ + ); + } + + // Start with both boundaries hidden. + const root = ReactDOMClient.createRoot(container); + await act(() => root.render()); + expect(container.innerHTML).toBe('
'); + expect(portalContainer.innerHTML).toBe( + '
Portal contents
', + ); + + // Reveal the inner Activity boundary. It should not reveal its children, + // because there's a parent Activity boundary that is still hidden. + await act(() => setShow(true)); + expect(container.innerHTML).toBe('
'); + expect(portalContainer.innerHTML).toBe( + '
Portal contents
', + ); + }, + ); +}); diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 87764464c400f..ecacb0c158503 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -117,6 +117,7 @@ import { DidCapture, AffectedParentLayout, ViewTransitionNamedStatic, + PortalStatic, } from './ReactFiberFlags'; import { commitStartTime, @@ -1182,66 +1183,104 @@ function commitTransitionProgress(offscreenFiber: Fiber) { } } -function hideOrUnhideAllChildren(finishedWork: Fiber, isHidden: boolean) { - // Only hide or unhide the top-most host nodes. - let hostSubtreeRoot = null; +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; + } +} - 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 - ) { +function hideOrUnhideAllChildrenOnFiber(fiber: Fiber, isHidden: boolean) { + if (!supportsMutation) { + return; + } + switch (fiber.tag) { + case HostComponent: + case HostHoistable: { + // Found the nearest host component. Hide it. + commitShowHideHostInstance(fiber, isHidden); + // Typically, only the nearest host nodes need to be hidden, since that + // has the effect of also hiding everything inside of them. + // + // However, there's a special case for portals, because portals do not + // exist in 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, if any portals exist within the tree, regardless of how deeply + // nested they are, we need to repeat this algorithm for its children. + 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 if (node.child !== null) { - node.child.return = node; - node = node.child; - continue; - } - - if (node === finishedWork) { - return; + } else { + hideOrUnhideAllChildren(fiber, isHidden); } - while (node.sibling === null) { - if (node.return === null || node.return === finishedWork) { - return; - } - - if (hostSubtreeRoot === node) { - hostSubtreeRoot = null; - } + return; + } + default: { + hideOrUnhideAllChildren(fiber, isHidden); + return; + } + } +} - node = node.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; + } + } +} - if (hostSubtreeRoot === node) { - hostSubtreeRoot = null; +function hideOrUnhideNearestPortalsOnFiber(fiber: Fiber, isHidden: boolean) { + if (!supportsMutation) { + return; + } + switch (fiber.tag) { + case HostPortal: { + // Found a portal. Switch back to the normal hide/unhide algorithm to + // toggle the visibility of its children. + 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); } - - node.sibling.return = node.return; - node = node.sibling; + return; + } + default: { + hideOrUnhideNearestPortals(fiber, isHidden); + return; } } } @@ -2305,6 +2344,15 @@ function commitMutationEffectsOnFiber( break; } case HostPortal: { + // 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; const prevMutationContext = pushMutationContext(); if (supportsResources) { const previousHoistableRoot = currentHoistableRoot; @@ -2326,6 +2374,7 @@ function commitMutationEffectsOnFiber( rootViewTransitionAffected = true; } popMutationContext(prevMutationContext); + offscreenDirectParentIsHidden = prevOffscreenDirectParentIsHidden; if (flags & Update) { if (supportsPersistence) { diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index dab1b2272bd9e..9b8c4a21bd8a5 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -99,6 +99,7 @@ import { Cloned, ViewTransitionStatic, Hydrate, + PortalStatic, } from './ReactFiberFlags'; import { @@ -1665,6 +1666,7 @@ function completeWork( if (current === null) { preparePortalMount(workInProgress.stateNode.containerInfo); } + workInProgress.flags |= PortalStatic; bubbleProperties(workInProgress); return null; case ContextProvider: diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index e44301d4ed2d2..cc43edc66b6e0 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -83,11 +83,13 @@ export const ViewTransitionNamedStatic = // ViewTransitionStatic tracks whether there are an ViewTransition components from // the nearest HostComponent down. It resets at every HostComponent level. export const ViewTransitionStatic = /* */ 0b0000010000000000000000000000000; +// Tracks whether a HostPortal is present in the tree. +export const PortalStatic = /* */ 0b0000100000000000000000000000000; // Flag used to identify newly inserted fibers. It isn't reset after commit unlike `Placement`. -export const PlacementDEV = /* */ 0b0000100000000000000000000000000; -export const MountLayoutDev = /* */ 0b0001000000000000000000000000000; -export const MountPassiveDev = /* */ 0b0010000000000000000000000000000; +export const PlacementDEV = /* */ 0b0001000000000000000000000000000; +export const MountLayoutDev = /* */ 0b0010000000000000000000000000000; +export const MountPassiveDev = /* */ 0b0100000000000000000000000000000; // Groups of flags that are used in the commit phase to skip over trees that // don't contain effects, by checking subtreeFlags. @@ -139,4 +141,5 @@ export const StaticMask = RefStatic | MaySuspendCommit | ViewTransitionStatic | - ViewTransitionNamedStatic; + ViewTransitionNamedStatic | + PortalStatic; From be48396dbd77a54e86dd4622678559be96706991 Mon Sep 17 00:00:00 2001 From: Facebook Community Bot Date: Mon, 10 Nov 2025 08:34:01 -0800 Subject: [PATCH 2/2] Remove Dead Code in WWW JS Differential Revision: D86593830 Pull Request resolved: https://github.com/facebook/react/pull/35085 --- .../react-dom-bindings/src/client/ReactDOMComponentTree.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponentTree.js b/packages/react-dom-bindings/src/client/ReactDOMComponentTree.js index 8a07dfefb0fe7..2063e5ef2f5e9 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponentTree.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponentTree.js @@ -293,8 +293,9 @@ export function updateFiberProps(node: Instance, props: Props): void { } export function getEventListenerSet(node: EventTarget): Set { - let elementListenerSet: Set | void; - elementListenerSet = (node: any)[internalEventHandlersKey]; + let elementListenerSet: Set | void = (node: any)[ + internalEventHandlersKey + ]; if (elementListenerSet === undefined) { elementListenerSet = (node: any)[internalEventHandlersKey] = new Set(); }