Unify BatchConfigTransition and Transition types #32783
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These are the same object. Never made sense for there to be two types. Need to clean up the usage a bit due to wrong types and inconsistent hidden classes.
github-actions
bot
added
the
React Core Team
|
Comparing: 6377903...bc93f06 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
|
This was really just part of #32785 so I'll land. |
github-actions bot
pushed a commit
that referenced
this pull request
Apr 1, 2025
This is some overdue refactoring. The two types never made sense. It also should be defined by isomorphic since it defines how it should be used by renderers rather than isomorphic depending on Fiber. Clean up hidden classes to be consistent. Fix missing name due to wrong types. I choose not to invoke the transition tracing callbacks if there's no name since the name is required there. DiffTrain build for [d3b8ff6](d3b8ff6)
sebmarkbage
added a commit
that referenced
this pull request
Apr 1, 2025
Stacked on #32783. This will replace [the `useSwipeTransition` API](#32373). Instead, of a special Hook, you can make updates to `useOptimistic` Hooks within the `startGestureTransition` scope. ``` import {unstable_startGestureTransition as startGestureTransition} from 'react'; const cancel = startGestureTransition(timeline, () => { setOptimistic(...); }, options); ``` There are some downsides to this like you can't define two directions as once and there's no "standard" direction protocol. It's instead up to libraries to come up with their own conventions (although we can suggest some). The convention is still that a gesture recognizer has two props `action` and `gesture`. The `gesture` prop is a Gesture concept which now behaves more like an Action but 1) it can't be async 2) it shouldn't have side-effects. For example you can't call `setState()` in it except on `useOptimistic` since those can be reverted if needed. The `action` is invoked with whatever side-effects you want after the gesture fulfills. This is isomorphic and not associated with a specific renderer nor root so it's a bit more complicated. To implement this I unify with the `ReactSharedInternal.T` property to contain a regular Transition or a Gesture Transition (the `gesture` field). The benefit of this unification means that every time we override this based on some scope like entering `flushSync` we also override the `startGestureTransition` scope. We just have to be careful when we read it to check the `gesture` field to know which one it is. (E.g. I error for setState / requestFormReset.) The other thing that's unique is the `cancel` return value to know when to stop the gesture. That cancellation is no longer associated with any particular Hook. It's more associated with the scope of the `startGestureTransition`. Since the schedule of whether a particular gesture has rendered or committed is associated with a root, we need to somehow associate any scheduled gestures with a root. We could track which roots we update inside the scope but instead, I went with a model where I check all the roots and see if there's a scheduled gesture matching the timeline. This means that you could "retain" a gesture across roots. Meaning this wouldn't cancel until both are cancelled: ``` const cancelA = startGestureTransition(timeline, () => { setOptimisticOnRootA(...); }, options); const cancelB = startGestureTransition(timeline, () => { setOptimisticOnRootB(...); }, options); ``` It's more like it's a global transition than associated with the roots that were updated. Optimistic updates mostly just work but I now associate them with a specific "ScheduledGesture" instance since we can only render one at a time and so if it's not the current one, we leave it for later. Clean up of optimistic updates is now lazy rather than when we cancel. Allowing the cancel closure not to have to be associated with each particular update.
github-actions bot
pushed a commit
that referenced
this pull request
Apr 1, 2025
This is some overdue refactoring. The two types never made sense. It also should be defined by isomorphic since it defines how it should be used by renderers rather than isomorphic depending on Fiber. Clean up hidden classes to be consistent. Fix missing name due to wrong types. I choose not to invoke the transition tracing callbacks if there's no name since the name is required there. DiffTrain build for [d3b8ff6](d3b8ff6)
github-actions bot
pushed a commit
that referenced
this pull request
Apr 1, 2025
Stacked on #32783. This will replace [the `useSwipeTransition` API](#32373). Instead, of a special Hook, you can make updates to `useOptimistic` Hooks within the `startGestureTransition` scope. ``` import {unstable_startGestureTransition as startGestureTransition} from 'react'; const cancel = startGestureTransition(timeline, () => { setOptimistic(...); }, options); ``` There are some downsides to this like you can't define two directions as once and there's no "standard" direction protocol. It's instead up to libraries to come up with their own conventions (although we can suggest some). The convention is still that a gesture recognizer has two props `action` and `gesture`. The `gesture` prop is a Gesture concept which now behaves more like an Action but 1) it can't be async 2) it shouldn't have side-effects. For example you can't call `setState()` in it except on `useOptimistic` since those can be reverted if needed. The `action` is invoked with whatever side-effects you want after the gesture fulfills. This is isomorphic and not associated with a specific renderer nor root so it's a bit more complicated. To implement this I unify with the `ReactSharedInternal.T` property to contain a regular Transition or a Gesture Transition (the `gesture` field). The benefit of this unification means that every time we override this based on some scope like entering `flushSync` we also override the `startGestureTransition` scope. We just have to be careful when we read it to check the `gesture` field to know which one it is. (E.g. I error for setState / requestFormReset.) The other thing that's unique is the `cancel` return value to know when to stop the gesture. That cancellation is no longer associated with any particular Hook. It's more associated with the scope of the `startGestureTransition`. Since the schedule of whether a particular gesture has rendered or committed is associated with a root, we need to somehow associate any scheduled gestures with a root. We could track which roots we update inside the scope but instead, I went with a model where I check all the roots and see if there's a scheduled gesture matching the timeline. This means that you could "retain" a gesture across roots. Meaning this wouldn't cancel until both are cancelled: ``` const cancelA = startGestureTransition(timeline, () => { setOptimisticOnRootA(...); }, options); const cancelB = startGestureTransition(timeline, () => { setOptimisticOnRootB(...); }, options); ``` It's more like it's a global transition than associated with the roots that were updated. Optimistic updates mostly just work but I now associate them with a specific "ScheduledGesture" instance since we can only render one at a time and so if it's not the current one, we leave it for later. Clean up of optimistic updates is now lazy rather than when we cancel. Allowing the cancel closure not to have to be associated with each particular update. DiffTrain build for [b286430](b286430)
github-actions bot
pushed a commit
that referenced
this pull request
Apr 1, 2025
Stacked on #32783. This will replace [the `useSwipeTransition` API](#32373). Instead, of a special Hook, you can make updates to `useOptimistic` Hooks within the `startGestureTransition` scope. ``` import {unstable_startGestureTransition as startGestureTransition} from 'react'; const cancel = startGestureTransition(timeline, () => { setOptimistic(...); }, options); ``` There are some downsides to this like you can't define two directions as once and there's no "standard" direction protocol. It's instead up to libraries to come up with their own conventions (although we can suggest some). The convention is still that a gesture recognizer has two props `action` and `gesture`. The `gesture` prop is a Gesture concept which now behaves more like an Action but 1) it can't be async 2) it shouldn't have side-effects. For example you can't call `setState()` in it except on `useOptimistic` since those can be reverted if needed. The `action` is invoked with whatever side-effects you want after the gesture fulfills. This is isomorphic and not associated with a specific renderer nor root so it's a bit more complicated. To implement this I unify with the `ReactSharedInternal.T` property to contain a regular Transition or a Gesture Transition (the `gesture` field). The benefit of this unification means that every time we override this based on some scope like entering `flushSync` we also override the `startGestureTransition` scope. We just have to be careful when we read it to check the `gesture` field to know which one it is. (E.g. I error for setState / requestFormReset.) The other thing that's unique is the `cancel` return value to know when to stop the gesture. That cancellation is no longer associated with any particular Hook. It's more associated with the scope of the `startGestureTransition`. Since the schedule of whether a particular gesture has rendered or committed is associated with a root, we need to somehow associate any scheduled gestures with a root. We could track which roots we update inside the scope but instead, I went with a model where I check all the roots and see if there's a scheduled gesture matching the timeline. This means that you could "retain" a gesture across roots. Meaning this wouldn't cancel until both are cancelled: ``` const cancelA = startGestureTransition(timeline, () => { setOptimisticOnRootA(...); }, options); const cancelB = startGestureTransition(timeline, () => { setOptimisticOnRootB(...); }, options); ``` It's more like it's a global transition than associated with the roots that were updated. Optimistic updates mostly just work but I now associate them with a specific "ScheduledGesture" instance since we can only render one at a time and so if it's not the current one, we leave it for later. Clean up of optimistic updates is now lazy rather than when we cancel. Allowing the cancel closure not to have to be associated with each particular update. DiffTrain build for [b286430](b286430)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is some overdue refactoring. The two types never made sense. It also should be defined by isomorphic since it defines how it should be used by renderers rather than isomorphic depending on Fiber.
Clean up hidden classes to be consistent.
Fix missing name due to wrong types. I choose not to invoke the transition tracing callbacks if there's no name since the name is required there.