Fix effect-flushing for radix and animations#5163
Open
JoviDeCroock wants to merge 3 commits into
Open
Conversation
📊 Tachometer Benchmark ResultsSummaryduration
usedJSHeapSize
Resultscreate10kduration
usedJSHeapSize
filter-listduration
usedJSHeapSize
hydrate1kduration
usedJSHeapSize
many-updatesduration
usedJSHeapSize
replace1kduration
usedJSHeapSize
run-warmup-0
run-warmup-1
run-warmup-2
run-warmup-3
run-warmup-4
run-final
text-updateduration
usedJSHeapSize
tododuration
usedJSHeapSize
update10th1kduration
usedJSHeapSize
|
|
Size Change: +31 B (+0.2%) Total Size: 15.8 kB 📦 View Changed
ℹ️ View Unchanged
|
|
coverage: 98.536% (+0.004%) from 98.532% — JoviDeCroock/investigate-3666 into main |
Now that pending passive effects are flushed before every scheduled render, the flush in options._render only served renders that bypass the scheduler. Move that responsibility to options._root so explicit render() calls flush pending effects before entering the diff instead of during it, skipping unmounting renders (vnode == null) to keep pending effects of unmounting components cancelable. This also fixes test-utils resetting options.debounceRendering to undefined on teardown, which silently uninstalled the effect-flushing scheduler. Teardown now distinguishes a saved undefined (act must restore it) from "setupRerender was never called" and falls back to the scheduler that was present at load time. The suspense effect-cleanup and useSyncExternalStore tests asserted flush-before-render ordering through rerender(), which replaces the flushing scheduler; they now exercise the default scheduler instead.
JoviDeCroock
commented
Jul 16, 2026
| } | ||
| hookItem._pendingArgs = hookItem._nextValue = undefined; | ||
| }); | ||
| } else { |
Member
Author
There was a problem hiding this comment.
Now because we flush when we start a new render cycle this isn't needed anymore. It was basically a hack for what we're adding. For predictability though I would love to also add this into signals, that would however require a major and it's already behaving like this currently so... it's not pressing for v11 itself
JoviDeCroock
marked this pull request as ready for review
July 16, 2026 18:09
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
2 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.
Fixes #3666.
Flushes pending passive effects before scheduled renders.
Fixes the Radix Slider reproduction and the Base UI + Motion exit-animation reproduction from #5055.