Skip to content

[Fiber] Trigger default transition indicator if needed #33160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented May 9, 2025

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 Reset currentEventTransitionLane after flushing sync work #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:

  • Default updates are currently not enough to cancel because those aren't flush in the same microtask. That's unfortunate.
  • Handle async actions before the setState. Since these don't necessarily have a root this is tricky.
  • 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.
  • Implement built-in DOM version by default. Implement Navigation API backed default indicator for DOM renderer #33162

@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label May 9, 2025
@react-sizebot
Copy link

react-sizebot commented May 9, 2025

Comparing: 21fdf30...c98d396

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB +0.05% 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js +0.02% 528.09 kB 528.18 kB = 93.17 kB 93.15 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB +0.11% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js +0.28% 646.40 kB 648.24 kB +0.24% 113.71 kB 113.99 kB
facebook-www/ReactDOM-prod.classic.js +0.03% 674.15 kB 674.37 kB +0.03% 118.41 kB 118.45 kB
facebook-www/ReactDOM-prod.modern.js +0.03% 664.43 kB 664.65 kB +0.02% 116.81 kB 116.83 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-art/cjs/react-art.production.js +0.53% 348.50 kB 350.33 kB +0.54% 58.77 kB 59.09 kB
oss-experimental/react-reconciler/cjs/react-reconciler.production.js +0.40% 475.65 kB 477.54 kB +0.44% 75.91 kB 76.25 kB
oss-experimental/react-reconciler/cjs/react-reconciler.profiling.js +0.35% 535.70 kB 537.60 kB +0.46% 83.95 kB 84.33 kB
oss-experimental/react-art/cjs/react-art.development.js +0.30% 669.99 kB 672.01 kB +0.29% 105.48 kB 105.78 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js +0.28% 646.40 kB 648.24 kB +0.24% 113.71 kB 113.99 kB
oss-experimental/react-dom/cjs/react-dom-unstable_testing.production.js +0.28% 660.82 kB 662.65 kB +0.24% 117.24 kB 117.53 kB
oss-experimental/react-dom/cjs/react-dom-profiling.profiling.js +0.26% 708.55 kB 710.38 kB +0.25% 122.75 kB 123.05 kB
oss-experimental/react-reconciler/cjs/react-reconciler.development.js +0.25% 795.20 kB 797.21 kB +0.33% 123.79 kB 124.20 kB

Generated by 🚫 dangerJS against c98d396

@sebmarkbage sebmarkbage force-pushed the transitionindicator2 branch 4 times, most recently from a653f6c to b80e413 Compare May 9, 2025 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants