Conversation
WalkthroughA new Changes
Sequence DiagramsequenceDiagram
participant Render as Bar Render
participant ComputeBar as computeBarRectangles
participant Scale as numericAxis.scale
participant Interp as Bar Animation<br/>(interpolation)
participant Output as Animated Bar
Render->>ComputeBar: Calculate bar rectangles<br/>with baseValue
ComputeBar->>Scale: scale(baseValue)
Scale-->>ComputeBar: stackedBarStart coordinate
ComputeBar->>ComputeBar: Assign stackedBarStart<br/>to each BarRectangleItem
ComputeBar-->>Render: BarRectangleItem with<br/>stackedBarStart
Render->>Interp: Interpolate entry.height,<br/>entry.stackedBarStart
rect rgba(76, 175, 80, 0.1)
note over Interp: New: y/x adjusted via<br/>stackedBarStart interpolation
end
Interp-->>Output: Coordinated stack<br/>animation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/cartesian/Bar.tsx(4 hunks)test/cartesian/Bar.spec.tsx(5 hunks)test/cartesian/Bar.truncateByDomain.spec.tsx(2 hunks)test/chart/BarChart.spec.tsx(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not hardcode any user-facing strings or formatting choices in the library code; leave internationalization to library users
Files:
src/cartesian/Bar.tsx
🧠 Learnings (1)
📚 Learning: 2025-10-25T07:35:46.188Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.188Z
Learning: Applies to test/**/*.test.{ts,tsx} : Mock getBoundingClientRect using test/helper/MockGetBoundingClientRect.ts when rendering Recharts components (e.g., Tooltip, Legend, charts)
Applied to files:
test/cartesian/Bar.spec.tsxtest/cartesian/Bar.truncateByDomain.spec.tsxtest/chart/BarChart.spec.tsx
🧬 Code graph analysis (1)
src/cartesian/Bar.tsx (1)
src/util/DataUtils.ts (1)
interpolate(105-110)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build, Test, Pack
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
src/cartesian/Bar.tsx (4)
569-575: Animation logic correctly implements the fix for stacked bars.For horizontal bars, the animation now:
- Interpolates
yfromstackedBarStart(base position) toentry.y(final position)- Interpolates
heightfrom 0 toentry.heightThis makes stacked bar segments appear to grow from a common baseline rather than all growing simultaneously from their individual positions, addressing the issue described in #523.
580-582: Vertical layout animation mirrors horizontal logic correctly.The vertical animation interpolates
xfromstackedBarStarttoentry.xwhile width grows from 0 toentry.width, maintaining consistency with the horizontal layout approach.
839-839: stackedBarStart properly propagated to all bar items.The computed
stackedBarStartis included in everyBarRectangleItem, ensuring it's available for animation, tooltips, and event handlers.
106-109: No public API breaking change — BarRectangleItem is constructed only internally.Search results confirm that
BarRectangleItemis never manually constructed by external code. The type is only used in type annotations for callback parameters and return types. The only place whereBarRectangleItemobjects are created is insidecomputeBarRectangles()(lines 837–850), wherestackedBarStartis already properly assigned. This change is safe and poses no burden on library consumers.test/cartesian/Bar.truncateByDomain.spec.tsx (1)
108-108: Test expectations correctly updated for new stackedBarStart field.The expected
stackedBarStart: 275value is added to both bar data objects in the stacked bar test. This value represents the chart coordinate of the baseValue for this specific test configuration.Also applies to: 144-144
test/chart/BarChart.spec.tsx (3)
796-796: Test expectations correctly include stackedBarStart for stacked bars.The tests now expect
stackedBarStart: 45for stacked bar scenarios. All bars in the same stack share the samestackedBarStartvalue, which is correct - they all animate from the common base of the stack.Also applies to: 829-829, 862-862, 895-895
856-856: Generic number expectation appropriate for background props test.Using
expect.any(Number)is appropriate here since the background component test focuses on prop passing behavior rather than specific coordinate values.
2471-2471: Boxplot/whisker test expectations updated consistently.The
stackedBarStart: 165value is correctly added to all three expected bar items in the boxplot simulation test, reflecting the base coordinate for this multi-bar stacked scenario.Also applies to: 2511-2511, 2551-2551
test/cartesian/Bar.spec.tsx (2)
856-856: Background component receives stackedBarStart in props.The test correctly expects
stackedBarStart: expect.any(Number)to be passed to custom background components, ensuring consistency across all bar-related components.
1788-1788: Event handlers receive stackedBarStart in bar data payload.All bar event handlers (onClick, onMouseOver, onMouseOut, onTouchMove) now include
stackedBarStart: 195in their payload. This confirms the field is part of the public API exposed to users, who may rely on it for custom animations or visual effects.Also applies to: 1842-1842, 1883-1883, 1936-1936
| // @ts-expect-error this assumes that the domain is always numeric, but doesn't check for it | ||
| const stackedDomain: ReadonlyArray<number> = stackedData ? numericAxis.scale.domain() : null; | ||
| const baseValue = getBaseValueOfBar({ numericAxis }); | ||
| const stackedBarStart: number = numericAxis.scale(baseValue); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify scale function never returns null or undefined for baseValue.
The numericAxis.scale(baseValue) result is assigned to a number-typed variable without null checking. While the interpolate function handles non-number start values gracefully (returning the end value), this could silently skip the animation improvement if the scale returns null/undefined.
🏁 Script executed:
#!/bin/bash
# Check if scale functions are expected to return nullable values
rg -n -B5 -A5 'numericAxis\.scale' --type=ts --type=tsx | head -50Length of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for numericAxis.scale usage with correct ripgrep syntax
rg -n -B5 -A5 'numericAxis\.scale' --type=typescript
Length of output: 34
🏁 Script executed:
#!/bin/bash
# View the Bar.tsx file around line 771 for context
fd 'Bar\.tsx' --type f -x cat {} | sed -n '765,780p'Length of output: 820
🏁 Script executed:
#!/bin/bash
# Search for scale method/function definitions to understand return type
rg -n 'scale\s*\(' --type=typescript -A3 | grep -E '(scale|return|:)' | head -40Length of output: 34
🏁 Script executed:
#!/bin/bash
# Look for Scale type or interface definitions
rg -n -B2 -A5 'interface.*Scale|type.*Scale|\.scale\s*:' --type=typescript | head -60Length of output: 34
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for numericAxis.scale without type filtering
rg -n 'numericAxis\.scale' | head -30
Length of output: 631
🏁 Script executed:
#!/bin/bash
# Find the function containing line 771 - search backwards for function start
fd 'Bar\.tsx' --type f -x cat {} | sed -n '730,775p'Length of output: 2106
🏁 Script executed:
#!/bin/bash
# Search for scale method in axis-related files
rg -n '\.scale\s*[:=].*function|scale.*:.*function' -A5 | head -60Length of output: 571
🏁 Script executed:
#!/bin/bash
# Look for d3-scale or scale-related type imports
rg -n 'import.*scale|from.*scale|Scale' src/cartesian/Bar.tsxLength of output: 569
---
🏁 Script executed:
```shell
#!/bin/bash
# Find BaseAxisWithScale type definition
rg -n -B3 -A10 'type BaseAxisWithScale|interface BaseAxisWithScale'
Length of output: 971
🏁 Script executed:
#!/bin/bash
# Look at axisSelectors to understand the scale property type
rg -n -B5 -A15 'BaseAxisWithScale' src/state/selectors/axisSelectors.ts | head -80Length of output: 803
🏁 Script executed:
#!/bin/bash
# Check the actual usage patterns around line 791 and 800 to see how scale results are handled
fd 'Bar\.tsx' --type f -x cat {} | sed -n '785,825p'Length of output: 1587
---
🏁 Script executed:
```shell
#!/bin/bash
# Find RechartsScale type definition
rg -n -B2 -A8 'type RechartsScale|interface RechartsScale' src/state/selectors/axisSelectors.ts
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for the interpolate function mentioned in the review comment
rg -n -B3 -A10 'function interpolate|const interpolate' src/cartesian/Bar.tsxLength of output: 0
🏁 Script executed:
#!/bin/bash
# Look at context around line 769-771 to see the full scope of baseValue handling
fd 'Bar\.tsx' --type f -x cat {} | sed -n '760,805p'Length of output: 1125
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for RechartsScale type across all files
rg -n 'type RechartsScale|interface RechartsScale'
Length of output: 59
🏁 Script executed:
#!/bin/bash
# Get the exact line 771 from Bar.tsx with surrounding context
grep -n 'const stackedBarStart: number = numericAxis.scale' src/cartesian/Bar.tsx -B10 -A10Length of output: 1103
🏁 Script executed:
#!/bin/bash
# Search for RechartsScale to understand the scale function's return type
rg -n 'RechartsScale' src/state/selectors/axisSelectors.ts | head -20Length of output: 543
🏁 Script executed:
#!/bin/bash
# Search for the interpolate function mentioned in the review comment
rg -n 'interpolate' src/cartesian/Bar.tsx -B2 -A5Length of output: 1334
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the full RechartsScale interface definition
sed -n '152,200p' src/util/ChartUtils.ts
Length of output: 1433
🏁 Script executed:
#!/bin/bash
# Check the interpolate function signature to see how it handles non-number values
rg -n -B2 -A8 'function interpolate|export.*interpolate' src/util/DataUtils.tsLength of output: 555
Add null checking for stackedBarStart assignment at line 771, or verify that baseValue always produces a defined scale result.
The code assigns numericAxis.scale(baseValue) to a number-typed variable without null checking. However, evidence from the codebase shows scale() can return undefined:
- Line 800 uses nullish coalescing:
currentValueScale ?? baseValueScale ?? undefined - The RechartsScale interface's return type signature appears incomplete
If scale returns undefined, stackedBarStart becomes undefined. When passed to interpolate() as the start parameter (lines 570, 580), this causes it to silently return the end value, potentially skipping the animation improvement.
Either: (1) add null checking similar to line 800, or (2) verify and document why baseValue guarantees a defined scale result.
🤖 Prompt for AI Agents
In src/cartesian/Bar.tsx around line 771, the assignment const stackedBarStart:
number = numericAxis.scale(baseValue); assumes scale() returns a number but
scale can be undefined; change this to guard against undefined like other places
(e.g., use nullish coalescing or a fallback) or assert/ensure baseValue yields a
defined scale. Specifically, replace the direct assignment with a nullable typed
value or use numericAxis.scale(baseValue) ?? fallbackValue (or propagate
undefined and handle it before calling interpolate), and update the variable
type accordingly so subsequent calls to interpolate() receive a defined start or
are skipped/handled when undefined.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6593 +/- ##
=======================================
Coverage 94.57% 94.57%
=======================================
Files 491 491
Lines 40687 40691 +4
Branches 4768 4768
=======================================
+ Hits 38481 38485 +4
Misses 2201 2201
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will increase total bundle size by 486 bytes (0.02%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-cjsAssets Changed:
view changes for bundle: recharts/bundle-es6Assets Changed:
view changes for bundle: recharts/bundle-umdAssets Changed:
|
Description
Fix initial animation of stacked Bars.
At first I have considered rendering them at full height which would also solve this problem but that turned out to be much larger change than just animating the x/y position too.
Related Issue
Fixes #523
Screenshots (if appropriate):
Before
Screen.Recording.2025-11-10.at.11.16.10.mov
Screen.Recording.2025-11-10.at.11.17.00.mov
After
Screen.Recording.2025-11-10.at.11.12.20.mov
Screen.Recording.2025-11-10.at.11.17.23.mov
Types of changes
Checklist:
Summary by CodeRabbit
New Features
stackedBarStartproperty to bar chart data structures, exposing the base coordinate for stacked bars.Tests
stackedBarStartproperty is correctly included in bar chart payloads and event handlers across various stacking scenarios.