Skip to content

Flush subtree effects#5055

Open
JoviDeCroock wants to merge 1 commit intov10.xfrom
flush-subtree-effects
Open

Flush subtree effects#5055
JoviDeCroock wants to merge 1 commit intov10.xfrom
flush-subtree-effects

Conversation

@JoviDeCroock
Copy link
Member

Reproduction at https://stackblitz.com/edit/vitejs-vite-yxpjutvm?file=package.json - notice how the fade-out animation never triggers and instead it just dissapears

Problem

When libraries like motion/react (framer-motion) and @base-ui/react are used together with Preact/compat, exit animations break. The popup disappears instantly instead of animating out.

The root cause is a timing issue: when a component re-renders before its useEffect callbacks have fired (via afterPaint), Preact flushes only that component's stale effects during options._render. This causes effects from different components to run in separate render cycles instead of as a single batch.

In the concrete case:

  1. Select closes → base-ui store updates → multiple components re-render
  2. Base-ui's stale useEffect flushes during one render cycle → calls getAnimations() → finds 0 animations
  3. Motion's stale useEffect flushes during a later render cycle → starts WAAPI animation → too late
  4. Base-ui has already concluded no animations are running → sets hidden → popup disappears

How React handles this

React calls flushPendingEffects() at the start of performSyncWorkOnRoot before beginning any new render work. This flushes all pending passive effects (from all components) as a batch, not just the re-rendering component's effects.

From React's ReactFiberRootScheduler.js:

// In performSyncWorkOnRoot:
const didFlushPassiveEffects = flushPendingEffects();
if (didFlushPassiveEffects) {
  return null; // Exit to recompute priority
}
// Then proceed with rendering
// In performWorkOnRootViaSchedulerTask (concurrent path):
const didFlushPassiveEffects = flushPendingEffectsDelayed();
if (didFlushPassiveEffects) {
  if (root.callbackNode !== originalCallbackNode) {
    return null;
  }
}
// Then proceed with rendering

This guarantees that all useEffect callbacks from render N complete before render N+1 begins, regardless of which component is re-rendering.

What Preact did before

In options._render, when a component re-renders with unflushed effects (previousComponent !== currentComponent):

// Old behavior: flush ONLY this component's stale effects
hooks._pendingEffects.some(invokeCleanup);
hooks._pendingEffects.some(invokeEffect);
hooks._pendingEffects = [];

This meant effects from different components could be spread across multiple render cycles when intermediate setState calls triggered cascading re-renders.

The fix

Replace the per-component flush with a call to flushAfterPaintEffects(), which drains the entire pending effects queue:

// New behavior: flush ALL pending effects (matching React)
if (hooks._pendingEffects.length) {
  flushAfterPaintEffects();
}

This matches React's approach: before starting new render work, ensure all effects from previous renders have completed.

Behavioral differences from the old code

Aspect Old behavior New behavior (matches React)
Which effects flush Only the re-rendering component's All components with pending effects
Flush order Re-rendering component first Tree order (children before parents, as queued by diffed)
When sibling effects run Deferred to their own re-render or afterPaint Eagerly, before any re-render proceeds
  • Parent-effects may fire earlier than before, but at the correct time per React's contract
  • Effect ordering changes from "re-rendering component first" to "tree order" which matches React

@github-actions
Copy link

github-actions bot commented Mar 17, 2026

📊 Tachometer Benchmark Results

Summary

duration

  • create10k: unsure 🔍 -2% - +0% (-12.54ms - +1.82ms)
    preact-local vs preact-main
  • filter-list: unsure 🔍 -0% - +0% (-0.02ms - +0.04ms)
    preact-local vs preact-main
  • hydrate1k: unsure 🔍 -1% - +3% (-0.73ms - +1.53ms)
    preact-local vs preact-main
  • many-updates: unsure 🔍 -1% - +3% (-0.23ms - +0.44ms)
    preact-local vs preact-main
  • replace1k: unsure 🔍 -1% - +1% (-0.41ms - +0.58ms)
    preact-local vs preact-main
  • text-update: unsure 🔍 -7% - +1% (-0.14ms - +0.03ms)
    preact-local vs preact-main
  • todo: unsure 🔍 -1% - +1% (-0.22ms - +0.18ms)
    preact-local vs preact-main
  • update10th1k: unsure 🔍 -3% - +5% (-1.09ms - +1.51ms)
    preact-local vs preact-main

usedJSHeapSize

  • create10k: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-main
  • filter-list: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-main
  • hydrate1k: unsure 🔍 -4% - +3% (-0.26ms - +0.18ms)
    preact-local vs preact-main
  • many-updates: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-main
  • replace1k: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-main
  • text-update: unsure 🔍 -1% - +6% (-0.01ms - +0.06ms)
    preact-local vs preact-main
  • todo: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-main
  • update10th1k: unsure 🔍 -0% - +0% (-0.01ms - +0.01ms)
    preact-local vs preact-main

Results

create10k

duration

VersionAvg timevs preact-localvs preact-main
preact-local759.06ms - 767.32ms-unsure 🔍
-2% - +0%
-12.54ms - +1.82ms
preact-main762.67ms - 774.43msunsure 🔍
-0% - +2%
-1.82ms - +12.54ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local19.20ms - 19.20ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-main19.20ms - 19.20msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms
-
filter-list

duration

VersionAvg timevs preact-localvs preact-main
preact-local16.46ms - 16.50ms-unsure 🔍
-0% - +0%
-0.02ms - +0.04ms
preact-main16.45ms - 16.49msunsure 🔍
-0% - +0%
-0.04ms - +0.02ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local1.56ms - 1.56ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-main1.55ms - 1.56msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms
-
hydrate1k

duration

VersionAvg timevs preact-localvs preact-main
preact-local60.96ms - 62.48ms-unsure 🔍
-1% - +3%
-0.73ms - +1.53ms
preact-main60.48ms - 62.15msunsure 🔍
-2% - +1%
-1.53ms - +0.73ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local6.78ms - 7.07ms-unsure 🔍
-4% - +3%
-0.26ms - +0.18ms
preact-main6.80ms - 7.13msunsure 🔍
-3% - +4%
-0.18ms - +0.26ms
-
many-updates

duration

VersionAvg timevs preact-localvs preact-main
preact-local16.03ms - 16.49ms-unsure 🔍
-1% - +3%
-0.23ms - +0.44ms
preact-main15.91ms - 16.40msunsure 🔍
-3% - +1%
-0.44ms - +0.23ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local3.72ms - 3.73ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-main3.72ms - 3.73msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms
-
replace1k
  • Browser: chrome-headless
  • Sample size: 100
  • Built by: CI #5442
  • Commit: 165ffdd

duration

VersionAvg timevs preact-localvs preact-main
preact-local59.57ms - 60.29ms-unsure 🔍
-1% - +1%
-0.41ms - +0.58ms
preact-main59.51ms - 60.19msunsure 🔍
-1% - +1%
-0.58ms - +0.41ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local3.01ms - 3.01ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-main3.01ms - 3.01msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms
-

run-warmup-0

VersionAvg timevs preact-localvs preact-main
preact-local27.01ms - 27.87ms-unsure 🔍
-2% - +2%
-0.60ms - +0.57ms
preact-main27.05ms - 27.86msunsure 🔍
-2% - +2%
-0.57ms - +0.60ms
-

run-warmup-1

VersionAvg timevs preact-localvs preact-main
preact-local32.55ms - 33.96ms-unsure 🔍
-2% - +3%
-0.76ms - +1.15ms
preact-main32.41ms - 33.70msunsure 🔍
-3% - +2%
-1.15ms - +0.76ms
-

run-warmup-2

VersionAvg timevs preact-localvs preact-main
preact-local33.71ms - 34.89ms-unsure 🔍
-2% - +3%
-0.62ms - +1.13ms
preact-main33.39ms - 34.69msunsure 🔍
-3% - +2%
-1.13ms - +0.62ms
-

run-warmup-3

VersionAvg timevs preact-localvs preact-main
preact-local26.33ms - 26.81ms-unsure 🔍
-0% - +2%
-0.12ms - +0.47ms
preact-main26.22ms - 26.57msunsure 🔍
-2% - +0%
-0.47ms - +0.12ms
-

run-warmup-4

VersionAvg timevs preact-localvs preact-main
preact-local23.31ms - 24.41ms-faster ✔
0% - 7%
0.05ms - 1.67ms
preact-main24.13ms - 25.32msslower ❌
0% - 7%
0.05ms - 1.67ms
-

run-final

VersionAvg timevs preact-localvs preact-main
preact-local22.11ms - 22.43ms-unsure 🔍
-1% - +1%
-0.14ms - +0.30ms
preact-main22.04ms - 22.33msunsure 🔍
-1% - +1%
-0.30ms - +0.14ms
-
text-update
  • Browser: chrome-headless
  • Sample size: 220
  • Built by: CI #5442
  • Commit: 165ffdd

duration

VersionAvg timevs preact-localvs preact-main
preact-local1.95ms - 2.05ms-unsure 🔍
-7% - +1%
-0.14ms - +0.03ms
preact-main1.99ms - 2.12msunsure 🔍
-1% - +7%
-0.03ms - +0.14ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local1.02ms - 1.07ms-unsure 🔍
-1% - +6%
-0.01ms - +0.06ms
preact-main1.00ms - 1.04msunsure 🔍
-6% - +1%
-0.06ms - +0.01ms
-
todo

duration

VersionAvg timevs preact-localvs preact-main
preact-local33.29ms - 33.59ms-unsure 🔍
-1% - +1%
-0.22ms - +0.18ms
preact-main33.32ms - 33.59msunsure 🔍
-1% - +1%
-0.18ms - +0.22ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local1.25ms - 1.25ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-main1.25ms - 1.25msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms
-
update10th1k

duration

VersionAvg timevs preact-localvs preact-main
preact-local31.47ms - 33.19ms-unsure 🔍
-3% - +5%
-1.09ms - +1.51ms
preact-main31.14ms - 33.09msunsure 🔍
-5% - +3%
-1.51ms - +1.09ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local2.97ms - 2.98ms-unsure 🔍
-0% - +0%
-0.01ms - +0.01ms
preact-main2.97ms - 2.98msunsure 🔍
-0% - +0%
-0.01ms - +0.01ms
-

tachometer-reporter-action v2 for CI

@github-actions
Copy link

Size Change: -15 B (-0.02%)

Total Size: 78.7 kB

Filename Size Change
hooks/dist/hooks.js 1.52 kB -7 B (-0.46%)
hooks/dist/hooks.mjs 1.55 kB -3 B (-0.19%)
hooks/dist/hooks.module.js 1.55 kB -3 B (-0.19%)
hooks/dist/hooks.umd.js 1.59 kB -2 B (-0.13%)
ℹ️ View Unchanged
Filename Size
compat/dist/compat.js 4.19 kB
compat/dist/compat.mjs 4.11 kB
compat/dist/compat.module.js 4.11 kB
compat/dist/compat.umd.js 4.25 kB
debug/dist/debug.js 3.85 kB
debug/dist/debug.mjs 3.85 kB
debug/dist/debug.module.js 3.85 kB
debug/dist/debug.umd.js 3.93 kB
devtools/dist/devtools.js 260 B
devtools/dist/devtools.mjs 273 B
devtools/dist/devtools.module.js 273 B
devtools/dist/devtools.umd.js 345 B
dist/preact.js 4.72 kB
dist/preact.min.js 4.74 kB
dist/preact.min.module.js 4.75 kB
dist/preact.min.umd.js 4.76 kB
dist/preact.mjs 4.74 kB
dist/preact.module.js 4.74 kB
dist/preact.umd.js 4.76 kB
jsx-runtime/dist/jsxRuntime.js 1.01 kB
jsx-runtime/dist/jsxRuntime.mjs 985 B
jsx-runtime/dist/jsxRuntime.module.js 985 B
jsx-runtime/dist/jsxRuntime.umd.js 1.08 kB
test-utils/dist/testUtils.js 473 B
test-utils/dist/testUtils.mjs 477 B
test-utils/dist/testUtils.module.js 477 B
test-utils/dist/testUtils.umd.js 555 B

compressed-size-action

Copy link
Member

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM nice find!

hooks._pendingEffects.some(invokeCleanup);
hooks._pendingEffects.some(invokeEffect);
hooks._pendingEffects = [];
if (hooks._pendingEffects.length) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this the wrong branch of this condition? The fix should be in the branch where currentComponent === previousComponent since it's for the case where the component is dirtied mid-render?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in the right branch, when we immediately re-render due to an in-render setState we will reset the components hooks state. When we however have overlapping renders where we see that we have rendered this component before and it still has pending-effects to flush we will flush it. Now instead of flushing the component only we flush the entire subtree.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main reason for this is that nothing currently stops the following from happening

  • Queue update
  • Update runs, our root and an intermediary component enqueues an effect run
  • Child queues an update for parent
    • BEFORE: we flush the intermediary effect
    • AFTER: we flush both intermediary as well as root effect
  • intermediary renders again

Between 3 and 4 we would flush the parent updates

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants