fix(memory-graph): stop hit-testing against stale nodes after resize or theme change#1284
Open
abhay-codes07 wants to merge 1 commit into
Open
Conversation
…ay regenerates
SpatialIndex.rebuild() early-returns on an unchanged content hash, and
the hash only covers node ids and rounded positions. useGraphData
produces a brand-new generation of node objects whenever its memo deps
change — including on container resize and light/dark theme switches —
with identical ids and coordinates. The GraphCanvas [nodes] effect then
called rebuild(), the hash matched, and the grid kept serving the
previous generation of objects while the renderer and nodeMap moved on.
The first drag after any resize or theme toggle therefore grabbed a
detached object: onMouseDown pinned fx/fy on a node the renderer no
longer draws, the visible node ignored the cursor, and the drag-start
reheat() let it drift freely. Hover and click were unaffected (they
only use node.id), which is what kept this hidden.
Give rebuild() a force parameter and pass it from the identity-keyed
effect, keeping the cheap hash guard for the per-frame rebuilds in the
rAF loop. Covered with SpatialIndex tests that pin both behaviours:
the unforced hash guard keeps the old generation (documenting why force
exists), and a forced rebuild swaps the grid to the new objects and
refreshes the hash.
Also remove the colors re-merge in MemoryGraph: useGraphTheme already
returns the overrides merged and referentially stable by value, and
re-merging keyed on the raw prop identity made `colors` a fresh object
every render for inline `colors={{...}}` consumers — rebuilding the
entire node array each render and multiplying how often the stale-index
window occurred.
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
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.
What
The first drag after any window resize or light/dark theme switch grabs a stale node object: the visible node ignores the cursor, and because drag-start reheats the simulation, it drifts off on its own.
Why:
SpatialIndex.rebuild()early-returns when its content hash is unchanged, and the hash only covers node ids and rounded positions.useGraphDataproduces a brand-new generation of node objects whenever its memo deps change — which includescanvasWidth/Height(resize) andcolors(theme switch) — with identical ids and coordinates. The[nodes]effect inGraphCanvascalledrebuild(), the hash matched, and the grid kept serving the previous generation while the renderer andnodeMapmoved on.InputHandler.onMouseDownthen pinsfx/fyon an object the renderer no longer draws. Hover and click still work (they only readnode.id), which is exactly what kept this hidden.Fix
rebuild()gains aforceparameter; the identity-keyed[nodes]effect passes it. The per-frame rebuilds in the rAF loop keep the cheap hash guard unchanged.colorsre-merge inMemoryGraph:useGraphThemealready returns the overrides merged and referentially stable by value, and the re-merge — keyed on the raw prop identity — madecolorsa fresh object on every render for inlinecolors={{...}}consumers. That rebuilt the whole node array each render and multiplied how often the stale-index window occurred.Testing
SpatialIndextests pin both sides: the unforced hash guard keeps the old generation for identical ids/positions (documenting exactly whyforceexists), a forced rebuild swaps the grid to the new objects, and a forced rebuild still refreshes the hash for subsequent unforced callsbun run testinpackages/memory-graph— 192 passbun run check-types— cleancc @MaheshtheDev