Conversation
WalkthroughIntroduces generic type parameters ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6993 +/- ##
=======================================
Coverage 90.10% 90.10%
=======================================
Files 522 522
Lines 38848 38850 +2
Branches 5347 5347
=======================================
+ Hits 35004 35006 +2
Misses 3835 3835
Partials 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cartesian/Area.tsx (1)
512-607:⚠️ Potential issue | 🟡 MinorHandle decimal
strokeWidthvalues without truncation.The current implementation uses
parseInt, which truncates fractional string values (e.g.,"1.5"→1). SincestrokeWidthacceptsstring | numberand decimal values likestrokeWidth={1.5}appear in the codebase, this causes incorrect clip rect sizing. Replace withNumber.parseFloatand a finite check as proposed.Proposed fix
function VerticalRect({ alpha, baseLine, points, strokeWidth, }: { alpha: number; points: ReadonlyArray<AreaPointItem>; baseLine: BaseLineType | undefined; strokeWidth: string | number | undefined; }) { + const strokeWidthValue = Number.parseFloat(`${strokeWidth}`); + const safeStrokeWidth = Number.isFinite(strokeWidthValue) ? strokeWidthValue : 1; const startY = points[0]?.y; const endY = points[points.length - 1]?.y; if (!isWellBehavedNumber(startY) || !isWellBehavedNumber(endY)) { return null; } const height = alpha * Math.abs(startY - endY); let maxX = Math.max(...points.map(entry => entry.x || 0)); @@ <rect x={0} y={startY < endY ? startY : startY - height} - width={maxX + (strokeWidth ? parseInt(`${strokeWidth}`, 10) : 1)} + width={maxX + safeStrokeWidth} height={Math.floor(height)} /> ); } @@ function HorizontalRect({ alpha, baseLine, points, strokeWidth, }: { alpha: number; points: ReadonlyArray<AreaPointItem>; baseLine: BaseLineType | undefined; strokeWidth: string | number | undefined; }) { + const strokeWidthValue = Number.parseFloat(`${strokeWidth}`); + const safeStrokeWidth = Number.isFinite(strokeWidthValue) ? strokeWidthValue : 1; const startX = points[0]?.x; const endX = points[points.length - 1]?.x; if (!isWellBehavedNumber(startX) || !isWellBehavedNumber(endX)) { return null; } const width = alpha * Math.abs(startX - endX); let maxY = Math.max(...points.map(entry => entry.y || 0)); @@ <rect x={startX < endX ? startX : startX - width} y={0} width={width} - height={Math.floor(maxY + (strokeWidth ? parseInt(`${strokeWidth}`, 10) : 1))} + height={Math.floor(maxY + safeStrokeWidth)} /> ); }
🤖 Fix all issues with AI agents
In `@src/cartesian/Area.tsx`:
- Around line 1140-1143: Remove the `any` defaults and the `as` assertion on the
exported Area component: replace the current "export const Area =
React.memo(AreaFn, propsAreEqual) as <DataPointType = any, ValueAxisType =
any>(...)" pattern by giving Area an explicit generic type annotation (no `any`
defaults) and assign the memoized value to that typed const (e.g. declare export
const Area: <DataPointType, ValueAxisType>(props: Props<DataPointType,
ValueAxisType>) => ReactElement = React.memo(AreaFn, propsAreEqual)); keep the
existing AreaFn and propsAreEqual references and preserve the displayName
comment.
| export const Area = React.memo(AreaFn, propsAreEqual) as <DataPointType = any, ValueAxisType = any>( | ||
| props: Props<DataPointType, ValueAxisType>, | ||
| ) => ReactElement; | ||
| // @ts-expect-error we need to set the displayName for debugging purposes |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the Area.tsx file around the flagged lines
head -n 1150 src/cartesian/Area.tsx | tail -n 20Repository: recharts/recharts
Length of output: 637
🏁 Script executed:
# Let's search for the AreaFn definition and understand the component structure
rg "const AreaFn|function AreaFn" src/cartesian/Area.tsx -A 5 -B 5Repository: recharts/recharts
Length of output: 481
🏁 Script executed:
# Check if forwardRef is used in this file
rg "forwardRef" src/cartesian/Area.tsx -B 5 -A 5Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Let's also check other chart components to see if they use the same pattern
rg "export const.*React.memo.*as <" src/cartesian/ -A 3Repository: recharts/recharts
Length of output: 392
🏁 Script executed:
# Check AreaChart to see the forwardRef pattern mentioned in learnings
head -n 50 src/chart/AreaChart.tsx | tail -n 30Repository: recharts/recharts
Length of output: 1057
🏁 Script executed:
# Search for the export of AreaChart to see the pattern
rg "export const AreaChart" src/chart/AreaChart.tsx -A 3Repository: recharts/recharts
Length of output: 227
🏁 Script executed:
# Search for other exported memoized components with explicit type annotations
rg "export const.*React.memo.*:" src/ -A 3 | head -50Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Check the current TypeScript version in the project
cat tsconfig.json | grep -A 2 "compilerOptions"Repository: recharts/recharts
Length of output: 256
🏁 Script executed:
# Search for memoized components to understand the pattern better
rg "React.memo" src/ -A 5 | grep -A 5 "export const" | head -60Repository: recharts/recharts
Length of output: 2201
🏁 Script executed:
# Check package.json for TypeScript version
grep -A 2 '"typescript"' package.jsonRepository: recharts/recharts
Length of output: 164
🏁 Script executed:
# Check the Props type definition in Area.tsx to understand the generic structure
rg "type Props|interface Props" src/cartesian/Area.tsx -B 5 -A 10 | head -40Repository: recharts/recharts
Length of output: 796
🏁 Script executed:
# Compare with another component like Scatter to see their Props definition
rg "type Props|interface Props" src/cartesian/Scatter.tsx -B 5 -A 10 | head -40Repository: recharts/recharts
Length of output: 546
🏁 Script executed:
# Search for other generic memoized components with explicit type annotations
rg "React.memo.*\)" src/ -B 2 | grep -B 10 "as <" | head -80Repository: recharts/recharts
Length of output: 608
🏁 Script executed:
# Look for any other components using generic type parameters with explicit annotations
rg "export const.*ComponentType<.*<" src/ -A 2Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Check if Area needs to be generic or if it can be simplified like Scatter
rg "AreaFn<" src/cartesian/Area.tsx -B 5 -A 10Repository: recharts/recharts
Length of output: 669
🏁 Script executed:
# Check if any component uses explicit generic type annotation on React.memo export
rg "export const.*React.memo.*<.*>" src/ -A 3 | head -50Repository: recharts/recharts
Length of output: 392
🏁 Script executed:
# Let's verify if the proposed syntax would work by checking TypeScript docs examples
# For now, let's see if there are any comments explaining why Area uses this pattern
rg -B 10 "export const Area = React.memo" src/cartesian/Area.tsx | head -30Repository: recharts/recharts
Length of output: 294
🏁 Script executed:
# Final check: look for any explicit generic function type annotations in the codebase to validate the proposed syntax
rg ": <.*>(.*) => " src/ | head -20Repository: recharts/recharts
Length of output: 43
Remove the any defaults and the as assertion on the exported Area component.
The any defaults and as assertion violate the no-any and no-as coding guidelines. The documented exception for as assertions applies only to generic forwardRef chart components (like AreaChart), not to memoized components. Use an explicit generic type annotation instead:
-export const Area = React.memo(AreaFn, propsAreEqual) as <DataPointType = any, ValueAxisType = any>(
- props: Props<DataPointType, ValueAxisType>,
-) => ReactElement;
+export const Area: <DataPointType = unknown, ValueAxisType = unknown>(
+ props: Props<DataPointType, ValueAxisType>,
+) => ReactElement = React.memo(AreaFn, propsAreEqual);🤖 Prompt for AI Agents
In `@src/cartesian/Area.tsx` around lines 1140 - 1143, Remove the `any` defaults
and the `as` assertion on the exported Area component: replace the current
"export const Area = React.memo(AreaFn, propsAreEqual) as <DataPointType = any,
ValueAxisType = any>(...)" pattern by giving Area an explicit generic type
annotation (no `any` defaults) and assign the memoized value to that typed const
(e.g. declare export const Area: <DataPointType, ValueAxisType>(props:
Props<DataPointType, ValueAxisType>) => ReactElement = React.memo(AreaFn,
propsAreEqual)); keep the existing AreaFn and propsAreEqual references and
preserve the displayName comment.
Bundle ReportChanges will decrease total bundle size by 106 bytes (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-es6Assets Changed:
view changes for bundle: recharts/bundle-umdAssets Changed:
view changes for bundle: recharts/bundle-cjsAssets Changed:
|
|
Staging Deployment Details
These deployments will remain available for 30 days. To update snapshots: Comment |
## Description Same as #6993 but for Bar <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Bar component TypeScript typing enhanced with improved generic type parameter support, providing better type inference and validation. * **Tests** * Added extensive type safety test coverage for the Bar component across multiple data typing scenarios to ensure compile-time correctness. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Use the new generics to allow strong typing for Area. Other graphical items are coming in separate PR.
Later I will also add a new guide on how to use these.
Related Issue
#6645
Types of changes
Checklist:
Summary by CodeRabbit
Refactor
Tests