fix(apollo-react): stop node height flicker on handle count change [MST-11677]#867
fix(apollo-react): stop node height flicker on handle count change [MST-11677]#867KodudulaAshishUiPath wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Dependency License Review
License distribution
Excluded packages
|
There was a problem hiding this comment.
Pull request overview
Fixes a React Flow node-height oscillation in apollo-react canvas BaseNode caused by a deferred requestAnimationFrame height sync briefly stamping a “synced” height before the write actually committed, allowing a stale in-flight height to be treated as an external resize.
Changes:
- Move
syncedHeightRefstamping into the rAF callback so it only records heights that were actually committed viaupdateNode. - Tighten comments around
syncedHeightRef/baseHeightRefsemantics to reflect commit-time stamping and the external-vs-internal height distinction. - Add a regression test that keeps rAF deferred and reproduces the stale interleaved render scenario, asserting the node deflates and remains stable.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/apollo-react/src/canvas/components/BaseNode/BaseNode.tsx | Stamps syncedHeightRef at rAF commit time (paired with updateNode) to prevent stale in-flight heights from poisoning baseHeightRef. |
| packages/apollo-react/src/canvas/components/BaseNode/BaseNode.test.tsx | Adds a deferred-rAF regression test to reproduce and prevent the 96px ↔ 128px flicker loop. |
📊 Coverage + size by packagePer-package coverage and bundle size on this PR. New-line coverage = of the source lines this PR adds or changes, the % hit by tests.
"Coverage" is each package's own |
| const frameId = requestAnimationFrame(() => { | ||
| // Stamp at commit time, paired with the write, so the guard above never | ||
| // mistakes an in-flight stale height for an external resize. | ||
| syncedHeightRef.current = computedHeight; |
There was a problem hiding this comment.
[P1] This can still bounce back to the stale height if another render lands after this rAF stamps syncedHeightRef but before React Flow propagates the new height prop. In that window, the guard above treats the stale height as an external resize, restores baseHeightRef to the old value, and schedules the old height again.
There was a problem hiding this comment.
Suggested change:
Keep the previous synced height ignored until that same value is observed through props, or track a separate pending synced target/source pair so stale post-commit renders cannot rewrite baseHeightRef. Please also add coverage for a stale rerender after the rAF callback fires and before the new height prop arrives.
Problem
Deleting a case from an unconnected Switch node could make it continuously flicker between the 2-handle (96px) and 3-handle (128px) sizes.
BaseNodederives node height from the visible handle count and writes it back to React Flow viaupdateNode.Root cause
The write is deferred a frame via
requestAnimationFrame(to avoid a synchronous effect → store-write → re-render cascade and to let React Flow's measurement settle).syncedHeightRef— the "last height we wrote", used by a render-time guard to keep our own handle-inflation writes out ofbaseHeightRef— was being stamped at schedule time (effect body) rather than commit time. That opened a one-frame window where a stale in-flightheightwas misread as an external resize, poisoningbaseHeightRefand driving the 96px↔128px loop.Demo
Screen.Recording.2026-06-30.at.00.02.48.mov
Tests
Adds a regression test (
Height computation (deferred rAF race)) that keeps the rAF deferred to expose the schedule-vs-commit window, removes a handle with an interleaved stale render, and asserts the node deflates to 96 and settles without bouncing back to 128. Verified it fails on the unfixed code and passes with the fix. Full apollo-react suite green (1884 tests).