Add optional explicit generics to graphical items and charts#7035
Conversation
WalkthroughThis PR introduces generic type parameters with default values across 20+ chart, axis, and component files, removes omnidoc utility methods for property extraction, and adds comprehensive TypeScript typing tests to validate generic parameter usage and type safety. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cartesian/ReferenceLine.tsx (1)
40-49:⚠️ Potential issue | 🟠 Major
ReferenceLineSegmenttype is now inconsistent with the genericsegmentprop.The exported
ReferenceLineSegmenttype uses hardcodednumber | string, while thesegmentprop inReferenceLinePropsnow accepts genericXValueTypeandYValueType. This creates an API inconsistency: consumers using generic props can pass custom types insegment, but consumers using the exportedReferenceLineSegmenttype directly are locked tonumber | string. SinceReferenceLineSegmentis part of the public API (exported insrc/index.ts, used in state slices, and used in tests), this needs resolution—either make the type generic, or reconcile the prop definition to use the non-generic type consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cartesian/ReferenceLine.tsx` around lines 40 - 49, The exported ReferenceLineSegment type is fixed to number|string while ReferenceLineProps.segment is generic (XValueType/YValueType), causing a public API mismatch; make ReferenceLineSegment generic (e.g., ReferenceLineSegment<XValueType, YValueType>) and update all usages (exports, state slices, tests) to reference the generic form, or alternatively change ReferenceLineProps.segment to the non-generic number|string pair to match the exported type—ensure consistency between ReferenceLineSegment and the segment prop by aligning their type parameters (update ReferenceLineProps, any exports in src/index.ts, and tests/state slices that import ReferenceLineSegment or segment).
🧹 Nitpick comments (6)
src/cartesian/ReferenceDot.tsx (1)
23-23:ReferenceCoordinateValueis defined separately inReferenceArea.tsx,ReferenceLine.tsx, andReferenceDot.tsx.All three definitions are identical (
type ReferenceCoordinateValue = number | string;) and none are exported. Since the type is used in the exportedPropsconstraints of all three components, extracting it to a shared location (e.g.,src/util/types.ts) would eliminate duplication and allow consumers to reuse the type when creating wrappers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cartesian/ReferenceDot.tsx` at line 23, Duplicate type alias ReferenceCoordinateValue (number | string) is declared in ReferenceDot.tsx, ReferenceArea.tsx, and ReferenceLine.tsx; extract it into a single shared exported type (e.g., add it to src/util/types.ts) and update each file to import the exported ReferenceCoordinateValue and use it in their exported Props types; ensure the new type is exported so consumers can import it when building wrappers and update import statements in ReferenceDot, ReferenceArea, and ReferenceLine to reference the centralized symbol ReferenceCoordinateValue.test/cartesian/ReferenceDot.typed.spec.tsx (1)
49-71: Event handler test doesn't render — type-check only, but thereturnpattern is fragile.The test at line 50 returns JSX without rendering it via
rechartsTestRender. This works as a compile-time type check, but the test body is technically returning a truthy value (a React element) rather than asserting anything. If the types ever becomeanyby accident, the test would still pass silently.This is consistent with the pattern used across other
.typed.spec.tsxfiles in this PR, so it's not a blocker — just noting the limitation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/cartesian/ReferenceDot.typed.spec.tsx` around lines 49 - 71, The test currently returns a JSX element (LineChart with ReferenceDot) which only provides a compile-time type check; replace the fragile "return (<LineChart ... />)" pattern with an actual render call using rechartsTestRender so the test executes at runtime — e.g., call rechartsTestRender(<LineChart ...>) inside the it block (keeping the same props and handlers referencing getRelativeCoordinate) to ensure the event handler typing is exercised at runtime rather than merely returning a React element.src/cartesian/ReferenceArea.tsx (1)
25-26: ExtractReferenceCoordinateValueto a shared location to reduce duplication.This type alias is duplicated identically in
ReferenceLine.tsx(line 51),ReferenceDot.tsx(line 23), andReferenceArea.tsx(line 25). Moving it to a shared types file would eliminate redundancy and centralize this common constraint used across all three Reference* components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cartesian/ReferenceArea.tsx` around lines 25 - 26, Extract the duplicated type alias ReferenceCoordinateValue into a shared types module (e.g., create and export it from a new/shared file) and replace the local declarations in ReferenceLine.tsx (the type at line ~51), ReferenceDot.tsx (line ~23), and ReferenceArea.tsx (line ~25) with an import of that exported type; ensure each file imports the shared ReferenceCoordinateValue and remove the local alias to eliminate duplication and centralize the constraint used by the Reference* components.test/chart/LineChart.typed.spec.tsx (1)
2-2: Missingexpectimport from vitest.Line 36 uses
expect(invalidChart).toBeDefined()butexpectis not imported on line 2. While vitest globals may cover this, other typed test files in this PR (e.g.,ReferenceLine.typed.spec.tsx) explicitly importexpectfrom vitest. Consider adding it for consistency.Suggested fix
-import { describe, it } from 'vitest'; +import { describe, expect, it } from 'vitest';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/chart/LineChart.typed.spec.tsx` at line 2, The test file imports only describe and it from vitest but uses expect (e.g., expect(invalidChart).toBeDefined() in the test referencing invalidChart), so update the import on the top of the file to include expect from 'vitest' (add expect to the existing import list) to match other typed tests like ReferenceLine.typed.spec.tsx and ensure the typed test compiles consistently.test/chart/PieChart.typed.spec.tsx (1)
2-2: Missingexpectimport from vitest.Same issue as
LineChart.typed.spec.tsx—expectis used on line 36 but not imported. Other files in this PR (e.g.,ReferenceLine.typed.spec.tsx) do import it.Suggested fix
-import { describe, it } from 'vitest'; +import { describe, expect, it } from 'vitest';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/chart/PieChart.typed.spec.tsx` at line 2, The test file imports only describe and it from vitest but uses expect (missing import), so update the import statement in PieChart.typed.spec.tsx to include expect (e.g., change the import that currently lists describe and it to also list expect); check other test files like LineChart.typed.spec.tsx for the same pattern and make the imports consistent with ReferenceLine.typed.spec.tsx by adding expect to the named imports from 'vitest'.src/polar/Pie.tsx (1)
1040-1086: Consider using a directly generic function likeReferenceLineinstead of theascast pattern.
ReferenceLinein this same PR uses a directly generic function signature (line 386), avoiding theasassertion entirely.Piecould potentially follow the same pattern:export function Pie<DataPointType = any, DataValueType = any>( outsideProps: Props<DataPointType, DataValueType>, ) { const props: PropsWithResolvedDefaults = resolveDefaultProps(outsideProps, defaultPieProps); // ... existing body }This would eliminate the
ascast and the@ts-expect-errorfordisplayName. That said, the current pattern is consistent with other components in the codebase that use the overloaded-cast approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/polar/Pie.tsx` around lines 1040 - 1086, The Pie component is exported via a casted constant Pie = PieFn as { ... } which forces a type assertion and requires a `@ts-expect-error` to set Pie.displayName; replace this with a directly generic function export like ReferenceLine does: turn PieFn into an exported generic function export function Pie<DataPointType = any, DataValueType = any>(outsideProps: Props<DataPointType, DataValueType>) { ... } (keep the existing body that calls resolveDefaultProps(outsideProps, defaultPieProps), RegisterGraphicalItemId, SetPolarGraphicalItem, SetPiePayloadLegend, and PieImpl) so you can remove the as-cast and the `@ts-expect-error` and set Pie.displayName normally.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/cartesian/Scatter.typed.spec.tsx`:
- Around line 129-139: The inline TypeScript suppression is using block comment
syntax between JSX attributes so the compiler sees a text node instead of the
directive; change the comment around the failing prop on the Scatter component
(the dataKey prop in the Scatter<ExampleDataPoint, number> element) from /*
`@ts-expect-error` ... */ to the JSX form {/* `@ts-expect-error` ... */} so
TypeScript recognizes the `@ts-expect-error` for that prop.
In `@test/chart/AreaChart.typed.spec.tsx`:
- Line 2: Update the top import so `expect` is explicitly imported from vitest
alongside `describe` and `it`: modify the import statement that currently lists
describe and it to also include expect (matching the pattern used in other typed
spec files such as the ones that import expect explicitly) so assertions on line
36 use the explicitly imported expect.
In `@test/chart/FunnelChart.typed.spec.tsx`:
- Around line 1-4: The test file is missing the expect import from vitest used
later in the spec; update the import statement that currently imports {
describe, it } from 'vitest' to also include expect so the test assertions
(references to expect) work—locate the top-level import line in
FunnelChart.typed.spec.tsx (the import that mentions describe and it) and add
expect to that import list.
In `@test/chart/RadarChart.typed.spec.tsx`:
- Line 2: The test file imports describe and it but omits expect while the test
body calls expect (in the RadarChart.typed.spec.tsx test), causing a reference
error; update the top import statement to include expect alongside describe and
it so the assertion calls succeed (i.e., add expect to the import that currently
lists describe and it).
In `@test/chart/ScatterChart.typed.spec.tsx`:
- Around line 1-4: The test file imports vitest helpers but omits expect,
causing inconsistency with other typed specs; update the import statement that
currently imports { describe, it } from 'vitest' to also include expect so it
reads { describe, expect, it } (look for the top-level import of vitest in
ScatterChart.typed.spec.tsx) and ensure the rest of the file uses
expect(invalidChart).toBeDefined() as intended.
In `@test/polar/PolarRadiusAxis.typed.spec.tsx`:
- Around line 38-50: The test uses single-arg handlers which bind the first
param (data) to getRelativeCoordinate rather than the actual event; update the
PolarRadiusAxis event handlers in PolarRadiusAxis.typed.spec.tsx to the
three-parameter form used in YAxis.typed.spec.tsx so the third arg is the React
event: replace onClick={e => getRelativeCoordinate(e)} (and the other handlers)
with onClick={(_tick, _i, e) => getRelativeCoordinate(e)} (or typed as (_tick:
any, _i: number, e: React.MouseEvent) => ...) to match PolarRadiusAxisProps and
properly validate the event parameter.
---
Outside diff comments:
In `@src/cartesian/ReferenceLine.tsx`:
- Around line 40-49: The exported ReferenceLineSegment type is fixed to
number|string while ReferenceLineProps.segment is generic
(XValueType/YValueType), causing a public API mismatch; make
ReferenceLineSegment generic (e.g., ReferenceLineSegment<XValueType,
YValueType>) and update all usages (exports, state slices, tests) to reference
the generic form, or alternatively change ReferenceLineProps.segment to the
non-generic number|string pair to match the exported type—ensure consistency
between ReferenceLineSegment and the segment prop by aligning their type
parameters (update ReferenceLineProps, any exports in src/index.ts, and
tests/state slices that import ReferenceLineSegment or segment).
---
Duplicate comments:
In `@test/chart/ComposedChart.typed.spec.tsx`:
- Around line 1-37: Add the same fake-timer setup/teardown as in the BarChart
typed test to avoid timing flakiness: import beforeEach, afterEach and vi from
'vitest' at the top, call beforeEach(() => vi.useFakeTimers()) and afterEach(()
=> vi.useRealTimers()) in this test file, and keep using rechartsTestRender with
ComposedChart and Line as before so timers are mocked during render and restored
after each spec.
---
Nitpick comments:
In `@src/cartesian/ReferenceArea.tsx`:
- Around line 25-26: Extract the duplicated type alias ReferenceCoordinateValue
into a shared types module (e.g., create and export it from a new/shared file)
and replace the local declarations in ReferenceLine.tsx (the type at line ~51),
ReferenceDot.tsx (line ~23), and ReferenceArea.tsx (line ~25) with an import of
that exported type; ensure each file imports the shared ReferenceCoordinateValue
and remove the local alias to eliminate duplication and centralize the
constraint used by the Reference* components.
In `@src/cartesian/ReferenceDot.tsx`:
- Line 23: Duplicate type alias ReferenceCoordinateValue (number | string) is
declared in ReferenceDot.tsx, ReferenceArea.tsx, and ReferenceLine.tsx; extract
it into a single shared exported type (e.g., add it to src/util/types.ts) and
update each file to import the exported ReferenceCoordinateValue and use it in
their exported Props types; ensure the new type is exported so consumers can
import it when building wrappers and update import statements in ReferenceDot,
ReferenceArea, and ReferenceLine to reference the centralized symbol
ReferenceCoordinateValue.
In `@src/polar/Pie.tsx`:
- Around line 1040-1086: The Pie component is exported via a casted constant Pie
= PieFn as { ... } which forces a type assertion and requires a `@ts-expect-error`
to set Pie.displayName; replace this with a directly generic function export
like ReferenceLine does: turn PieFn into an exported generic function export
function Pie<DataPointType = any, DataValueType = any>(outsideProps:
Props<DataPointType, DataValueType>) { ... } (keep the existing body that calls
resolveDefaultProps(outsideProps, defaultPieProps), RegisterGraphicalItemId,
SetPolarGraphicalItem, SetPiePayloadLegend, and PieImpl) so you can remove the
as-cast and the `@ts-expect-error` and set Pie.displayName normally.
In `@test/cartesian/ReferenceDot.typed.spec.tsx`:
- Around line 49-71: The test currently returns a JSX element (LineChart with
ReferenceDot) which only provides a compile-time type check; replace the fragile
"return (<LineChart ... />)" pattern with an actual render call using
rechartsTestRender so the test executes at runtime — e.g., call
rechartsTestRender(<LineChart ...>) inside the it block (keeping the same props
and handlers referencing getRelativeCoordinate) to ensure the event handler
typing is exercised at runtime rather than merely returning a React element.
In `@test/chart/LineChart.typed.spec.tsx`:
- Line 2: The test file imports only describe and it from vitest but uses expect
(e.g., expect(invalidChart).toBeDefined() in the test referencing invalidChart),
so update the import on the top of the file to include expect from 'vitest' (add
expect to the existing import list) to match other typed tests like
ReferenceLine.typed.spec.tsx and ensure the typed test compiles consistently.
In `@test/chart/PieChart.typed.spec.tsx`:
- Line 2: The test file imports only describe and it from vitest but uses expect
(missing import), so update the import statement in PieChart.typed.spec.tsx to
include expect (e.g., change the import that currently lists describe and it to
also list expect); check other test files like LineChart.typed.spec.tsx for the
same pattern and make the imports consistent with ReferenceLine.typed.spec.tsx
by adding expect to the named imports from 'vitest'.
| it('should show error when dataKey is inline typed to return wrong value', () => { | ||
| rechartsTestRender( | ||
| <ScatterChart data={data} width={400} height={400}> | ||
| <Scatter<ExampleDataPoint, number> | ||
| /* @ts-expect-error TypeScript is correct here - the dataKey return type does not match and this should be an error */ | ||
| dataKey={(entry: ExampleDataPoint) => entry.name} | ||
| isAnimationActive={false} | ||
| /> | ||
| </ScatterChart>, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for `@ts-expect-error` directives in tsx files with context
rg -n '@ts-expect-error' --type=tsx -A2 -B2 | head -60Repository: recharts/recharts
Length of output: 88
🏁 Script executed:
# Look specifically for `@ts-expect-error` comments placed between JSX attributes
rg -n '/\*.*@ts-expect-error' --type=tsx -A1 -B1Repository: recharts/recharts
Length of output: 88
🏁 Script executed:
# Check the specific file to understand the context better
cat -n test/cartesian/Scatter.typed.spec.tsx | sed -n '125,145p'Repository: recharts/recharts
Length of output: 1121
🏁 Script executed:
# Search for `@ts-expect-error` in all TypeScript files
fd -e ts -e tsx | xargs rg '@ts-expect-error' -B1 -A1 | head -80Repository: recharts/recharts
Length of output: 5811
🏁 Script executed:
# Count and categorize `@ts-expect-error` comment patterns
fd -e tsx | xargs cat | grep -n '@ts-expect-error' | head -30Repository: recharts/recharts
Length of output: 2584
Replace the plain comment with proper JSX comment syntax for @ts-expect-error to work correctly.
Line 133 uses /* @ts-expect-error ... */ between JSX attributes, which creates a text node rather than a comment. TypeScript cannot recognize the directive in this context. Use {/* @ts-expect-error ... */} instead (as already used correctly on line 145 in the same file).
Diff
<Scatter<ExampleDataPoint, number>
- /* `@ts-expect-error` TypeScript is correct here - the dataKey return type does not match and this should be an error */
+ {/* `@ts-expect-error` TypeScript is correct here - the dataKey return type does not match and this should be an error */}
dataKey={(entry: ExampleDataPoint) => entry.name}
isAnimationActive={false}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should show error when dataKey is inline typed to return wrong value', () => { | |
| rechartsTestRender( | |
| <ScatterChart data={data} width={400} height={400}> | |
| <Scatter<ExampleDataPoint, number> | |
| /* @ts-expect-error TypeScript is correct here - the dataKey return type does not match and this should be an error */ | |
| dataKey={(entry: ExampleDataPoint) => entry.name} | |
| isAnimationActive={false} | |
| /> | |
| </ScatterChart>, | |
| ); | |
| }); | |
| it('should show error when dataKey is inline typed to return wrong value', () => { | |
| rechartsTestRender( | |
| <ScatterChart data={data} width={400} height={400}> | |
| <Scatter<ExampleDataPoint, number> | |
| {/* `@ts-expect-error` TypeScript is correct here - the dataKey return type does not match and this should be an error */} | |
| dataKey={(entry: ExampleDataPoint) => entry.name} | |
| isAnimationActive={false} | |
| /> | |
| </ScatterChart>, | |
| ); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/cartesian/Scatter.typed.spec.tsx` around lines 129 - 139, The inline
TypeScript suppression is using block comment syntax between JSX attributes so
the compiler sees a text node instead of the directive; change the comment
around the failing prop on the Scatter component (the dataKey prop in the
Scatter<ExampleDataPoint, number> element) from /* `@ts-expect-error` ... */ to
the JSX form {/* `@ts-expect-error` ... */} so TypeScript recognizes the
`@ts-expect-error` for that prop.
| @@ -0,0 +1,38 @@ | |||
| import React from 'react'; | |||
| import { describe, it } from 'vitest'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and get its content
cat -n test/chart/AreaChart.typed.spec.tsx | head -40Repository: recharts/recharts
Length of output: 1627
🏁 Script executed:
# Check vitest configuration for globals setting
find . -type f -name "vitest.config.*" -o -name "vite.config.*" | head -5Repository: recharts/recharts
Length of output: 101
🏁 Script executed:
# Check for vitest configuration files
fd "vitest.config|vite.config" -e ts -e js -e mts -e mjsRepository: recharts/recharts
Length of output: 124
🏁 Script executed:
# Search for globals configuration in config files
rg "globals" -t ts -t js --max-count 5Repository: recharts/recharts
Length of output: 809
🏁 Script executed:
# Check the vitest configuration for globals settings
cat vitest.config.mtsRepository: recharts/recharts
Length of output: 2803
🏁 Script executed:
# Compare with other typed spec files to verify consistency
cat -n test/polar/Radar.typed.spec.tsx | head -5Repository: recharts/recharts
Length of output: 331
🏁 Script executed:
# Check another similar typed spec file
cat -n test/polar/RadialBar/RadialBar.typed.spec.tsx | head -5Repository: recharts/recharts
Length of output: 344
Add explicit expect import to match other typed spec files.
Line 2 imports describe and it explicitly, but expect is missing on line 36. While vitest globals are enabled (vitest.config.mts line 20), parallel typed spec files (test/polar/Radar.typed.spec.tsx, test/polar/RadialBar/RadialBar.typed.spec.tsx) explicitly import expect, indicating a codebase preference for explicit imports.
Suggested fix
-import { describe, it } from 'vitest';
+import { describe, expect, it } from 'vitest';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { describe, it } from 'vitest'; | |
| import { describe, expect, it } from 'vitest'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/chart/AreaChart.typed.spec.tsx` at line 2, Update the top import so
`expect` is explicitly imported from vitest alongside `describe` and `it`:
modify the import statement that currently lists describe and it to also include
expect (matching the pattern used in other typed spec files such as the ones
that import expect explicitly) so assertions on line 36 use the explicitly
imported expect.
| import React from 'react'; | ||
| import { describe, it } from 'vitest'; | ||
| import { rechartsTestRender } from '../helper/createSelectorTestCase'; | ||
| import { FunnelChart, Funnel } from '../../src'; |
There was a problem hiding this comment.
Missing expect import from vitest (same issue as ScatterChart.typed.spec.tsx).
Line 36 uses expect but it's not imported on line 2.
Proposed fix
-import { describe, it } from 'vitest';
+import { describe, expect, it } from 'vitest';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import React from 'react'; | |
| import { describe, it } from 'vitest'; | |
| import { rechartsTestRender } from '../helper/createSelectorTestCase'; | |
| import { FunnelChart, Funnel } from '../../src'; | |
| import React from 'react'; | |
| import { describe, expect, it } from 'vitest'; | |
| import { rechartsTestRender } from '../helper/createSelectorTestCase'; | |
| import { FunnelChart, Funnel } from '../../src'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/chart/FunnelChart.typed.spec.tsx` around lines 1 - 4, The test file is
missing the expect import from vitest used later in the spec; update the import
statement that currently imports { describe, it } from 'vitest' to also include
expect so the test assertions (references to expect) work—locate the top-level
import line in FunnelChart.typed.spec.tsx (the import that mentions describe and
it) and add expect to that import list.
| @@ -0,0 +1,38 @@ | |||
| import React from 'react'; | |||
| import { describe, it } from 'vitest'; | |||
There was a problem hiding this comment.
Same missing expect import as test/chart/AreaChart.typed.spec.tsx.
describe and it are explicitly imported on line 2 but expect is omitted, while it is called on line 36. Add expect to the import.
🐛 Proposed fix
-import { describe, it } from 'vitest';
+import { describe, expect, it } from 'vitest';Also applies to: 36-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/chart/RadarChart.typed.spec.tsx` at line 2, The test file imports
describe and it but omits expect while the test body calls expect (in the
RadarChart.typed.spec.tsx test), causing a reference error; update the top
import statement to include expect alongside describe and it so the assertion
calls succeed (i.e., add expect to the import that currently lists describe and
it).
| import React from 'react'; | ||
| import { describe, it } from 'vitest'; | ||
| import { rechartsTestRender } from '../helper/createSelectorTestCase'; | ||
| import { ScatterChart, Scatter } from '../../src'; |
There was a problem hiding this comment.
Missing expect import from vitest.
Line 36 uses expect(invalidChart).toBeDefined() but expect is not imported on line 2. Other typed spec files in this PR (e.g., ReferenceArea.typed.spec.tsx, Funnel.typed.spec.tsx, Line.typed.spec.tsx) explicitly import { describe, expect, it } from 'vitest'. This may still work if vitest globals are configured, but it's inconsistent.
Proposed fix
-import { describe, it } from 'vitest';
+import { describe, expect, it } from 'vitest';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/chart/ScatterChart.typed.spec.tsx` around lines 1 - 4, The test file
imports vitest helpers but omits expect, causing inconsistency with other typed
specs; update the import statement that currently imports { describe, it } from
'vitest' to also include expect so it reads { describe, expect, it } (look for
the top-level import of vitest in ScatterChart.typed.spec.tsx) and ensure the
rest of the file uses expect(invalidChart).toBeDefined() as intended.
| <PolarRadiusAxis | ||
| onClick={e => { | ||
| getRelativeCoordinate(e); | ||
| }} | ||
| onMouseDown={e => { | ||
| getRelativeCoordinate(e); | ||
| }} | ||
| onMouseUp={e => { | ||
| getRelativeCoordinate(e); | ||
| }} | ||
| onMouseMove={e => { | ||
| getRelativeCoordinate(e); | ||
| }} | ||
| onMouseLeave={e => { | ||
| getRelativeCoordinate(e); | ||
| }} | ||
| onMouseOver={e => { | ||
| getRelativeCoordinate(e); | ||
| }} | ||
| onMouseOut={e => { | ||
| getRelativeCoordinate(e); | ||
| }} | ||
| onMouseEnter={e => { | ||
| getRelativeCoordinate(e); | ||
| }} | ||
| onTouchStart={e => { | ||
| getRelativeCoordinate(e); | ||
| }} | ||
| onTouchMove={e => { | ||
| getRelativeCoordinate(e); | ||
| }} | ||
| onTouchEnd={e => { | ||
| getRelativeCoordinate(e); | ||
| }} | ||
| onClick={e => getRelativeCoordinate(e)} | ||
| onMouseDown={e => getRelativeCoordinate(e)} | ||
| onMouseUp={e => getRelativeCoordinate(e)} | ||
| onMouseMove={e => getRelativeCoordinate(e)} | ||
| onMouseLeave={e => getRelativeCoordinate(e)} | ||
| onMouseOver={e => getRelativeCoordinate(e)} | ||
| onMouseOut={e => getRelativeCoordinate(e)} | ||
| onMouseEnter={e => getRelativeCoordinate(e)} | ||
| onTouchStart={e => getRelativeCoordinate(e)} | ||
| onTouchMove={e => getRelativeCoordinate(e)} | ||
| onTouchEnd={e => getRelativeCoordinate(e)} | ||
| /> |
There was a problem hiding this comment.
Event handler signatures differ from YAxis test — verify this is intentional.
Here, event handlers use a single parameter (e) =>, while the YAxis.typed.spec.tsx test uses three parameters (_tick: TickItem, _i: number, e) =>. Looking at PolarRadiusAxisProps (lines 135–163 in PolarRadiusAxis.tsx), the custom event handlers are defined with three parameters: (data: any, index: number, e: React.MouseEvent) => void.
With the single-param form (e) =>, TypeScript binds e to the first parameter (data: any), not the mouse event. The test compiles because any is assignable to getRelativeCoordinate's parameter, but it doesn't actually validate that the event parameter's type is compatible with getRelativeCoordinate.
If the intent is to validate the event type (as in the YAxis test), the handlers should use the three-parameter form. If the intent is just to verify the prop accepts a callback, the current form works but is misleading.
#!/bin/bash
# Check whether PolarRadiusAxis event handler types match the 3-param pattern
# Look at how onClick etc. are typed in PolarRadiusAxisProps
ast-grep --pattern 'onClick?: ($$$) => $_'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/polar/PolarRadiusAxis.typed.spec.tsx` around lines 38 - 50, The test
uses single-arg handlers which bind the first param (data) to
getRelativeCoordinate rather than the actual event; update the PolarRadiusAxis
event handlers in PolarRadiusAxis.typed.spec.tsx to the three-parameter form
used in YAxis.typed.spec.tsx so the third arg is the React event: replace
onClick={e => getRelativeCoordinate(e)} (and the other handlers) with
onClick={(_tick, _i, e) => getRelativeCoordinate(e)} (or typed as (_tick: any,
_i: number, e: React.MouseEvent) => ...) to match PolarRadiusAxisProps and
properly validate the event parameter.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7035 +/- ##
==========================================
+ Coverage 90.12% 90.13% +0.01%
==========================================
Files 526 526
Lines 39183 39262 +79
Branches 5422 5439 +17
==========================================
+ Hits 35312 35390 +78
- Misses 3862 3863 +1
Partials 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Staging Deployment Details
These deployments will remain available for 30 days. To update snapshots: Comment |
Bundle ReportChanges will increase total bundle size by 1.01kB (0.03%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-umdAssets Changed:
view changes for bundle: recharts/bundle-cjsAssets Changed:
view changes for bundle: recharts/bundle-es6Assets Changed:
|
1 participant
Description
We had Area and Bar before, now the rest.
Related Issue
#6645
Types of changes
Checklist:
Summary by CodeRabbit
Improvements
Tests