[Fiber] Don't work on scheduled tasks while we're in an async commit but flush it eagerly if we're sync

[sebmarkbage]

This is a follow up to #31930 and a prerequisite for #31975.

With View Transitions, the commit phase becomes async which means that other work can sneak in between. We need to be resilient to that.

This PR first refactors the flushMutationEffects and flushLayoutEffects to use module scope variables to track its arguments so we can defer them. It shares these with how we were already doing it for flushPendingEffects.

We also track how far along the commit phase we are so we know what we have left to flush.

Then callers of flushPassiveEffects become flushPendingEffects. That helper synchronously flushes any remaining phases we've yet to commit. That ensure that things are at least consistent if that happens.

Finally, when we are using a scheduled task, we don't do any work. This ensures that we're not flushing any work too early if we could've deferred it. This still ensures that we always do flush it before starting any new work on any root so new roots observe the committed state.

There are some unfortunate effects that could happen from allowing things to flush eagerly. Such as if a flushSync sneaks in before startViewTransition, it'll skip the animation. If it's during a suspensey font it'll start the transition before the font has loaded which might be better than breaking flushSync. It'll also potentially flush passive effects inside the startViewTransition which should typically be ok.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 6, 2025 4:25pm

[react-sizebot]

Comparing: 3ce77d55a205065714fcf4b0533a4ac6d009992b...7adb0b253287be4c4f0e4845be32331f8b21e3bb

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	=	1.83 kB	1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	+0.16%	512.56 kB	513.40 kB	+0.12%	91.61 kB	91.72 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.69 kB	6.69 kB	=	1.83 kB	1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	+0.16%	517.34 kB	518.18 kB	+0.12%	92.46 kB	92.57 kB
facebook-www/ReactDOM-prod.classic.js	+0.18%	594.36 kB	595.45 kB	+0.12%	104.68 kB	104.81 kB
facebook-www/ReactDOM-prod.modern.js	+0.19%	584.62 kB	585.72 kB	+0.11%	103.14 kB	103.25 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable-semver/react-test-renderer/cjs/react-test-renderer.production.js	+0.27%	311.69 kB	312.53 kB	+0.35%	54.57 kB	54.76 kB
oss-stable/react-test-renderer/cjs/react-test-renderer.production.js	+0.27%	311.76 kB	312.61 kB	+0.35%	54.59 kB	54.78 kB
oss-experimental/react-test-renderer/cjs/react-test-renderer.production.js	+0.27%	311.93 kB	312.78 kB	+0.35%	54.62 kB	54.81 kB
oss-stable-semver/react-test-renderer/cjs/react-test-renderer.development.js	+0.24%	552.27 kB	553.57 kB	+0.33%	89.71 kB	90.01 kB
oss-experimental/react-test-renderer/cjs/react-test-renderer.development.js	+0.24%	552.30 kB	553.60 kB	+0.33%	89.72 kB	90.02 kB
oss-stable/react-test-renderer/cjs/react-test-renderer.development.js	+0.24%	552.34 kB	553.64 kB	+0.33%	89.73 kB	90.03 kB
facebook-www/ReactART-dev.modern.js	+0.23%	639.04 kB	640.54 kB	+0.29%	101.24 kB	101.53 kB
facebook-www/ReactART-prod.modern.js	+0.23%	357.10 kB	357.94 kB	+0.22%	59.80 kB	59.94 kB
facebook-www/ReactART-dev.classic.js	+0.23%	648.92 kB	650.42 kB	+0.26%	103.22 kB	103.49 kB
facebook-www/ReactART-prod.classic.js	+0.23%	366.83 kB	367.66 kB	+0.17%	61.40 kB	61.51 kB

Generated by 🚫 dangerJS against 424f0dd

[eps1lon]

I feel like we can remove this comment block now. We certainly don't stash a reference at this point anymore. And the scheduling is now obvious by advancing the phase.

[eps1lon]

Is it a problem that we potentially leave pendingEffectsRoot as non-nullabble but pendingEffectsStatus === NO_PENDING_EFFECTS when we throw in commitBeforeMutationEffects?

[sebmarkbage]

No, not really because it'll just leak until we update anything again.

A bigger problem would be leaving pendingEffectsStatus as something else. Leaving PENDING_PASSIVE_PHASE doesn't really happen because it's immediately reset and even if it did it would try again. Leaving PENDING_MUTATION_PHASE or PENDING_LAYOUT_PHASE would stop any new update except sync which would be bad.

So I'll reset it in the beginning in case React itself throws so it doesn't get stuck.