Implement Navigation API backed default indicator for DOM renderer#33162
Merged
sebmarkbage merged 3 commits intofacebook:mainfrom May 13, 2025
Merged
Implement Navigation API backed default indicator for DOM renderer#33162sebmarkbage merged 3 commits intofacebook:mainfrom
sebmarkbage merged 3 commits intofacebook:mainfrom
Conversation
github-actions
bot
added
the
React Core Team
|
Comparing: b480865...177844e Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
4 tasks
sebmarkbage
force-pushed
the
transitionindicator3
branch
from
b7ad698 to
1cc5d16
Compare
rickhanlonii
approved these changes
May 13, 2025
| } | ||
|
|
||
| // Delay the start a bit in case this is a fast navigation. | ||
| setTimeout(startFakeNavigation, 100); |
Member
There was a problem hiding this comment.
Should we do const setTimeout = window.setTimeout; in the module scope so we're more likely to have access to a non-patched timeout (like we do in Scheduler).
Contributor
Author
There was a problem hiding this comment.
We don't really know if it's better or worse to have a patched one. Depends on use case and up to the patcher to figure out. Arguably for priority purposes it is better to have it be idle.
sebmarkbage
added a commit
that referenced
this pull request
May 13, 2025
Stacked on #33159. This implements `onDefaultTransitionIndicator`. The sequence is: 1) In `markRootUpdated` we schedule Transition updates as needing `indicatorLanes` on the root. This tracks the lanes that currently need an indicator to either start or remain going until this lane commits. 2) Track mutations during any commit. We use the same hook that view transitions use here but instead of tracking it just per view transition scope, we also track a global boolean for the whole root. 3) If a sync/default commit had any mutations, then we clear the indicator lane for the `currentEventTransitionLane`. This requires that the lane is still active while we do these commits. See #33159. In other words, a sync update gets associated with the current transition and it is assumed to be rendering the loading state for that corresponding transition so we don't need a default indicator for this lane. 4) At the end of `processRootScheduleInMicrotask`, right before we're about to enter a new "event transition lane" scope, it is no longer possible to render any more loading states for the current transition lane. That's when we invoke `onDefaultTransitionIndicator` for any roots that have new indicator lanes. 5) When we commit, we remove the finished lanes from `indicatorLanes` and once that reaches zero again, then we can clean up the default indicator. This approach means that you can start multiple different transitions while an indicator is still going but it won't stop/restart each time. Instead, it'll wait until all are done before stopping. Follow ups: - [x] Default updates are currently not enough to cancel because those aren't flush in the same microtask. That's unfortunate. #33186 - [x] Handle async actions before the setState. Since these don't necessarily have a root this is tricky. #33190 - [x] Disable for `useDeferredValue`. ~Since it also goes through `markRootUpdated` and schedules a Transition lane it'll get a default indicator even though it probably shouldn't have one.~ EDIT: Turns out this just works because it doesn't go through `markRootUpdated` when work is left behind. - [x] Implement built-in DOM version by default. #33162
github-actions bot
pushed a commit
that referenced
this pull request
May 13, 2025
Stacked on #33159. This implements `onDefaultTransitionIndicator`. The sequence is: 1) In `markRootUpdated` we schedule Transition updates as needing `indicatorLanes` on the root. This tracks the lanes that currently need an indicator to either start or remain going until this lane commits. 2) Track mutations during any commit. We use the same hook that view transitions use here but instead of tracking it just per view transition scope, we also track a global boolean for the whole root. 3) If a sync/default commit had any mutations, then we clear the indicator lane for the `currentEventTransitionLane`. This requires that the lane is still active while we do these commits. See #33159. In other words, a sync update gets associated with the current transition and it is assumed to be rendering the loading state for that corresponding transition so we don't need a default indicator for this lane. 4) At the end of `processRootScheduleInMicrotask`, right before we're about to enter a new "event transition lane" scope, it is no longer possible to render any more loading states for the current transition lane. That's when we invoke `onDefaultTransitionIndicator` for any roots that have new indicator lanes. 5) When we commit, we remove the finished lanes from `indicatorLanes` and once that reaches zero again, then we can clean up the default indicator. This approach means that you can start multiple different transitions while an indicator is still going but it won't stop/restart each time. Instead, it'll wait until all are done before stopping. Follow ups: - [x] Default updates are currently not enough to cancel because those aren't flush in the same microtask. That's unfortunate. #33186 - [x] Handle async actions before the setState. Since these don't necessarily have a root this is tricky. #33190 - [x] Disable for `useDeferredValue`. ~Since it also goes through `markRootUpdated` and schedules a Transition lane it'll get a default indicator even though it probably shouldn't have one.~ EDIT: Turns out this just works because it doesn't go through `markRootUpdated` when work is left behind. - [x] Implement built-in DOM version by default. #33162 DiffTrain build for [62d3f36](62d3f36)
github-actions bot
pushed a commit
that referenced
this pull request
May 13, 2025
Stacked on #33159. This implements `onDefaultTransitionIndicator`. The sequence is: 1) In `markRootUpdated` we schedule Transition updates as needing `indicatorLanes` on the root. This tracks the lanes that currently need an indicator to either start or remain going until this lane commits. 2) Track mutations during any commit. We use the same hook that view transitions use here but instead of tracking it just per view transition scope, we also track a global boolean for the whole root. 3) If a sync/default commit had any mutations, then we clear the indicator lane for the `currentEventTransitionLane`. This requires that the lane is still active while we do these commits. See #33159. In other words, a sync update gets associated with the current transition and it is assumed to be rendering the loading state for that corresponding transition so we don't need a default indicator for this lane. 4) At the end of `processRootScheduleInMicrotask`, right before we're about to enter a new "event transition lane" scope, it is no longer possible to render any more loading states for the current transition lane. That's when we invoke `onDefaultTransitionIndicator` for any roots that have new indicator lanes. 5) When we commit, we remove the finished lanes from `indicatorLanes` and once that reaches zero again, then we can clean up the default indicator. This approach means that you can start multiple different transitions while an indicator is still going but it won't stop/restart each time. Instead, it'll wait until all are done before stopping. Follow ups: - [x] Default updates are currently not enough to cancel because those aren't flush in the same microtask. That's unfortunate. #33186 - [x] Handle async actions before the setState. Since these don't necessarily have a root this is tricky. #33190 - [x] Disable for `useDeferredValue`. ~Since it also goes through `markRootUpdated` and schedules a Transition lane it'll get a default indicator even though it probably shouldn't have one.~ EDIT: Turns out this just works because it doesn't go through `markRootUpdated` when work is left behind. - [x] Implement built-in DOM version by default. #33162 DiffTrain build for [62d3f36](62d3f36)
Seems like you can't extend addEventListener.
sebmarkbage
force-pushed
the
transitionindicator3
branch
from
1cc5d16 to
177844e
Compare
github-actions bot
pushed a commit
that referenced
this pull request
May 13, 2025
…33162) Stacked on #33160. By default, if `onDefaultTransitionIndicator` is not overridden, this will trigger a fake Navigation event using the Navigation API. This is intercepted to create an on-going navigation until we complete the Transition. Basically each default Transition is simulated as a Navigation. This triggers the native browser loading state (in Chrome at least). So now by default the browser spinner spins during a Transition if no other loading state is provided. Firefox and Safari hasn't shipped Navigation API yet and even in the flag Safari has, it doesn't actually trigger the native loading state. To ensures that you can still use other Navigations concurrently, we don't start our fake Navigation if there's one on-going already. Similarly if our fake Navigation gets interrupted by another. We wait for on-going ones to finish and then start a new fake one if we're supposed to be still pending. There might be other routers on the page that might listen to intercept Navigation Events. Typically you'd expect them not to trigger a refetch when navigating to the same state. However, if they want to detect this we provide the `"react-transition"` string in the `info` field for this purpose. DiffTrain build for [5944042](5944042)
github-actions bot
pushed a commit
that referenced
this pull request
May 13, 2025
…33162) Stacked on #33160. By default, if `onDefaultTransitionIndicator` is not overridden, this will trigger a fake Navigation event using the Navigation API. This is intercepted to create an on-going navigation until we complete the Transition. Basically each default Transition is simulated as a Navigation. This triggers the native browser loading state (in Chrome at least). So now by default the browser spinner spins during a Transition if no other loading state is provided. Firefox and Safari hasn't shipped Navigation API yet and even in the flag Safari has, it doesn't actually trigger the native loading state. To ensures that you can still use other Navigations concurrently, we don't start our fake Navigation if there's one on-going already. Similarly if our fake Navigation gets interrupted by another. We wait for on-going ones to finish and then start a new fake one if we're supposed to be still pending. There might be other routers on the page that might listen to intercept Navigation Events. Typically you'd expect them not to trigger a refetch when navigating to the same state. However, if they want to detect this we provide the `"react-transition"` string in the `info` field for this purpose. DiffTrain build for [5944042](5944042)
sebmarkbage
added a commit
that referenced
this pull request
May 13, 2025
…o root associated (#33190) Stacked on #33160, #33162, #33186 and #33188. We have a special case that's awkward for default indicators. When you start a new async Transition from `React.startTransition` then there's not yet any associated root with the Transition because you haven't necessarily `setState` on anything yet until the promise resolves. That's what `entangleAsyncAction` handles by creating a lane that everything entangles with until all async actions are done. If there are no sync updates before the end of the event, we should trigger a default indicator until either the async action completes without update or if it gets entangled with some roots we should keep it going until those roots are done.
github-actions bot
pushed a commit
that referenced
this pull request
May 13, 2025
…o root associated (#33190) Stacked on #33160, #33162, #33186 and #33188. We have a special case that's awkward for default indicators. When you start a new async Transition from `React.startTransition` then there's not yet any associated root with the Transition because you haven't necessarily `setState` on anything yet until the promise resolves. That's what `entangleAsyncAction` handles by creating a lane that everything entangles with until all async actions are done. If there are no sync updates before the end of the event, we should trigger a default indicator until either the async action completes without update or if it gets entangled with some roots we should keep it going until those roots are done. DiffTrain build for [3a5b326](3a5b326)
github-actions bot
pushed a commit
that referenced
this pull request
May 13, 2025
…o root associated (#33190) Stacked on #33160, #33162, #33186 and #33188. We have a special case that's awkward for default indicators. When you start a new async Transition from `React.startTransition` then there's not yet any associated root with the Transition because you haven't necessarily `setState` on anything yet until the promise resolves. That's what `entangleAsyncAction` handles by creating a lane that everything entangles with until all async actions are done. If there are no sync updates before the end of the event, we should trigger a default indicator until either the async action completes without update or if it gets entangled with some roots we should keep it going until those roots are done. DiffTrain build for [3a5b326](3a5b326)
github-actions bot
pushed a commit
to code/lib-react
that referenced
this pull request
May 14, 2025
…o root associated (facebook#33190) Stacked on facebook#33160, facebook#33162, facebook#33186 and facebook#33188. We have a special case that's awkward for default indicators. When you start a new async Transition from `React.startTransition` then there's not yet any associated root with the Transition because you haven't necessarily `setState` on anything yet until the promise resolves. That's what `entangleAsyncAction` handles by creating a lane that everything entangles with until all async actions are done. If there are no sync updates before the end of the event, we should trigger a default indicator until either the async action completes without update or if it gets entangled with some roots we should keep it going until those roots are done. DiffTrain build for [3a5b326](facebook@3a5b326)
github-actions bot
pushed a commit
to code/lib-react
that referenced
this pull request
May 14, 2025
…o root associated (facebook#33190) Stacked on facebook#33160, facebook#33162, facebook#33186 and facebook#33188. We have a special case that's awkward for default indicators. When you start a new async Transition from `React.startTransition` then there's not yet any associated root with the Transition because you haven't necessarily `setState` on anything yet until the promise resolves. That's what `entangleAsyncAction` handles by creating a lane that everything entangles with until all async actions are done. If there are no sync updates before the end of the event, we should trigger a default indicator until either the async action completes without update or if it gets entangled with some roots we should keep it going until those roots are done. DiffTrain build for [3a5b326](facebook@3a5b326)
This was referenced May 14, 2025
17 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Stacked on #33160.
By default, if
onDefaultTransitionIndicatoris not overridden, this will trigger a fake Navigation event using the Navigation API. This is intercepted to create an on-going navigation until we complete the Transition. Basically each default Transition is simulated as a Navigation.This triggers the native browser loading state (in Chrome at least). So now by default the browser spinner spins during a Transition if no other loading state is provided. Firefox and Safari hasn't shipped Navigation API yet and even in the flag Safari has, it doesn't actually trigger the native loading state.
To ensures that you can still use other Navigations concurrently, we don't start our fake Navigation if there's one on-going already. Similarly if our fake Navigation gets interrupted by another. We wait for on-going ones to finish and then start a new fake one if we're supposed to be still pending.
There might be other routers on the page that might listen to intercept Navigation Events. Typically you'd expect them not to trigger a refetch when navigating to the same state. However, if they want to detect this we provide the
"react-transition"string in theinfofield for this purpose.