fix(heatmap): persist drag-select rectangle across re-renders#2189
fix(heatmap): persist drag-select rectangle across re-renders#2189alex-fedotyev wants to merge 5 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: c7ee6cd The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
review/tier-2
🔵 Tier 2 — Low RiskSmall, isolated change with no API route or data model modifications. Why this tier:
Review process: AI review + quick human skim (target: 5–15 min). Reviewer validates AI assessment and checks for domain-specific concerns. Stats
|
PR Reviewfix(heatmap): persist drag-select rectangle across re-renders — the root cause analysis is accurate and the memoization fix is correct.
No blocking issues beyond the missing browser smoke-test. If a reviewer can confirm the visual fix in-browser, this is good to merge. |
E2E Test Results✅ All tests passed • 158 passed • 3 skipped • 1137s
Tests ran across 4 shards in parallel. |
The dashed selection rectangle on the Event Deltas heatmap collapses to a 2x2 px box the instant the user releases the mouse. The filter is applied correctly (URL params, comparison legend, bar charts all update), but visually you can no longer see what region you picked. Root cause: uplot-react's dataMatch deep-compares the data prop and calls chart.setData(data, true) when refs differ. setData resets uPlot's internal u.select rectangle. HeatmapContainer rebuilt the [time, bucket, count] arrays in its render body, so on every parent re-render those entries had new references with identical values, and dataMatch returned false. Memoize the data array with stable refs so dataMatch returns true, setData is skipped, and u.select survives the re-render that follows a drag (the URL param write triggers). Also memoize generatedTsBuckets so its fresh-Date-array each render doesn't propagate into the heatmapData useMemo deps. HDX-4147
Lint feedback from CI: - timestampColumn?.name dep is wrong since the closure uses the full timestampColumn object; pass the object instead. The ref is stable when data is stable, so this doesn't reintroduce the original bug. - Stop destructuring bucket/count when only time is needed for the not-enough-data short-circuit. Add changeset (patch on @hyperdx/app).
Bot review feedback: timestampColumn was a non-primitive useMemo dep derived entirely from data?.meta. Move the call inside the useMemo so data is the only relevant dep, which is the cleanest way to satisfy both exhaustive-deps and the ref-stability the fix relies on. Also drop the // x values / // y value series 1 / // y value series 2 labels — they describe what the line is rather than why.
alex-fedotyev
force-pushed
the
alex/HDX-4147-heatmap-select-persist
branch
from
b7962df to
3d04cba
Compare
prose-lint flags U+2014 anywhere; tighten the comment instead.
Drop the dataMatch description (it was element-wise === per series, not top-level === as written) and consolidate to the actionable mechanism: fresh data ref drives uplot-react to call setData(data, true) which wipes u.select. Add HDX-4147 ticket reference.
1 participant
Summary
The dashed selection rectangle on the Event Deltas heatmap collapses to a 2x2 px box the instant the user releases the mouse. The filter itself is applied correctly (URL gets
xMin/xMax/yMin/yMax, the comparison legend shows "Selection" vs "Background", and the bar charts below split into two colors), but visually the user has no idea what slice of the heatmap they picked.Repro on play-clickstack: open the Demo Traces source, switch to the Event Deltas tab, drag any region. Inspect the
.u-selectelement after mouse-up: width=2 px, height=2 px, position pinned to top-left of the canvas.Same component is used in the Search heatmap (
DBSearchHeatmapChart), so this fix applies there too. Dashboard tile heatmaps don't passonFilter, so they're not affected.Linked: HDX-4147.
Root cause
HeatmapContainerrebuilt the[time, bucket, count]arrays in its render body each render. After a drag:setSelecthook firesonFilter(...)->setFields({xMin, xMax, yMin, yMax})(nuqs URL update)[time, bucket, count]arrays are constructed with identical values but fresh refsdataprop ref changes, souplot-react's data effect runs (propDataRef.current !== data)setData(data, true)path is taken and the renderedu.selectrectangle ends up wiped (the visible 2x2 px residue)Stable data refs short-circuit step 4 before any setData call, leaving
u.selectuntouched.Fix
Memoize the
[time, bucket, count]data array inHeatmapContainerso the inner references stay stable across re-renders that don't change the underlying data.Also memoized
generatedTsBuckets(it's a freshDate[]fromtimeBucketByGranularityeach render); without that, including it in the heatmapData useMemo deps would defeat the memoization.Test plan
.u-selectcollapse to 2x2 px)yarn workspace @hyperdx/app run tsc --noEmitpasses for the modified fileyarn workspace @hyperdx/app jest --testPathPatterns heatmappasses (1537 tests)