From 540efebcc34357c98412a96805bfd9244d6aa678 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Sun, 12 Jan 2025 13:16:54 -0500 Subject: [PATCH 1/3] View Transition Events (#32041) This adds five events to `` that triggers when React wants to animate it. - `onEnter`: The `` or its parent Component is mounted and there's no other `` with the same name being deleted. - `onExit`: The `` or its parent Component is unmounted and there's no other `` with the same name being deleted. - `onLayout`: There are no updates to the content inside this `` boundary itself but the boundary has resized or moved due to other changes to siblings. - `onShare`: This `` is being mounted and another `` instance with the same name is being unmounted elsewhere. - `onUpdate`: The content of `` has changed either due to DOM mutations or because an inner child `` has resized. Only one of these events is fired per Transition. If you want to cover all updates you have to listen to `onLayout`, `onShare` and `onUpdate`. We could potentially do something like fire `onUpdate` if `onLayout` or `onShare` isn't specified but it's a little sketchy to have behavior based on if someone is listening since it limits adding wrappers that may or may not need it. Each takes a `ViewTransitionInstance` as an argument so you don't need a ref to animate it. ```js inst.new.animate(keyframes, options)}> ``` The timing of this event is after the View Transition's `ready` state which means that's too late to do any changes to the View Transition's snapshots but now both the new and old pseudo-elements are ready to animate. The order of `onExit` is parent first, where as the others are child first. This mimics effect mount/unmount. I implement this by adding to a queue in the commit phase and then call it while we're finishing up the commit. This is after layout effects but before passive effects since passive effects fire after the animation is `finished`. --- .../view-transition/src/components/Page.js | 26 ++++------ .../src/ReactFiberCommitWork.js | 33 ++++++++++++- .../src/ReactFiberViewTransitionComponent.js | 5 ++ .../src/ReactFiberWorkLoop.js | 48 ++++++++++++++++++- 4 files changed, 93 insertions(+), 19 deletions(-) diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index 8a3638b01f..8204643a3e 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -1,8 +1,6 @@ import React, { unstable_ViewTransition as ViewTransition, unstable_Activity as Activity, - useRef, - useLayoutEffect, } from 'react'; import './Page.css'; @@ -37,21 +35,17 @@ function Component() { } export default function Page({url, navigate}) { - const ref = useRef(); const show = url === '/?b'; - useLayoutEffect(() => { - const viewTransition = ref.current; - requestAnimationFrame(() => { - const keyframes = [ - {rotate: '0deg', transformOrigin: '30px 8px'}, - {rotate: '360deg', transformOrigin: '30px 8px'}, - ]; - viewTransition.old.animate(keyframes, 300); - viewTransition.new.animate(keyframes, 300); - }); - }, [show]); + function onTransition(viewTransition) { + const keyframes = [ + {rotate: '0deg', transformOrigin: '30px 8px'}, + {rotate: '360deg', transformOrigin: '30px 8px'}, + ]; + viewTransition.old.animate(keyframes, 250); + viewTransition.new.animate(keyframes, 250); + } const exclamation = ( - + ! ); @@ -76,7 +70,7 @@ export default function Page({url, navigate}) { {a} )} - + {show ?
hello{exclamation}
:
Loading
}

scroll me

diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 8b4baead09..8e98b763b7 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -186,6 +186,7 @@ import { addMarkerIncompleteCallbackToPendingTransition, addMarkerCompleteCallbackToPendingTransition, retryDehydratedSuspenseBoundary, + scheduleViewTransitionEvent, } from './ReactFiberWorkLoop'; import { HasEffect as HookHasEffect, @@ -649,6 +650,7 @@ function commitAppearingPairViewTransitions(placement: Fiber): void { if (child.tag === OffscreenComponent && child.memoizedState === null) { // This tree was already hidden so we skip it. } else { + commitAppearingPairViewTransitions(child); if ( child.tag === ViewTransitionComponent && (child.flags & ViewTransitionNamedStatic) !== NoFlags @@ -682,7 +684,6 @@ function commitAppearingPairViewTransitions(placement: Fiber): void { } } } - commitAppearingPairViewTransitions(child); } child = child.sibling; } @@ -701,12 +702,18 @@ function commitEnterViewTransitions(placement: Fiber): void { false, ); if (!inViewport) { + // TODO: If this was part of a pair we will still run the onShare callback. // Revert the transition names. This boundary is not in the viewport // so we won't bother animating it. restoreViewTransitionOnHostInstances(placement.child, false); // TODO: Should we still visit the children in case a named one was in the viewport? } else { commitAppearingPairViewTransitions(placement); + + const state: ViewTransitionState = placement.stateNode; + if (!state.paired) { + scheduleViewTransitionEvent(placement, props.onEnter); + } } } else if ((placement.subtreeFlags & ViewTransitionStatic) !== NoFlags) { let child = placement.child; @@ -764,6 +771,9 @@ function commitDeletedPairViewTransitions( const oldinstance: ViewTransitionState = child.stateNode; const newInstance: ViewTransitionState = pair; newInstance.paired = oldinstance; + // Note: If the other side ends up outside the viewport, we'll still run this. + // Therefore it's possible for onShare to be called with only an old snapshot. + scheduleViewTransitionEvent(child, props.onShare); } // Delete the entry so that we know when we've found all of them // and can stop searching (size reaches zero). @@ -811,9 +821,16 @@ function commitExitViewTransitions( // Delete the entry so that we know when we've found all of them // and can stop searching (size reaches zero). appearingViewTransitions.delete(name); + // Note: If the other side ends up outside the viewport, we'll still run this. + // Therefore it's possible for onShare to be called with only an old snapshot. + scheduleViewTransitionEvent(deletion, props.onShare); + } else { + scheduleViewTransitionEvent(deletion, props.onExit); } // Look for more pairs deeper in the tree. commitDeletedPairViewTransitions(deletion, appearingViewTransitions); + } else { + scheduleViewTransitionEvent(deletion, props.onExit); } } else if ((deletion.subtreeFlags & ViewTransitionStatic) !== NoFlags) { let child = deletion.child; @@ -1118,6 +1135,8 @@ function measureNestedViewTransitions(changedParent: Fiber): void { child.memoizedState, false, ); + const props: ViewTransitionProps = child.memoizedProps; + scheduleViewTransitionEvent(child, props.onLayout); } } else if ((child.subtreeFlags & ViewTransitionStatic) !== NoFlags) { measureNestedViewTransitions(child); @@ -3075,6 +3094,8 @@ function commitAfterMutationEffectsOnFiber( (Placement | Update | ChildDeletion | ContentReset | Visibility)) !== NoFlags ) { + const wasMutated = (finishedWork.flags & Update) !== NoFlags; + const prevContextChanged = viewTransitionContextChanged; const prevCancelableChildren = viewTransitionCancelableChildren; viewTransitionContextChanged = false; @@ -3103,7 +3124,17 @@ function commitAfterMutationEffectsOnFiber( ); viewTransitionCancelableChildren = 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. } else { + const props: ViewTransitionProps = finishedWork.memoizedProps; + scheduleViewTransitionEvent( + finishedWork, + wasMutated || viewTransitionContextChanged + ? props.onUpdate + : props.onLayout, + ); + // If this boundary did update, we cannot cancel its children so those are dropped. viewTransitionCancelableChildren = prevCancelableChildren; } diff --git a/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js b/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js index 008a6992f2..bfbc1b0b2e 100644 --- a/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js +++ b/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js @@ -21,6 +21,11 @@ export type ViewTransitionProps = { name?: string, className?: string, children?: ReactNodeList, + onEnter?: (instance: ViewTransitionInstance) => void, + onExit?: (instance: ViewTransitionInstance) => void, + onLayout?: (instance: ViewTransitionInstance) => void, + onShare?: (instance: ViewTransitionInstance) => void, + onUpdate?: (instance: ViewTransitionInstance) => void, }; export type ViewTransitionState = { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index f3ba210bf9..43cba4586f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -21,9 +21,12 @@ import type { TransitionAbort, } from './ReactFiberTracingMarkerComponent'; import type {OffscreenInstance} from './ReactFiberActivityComponent'; -import type {Resource} from './ReactFiberConfig'; +import type {Resource, ViewTransitionInstance} from './ReactFiberConfig'; import type {RootState} from './ReactFiberRoot'; -import type {ViewTransitionState} from './ReactFiberViewTransitionComponent'; +import { + getViewTransitionName, + type ViewTransitionState, +} from './ReactFiberViewTransitionComponent'; import { enableCreateEventHandleAPI, @@ -95,6 +98,7 @@ import { resolveUpdatePriority, trackSchedulerEvent, startViewTransition, + createViewTransitionInstance, } from './ReactFiberConfig'; import {createWorkInProgress, resetWorkInProgress} from './ReactFiber'; @@ -649,6 +653,7 @@ let pendingEffectsRemainingLanes: Lanes = NoLanes; let pendingEffectsRenderEndTime: number = -0; // Profiling-only let pendingPassiveTransitions: Array | null = null; let pendingRecoverableErrors: null | Array> = null; +let pendingViewTransitionEvents: Array<() => void> | null = null; let pendingDidIncludeRenderPhaseUpdate: boolean = false; let pendingSuspendedCommitReason: SuspendedCommitReason = IMMEDIATE_COMMIT; // Profiling-only @@ -797,6 +802,27 @@ export function requestDeferredLane(): Lane { return workInProgressDeferredLane; } +export function scheduleViewTransitionEvent( + fiber: Fiber, + callback: ?(instance: ViewTransitionInstance) => void, +): void { + if (enableViewTransition) { + if (callback != null) { + const state: ViewTransitionState = fiber.stateNode; + let instance = state.ref; + if (instance === null) { + instance = state.ref = createViewTransitionInstance( + getViewTransitionName(fiber.memoizedProps, state), + ); + } + if (pendingViewTransitionEvents === null) { + pendingViewTransitionEvents = []; + } + pendingViewTransitionEvents.push(callback.bind(null, instance)); + } + } +} + export function peekDeferredLane(): Lane { return workInProgressDeferredLane; } @@ -3322,6 +3348,9 @@ function commitRoot( pendingEffectsRemainingLanes = remainingLanes; pendingPassiveTransitions = transitions; pendingRecoverableErrors = recoverableErrors; + if (enableViewTransition) { + pendingViewTransitionEvents = null; + } pendingDidIncludeRenderPhaseUpdate = didIncludeRenderPhaseUpdate; if (enableProfilerTimer) { pendingEffectsRenderEndTime = completedRenderEndTime; @@ -3673,6 +3702,21 @@ function flushSpawnedWork(): void { } } + if (enableViewTransition) { + // We should now be after the startViewTransition's .ready call which is late enough + // to start animating any pseudo-elements. We do this before flushing any passive + // effects or spawned sync work since this is still part of the previous commit. + // Even though conceptually it's like its own task between layout effets and passive. + const pendingEvents = pendingViewTransitionEvents; + if (pendingEvents !== null) { + pendingViewTransitionEvents = null; + for (let i = 0; i < pendingEvents.length; i++) { + const viewTransitionEvent = pendingEvents[i]; + viewTransitionEvent(); + } + } + } + // If the passive effects are the result of a discrete render, flush them // synchronously at the end of the current task so that the result is // immediately observable. Otherwise, we assume that they are not From cabd8a0e700713900d9573edb162e608268d09ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 13 Jan 2025 09:45:53 -0500 Subject: [PATCH 2/3] View Transition Class Names based on event kind (#32050) This adds five props to `` that adds a specific `view-transition-class` when React wants to animate it based on the heuristic that triggers. ```js ``` - `enter`: The or its parent Component is mounted and there's no other with the same name being deleted. - `exit`: The or its parent Component is unmounted and there's no other with the same name being deleted. - `layout`: There are no updates to the content inside this boundary itself but the boundary has resized or moved due to other changes to siblings. - `share`: This is being mounted and another instance with the same name is being unmounted elsewhere. - `update`: The content of has changed either due to DOM mutations or because an inner child has resized. The existing `className` is the baseline and the others are added to it to combine. This is convenient to distinguish things like `enter` / `exit` but that can already be expressed as CSS. The other cases can't be expressed as purely CSS. `"none"` is a special value that deactivates the view transition name under that condition. The most important feature of this is that you can now limit View Transitions to only tigger when a particular DOM node is affected, not when just any child updates, by opt-ing out a subtree. This is safer when added to shared parent. ```js
{children}
``` This can't be fully expressed using neither just CSS nor the imperative refs API since we need some way to have already removed the `view-transition-name` when this happens. When you think about the implementation details it might seem a bit strange that you specify the `class` to `none` to remove the `name` but it's really about picking which animation should happen for that case default (`undefined`), a specific one (class) or none (`"none"`). --- .../view-transition/src/components/Page.js | 4 +- .../src/ReactFiberCommitWork.js | 347 +++++++++++++----- .../src/ReactFiberViewTransitionComponent.js | 23 +- 3 files changed, 272 insertions(+), 102 deletions(-) diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index 8204643a3e..688688bcd6 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -57,7 +57,7 @@ export default function Page({url, navigate}) { }}> {show ? 'A' : 'B'} - +
{show ? (
@@ -92,7 +92,7 @@ export default function Page({url, navigate}) {
!!
- {show ? :

 

} + {show ? : null}
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 8e98b763b7..e57f54e668 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -203,7 +203,10 @@ import { OffscreenDetached, OffscreenPassiveEffectsConnected, } from './ReactFiberActivityComponent'; -import {getViewTransitionName} from './ReactFiberViewTransitionComponent'; +import { + getViewTransitionName, + getViewTransitionClassName, +} from './ReactFiberViewTransitionComponent'; import { TransitionRoot, TransitionTracingMarker, @@ -504,7 +507,7 @@ function commitBeforeMutationEffectsOnFiber( // We should just stash the parent ViewTransitionComponent and continue // walking the tree until we find HostComponent but to do that we need // to use a stack which requires refactoring this phase. - commitBeforeUpdateViewTransition(current); + commitBeforeUpdateViewTransition(current, finishedWork); } } break; @@ -663,24 +666,31 @@ function commitAppearingPairViewTransitions(placement: Fiber): void { 'Found a pair with an auto name. This is a bug in React.', ); } - // 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, - props.name, + const name = props.name; + const className: ?string = getViewTransitionClassName( props.className, - null, - false, + props.share, ); - if (!inViewport) { - // This boundary is exiting within the viewport but is going to leave the viewport. - // Instead, we treat this as an exit of the previous entry by reverting the new name. - // Ideally we could undo the old transition but it's now too late. It's also on its - // on snapshot. We have know was for it to paint onto the original group. - // TODO: This will lead to things unexpectedly having exit animations that normally - // wouldn't happen. Consider if we should just let this fly off the screen instead. - restoreViewTransitionOnHostInstances(child.child, false); + 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, + className, + null, + false, + ); + if (!inViewport) { + // This boundary is exiting within the viewport but is going to leave the viewport. + // Instead, we treat this as an exit of the previous entry by reverting the new name. + // Ideally we could undo the old transition but it's now too late. It's also on its + // on snapshot. We have know was for it to paint onto the original group. + // TODO: This will lead to things unexpectedly having exit animations that normally + // wouldn't happen. Consider if we should just let this fly off the screen instead. + restoreViewTransitionOnHostInstances(child.child, false); + } } } } @@ -691,29 +701,37 @@ function commitAppearingPairViewTransitions(placement: Fiber): void { function commitEnterViewTransitions(placement: Fiber): void { if (placement.tag === ViewTransitionComponent) { + const state: ViewTransitionState = placement.stateNode; const props: ViewTransitionProps = placement.memoizedProps; - const name = getViewTransitionName(props, placement.stateNode); - viewTransitionHostInstanceIdx = 0; - const inViewport = applyViewTransitionToHostInstances( - placement.child, - name, + const name = getViewTransitionName(props, state); + const className: ?string = getViewTransitionClassName( props.className, - null, - false, + state.paired ? props.share : props.enter, ); - if (!inViewport) { - // TODO: If this was part of a pair we will still run the onShare callback. - // Revert the transition names. This boundary is not in the viewport - // so we won't bother animating it. - restoreViewTransitionOnHostInstances(placement.child, false); - // TODO: Should we still visit the children in case a named one was in the viewport? + if (className !== 'none') { + viewTransitionHostInstanceIdx = 0; + const inViewport = applyViewTransitionToHostInstances( + placement.child, + name, + className, + null, + false, + ); + if (!inViewport) { + // TODO: If this was part of a pair we will still run the onShare callback. + // Revert the transition names. This boundary is not in the viewport + // so we won't bother animating it. + restoreViewTransitionOnHostInstances(placement.child, false); + // TODO: Should we still visit the children in case a named one was in the viewport? + } else { + commitAppearingPairViewTransitions(placement); + + if (!state.paired) { + scheduleViewTransitionEvent(placement, props.onEnter); + } + } } else { commitAppearingPairViewTransitions(placement); - - const state: ViewTransitionState = placement.stateNode; - if (!state.paired) { - scheduleViewTransitionEvent(placement, props.onEnter); - } } } else if ((placement.subtreeFlags & ViewTransitionStatic) !== NoFlags) { let child = placement.child; @@ -752,28 +770,34 @@ function commitDeletedPairViewTransitions( if (name != null && name !== 'auto') { const pair = appearingViewTransitions.get(name); if (pair !== undefined) { - // We found a new appearing view transition with the same name as this deletion. - viewTransitionHostInstanceIdx = 0; - const inViewport = applyViewTransitionToHostInstances( - child.child, - name, + const className: ?string = getViewTransitionClassName( props.className, - null, - false, + props.share, ); - if (!inViewport) { - // This boundary is not in the viewport so we won't treat it as a matched pair. - // Revert the transition names. This avoids it flying onto the screen which can - // be disruptive and doesn't really preserve any continuity anyway. - restoreViewTransitionOnHostInstances(child.child, false); - } else { - // We'll transition between them. - const oldinstance: ViewTransitionState = child.stateNode; - const newInstance: ViewTransitionState = pair; - newInstance.paired = oldinstance; - // Note: If the other side ends up outside the viewport, we'll still run this. - // Therefore it's possible for onShare to be called with only an old snapshot. - scheduleViewTransitionEvent(child, props.onShare); + 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, + className, + null, + false, + ); + if (!inViewport) { + // This boundary is not in the viewport so we won't treat it as a matched pair. + // Revert the transition names. This avoids it flying onto the screen which can + // be disruptive and doesn't really preserve any continuity anyway. + restoreViewTransitionOnHostInstances(child.child, false); + } else { + // We'll transition between them. + const oldinstance: ViewTransitionState = child.stateNode; + const newInstance: ViewTransitionState = pair; + newInstance.paired = oldinstance; + // Note: If the other side ends up outside the viewport, we'll still run this. + // Therefore it's possible for onShare to be called with only an old snapshot. + scheduleViewTransitionEvent(child, props.onShare); + } } // Delete the entry so that we know when we've found all of them // and can stop searching (size reaches zero). @@ -797,22 +821,29 @@ function commitExitViewTransitions( if (deletion.tag === ViewTransitionComponent) { const props: ViewTransitionProps = deletion.memoizedProps; const name = getViewTransitionName(props, deletion.stateNode); - viewTransitionHostInstanceIdx = 0; - const inViewport = applyViewTransitionToHostInstances( - deletion.child, - name, + const pair = + appearingViewTransitions !== null + ? appearingViewTransitions.get(name) + : undefined; + const className: ?string = getViewTransitionClassName( props.className, - null, - false, + pair !== undefined ? props.share : props.exit, ); - if (!inViewport) { - // Revert the transition names. This boundary is not in the viewport - // so we won't bother animating it. - restoreViewTransitionOnHostInstances(deletion.child, false); - // TODO: Should we still visit the children in case a named one was in the viewport? - } else if (appearingViewTransitions !== null) { - const pair = appearingViewTransitions.get(name); - if (pair !== undefined) { + if (className !== 'none') { + viewTransitionHostInstanceIdx = 0; + const inViewport = applyViewTransitionToHostInstances( + deletion.child, + name, + className, + null, + false, + ); + if (!inViewport) { + // Revert the transition names. This boundary is not in the viewport + // so we won't bother animating it. + restoreViewTransitionOnHostInstances(deletion.child, false); + // TODO: Should we still visit the children in case a named one was in the viewport? + } else if (pair !== undefined) { // We found a new appearing view transition with the same name as this deletion. // We'll transition between them instead of running the normal exit. const oldinstance: ViewTransitionState = deletion.stateNode; @@ -820,6 +851,7 @@ function commitExitViewTransitions( newInstance.paired = oldinstance; // Delete the entry so that we know when we've found all of them // and can stop searching (size reaches zero). + // $FlowFixMe[incompatible-use]: Refined by the pair. appearingViewTransitions.delete(name); // Note: If the other side ends up outside the viewport, we'll still run this. // Therefore it's possible for onShare to be called with only an old snapshot. @@ -827,10 +859,10 @@ function commitExitViewTransitions( } else { scheduleViewTransitionEvent(deletion, props.onExit); } + } + if (appearingViewTransitions !== null) { // Look for more pairs deeper in the tree. commitDeletedPairViewTransitions(deletion, appearingViewTransitions); - } else { - scheduleViewTransitionEvent(deletion, props.onExit); } } else if ((deletion.subtreeFlags & ViewTransitionStatic) !== NoFlags) { let child = deletion.child; @@ -845,7 +877,10 @@ function commitExitViewTransitions( } } -function commitBeforeUpdateViewTransition(current: Fiber): void { +function commitBeforeUpdateViewTransition( + current: Fiber, + finishedWork: Fiber, +): void { // The way we deal with multiple HostInstances as children of a View Transition in an // update can get tricky. The important bit is that if you swap out n HostInstances // from n HostInstances then they match up in order. Similarly, if you don't swap @@ -862,13 +897,32 @@ function commitBeforeUpdateViewTransition(current: Fiber): void { // be unexpected but it is in line with the semantics that the ViewTransition is its // own layer that cross-fades its content when it updates. If you want to reorder then // each child needs its own ViewTransition. - const props: ViewTransitionProps = current.memoizedProps; - const name = getViewTransitionName(props, current.stateNode); + const oldProps: ViewTransitionProps = current.memoizedProps; + const oldName = getViewTransitionName(oldProps, current.stateNode); + const newProps: ViewTransitionProps = finishedWork.memoizedProps; + // This className applies only if there are fewer child DOM nodes than + // before or if this update should've been cancelled but we ended up with + // a parent animating so we need to animate the child too. + // For example, if update="foo" layout="none" and it turns out this was + // a layout only change, then the "foo" class will be applied even though + // it was not actually an update. Which is a bug. + let className: ?string = getViewTransitionClassName( + newProps.className, + newProps.update, + ); + if (className === 'none') { + className = getViewTransitionClassName(newProps.className, newProps.layout); + if (className === 'none') { + // If both update and layout are both "none" then we don't have to + // apply a name. Since we won't animate this boundary. + return; + } + } viewTransitionHostInstanceIdx = 0; applyViewTransitionToHostInstances( current.child, - name, - props.className, + oldName, + className, (current.memoizedState = []), true, ); @@ -882,14 +936,20 @@ function commitNestedViewTransitions(changedParent: Fiber): void { // was an update through this component then the inner one wins. const props: ViewTransitionProps = child.memoizedProps; const name = getViewTransitionName(props, child.stateNode); - viewTransitionHostInstanceIdx = 0; - applyViewTransitionToHostInstances( - child.child, - name, + const className: ?string = getViewTransitionClassName( props.className, - (child.memoizedState = []), - false, + props.layout, ); + if (className !== 'none') { + viewTransitionHostInstanceIdx = 0; + applyViewTransitionToHostInstances( + child.child, + name, + className, + (child.memoizedState = []), + false, + ); + } } else if ((child.subtreeFlags & ViewTransitionStatic) !== NoFlags) { commitNestedViewTransitions(child); } @@ -979,10 +1039,58 @@ function restoreNestedViewTransitions(changedParent: Fiber): void { } } +function cancelViewTransitionHostInstances( + currentViewTransition: Fiber, + child: null | Fiber, + stopAtNestedViewTransitions: boolean, +): void { + if (!supportsMutation) { + return; + } + while (child !== null) { + if (child.tag === HostComponent) { + const instance: Instance = child.stateNode; + const oldName = getViewTransitionName( + currentViewTransition.memoizedProps, + currentViewTransition.stateNode, + ); + if (viewTransitionCancelableChildren === null) { + viewTransitionCancelableChildren = []; + } + viewTransitionCancelableChildren.push( + instance, + oldName, + child.memoizedProps, + ); + viewTransitionHostInstanceIdx++; + } else if ( + child.tag === OffscreenComponent && + child.memoizedState !== null + ) { + // Skip any hidden subtrees. They were or are effectively not there. + } else if ( + child.tag === ViewTransitionComponent && + stopAtNestedViewTransitions + ) { + // Skip any nested view transitions for updates since in that case the + // inner most one is the one that handles the update. + } else { + cancelViewTransitionHostInstances( + currentViewTransition, + child.child, + stopAtNestedViewTransitions, + ); + } + child = child.sibling; + } +} + function measureViewTransitionHostInstances( currentViewTransition: Fiber, parentViewTransition: Fiber, child: null | Fiber, + name: string, + className: ?string, previousMeasurements: null | Array, stopAtNestedViewTransitions: boolean, ): boolean { @@ -1028,20 +1136,15 @@ function measureViewTransitionHostInstances( parentViewTransition.flags |= AffectedParentLayout; } if ((parentViewTransition.flags & Update) !== NoFlags) { - const props: ViewTransitionProps = parentViewTransition.memoizedProps; // We might update this node so we need to apply its new name for the new state. - const newName = getViewTransitionName( - props, - parentViewTransition.stateNode, - ); applyViewTransitionName( instance, viewTransitionHostInstanceIdx === 0 - ? newName + ? name : // If we have multiple Host Instances below, we add a suffix to the name to give // each one a unique name. - newName + '_' + viewTransitionHostInstanceIdx, - props.className, + name + '_' + viewTransitionHostInstanceIdx, + className, ); } if (!inViewport || (parentViewTransition.flags & Update) === NoFlags) { @@ -1083,6 +1186,8 @@ function measureViewTransitionHostInstances( currentViewTransition, parentViewTransition, child.child, + name, + className, previousMeasurements, stopAtNestedViewTransitions, ) @@ -1099,6 +1204,42 @@ function measureUpdateViewTransition( current: Fiber, finishedWork: Fiber, ): boolean { + const props: ViewTransitionProps = finishedWork.memoizedProps; + const updateClassName: ?string = getViewTransitionClassName( + props.className, + props.update, + ); + const layoutClassName: ?string = getViewTransitionClassName( + props.className, + props.update, + ); + let className: ?string; + if (updateClassName === 'none') { + if (layoutClassName === 'none') { + // If both update and layout class name were none, then we didn't apply any + // names in the before update phase so we shouldn't now neither. + return false; + } + // We don't care if this is mutated or children layout changed, but we still + // measure each instance to see if it moved and therefore should apply layout. + finishedWork.flags &= ~Update; + className = layoutClassName; + } else if ((finishedWork.flags & Update) !== NoFlags) { + // It was updated and we have an appropriate class name to apply. + className = updateClassName; + } else { + 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; + } + // 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. viewTransitionHostInstanceIdx = 0; @@ -1107,6 +1248,8 @@ function measureUpdateViewTransition( current, finishedWork, finishedWork.child, + name, + className, previousMeasurements, true, ); @@ -1127,16 +1270,27 @@ function measureNestedViewTransitions(changedParent: Fiber): void { 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, + ); viewTransitionHostInstanceIdx = 0; - measureViewTransitionHostInstances( + const inViewport = measureViewTransitionHostInstances( current, child, child.child, + name, + className, child.memoizedState, false, ); - const props: ViewTransitionProps = child.memoizedProps; - scheduleViewTransitionEvent(child, props.onLayout); + if ((child.flags & Update) === NoFlags || !inViewport) { + // Nothing changed. + } else { + scheduleViewTransitionEvent(child, props.onLayout); + } } } else if ((child.subtreeFlags & ViewTransitionStatic) !== NoFlags) { measureNestedViewTransitions(child); @@ -3010,11 +3164,6 @@ function recursivelyTraverseAfterMutationEffects( // its size and position. We need to measure this and if not, restore it to // not animate. measureNestedViewTransitions(parentFiber); - if ((parentFiber.flags & AffectedParentLayout) !== NoFlags) { - // This boundary changed size in a way that may have caused its parent to - // relayout. We need to bubble this information up to the parent. - viewTransitionContextChanged = true; - } } } diff --git a/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js b/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js index bfbc1b0b2e..65e3ffc459 100644 --- a/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js +++ b/packages/react-reconciler/src/ReactFiberViewTransitionComponent.js @@ -19,8 +19,13 @@ import {getTreeId} from './ReactFiberTreeContext'; export type ViewTransitionProps = { name?: string, - className?: string, children?: ReactNodeList, + className?: 'none' | string, + enter?: 'none' | string, + exit?: 'none' | string, + layout?: 'none' | string, + share?: 'none' | string, + update?: 'none' | string, onEnter?: (instance: ViewTransitionInstance) => void, onExit?: (instance: ViewTransitionInstance) => void, onLayout?: (instance: ViewTransitionInstance) => void, @@ -76,3 +81,19 @@ export function getViewTransitionName( // We should have assigned a name by now. return (instance.autoName: any); } + +export function getViewTransitionClassName( + className: ?string, + eventClassName: ?string, +): ?string { + if (eventClassName == null) { + return className; + } + if (eventClassName === 'none') { + return eventClassName; + } + if (className != null) { + return className + ' ' + eventClassName; + } + return eventClassName; +} From 6e1e8fba1ece96e41157af125594bded29de17e6 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Mon, 13 Jan 2025 12:05:59 -0500 Subject: [PATCH 3/3] [compiler][ez] Patch compilationMode:infer object method edge case Fix for https://github.com/facebook/react/issues/31180 --- .../src/Entrypoint/Program.ts | 2 + .../infer-nested-object-method.expect.md | 63 +++++++++++++++++++ .../compiler/infer-nested-object-method.jsx | 19 ++++++ 3 files changed, 84 insertions(+) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-nested-object-method.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-nested-object-method.jsx diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts index bb0d662c4f..6ab9ee79c7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Program.ts @@ -992,9 +992,11 @@ function returnsNonNode( } } }, + // Skip traversing all nested functions and their return statements ArrowFunctionExpression: skipNestedFunctions(node), FunctionExpression: skipNestedFunctions(node), FunctionDeclaration: skipNestedFunctions(node), + ObjectMethod: node => node.skip(), }); return !hasReturn || returnsNonNode; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-nested-object-method.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-nested-object-method.expect.md new file mode 100644 index 0000000000..4c89f59919 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-nested-object-method.expect.md @@ -0,0 +1,63 @@ + +## Input + +```javascript +// @compilationMode(infer) + +import {Stringify} from 'shared-runtime'; + +function Test() { + const context = { + testFn() { + // if it is an arrow function its work + return () => 'test'; // it will break compile if returns an arrow fn + }, + }; + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Test, + params: [{}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @compilationMode(infer) + +import { Stringify } from "shared-runtime"; + +function Test() { + const $ = _c(1); + let t0; + if ($[0] === Symbol.for("react.memo_cache_sentinel")) { + const context = { + testFn() { + return _temp; + }, + }; + + t0 = ; + $[0] = t0; + } else { + t0 = $[0]; + } + return t0; +} +function _temp() { + return "test"; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Test, + params: [{}], +}; + +``` + +### Eval output +(kind: ok)
{"value":{"testFn":{"kind":"Function","result":{"kind":"Function","result":"test"}}},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-nested-object-method.jsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-nested-object-method.jsx new file mode 100644 index 0000000000..c8e396b057 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/infer-nested-object-method.jsx @@ -0,0 +1,19 @@ +// @compilationMode(infer) + +import {Stringify} from 'shared-runtime'; + +function Test() { + const context = { + testFn() { + // if it is an arrow function its work + return () => 'test'; // it will break compile if returns an arrow fn + }, + }; + + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Test, + params: [{}], +};