[scheduler] Absorb un-acted ResizeObserver deliveries after rendering the timeline - #23444
Open
rita-codes wants to merge 8 commits into
Open
[scheduler] Absorb un-acted ResizeObserver deliveries after rendering the timeline#23444rita-codes wants to merge 8 commits into
rita-codes wants to merge 8 commits into
Conversation
The title-column and viewport observers deliver on the frames after the render, outside act; depending on runner timing the un-acted update fails the run through the console guard (the recurring test_browser_react_18 header failure). The absorb helper waits two native frames inside act — rAF captured at module load so fake timers cannot stall it — and now lives in test/utils/scheduler for any suite rendering the timeline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEgka4iDRFrenWeWFhunq
rita-codes
added
test
scope: scheduler
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
rita-codes
changed the title
Every suite mounting the timeline in browser mode is exposed to the same un-acted delivery; renderSettled packages render + absorb in the shared renderer so adopting it is one call, and the seven remaining timeline suites now settle their renders. Drag/scroll perturbations keep the explicit absorbObserverFrames calls. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEgka4iDRFrenWeWFhunq
The dependency harness still rendered synchronously with no absorb, so the arrows and terminals suites were failing test_browser_react_18 on CI for every branch (build 902915): the muting drag tests landed with master through mui#23441, but their absorb harness only exists on the auto-scheduling branch. The harness now settles its renders and the drag/scroll steps absorb explicitly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RDEgka4iDRFrenWeWFhunq
…' into scheduler-shared-observer-absorb
github-actions
Bot
added
the
PR: out-of-date
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
1 similar comment
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
…observer-absorb # Conflicts: # packages/x-scheduler-premium/src/event-timeline-premium/EventTimelinePremium.test.tsx # packages/x-scheduler-premium/src/event-timeline-premium/tests/EventDialog.test.tsx # packages/x-scheduler-premium/src/event-timeline-premium/tests/dependencyTerminals.EventTimelinePremium.test.tsx
github-actions
Bot
removed
the
PR: out-of-date
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CC6LfHbZXD549R8qtmQojJ
noraleonte
approved these changes
Sep 1, 2026
noraleonte
left a comment
noraleonte
left a comment
Contributor
There was a problem hiding this comment.
LGTM 🎉 Thanks for fixing it 🙏
| * Renders and absorbs the post-render ResizeObserver deliveries (see | ||
| * `absorbObserverFrames`), so browser-mode tests start from an acted, settled | ||
| * layout. Use it for anything mounting a scheduler surface. | ||
| */ |
Contributor
There was a problem hiding this comment.
Nit:
Nothing stops the flake from coming back - createSchedulerRenderer now exposes both render and renderSettled, and a new timeline test reaching for render silently reintroduces the race.
We can't just make render absorb (conformance needs the sync one), but a one-line comment on render in createSchedulerRenderer.tsx pointing at renderSettled for timeline surfaces — or an eslint no-restricted-syntax scoped to event-timeline-premium/ — would make the invariant enforceable rather than tribal.
| * Waits two native frames inside act, so pending ResizeObserver deliveries land as | ||
| * acted updates instead of between test steps. Call it after rendering a scheduler | ||
| * surface (prefer `renderSettled`) or after a scroll that mounts observed elements. | ||
| * No-op in jsdom, which has no ResizeObserver. |
Contributor
There was a problem hiding this comment.
Claude highlighted this: the branch below awaits act(async () => {}) and flushes pending microtasks — the inline comment says so directly. That's a real behavior change applied after every render across ~13 jsdom suites, not a no-op. The behavior looks deliberate and useful; we should just fix the doc line so nobody debugging jsdom timing is misled by it.
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 the recurring
test_browser_react_18failures in the scheduler browser suites: the dependency drag suites have been red on every branch since #23441 landed, plus the header act-warning flake.Why it flakes: the timeline measures itself with ResizeObservers. The browser delivers those callbacks on the frames after the render, and each delivery sets React state. Whether that update lands inside an
act()window depends on runner timing — when it doesn't, React warns, andvitest-fail-on-consoleturns the warning into a failure. The race predates #23441; that PR added the first tests shaped to lose it almost every run (perturb layout, assert synchronously, end — nothing act-covered follows), and only the React 18 scheduler leaves that window uncovered, which is why the other jobs stay green.The fix: drain the pending deliveries inside
actbefore the test moves on, so they can never land un-acted.absorbObserverFrames()(new, intest/utils/scheduler): waits two native frames insideact. The rAF is captured at module load so fake-timer tests can't stall it — and it hard-fails with a descriptive error if a test ever leaks fake timers into module collection, instead of hanging. In jsdom (no ResizeObserver) it flushes pending microtasks insideact, so async work started by the render (e.g. a data source fetch) lands acted in both environments.renderSettledoncreateSchedulerRenderer(render + absorb in one call) is the single render path in the adopted suites; explicitabsorbObserverFrames()calls remain only where a render can't be awaited (toErrorDev) or after scrolls that mount observed elements.renderSettledthe day it flakes.Verified deterministically, not by rerunning until green: a probe forcing the delivery to land outside
act(two raw frames after render) reproduced the exact CI error 3/3 runs; withabsorbObserverFrames()first it passed 3/3. That probe is committed as a regression test (absorbObserverFrames.test.tsx) — if the absorb ever stops draining the queue, the console guard fails it automatically — alongside pins for the fake-timers and jsdom branches.