fix(apollo-react): compute node height from handle count to stop flicker [MST-11677]#867
fix(apollo-react): compute node height from handle count to stop flicker [MST-11677]#867KodudulaAshishUiPath wants to merge 2 commits 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 |
04c3ca9 to
7bae0a7
Compare
7bae0a7 to
ed95b26
Compare
ed95b26 to
ad23359
Compare
4a3a6a6 to
d887e35
Compare
d887e35 to
c8a66b4
Compare
| // Write computedHeight to node.height and recalculate handle positions. Compare | ||
| // against node.height (not the measured `height` prop) so a lagging measurement | ||
| // can't retrigger the write. | ||
| // biome-ignore lint/correctness/useExhaustiveDependencies: handleConfigurations triggers handle recalculation. |
| // Intrinsic height: the fixed footer height when a footer is present, else the default. | ||
| const getIntrinsicHeight = ( | ||
| hasFooter: boolean, | ||
| footerVariant: FooterVariant | undefined | ||
| ): number | 'auto' => { | ||
| ): number => { |
Summary
Fixes the Switch-node height flicker (MST-11677): deleting a case from an unconnected Switch made it oscillate between the 2-handle (96px) and 3-handle (128px) sizes. Root cause was a feedback loop where
BaseNodederived its height from the measuredheightprop and wrote it back viaupdateNodeinside arequestAnimationFrame.Changes
computedHeightis now a pure function of visible handle count + intrinsic footer/default height (getIntrinsicHeight). It never reads the measuredheightprop, so the value written back can't feed into its own input — the loop is structurally gone.useEffectwritescomputedHeighttonode.height(no rAF), only when it differs from the explicitgetNode(id)?.height, then callsupdateNodeInternals. Comparing against the explicit height (not the measured prop) means a lagging ResizeObserver measurement can't retrigger the write. Convergent and idempotent.node.heightauthoritative fixes the downstream model: measured height, edge anchoring, fit-view, selection bounds, and handle spacing all agree with the rendered body (fixes handles overflowing on high-fan-out nodes).animatedViewportManagernow readsnode.measured?.height ?? node.height ?? …, matching the package's dominant convention (previouslynode.height || 100, which went stale when height wasn't written).baseHeightRef/syncedHeightRefbookkeeping and the deadgetContainerHeightheight param.Flow
flowchart TD A[handleConfigurations / footer] --> B[computedHeight - pure] B --> C{getNode.height != computedHeight?} C -- yes --> D[updateNode height + updateNodeInternals] C -- no --> E[no-op: converged] D --> F[node.height authoritative] F --> G[measured height / edges / handle spacing agree]Testing
pnpm lint(Biome) passespnpm typecheckpassespnpm test:visual— runs in CIas any/ type suppressions added