From e5cbce6eadebbac0396bf5cc255312e26affabd4 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 13 Mar 2025 18:38:45 -0400 Subject: [PATCH 1/3] Make wrapper around the xViewTransitionToHostInstances helpers These all start with resetting a counter but it's tricky to have to remember to do this and tricky to do from the outside of this module. So we make an exported helper that does the resetting. Ideally it gets inlined. --- .../src/ReactFiberCommitViewTransitions.js | 72 +++++++++++++++---- 1 file changed, 57 insertions(+), 15 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js b/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js index 22b9d11ac67d2..58af129b44caf 100644 --- a/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js +++ b/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js @@ -79,7 +79,24 @@ export function setViewTransitionCancelableChildren( let viewTransitionHostInstanceIdx = 0; -function applyViewTransitionToHostInstances( +export function applyViewTransitionToHostInstances( + child: null | Fiber, + name: string, + className: ?string, + collectMeasurements: null | Array, + stopAtNestedViewTransitions: boolean, +): boolean { + viewTransitionHostInstanceIdx = 0; + return applyViewTransitionToHostInstancesRecursive( + child, + name, + className, + collectMeasurements, + stopAtNestedViewTransitions, + ); +} + +function applyViewTransitionToHostInstancesRecursive( child: null | Fiber, name: string, className: ?string, @@ -128,7 +145,7 @@ function applyViewTransitionToHostInstances( // inner most one is the one that handles the update. } else { if ( - applyViewTransitionToHostInstances( + applyViewTransitionToHostInstancesRecursive( child.child, name, className, @@ -207,7 +224,6 @@ function commitAppearingPairViewTransitions(placement: Fiber): void { if (className !== 'none') { // We found a new appearing view transition with the same name as this deletion. // We'll transition between them. - viewTransitionHostInstanceIdx = 0; const inViewport = applyViewTransitionToHostInstances( child.child, name, @@ -242,7 +258,6 @@ export function commitEnterViewTransitions(placement: Fiber): void { state.paired ? props.share : props.enter, ); if (className !== 'none') { - viewTransitionHostInstanceIdx = 0; const inViewport = applyViewTransitionToHostInstances( placement.child, name, @@ -310,7 +325,6 @@ function commitDeletedPairViewTransitions(deletion: Fiber): void { ); if (className !== 'none') { // We found a new appearing view transition with the same name as this deletion. - viewTransitionHostInstanceIdx = 0; const inViewport = applyViewTransitionToHostInstances( child.child, name, @@ -361,7 +375,6 @@ export function commitExitViewTransitions(deletion: Fiber): void { pair !== undefined ? props.share : props.exit, ); if (className !== 'none') { - viewTransitionHostInstanceIdx = 0; const inViewport = applyViewTransitionToHostInstances( deletion.child, name, @@ -449,7 +462,6 @@ export function commitBeforeUpdateViewTransition( return; } } - viewTransitionHostInstanceIdx = 0; applyViewTransitionToHostInstances( current.child, oldName, @@ -472,7 +484,6 @@ export function commitNestedViewTransitions(changedParent: Fiber): void { props.layout, ); if (className !== 'none') { - viewTransitionHostInstanceIdx = 0; applyViewTransitionToHostInstances( child.child, name, @@ -570,7 +581,20 @@ export function restoreNestedViewTransitions(changedParent: Fiber): void { } } -function cancelViewTransitionHostInstances( +export function cancelViewTransitionHostInstances( + currentViewTransition: Fiber, + child: null | Fiber, + stopAtNestedViewTransitions: boolean, +): void { + viewTransitionHostInstanceIdx = 0; + cancelViewTransitionHostInstancesRecursive( + currentViewTransition, + child, + stopAtNestedViewTransitions, + ); +} + +function cancelViewTransitionHostInstancesRecursive( currentViewTransition: Fiber, child: null | Fiber, stopAtNestedViewTransitions: boolean, @@ -606,7 +630,7 @@ function cancelViewTransitionHostInstances( // Skip any nested view transitions for updates since in that case the // inner most one is the one that handles the update. } else { - cancelViewTransitionHostInstances( + cancelViewTransitionHostInstancesRecursive( currentViewTransition, child.child, stopAtNestedViewTransitions, @@ -616,7 +640,28 @@ function cancelViewTransitionHostInstances( } } -function measureViewTransitionHostInstances( +export function measureViewTransitionHostInstances( + currentViewTransition: Fiber, + parentViewTransition: Fiber, + child: null | Fiber, + name: string, + className: ?string, + previousMeasurements: null | Array, + stopAtNestedViewTransitions: boolean, +): boolean { + viewTransitionHostInstanceIdx = 0; + return measureViewTransitionHostInstancesRecursive( + currentViewTransition, + parentViewTransition, + child, + name, + className, + previousMeasurements, + stopAtNestedViewTransitions, + ); +} + +function measureViewTransitionHostInstancesRecursive( currentViewTransition: Fiber, parentViewTransition: Fiber, child: null | Fiber, @@ -713,7 +758,7 @@ function measureViewTransitionHostInstances( parentViewTransition.flags |= child.flags & AffectedParentLayout; } else { if ( - measureViewTransitionHostInstances( + measureViewTransitionHostInstancesRecursive( currentViewTransition, parentViewTransition, child.child, @@ -762,7 +807,6 @@ export function measureUpdateViewTransition( if (layoutClassName === 'none') { // If we did not update, then all changes are considered a layout. We'll // attempt to cancel. - viewTransitionHostInstanceIdx = 0; cancelViewTransitionHostInstances(current, finishedWork.child, true); return false; } @@ -773,7 +817,6 @@ export function measureUpdateViewTransition( const name = getViewTransitionName(props, finishedWork.stateNode); // If nothing changed due to a mutation, or children changing size // and the measurements end up unchanged, we should restore it to not animate. - viewTransitionHostInstanceIdx = 0; const previousMeasurements = current.memoizedState; const inViewport = measureViewTransitionHostInstances( current, @@ -807,7 +850,6 @@ export function measureNestedViewTransitions(changedParent: Fiber): void { props.className, props.layout, ); - viewTransitionHostInstanceIdx = 0; const inViewport = measureViewTransitionHostInstances( current, child, From 8ade44278e2186a35264710f68a781f6fa0d7eb2 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 13 Mar 2025 23:05:30 -0400 Subject: [PATCH 2/3] Don't pass "current" to measureViewTransitionHostInstances Same thing for cancelViewTransitionHostInstances This doesn't make sense for "nested" which has not updated and so might not have an alternate. Instead we pass in the old and new name if they might be different. --- .../src/ReactFiberCommitViewTransitions.js | 89 +++++++++---------- 1 file changed, 41 insertions(+), 48 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js b/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js index 58af129b44caf..506f437562e2b 100644 --- a/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js +++ b/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js @@ -582,21 +582,21 @@ export function restoreNestedViewTransitions(changedParent: Fiber): void { } export function cancelViewTransitionHostInstances( - currentViewTransition: Fiber, child: null | Fiber, + oldName: string, stopAtNestedViewTransitions: boolean, ): void { viewTransitionHostInstanceIdx = 0; cancelViewTransitionHostInstancesRecursive( - currentViewTransition, child, + oldName, stopAtNestedViewTransitions, ); } function cancelViewTransitionHostInstancesRecursive( - currentViewTransition: Fiber, child: null | Fiber, + oldName: string, stopAtNestedViewTransitions: boolean, ): void { if (!supportsMutation) { @@ -605,10 +605,6 @@ function cancelViewTransitionHostInstancesRecursive( while (child !== null) { if (child.tag === HostComponent) { const instance: Instance = child.stateNode; - const oldName = getViewTransitionName( - currentViewTransition.memoizedProps, - currentViewTransition.stateNode, - ); if (viewTransitionCancelableChildren === null) { viewTransitionCancelableChildren = []; } @@ -631,8 +627,8 @@ function cancelViewTransitionHostInstancesRecursive( // inner most one is the one that handles the update. } else { cancelViewTransitionHostInstancesRecursive( - currentViewTransition, child.child, + oldName, stopAtNestedViewTransitions, ); } @@ -641,20 +637,20 @@ function cancelViewTransitionHostInstancesRecursive( } export function measureViewTransitionHostInstances( - currentViewTransition: Fiber, parentViewTransition: Fiber, child: null | Fiber, - name: string, + newName: string, + oldName: string, className: ?string, previousMeasurements: null | Array, stopAtNestedViewTransitions: boolean, ): boolean { viewTransitionHostInstanceIdx = 0; return measureViewTransitionHostInstancesRecursive( - currentViewTransition, parentViewTransition, child, - name, + newName, + oldName, className, previousMeasurements, stopAtNestedViewTransitions, @@ -662,10 +658,10 @@ export function measureViewTransitionHostInstances( } function measureViewTransitionHostInstancesRecursive( - currentViewTransition: Fiber, parentViewTransition: Fiber, child: null | Fiber, - name: string, + newName: string, + oldName: string, className: ?string, previousMeasurements: null | Array, stopAtNestedViewTransitions: boolean, @@ -716,10 +712,10 @@ function measureViewTransitionHostInstancesRecursive( applyViewTransitionName( instance, viewTransitionHostInstanceIdx === 0 - ? name + ? newName : // If we have multiple Host Instances below, we add a suffix to the name to give // each one a unique name. - name + '_' + viewTransitionHostInstanceIdx, + newName + '_' + viewTransitionHostInstanceIdx, className, ); } @@ -729,10 +725,6 @@ function measureViewTransitionHostInstancesRecursive( // animating it. However, in the current model this only works if the parent also // doesn't animate. So we have to queue these and wait until we complete the parent // to cancel them. - const oldName = getViewTransitionName( - currentViewTransition.memoizedProps, - currentViewTransition.stateNode, - ); if (viewTransitionCancelableChildren === null) { viewTransitionCancelableChildren = []; } @@ -759,10 +751,10 @@ function measureViewTransitionHostInstancesRecursive( } else { if ( measureViewTransitionHostInstancesRecursive( - currentViewTransition, parentViewTransition, child.child, - name, + newName, + oldName, className, previousMeasurements, stopAtNestedViewTransitions, @@ -781,6 +773,11 @@ export function measureUpdateViewTransition( finishedWork: Fiber, ): boolean { const props: ViewTransitionProps = finishedWork.memoizedProps; + const newName = getViewTransitionName(props, finishedWork.stateNode); + const oldName = getViewTransitionName( + current.memoizedProps, + current.stateNode, + ); const updateClassName: ?string = getViewTransitionClassName( props.className, props.update, @@ -807,22 +804,21 @@ export function measureUpdateViewTransition( if (layoutClassName === 'none') { // If we did not update, then all changes are considered a layout. We'll // attempt to cancel. - cancelViewTransitionHostInstances(current, finishedWork.child, true); + cancelViewTransitionHostInstances(finishedWork.child, oldName, true); return false; } // We didn't update but we might still apply layout so we measure each // instance to see if it moved or resized. className = layoutClassName; } - const name = getViewTransitionName(props, finishedWork.stateNode); // If nothing changed due to a mutation, or children changing size // and the measurements end up unchanged, we should restore it to not animate. const previousMeasurements = current.memoizedState; const inViewport = measureViewTransitionHostInstances( - current, finishedWork, finishedWork.child, - name, + newName, + oldName, className, previousMeasurements, true, @@ -842,28 +838,25 @@ export function measureNestedViewTransitions(changedParent: Fiber): void { let child = changedParent.child; while (child !== null) { if (child.tag === ViewTransitionComponent) { - const current = child.alternate; - if (current !== null) { - const props: ViewTransitionProps = child.memoizedProps; - const name = getViewTransitionName(props, child.stateNode); - const className: ?string = getViewTransitionClassName( - props.className, - props.layout, - ); - const inViewport = measureViewTransitionHostInstances( - current, - child, - child.child, - name, - className, - child.memoizedState, - false, - ); - if ((child.flags & Update) === NoFlags || !inViewport) { - // Nothing changed. - } else { - scheduleViewTransitionEvent(child, props.onLayout); - } + const props: ViewTransitionProps = child.memoizedProps; + const name = getViewTransitionName(props, child.stateNode); + const className: ?string = getViewTransitionClassName( + props.className, + props.layout, + ); + const inViewport = measureViewTransitionHostInstances( + child, + child.child, + name, + name, // Since this is unchanged, new and old name is the same. + className, + child.memoizedState, + false, + ); + if ((child.flags & Update) === NoFlags || !inViewport) { + // Nothing changed. + } else { + scheduleViewTransitionEvent(child, props.onLayout); } } else if ((child.subtreeFlags & ViewTransitionStatic) !== NoFlags) { measureNestedViewTransitions(child); From 8cebae4c0713d855bd4c6b0dd160e7c1438ff9c9 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 14 Mar 2025 00:03:36 -0400 Subject: [PATCH 3/3] Clarify that setViewTransitionCancelableChildren should follow the push/pop pattern This was moved to be a setter when it was extracted to another module but really it should be treated as push/pop. --- .../src/ReactFiberCommitViewTransitions.js | 14 +++++++++++--- .../react-reconciler/src/ReactFiberCommitWork.js | 14 +++++++------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js b/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js index 506f437562e2b..ca3df168dc30a 100644 --- a/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js +++ b/packages/react-reconciler/src/ReactFiberCommitViewTransitions.js @@ -71,10 +71,18 @@ export let viewTransitionCancelableChildren: null | Array< Instance | string | Props, > = null; // tupled array where each entry is [instance: Instance, oldName: string, props: Props] -export function setViewTransitionCancelableChildren( - children: null | Array, +export function pushViewTransitionCancelableScope(): null | Array< + Instance | string | Props, +> { + const prevChildren = viewTransitionCancelableChildren; + viewTransitionCancelableChildren = null; + return prevChildren; +} + +export function popViewTransitionCancelableScope( + prevChildren: null | Array, ): void { - viewTransitionCancelableChildren = children; + viewTransitionCancelableChildren = prevChildren; } let viewTransitionHostInstanceIdx = 0; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 8b82e6a7e713c..168a5d80f65b3 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -255,7 +255,8 @@ import { resetAppearingViewTransitions, trackAppearingViewTransition, viewTransitionCancelableChildren, - setViewTransitionCancelableChildren, + pushViewTransitionCancelableScope, + popViewTransitionCancelableScope, } from './ReactFiberCommitViewTransitions'; import { viewTransitionMutationContext, @@ -2475,14 +2476,14 @@ function commitAfterMutationEffectsOnFiber( switch (finishedWork.tag) { case HostRoot: { viewTransitionContextChanged = false; - setViewTransitionCancelableChildren(null); + pushViewTransitionCancelableScope(); recursivelyTraverseAfterMutationEffects(root, finishedWork, lanes); if (!viewTransitionContextChanged) { // If we didn't leak any resizing out to the root, we don't have to transition // the root itself. This means that we can now safely cancel any cancellations // that bubbled all the way up. const cancelableChildren = viewTransitionCancelableChildren; - setViewTransitionCancelableChildren(null); + popViewTransitionCancelableScope(null); if (cancelableChildren !== null) { for (let i = 0; i < cancelableChildren.length; i += 3) { cancelViewTransitionName( @@ -2533,9 +2534,8 @@ function commitAfterMutationEffectsOnFiber( const wasMutated = (finishedWork.flags & Update) !== NoFlags; const prevContextChanged = viewTransitionContextChanged; - const prevCancelableChildren = viewTransitionCancelableChildren; + const prevCancelableChildren = pushViewTransitionCancelableScope(); viewTransitionContextChanged = false; - setViewTransitionCancelableChildren(null); recursivelyTraverseAfterMutationEffects(root, finishedWork, lanes); if (viewTransitionContextChanged) { @@ -2558,7 +2558,7 @@ function commitAfterMutationEffectsOnFiber( prevCancelableChildren, viewTransitionCancelableChildren, ); - setViewTransitionCancelableChildren(prevCancelableChildren); + popViewTransitionCancelableScope(prevCancelableChildren); } // TODO: If this doesn't end up canceled, because a parent animates, // then we should probably issue an event since this instance is part of it. @@ -2572,7 +2572,7 @@ function commitAfterMutationEffectsOnFiber( ); // If this boundary did update, we cannot cancel its children so those are dropped. - setViewTransitionCancelableChildren(prevCancelableChildren); + popViewTransitionCancelableScope(prevCancelableChildren); } if ((finishedWork.flags & AffectedParentLayout) !== NoFlags) {