chore(charts): upgrade Recharts from 2.13 to 3.x#2610
Open
brandon-pereira wants to merge 13 commits into
Open
Conversation
Recharts 3 reworked its internal state management, which removes the CategoricalChartState fields (chartX/chartY/activePayload) from chart event handlers and the imperative chart.setState escape hatch. Update the chart components to the new API while preserving existing behavior: - HDXMultiSeriesTimeChart: derive click/drag position from activeCoordinate and rebuild the drill-down per-series payload from graphResults, keeping zoom-brush selection and the click drill-down popover working. - DBHistogramChart: replace the chart.setState tooltip-pin hack with the controlled active/defaultIndex Tooltip props. - PropertyComparisonChart: update custom tooltip/shape typings to the renamed TooltipContentProps and select the clicked bar via activeLabel. - app.scss: hide the browser focus ring that Recharts 3's default accessibilityLayer paints when a chart is clicked, keeping charts keyboard-focusable.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 0bdf0f0 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 |
github-actions
Bot
added
the
review/tier-2
Contributor
🟡 Tier 3 — StandardIntroduces new logic, modifies core functionality, or touches areas with non-trivial risk. Why this tier:
Review process: Full human review — logic, architecture, edge cases. Stats
|
Contributor
E2E Test Results✅ All tests passed • 236 passed • 3 skipped • 1531s
Tests ran across 4 shards in parallel. |
Contributor
Greptile SummaryThis PR upgrades Recharts from
Confidence Score: 5/5Safe to merge — all behavioral changes preserve existing user-facing functionality and the key edge cases (frozen Immer arrays, mixed coordinate spaces, synthetic post-zoom click, stale positional pin) are handled and unit-tested. The upgrade adapts three complex chart components to Recharts 3's new API without introducing regressions. Each changed interaction path (drag-to-zoom, click drill-down, histogram pin, bar selection) has either been made more correct than the v2 version or is covered by new unit tests. No logic gaps were found. No files require special attention. The most involved file is HDXMultiSeriesTimeChart.tsx, but its changes are well-commented and covered by tests. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
MD[onMouseDown\nrecord containerRef-relative X\n+ activeLabel as highlightStart] --> MM
MM[onMouseMove\nupdate highlightEnd\nclear isClickActive] --> MU
MU[onMouseUp\nmeasure drag distance\nusing same containerRef-relative X] --> DragCheck
DragCheck{drag >\nMIN_DRAG_DISTANCE?}
DragCheck -- Yes --> Zoom[call onTimeRangeSelect\nset suppressNextClickRef=true\nset justZoomedRef=true]
DragCheck -- No,\nsame label --> ClearHL[clear highlightStart/End\nno zoom]
DragCheck -- No,\ndifferent label --> ClearHL2[clear highlight\nno zoom]
Zoom --> SyntheticClick
SyntheticClick[onClick fires\nsuppressNextClickRef?] -- true --> Swallow[set false\nstopPropagation\nreturn]
SyntheticClick -- false --> ClickGuard
ClearHL --> ClickGuard
ClearHL2 --> ClickGuard
ClickGuard{activeCoordinate\n+ activeLabel\n+ highlightStart==null?}
ClickGuard -- No --> ClearClick[setIsClickActive undefined]
ClickGuard -- Yes --> BuildPayload[graphResults.find by timestampKey\nbuildActiveClickSeries from visibleLineData\nsetIsClickActive with x/y/xPerc/yPerc/activePayload]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
MD[onMouseDown\nrecord containerRef-relative X\n+ activeLabel as highlightStart] --> MM
MM[onMouseMove\nupdate highlightEnd\nclear isClickActive] --> MU
MU[onMouseUp\nmeasure drag distance\nusing same containerRef-relative X] --> DragCheck
DragCheck{drag >\nMIN_DRAG_DISTANCE?}
DragCheck -- Yes --> Zoom[call onTimeRangeSelect\nset suppressNextClickRef=true\nset justZoomedRef=true]
DragCheck -- No,\nsame label --> ClearHL[clear highlightStart/End\nno zoom]
DragCheck -- No,\ndifferent label --> ClearHL2[clear highlight\nno zoom]
Zoom --> SyntheticClick
SyntheticClick[onClick fires\nsuppressNextClickRef?] -- true --> Swallow[set false\nstopPropagation\nreturn]
SyntheticClick -- false --> ClickGuard
ClearHL --> ClickGuard
ClearHL2 --> ClickGuard
ClickGuard{activeCoordinate\n+ activeLabel\n+ highlightStart==null?}
ClickGuard -- No --> ClearClick[setIsClickActive undefined]
ClickGuard -- Yes --> BuildPayload[graphResults.find by timestampKey\nbuildActiveClickSeries from visibleLineData\nsetIsClickActive with x/y/xPerc/yPerc/activePayload]
Reviews (12): Last reviewed commit: "Merge branch 'main' into brandon/upgrade..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Deep Review✅ No critical issues found. This is a well-executed major-version upgrade: event handlers are cleanly migrated off the removed 🟡 P2 -- recommended
🔵 P3 nitpicks (3)
Reviewers (10): correctness, testing, maintainability, project-standards, kieran-typescript, julik-frontend-races, adversarial, api-contract, agent-native, learnings-researcher. Testing gaps:
|
- DBHistogramChart: lock the pinned tooltip to the clicked bar by adding trigger: 'click' alongside active/defaultIndex, so hovering another bar no longer swaps the tooltip's data and View Events link. - HDXMultiSeriesTimeChart: measure zoom drag distance consistently from the native event's offsetX on both mouse down and up, avoiding a coordinate- space mismatch with activeCoordinate that could turn a click into a zoom. - app.scss: scope the chart focus-ring suppression to the surface and its descendants, leaving the wrapper's keyboard focus ring intact (WCAG 2.4.11). - Sparkline: type the height prop as ResponsiveContainer expects.
- HDXMultiSeriesTimeChart: rebuild the click drill-down payload from the same visible series set used to draw the lines (legend selection + HARD_LINES_LIMIT), so the popover never lists deselected or over-limit series. Extracts getVisibleLineData / getSeriesDisplayName helpers reused by the lines, payload, and (indirectly) the y-axis domain. - HDXMultiSeriesTimeChart: measure zoom drag distance from activeCoordinate.x on both mouse down and up (offsetX is relative to whichever child SVG element is under the pointer, so it mismatched across bars/paths). - HDXMultiSeriesTimeChart: export a shared ActiveClickPayload type and use it for isClickActive/setIsClickActive instead of any; DBTimeChart imports it. - DBHistogramChart: ignore a non-integer activeIndex so a click resolving to no bar can't leave a stuck pin. - app.scss: suppress the chart focus ring only for pointer focus (:focus:not(:focus-visible)), preserving the keyboard focus indicator (WCAG 2.4.11). Verified in a browser that click shows no ring. - Add getVisibleLineData unit tests (selection filtering + line limit).
github-actions
Bot
added
review/tier-3
Knip flagged it as an unused export; it's only referenced by the exported ActiveClickPayload, so keep it module-private.
…, nits) - Zoom-drag distance is measured from a container-relative pointer X (clientX minus the chart container's bounding rect) captured on both mouse down and up, so it has one stable origin and is never dropped when the pointer maps to no data point. - Histogram: reset the pinned tooltip when the buckets change (a background refetch would otherwise repoint the positional pin to the wrong time range) and remount the Tooltip on pin change so defaultIndex re-seeds. Ignore a non-integer activeIndex. - Extract buildActiveClickSeries (pure) for the drill-down payload rebuild and getActiveLabel to normalize the event label; add unit tests for the payload builder and the visible-series set. - Nits: numeric ReferenceLine x on the time axis, type-guard narrowing instead of an as-number cast, type the comparison-chart click arg, drop an unreachable String(... ?? '').
brandon-pereira
requested review from
a team and
bot-hyperdx
and removed request for
a team and
bot-hyperdx
brandon-pereira
requested review from
a team and
jordan-simonovski
and removed request for
a team
- Debounce the chart ResponsiveContainer resize observer (50ms) so a resize -> re-render -> resize cycle can't keep the chart (and the tile editor form around it) from settling. - Histogram pin: only accept a real, in-range integer bar index; null/''/-1 clear the pin instead of defaulting to bar 0. - Skip the synthetic click that ends a brush-to-zoom so it can't freeze a drill-down tooltip with stale pre-zoom data. - Order the zoom range numerically (labels are epoch-second strings; a lexicographic compare misorders differing digit lengths). - Doc fix: getVisibleLineData no longer claims to back the y-axis domain.
Recharts 3 keeps chart data, payloads, and graphical-item element props in an Immer-backed store and freezes them. Three spots mutated those frozen objects, which threw "This object has been frozen" and crashed the chart in production builds (dev builds masked it), destabilizing the tile editor and failing the dashboard/alert E2E suites: - Tooltip and legend sorted the Recharts payload in place -> copy first ([...payload].sort()). - The active-dot element received the capture Map as a prop, so Recharts froze it and dot.set() threw -> pass a stable onCapture callback that writes to the closed-over ref instead. Verified in production E2E mode: dashboard.spec (30 passed) and alerts.spec (15 passed) are green; the previously deterministic failures now pass.
- P1: only set justZoomedRef when onTimeRangeSelect is defined. On charts without it (side panels, alert preview, infra panels) a >=20px drag changed no date range, so the [dateRange] effect never reset the flag and the post-zoom onClick guard then suppressed every later click, permanently disabling the drill-down tooltip. - ReferenceArea drag band now uses numeric x1/x2 on the numeric time axis, matching the click-marker ReferenceLine. - Extract resolvePinnedBarIndex (histogram pin) and resolveComparisonClick (comparison bar selection) as pure exported helpers with unit tests (numeric/string/NaN/negative/other-bucket/toggle paths).
…oom click The onClick guard that skips the synthetic click after a brush-zoom relied on the [dateRange] effect to clear justZoomedRef, but that effect early-returns without clearing when the post-zoom range is value-equal to the current one (e.g. zooming at the min-granularity floor, or a parent that clamps to the same range) — leaving the flag stuck and swallowing every later drill-down click. Split the two concerns into separate refs: - suppressNextClickRef: set on a completed zoom, consumed and cleared by the next onClick itself, so it can never leak regardless of the range change. - justZoomedRef: still tells the [dateRange] effect to preserve zoomOrigin, and is now cleared even on the value-equal (no-change) path so it can't leak into a later unrelated range change. Verified drag-to-zoom still passes in production E2E.
…rts-3 # Conflicts: # packages/app/src/components/DBTimeChart.tsx
Recharts 3 hoists axis tick labels into a dedicated `.recharts-<axis>-tick-labels` z-index layer, so they are no longer nested inside `.recharts-xAxis`. Update getBarLabels() to scope to the new x-axis tick-labels group, fixing the timeout in the categorical bar chart ORDER BY E2E test.
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
1 participant
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.
Why
Bumps
rechartsfrom2.13.3to3.x(^3.2.1). Recharts 3 is a major release that rewrote the library's internal state management. This upgrade keeps us on a maintained major and unblocks features that only exist in 3.x (portals, auto-width axes, per-axis tooltips), while preserving all existing chart behavior.This branch is intentionally scoped to only the dependency upgrade. (The unrelated drill-down menu work that previously shared this worktree has been moved to its own branch.)
Recharts v3 Migration Guide
What changed
Recharts 3 removed
CategoricalChartState(thechartX/chartY/activePayloadfields) from chart event handlers, and removed the imperativechart.setStateescape hatch. The chart components were updated to the new API with behavior preserved:HDXMultiSeriesTimeChart— event handlers now derive the click/drag position fromactiveCoordinate(falling back to the native event'soffsetX) and rebuild the drill-down per-series payload fromgraphResults. Zoom-brush selection and the click drill-down popover are unchanged from a user's perspective.DBHistogramChart— thechart.setStatetooltip-pin hack is replaced with the controlledactive/defaultIndexTooltipprops. Click-to-pin, click-again-to-unpin, and Esc-to-unpin all still work.PropertyComparisonChart— custom tooltip/shape typings updated to the renamedTooltipContentProps; the clicked bar is now selected viaactiveLabel+ achartDatalookup.app.scss— hides the browser focus ring that Recharts 3's defaultaccessibilityLayerpaints when a chart is clicked. Charts remain keyboard-focusable; only the visual outline is suppressed.package.json/yarn.lock—recharts@^3.2.1andreact-is@^19.0.0(added to the app to satisfy recharts' peer requirement under React 19).Testing
I have done a lot of testing on this, but we have lots of charts and lots of edge cases. Please check charts generally, specific things that were refactored below
CaptureActiveDotnearest-series bolding depends on recharts' internal render order; it type-checks and works in dev but is worth an explicit look on a multi-series line chart.connectNullsnull handling changed in 3.x (nulls treated as 0 when enabled) — verify series with gaps render as before.