Enables Basic View Transition support for React Native Fabric renderer#504
Enables Basic View Transition support for React Native Fabric renderer#504everettbu wants to merge 15 commits into
Conversation
Greptile SummaryThis PR wires up basic View Transition support for the React Native Fabric renderer by adding Key concerns identified in this review (in addition to those already raised in prior threads):
Confidence Score: 2/5
Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| export function cancelRootViewTransitionName(rootContainer: Container): void { | ||
| if (__DEV__) { | ||
| console.warn('cancelRootViewTransitionName is not implemented'); | ||
| } |
There was a problem hiding this comment.
Root APIs are no-ops
cancelRootViewTransitionName/restoreRootViewTransitionName are stubbed (only __DEV__ warnings), but the reconciler calls these for persistence renderers during commit/gesture flows. As a result, root-level view-transition-name cancellation/restoration won’t happen on Fabric, leaving root transition state inconsistent with what the reconciler assumes (e.g., cancellations that bubbled to the root never get undone).
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-native-renderer/src/ReactFiberConfigFabric.js
Line: 243:246
Comment:
**Root APIs are no-ops**
`cancelRootViewTransitionName`/`restoreRootViewTransitionName` are stubbed (only `__DEV__` warnings), but the reconciler calls these for persistence renderers during commit/gesture flows. As a result, root-level view-transition-name cancellation/restoration won’t happen on Fabric, leaving root transition state inconsistent with what the reconciler assumes (e.g., cancellations that bubbled to the root never get undone).
How can I resolve this? If you propose a fix, please make it concise.
| export function startGestureTransition( | ||
| suspendedState: null | SuspendedState, | ||
| rootContainer: Container, | ||
| timeline: GestureTimeline, | ||
| rangeStart: number, | ||
| rangeEnd: number, | ||
| transitionTypes: null | TransitionTypes, | ||
| mutationCallback: () => void, | ||
| animateCallback: () => void, | ||
| errorCallback: (error: mixed) => void, | ||
| finishedAnimation: () => void, | ||
| ): RunningViewTransition { | ||
| if (__DEV__) { | ||
| console.warn('startGestureTransition is not implemented'); | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
startGestureTransition returns null despite non-nullable type
The return type is declared as RunningViewTransition (non-nullable, per line 218-223: an object with skipTransition(), finished, and ready), but this function returns null. The reconciler at ReactFiberWorkLoop.js:4432 assigns the result to both pendingViewTransition and finishedGesture.running, which are later used by stopViewTransition() and for cancellation logic.
Returning null here means finishedGesture.running will be null, and any attempt to stop or cancel the gesture transition will silently fail or error. Either the return type should be null | RunningViewTransition, or this stub needs to return a valid RunningViewTransition object (e.g., with no-op skipTransition and pre-resolved promises).
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-native-renderer/src/ReactFiberConfigFabric.js
Line: 332:348
Comment:
**`startGestureTransition` returns null despite non-nullable type**
The return type is declared as `RunningViewTransition` (non-nullable, per line 218-223: an object with `skipTransition()`, `finished`, and `ready`), but this function returns `null`. The reconciler at `ReactFiberWorkLoop.js:4432` assigns the result to both `pendingViewTransition` and `finishedGesture.running`, which are later used by `stopViewTransition()` and for cancellation logic.
Returning `null` here means `finishedGesture.running` will be `null`, and any attempt to stop or cancel the gesture transition will silently fail or error. Either the return type should be `null | RunningViewTransition`, or this stub needs to return a valid `RunningViewTransition` object (e.g., with no-op `skipTransition` and pre-resolved promises).
How can I resolve this? If you propose a fix, please make it concise.| export function addViewTransitionFinishedListener( | ||
| transition: RunningViewTransition, | ||
| callback: () => void, | ||
| ): void { | ||
| callback(); | ||
| } |
There was a problem hiding this comment.
Immediate callback invocation may cause premature cleanup
addViewTransitionFinishedListener calls callback() immediately, but the reconciler uses this to register cleanup functions that should run when the view transition animation finishes (see ReactFiberWorkLoop.js:4278-4284). In the DOM renderer, this is transition.finished.finally(callback).
Since Fabric's startViewTransition now returns a RunningViewTransition with a finished promise (line 218-223), this implementation should use transition.finished.finally(callback) (or .then(callback)) like the DOM renderer does, so cleanup runs after the native animation completes rather than immediately.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-native-renderer/src/ReactFiberConfigFabric.js
Line: 356:361
Comment:
**Immediate callback invocation may cause premature cleanup**
`addViewTransitionFinishedListener` calls `callback()` immediately, but the reconciler uses this to register *cleanup* functions that should run when the view transition animation finishes (see `ReactFiberWorkLoop.js:4278-4284`). In the DOM renderer, this is `transition.finished.finally(callback)`.
Since Fabric's `startViewTransition` now returns a `RunningViewTransition` with a `finished` promise (line 218-223), this implementation should use `transition.finished.finally(callback)` (or `.then(callback)`) like the DOM renderer does, so cleanup runs after the native animation completes rather than immediately.
How can I resolve this? If you propose a fix, please make it concise.
| transition.ready.then(() => { | ||
| spawnedWorkCallback(); | ||
| }); |
There was a problem hiding this comment.
Missing error handler on ready promise
The DOM renderer handles transition.ready rejection with a dedicated error handler (transition.ready.then(readyCallback, handleError) in ReactFiberConfigDOM.js:2322) that calls errorCallback and flushes remaining work synchronously as a recovery path. Here, if transition.ready rejects (e.g., due to a native-side error), spawnedWorkCallback will never be called, the errorCallback parameter is silently ignored, and the unhandled promise rejection could crash the app or leave the commit lifecycle in an inconsistent state where spawned work is never flushed.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-native-renderer/src/ReactFiberConfigFabric.js
Line: 422-424
Comment:
**Missing error handler on `ready` promise**
The DOM renderer handles `transition.ready` rejection with a dedicated error handler (`transition.ready.then(readyCallback, handleError)` in `ReactFiberConfigDOM.js:2322`) that calls `errorCallback` and flushes remaining work synchronously as a recovery path. Here, if `transition.ready` rejects (e.g., due to a native-side error), `spawnedWorkCallback` will never be called, the `errorCallback` parameter is silently ignored, and the unhandled promise rejection could crash the app or leave the commit lifecycle in an inconsistent state where spawned work is never flushed.
How can I resolve this? If you propose a fix, please make it concise.
3b94734 to
def44a1
Compare
c730c17 to
efe355f
Compare
| export const stopViewTransition = shim; | ||
| export const addViewTransitionFinishedListener = shim; | ||
| export type ViewTransitionInstance = null | {name: string, ...}; | ||
| export const createViewTransitionInstance = shim; |
There was a problem hiding this comment.
Missing GestureTimeline and getCurrentGestureOffset exports
This module is missing exports for GestureTimeline (type) and getCurrentGestureOffset (function), which the reconciler imports from ReactFiberConfig at ReactFiberGestureScheduler.js:12,27:
import type {GestureTimeline, RunningViewTransition} from './ReactFiberConfig';
import {getCurrentGestureOffset, stopViewTransition} from './ReactFiberConfig';Prior to this PR, these were defined directly in each renderer config (ART, Native legacy, Test). This PR removes them from ART and Native (legacy) — those renderers now rely solely on ReactFiberConfigWithNoViewTransition for view-transition-related exports, but this module doesn't include these two. The result is that ART and the legacy Native renderer are missing required exports that the reconciler depends on.
The Test renderer still defines GestureTimeline and getCurrentGestureOffset locally (lines 371-376), so it's unaffected. Fabric gets them from ReactFiberConfigWithNoMutation. But ART and the legacy Native renderer have no source for these.
Consider adding the following to this module:
export type GestureTimeline = null;
export function getCurrentGestureOffset(provider: GestureTimeline): number {
throw new Error(
'The current renderer does not support view transitions. ' +
'This error is likely caused by a bug in React. ' +
'Please file an issue.',
);
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-reconciler/src/ReactFiberConfigWithNoViewTransition.js
Line: 42
Comment:
**Missing `GestureTimeline` and `getCurrentGestureOffset` exports**
This module is missing exports for `GestureTimeline` (type) and `getCurrentGestureOffset` (function), which the reconciler imports from `ReactFiberConfig` at `ReactFiberGestureScheduler.js:12,27`:
```js
import type {GestureTimeline, RunningViewTransition} from './ReactFiberConfig';
import {getCurrentGestureOffset, stopViewTransition} from './ReactFiberConfig';
```
Prior to this PR, these were defined directly in each renderer config (ART, Native legacy, Test). This PR removes them from ART and Native (legacy) — those renderers now rely solely on `ReactFiberConfigWithNoViewTransition` for view-transition-related exports, but this module doesn't include these two. The result is that ART and the legacy Native renderer are missing required exports that the reconciler depends on.
The Test renderer still defines `GestureTimeline` and `getCurrentGestureOffset` locally (lines 371-376), so it's unaffected. Fabric gets them from `ReactFiberConfigWithNoMutation`. But ART and the legacy Native renderer have no source for these.
Consider adding the following to this module:
```
export type GestureTimeline = null;
export function getCurrentGestureOffset(provider: GestureTimeline): number {
throw new Error(
'The current renderer does not support view transitions. ' +
'This error is likely caused by a bug in React. ' +
'Please file an issue.',
);
}
```
How can I resolve this? If you propose a fix, please make it concise.
Wherever the reconciler checks supportsMutation before running view transition logic, add an else-if branch gated on enableViewTransitionForPersistenceMode for persistent renderers (Fabric). This follows the pattern that supportsMutation guards mutation-mode-only logic, and persistent mode should have its own branch rather than being lumped under a single supportsViewTransition capability flag. For now the persistent mode branches duplicate the mutation logic. The flag defaults to false in all channels.
…n out of ReactFiberConfigWithNoMutation
…n out of ReactFiberConfigWithNoMutation
Summary: - turn on enableViewTransition feature - stub shim - run some mutation config fn at persistence mode too
Better reflects that this signals the transition has started rather than initiating it.
…enable startViewTransition
e5a7693 to
aff6b07
Compare
2 participants
Mirror of facebook/react#35764
Original author: zeyap
Summary
Enables Basic View Transition support for React Native Fabric renderer.
Implemented:
measureInstance,applyViewTransitionName,startViewTransition,restoreViewTransitionName, andcancelViewTransitionNamestartViewTransitionwith proper callback orchestration (mutation → layout → afterMutation → spawnedWork → passive)startViewTransitionreturns false (e.g., when the ViewTransition ReactNativeFeatureFlag is not enabled)Stubbed with
__DEV__warnings (not yet implemented):cancelRootViewTransitionNamerestoreRootViewTransitionNamecloneRootViewTransitionContainerremoveRootViewTransitionClonemeasureClonedInstancehasInstanceChangedhasInstanceAffectedParentstartGestureTransitionstopViewTransitiongetCurrentGestureOffsetThis allows React Native apps using Fabric to leverage the View Transition API for coordinated animations during state transitions, with graceful degradation when the native side doesn't support it.
Below are diagrams of proposed architecture in fabric, and observation of what/when config functions get called during a basic shared transition example
How did you test this change?
yarn flow fabric- Flow type checks passyarn lint- Lint checks passReactFeatureFlags.test-renderer.native-fb.jsand View Transition enabled via ReactNativeFeatureFlagReactFabric-dev.fb.jsthat the 'shim' config functions are not included__DEV__and flushes work synchronously when ViewTransition flag isn't enabled in Fabric