Conversation
WalkthroughRefines Pie chart typings and exports: restructures PieSectorData and PieSectorDataItem, adjusts computePieSectors merging and cornerRadius handling, re-exports new types (PieSectorDataItem, RadialBarProps) from package root, tightens tests, and refactors example Needle label rendering and demo components for null-safety. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Consumer as Consumer Code / Examples
participant Pie as Pie Component
participant compute as computePieSectors
participant Type as Type Exports
Consumer->>Pie: Render Pie with data, props
Pie->>compute: call computePieSectors(data, props)
compute->>compute: merge entryWithCellInfo early
compute->>compute: normalize percent, compute cornerRadius
compute-->>Pie: return array of PieSectorDataItem
Pie->>Type: surface PieSectorDataItem via public export
Consumer->>Consumer: consume strongly-typed sector data (labels, Needle, Tooltip)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas to focus review on:
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6533 +/- ##
==========================================
- Coverage 93.43% 93.43% -0.01%
==========================================
Files 436 436
Lines 39545 39548 +3
Branches 4599 4602 +3
==========================================
+ Hits 36950 36951 +1
- Misses 2579 2581 +2
Partials 16 16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will increase total bundle size by 918 bytes (0.04%) ⬆️. 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:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
www/src/docs/exampleComponents/PieChart/PieChartWithNeedle.tsx (1)
42-55: Consider whether HalfPie props should be configurable.The
HalfPiecomponent acceptsPiePropsbut then overrides most properties with hardcoded values. This pattern makes the component less reusable. If these values are intentionally fixed for this example, consider either:
- Removing the props spread and only accepting specific props like
isAnimationActiveandactiveShape- Making the component more flexible by not hardcoding all values
Example for option 1:
const HalfPie = (props: Pick<PieProps, 'isAnimationActive' | 'activeShape'>) => ( <Pie {...props} stroke="none" dataKey="value" startAngle={180} endAngle={0} data={chartData} cx={100} cy={100} innerRadius={50} outerRadius={100} /> );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
test-vr/__snapshots__/tests/www/PieChartApiExamples.spec-vr.tsx-snapshots/PieChartWithNeedle-1-chromium-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/www/PieChartApiExamples.spec-vr.tsx-snapshots/PieChartWithNeedle-1-firefox-linux.pngis excluded by!**/*.pngtest-vr/__snapshots__/tests/www/PieChartApiExamples.spec-vr.tsx-snapshots/PieChartWithNeedle-1-webkit-linux.pngis excluded by!**/*.png
📒 Files selected for processing (8)
src/index.ts(1 hunks)src/polar/Pie.tsx(2 hunks)src/util/types.ts(1 hunks)src/zindex/ZIndexLayer.tsx(1 hunks)storybook/stories/Examples/Pie/PieWithNeedle.stories.tsx(0 hunks)test/polar/Pie.spec.tsx(1 hunks)www/src/docs/exampleComponents/PieChart/PieChartWithCustomizedLabel.tsx(1 hunks)www/src/docs/exampleComponents/PieChart/PieChartWithNeedle.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- storybook/stories/Examples/Pie/PieWithNeedle.stories.tsx
🧰 Additional context used
📓 Path-based instructions (5)
{test,www/test}/**
📄 CodeRabbit inference engine (DEVELOPING.md)
Place unit tests in the test directory; some tests may also live in www/test
Files:
test/polar/Pie.spec.tsx
test/**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
test/**/*.spec.{ts,tsx}: When writing new code, aim for 100% unit test code coverage
When adding or changing functionality, add appropriate tests (prefer unit tests; use RTL if rendering is involved)
Files:
test/polar/Pie.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx}: Ensure code lints cleanly before submitting PRs (npm run lint)
Never use the TypeScript any type (implicit or explicit)
Prefer unknown over any and refine types appropriately
Explicitly type all function parameters and return values; do not rely on implicit any or inference
Do not use as type assertions; the only exception is as const
Files:
test/polar/Pie.spec.tsxsrc/zindex/ZIndexLayer.tsxsrc/index.tswww/src/docs/exampleComponents/PieChart/PieChartWithCustomizedLabel.tsxsrc/util/types.tssrc/polar/Pie.tsxwww/src/docs/exampleComponents/PieChart/PieChartWithNeedle.tsx
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/zindex/ZIndexLayer.tsxsrc/index.tssrc/util/types.tssrc/polar/Pie.tsx
www/**
📄 CodeRabbit inference engine (DEVELOPING.md)
Use the www directory to add and commit examples for the documentation website (recharts.github.io)
Files:
www/src/docs/exampleComponents/PieChart/PieChartWithCustomizedLabel.tsxwww/src/docs/exampleComponents/PieChart/PieChartWithNeedle.tsx
🧠 Learnings (5)
📚 Learning: 2025-10-25T07:35:14.559Z
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Applies to test/**/*.spec.{ts,tsx} : When adding or changing functionality, add appropriate tests (prefer unit tests; use RTL if rendering is involved)
Applied to files:
test/polar/Pie.spec.tsx
📚 Learning: 2025-10-25T07:35:14.559Z
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Applies to test/component/**/*.spec.tsx : Test rendering behavior and component interactions with React Testing Library
Applied to files:
test/polar/Pie.spec.tsx
📚 Learning: 2025-10-25T07:35:46.180Z
Learnt from: CR
PR: recharts/recharts#0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.180Z
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/polar/Pie.spec.tsxwww/src/docs/exampleComponents/PieChart/PieChartWithCustomizedLabel.tsxwww/src/docs/exampleComponents/PieChart/PieChartWithNeedle.tsx
📚 Learning: 2025-10-25T07:36:02.211Z
Learnt from: CR
PR: recharts/recharts#0
File: AGENTS.md:0-0
Timestamp: 2025-10-25T07:36:02.211Z
Learning: Recharts aims for simple, declarative, and composable charts; prioritize consistency, usability, performance, and accessibility
Applied to files:
www/src/docs/exampleComponents/PieChart/PieChartWithNeedle.tsx
📚 Learning: 2025-10-25T07:35:14.559Z
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Provide a reduced, reproducible test case (e.g., via CodeSandbox or fork of recharts.github.io) when reporting issues
Applied to files:
www/src/docs/exampleComponents/PieChart/PieChartWithNeedle.tsx
🧬 Code graph analysis (3)
www/src/docs/exampleComponents/PieChart/PieChartWithCustomizedLabel.tsx (2)
storybook/stories/API/props/DotProps.ts (2)
cx(18-24)cy(26-32)src/util/PolarUtils.ts (1)
RADIAN(4-4)
src/polar/Pie.tsx (4)
src/util/types.ts (2)
GeometrySector(637-644)Coordinate(86-89)src/state/tooltipSlice.ts (1)
TooltipPayload(29-29)src/state/types/PieSettings.ts (1)
PiePresentationProps(8-8)src/index.ts (1)
PieSectorDataItem(55-55)
www/src/docs/exampleComponents/PieChart/PieChartWithNeedle.tsx (2)
src/index.ts (4)
PieSectorDataItem(55-55)Pie(54-54)PieChart(92-92)Tooltip(15-15)src/polar/Pie.tsx (2)
PieSectorDataItem(158-158)Pie(785-825)
🔇 Additional comments (2)
test/polar/Pie.spec.tsx (1)
547-559: Good defensive typing test for PieLabelRenderProps.The runtime type assertions effectively verify that
innerRadius,outerRadius, andpercentare properly typed asnumberrather thanunknown. This directly addresses issue #6380 and ensures the type improvements don't regress.www/src/docs/exampleComponents/PieChart/PieChartWithNeedle.tsx (1)
62-62: Clarify the purpose of the Tooltip configuration.The Tooltip is configured with
content={() => null}(which renders nothing) andactive={true}(always active). This pattern is unusual and may confuse developers reading the example.If the intent is to trigger the
activeShape(Needle) behavior without displaying tooltip content, consider:
- Adding a comment explaining this pattern
- Verifying this is the simplest approach to achieve the desired behavior
Alternatively, if the Tooltip should display actual content, replace
content={() => null}with a proper content renderer or remove thecontentprop entirely to use the default.
src/polar/Pie.tsx
Outdated
| type PieSectorDataItemKnown = PiePresentationProps & | ||
| PieCoordinate & | ||
| PieSectorData & { | ||
| cornerRadius: number | undefined; | ||
| }; | ||
|
|
||
| export type PieSectorDataItem = PiePresentationProps & PieCoordinate & PieSectorData; | ||
| /** | ||
| * We spread the data object into the sector data item, | ||
| * so we can't really know what is going to be inside. | ||
| * There are however some known properties so let's omit those from RealPieData | ||
| * to prevent getting unknowns on the outside. | ||
| * https://github.com/recharts/recharts/issues/6380 | ||
| * https://github.com/recharts/recharts/discussions/6375 | ||
| */ | ||
| export type PieSectorDataItem = PieSectorDataItemKnown & Omit<RealPieData, keyof PieSectorDataItemKnown>; | ||
|
|
There was a problem hiding this comment.
Type intersection with any collapses the new Pie label types
Because RealPieData remains any, the intersection PieSectorDataItemKnown & Omit<RealPieData, ...> still resolves to any. Every consumer of PieSectorDataItem (and PieLabelRenderProps) therefore keeps getting any, so the PR does not actually fix the typing gaps from #6380. Please narrow RealPieData (for example, reuse ChartDataInput or another Record<string, unknown> shape) before intersecting, so the known fields stay strongly typed while still allowing extra keys.
Apply this diff to restore the intended typing:
-type RealPieData = any;
+type RealPieData = ChartDataInput;With RealPieData no longer any, PieSectorDataItem will expose the concrete fields you just defined while still allowing arbitrary user data to pass through.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/polar/Pie.tsx around lines 144 to 159, the current PieSectorDataItem uses
Omit<RealPieData, ...> while RealPieData is still typed as any, which collapses
the intersection back to any; change RealPieData from any to a narrower shape
(for example reuse the existing ChartDataInput type or declare it as
Record<string, unknown>) before performing the Omit so that
PieSectorDataItemKnown fields remain strongly typed while still permitting
arbitrary extra properties.
There was a problem hiding this comment.
Hm that is good observation but when I do that, the final type still becomes unknown and the tests I just added start failing again.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the test file to see what assertions were added
echo "=== Test file assertions ==="
rg -A 5 -B 2 "innerRadius|outerRadius|percent" test/polar/Pie.spec.tsx
Length of output: 18564
🏁 Script executed:
#!/bin/bash
# Look at how PieSectorDataItem and PieLabelRenderProps are used in the codebase
echo "=== Usage of PieLabelRenderProps ==="
rg -n "PieLabelRenderProps" --type ts --type tsx -C 2
echo -e "\n=== Usage of PieSectorDataItem ==="
rg -n "PieSectorDataItem" --type ts --type tsx -C 2
Length of output: 128
🏁 Script executed:
#!/bin/bash
# Check the example files to see how the label props are being used
echo "=== PieChartWithCustomizedLabel usage ==="
cat www/src/docs/exampleComponents/PieChart/PieChartWithCustomizedLabel.tsx | head -60
Length of output: 1642
🏁 Script executed:
#!/bin/bash
# Look at the computePieSectors function to understand how data is spread
echo "=== computePieSectors function - data spreading ==="
sed -n '/export function computePieSectors/,/^}/p' src/polar/Pie.tsx | head -100
Length of output: 143
🏁 Script executed:
#!/bin/bash
# Check how entryWithCellInfo is constructed and what properties it contains
echo "=== Looking for entryWithCellInfo and data spreading pattern ==="
rg -n "entryWithCellInfo|\.\.\.entry" src/polar/Pie.tsx -A 3 -B 3
Length of output: 2714
@PavelVanecek You're absolutely right! I apologize for the confusion. The issue is a fundamental TypeScript limitation with index signatures and intersection types.
When RealPieData = Record<string, unknown>, the index signature [key: string]: unknown takes precedence in the intersection, causing all properties—even the explicitly typed ones like innerRadius, outerRadius, and percent—to become unknown:
type Example = Record<string, unknown> & { innerRadius: number };
// innerRadius becomes `unknown` because the index signature dominatesThis is why your tests fail when trying to assign these properties to number variables.
The Dilemma
The current implementation with any has its own problem: intersecting with any collapses the entire type to any, so PieSectorDataItem becomes any and doesn't actually fix #6380. However, it makes the tests pass because any is assignable to everything.
Possible Solutions
Option 1: Keep RealPieData = any (current approach)
- Accept that we can't have perfect type safety for user-provided data properties
- The known properties in
PieSectorDataItemKnownare still well-typed in the code that constructs them - Add a JSDoc comment explaining this limitation
Option 2: Use a different type pattern
Instead of an intersection, we could use a mapped type approach, but this gets complex and may not work well with how the data is spread in computePieSectors.
Option 3: Don't type the spread data
Define PieSectorDataItem to only include the known properties and accept that additional properties from user data exist at runtime but aren't typed.
I'd recommend Option 1 with better documentation. The reality is that you're spreading arbitrary user data (...entryWithCellInfo) into the sector objects, so we can't know those types at compile time. The important part—which your PR does achieve—is that the known Recharts-added properties are properly typed.
Would you like to add a comment explaining this TypeScript limitation, or would you prefer to explore one of the other approaches?
🧬 Code Graph Analysis Results
<no_relevant_code_snippets>
🧠 Learnings used
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Applies to **/*.{ts,tsx} : Prefer unknown over any and refine types appropriately
| @@ -1,6 +1,6 @@ | |||
| import { Pie, PieChart } from 'recharts'; | |||
| import { Pie, PieChart, PieProps, Tooltip } from 'recharts'; | |||
| import { PieSectorDataItem } from 'recharts/types/polar/Pie'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
Verify import path is part of the public API.
The import from 'recharts/types/polar/Pie' appears to access internal TypeScript definitions. According to the relevant code snippets, PieSectorDataItem is exported from the main package in src/index.ts (line 54).
Please verify whether this import should use the public API path instead:
🏁 Script executed:
#!/bin/bash
# Check if PieSectorDataItem is exported from the main recharts package
rg -n "export.*PieSectorDataItem" src/index.ts
# Check other example files for similar imports
rg -n "from 'recharts/types" www/src/docs/exampleComponents/Length of output: 1281
Use the public API import path for PieSectorDataItem.
The type is exported as part of recharts' public API (src/index.ts line 55), so import it directly from the main package:
import { PieSectorDataItem } from 'recharts';Instead of the internal type path:
import { PieSectorDataItem } from 'recharts/types/polar/Pie';🤖 Prompt for AI Agents
In www/src/docs/exampleComponents/PieChart/PieChartWithNeedle.tsx around line 2,
the file imports PieSectorDataItem from an internal path
(recharts/types/polar/Pie); change the import to use the public API by importing
PieSectorDataItem directly from 'recharts' instead, updating the import
statement to import { PieSectorDataItem } from 'recharts'.
There was a problem hiding this comment.
I will do this once we publish the changes to npm.
There was a problem hiding this comment.
@PavelVanecek, that makes perfect sense! Since PieSectorDataItem is being added to the public API in this PR but won't be available from the main 'recharts' import until it's published to npm, waiting to update the import path is the right approach.
Would you like me to create a GitHub issue to track this follow-up task, or do you have another system for tracking post-publish cleanups?
🧠 Learnings used
Learnt from: CR
PR: recharts/recharts#0
File: AGENTS.md:0-0
Timestamp: 2025-10-25T07:36:02.211Z
Learning: Recharts aims for simple, declarative, and composable charts; prioritize consistency, usability, performance, and accessibility
Learnt from: CR
PR: recharts/recharts#0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.180Z
Learning: Applies to test/**/*.test.{ts,tsx} : Mock getBoundingClientRect using test/helper/MockGetBoundingClientRect.ts when rendering Recharts components (e.g., Tooltip, Legend, charts)
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Provide a reduced, reproducible test case (e.g., via CodeSandbox or fork of recharts.github.io) when reporting issues
There was a problem hiding this comment.
Yes create a GitHub issue please.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue to track the follow-up task
gh issue create \
--title "Update PieSectorDataItem import to use public API path after npm publish" \
--body "## Description
After PR #6533 is merged and published to npm, update the import path for \`PieSectorDataItem\` in the PieChartWithNeedle example to use the public API.
## Location
\`www/src/docs/exampleComponents/PieChart/PieChartWithNeedle.tsx\` (line 2)
## Current Import
\`\`\`typescript
import { PieSectorDataItem } from 'recharts/types/polar/Pie';
\`\`\`
## Desired Import
\`\`\`typescript
import { PieSectorDataItem } from 'recharts';
\`\`\`
## Context
The \`PieSectorDataItem\` type is being added to the public API in PR #6533 and is exported from \`src/index.ts\`. However, the example currently uses an internal import path because the changes haven't been published to npm yet.
## References
- PR: https://github.com/recharts/recharts/pull/6533
- Discussion: https://github.com/recharts/recharts/pull/6533#discussion_r2478305779
- Requested by: @PavelVanecek" \
--assignee PavelVanecek
Length of output: 48
@PavelVanecek, I've created issue #6535 to track updating the import path after the npm publish. The issue includes all the context and references to this PR and discussion thread.
🧬 Code Graph Analysis Results
[Piece 1] File: src/index.ts, lines 55-55
PieSectorDataItem
[Piece 2] File: src/index.ts, lines 54-54
Pie
[Piece 3] File: src/index.ts, lines 92-92
PieChart
[Piece 4] File: src/index.ts, lines 15-15
Tooltip
[Piece 5] File: storybook/stories/API/props/AnimationProps.ts, lines 28-36
export const isAnimationActive: StorybookArg = {
description: 'If set false, animation of component will be disabled.',
table: {
type: { summary: 'boolean' },
defaultValue: { summary: 'true in CSR, and false in SSR' },
category: 'Animation',
},
defaultValue: true,
};
[Piece 6] File: storybook/stories/API/props/DotProps.ts, lines 18-24
export const cx: StorybookArg = {
description: 'The x-coordinate of the dots center.',
table: {
type: { summary: 'number' },
category: 'General',
},
};
[Piece 7] File: storybook/stories/API/props/DotProps.ts, lines 26-32
export const cy: StorybookArg = {
description: 'The y-coordinate of the dots center.',
table: {
type: { summary: 'number' },
category: 'General',
},
};
[Piece 8] File: src/polar/Pie.tsx, lines 158-158
export type PieSectorDataItem = PieSectorDataItemKnown & Omit<RealPieData, keyof PieSectorDataItemKnown>;
[Piece 9] File: src/polar/Pie.tsx, lines 785-825
export function Pie(outsideProps: Props) {
const props: PropsWithResolvedDefaults = resolveDefaultProps(outsideProps, defaultPieProps);
const { id: externalId, ...propsWithoutId } = props;
const presentationProps: PiePresentationProps | null = svgPropertiesNoEvents(propsWithoutId);
return (
<RegisterGraphicalItemId id={externalId} type="pie">
{id => (
<>
<SetPolarGraphicalItem
type="pie"
id={id}
data={propsWithoutId.data}
dataKey={propsWithoutId.dataKey}
hide={propsWithoutId.hide}
angleAxisId={0}
radiusAxisId={0}
name={propsWithoutId.name}
nameKey={propsWithoutId.nameKey}
tooltipType={propsWithoutId.tooltipType}
legendType={propsWithoutId.legendType}
fill={propsWithoutId.fill}
cx={propsWithoutId.cx}
cy={propsWithoutId.cy}
startAngle={propsWithoutId.startAngle}
endAngle={propsWithoutId.endAngle}
paddingAngle={propsWithoutId.paddingAngle}
minAngle={propsWithoutId.minAngle}
innerRadius={propsWithoutId.innerRadius}
outerRadius={propsWithoutId.outerRadius}
cornerRadius={propsWithoutId.cornerRadius}
presentationProps={presentationProps}
maxRadius={props.maxRadius}
/>
<SetPiePayloadLegend {...propsWithoutId} id={id} />
<PieImpl {...propsWithoutId} id={id} />
</>
)}
</RegisterGraphicalItemId>
);
}
🧠 Learnings used
Learnt from: CR
PR: recharts/recharts#0
File: AGENTS.md:0-0
Timestamp: 2025-10-25T07:36:02.211Z
Learning: Recharts aims for simple, declarative, and composable charts; prioritize consistency, usability, performance, and accessibility
Learnt from: CR
PR: recharts/recharts#0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.180Z
Learning: Applies to test/**/*.test.{ts,tsx} : Mock getBoundingClientRect using test/helper/MockGetBoundingClientRect.ts when rendering Recharts components (e.g., Tooltip, Legend, charts)
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Provide a reduced, reproducible test case (e.g., via CodeSandbox or fork of recharts.github.io) when reporting issues
| const Needle = ({ cx, cy, midAngle, innerRadius, outerRadius }: PieSectorDataItem) => { | ||
| const needleBaseCenterX = cx; | ||
| const needleBaseCenterY = cy; | ||
| const needleLength = innerRadius + (outerRadius - innerRadius) / 2; | ||
|
|
||
| return [ | ||
| <circle key="needle-circle" cx={x0} cy={y0} r={r} fill={color} stroke="none" />, | ||
| <path | ||
| key="needle-path" | ||
| d={`M${xba} ${yba}L${xbb} ${ybb} L${xp} ${yp} L${xba} ${yba}`} | ||
| stroke="#none" | ||
| fill={color} | ||
| />, | ||
| ]; | ||
| return ( | ||
| <g> | ||
| <circle | ||
| cx={needleBaseCenterX} | ||
| cy={needleBaseCenterY} | ||
| r={NEEDLE_BASE_RADIUS_PX} | ||
| fill={NEEDLE_COLOR} | ||
| stroke="none" | ||
| /> | ||
| <path | ||
| d={`M${needleBaseCenterX},${needleBaseCenterY}l${needleLength},0`} | ||
| strokeWidth={2} | ||
| stroke={NEEDLE_COLOR} | ||
| fill={NEEDLE_COLOR} | ||
| style={{ | ||
| transform: `rotate(-${midAngle}deg)`, | ||
| transformOrigin: `${needleBaseCenterX}px ${needleBaseCenterY}px`, | ||
| }} | ||
| /> | ||
| </g> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
Add null safety and explicit return type annotation.
The Needle component destructures properties from PieSectorDataItem without verifying they exist. Based on the type definition, some properties may be undefined. Additionally, per coding guidelines, function return values should be explicitly typed.
Apply this diff to add null safety and type annotation:
-const Needle = ({ cx, cy, midAngle, innerRadius, outerRadius }: PieSectorDataItem) => {
+const Needle = ({ cx, cy, midAngle, innerRadius, outerRadius }: PieSectorDataItem): JSX.Element | null => {
+ if (cx == null || cy == null || midAngle == null || innerRadius == null || outerRadius == null) {
+ return null;
+ }
+
const needleBaseCenterX = cx;
const needleBaseCenterY = cy;
const needleLength = innerRadius + (outerRadius - innerRadius) / 2;📝 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.
| const Needle = ({ cx, cy, midAngle, innerRadius, outerRadius }: PieSectorDataItem) => { | |
| const needleBaseCenterX = cx; | |
| const needleBaseCenterY = cy; | |
| const needleLength = innerRadius + (outerRadius - innerRadius) / 2; | |
| return [ | |
| <circle key="needle-circle" cx={x0} cy={y0} r={r} fill={color} stroke="none" />, | |
| <path | |
| key="needle-path" | |
| d={`M${xba} ${yba}L${xbb} ${ybb} L${xp} ${yp} L${xba} ${yba}`} | |
| stroke="#none" | |
| fill={color} | |
| />, | |
| ]; | |
| return ( | |
| <g> | |
| <circle | |
| cx={needleBaseCenterX} | |
| cy={needleBaseCenterY} | |
| r={NEEDLE_BASE_RADIUS_PX} | |
| fill={NEEDLE_COLOR} | |
| stroke="none" | |
| /> | |
| <path | |
| d={`M${needleBaseCenterX},${needleBaseCenterY}l${needleLength},0`} | |
| strokeWidth={2} | |
| stroke={NEEDLE_COLOR} | |
| fill={NEEDLE_COLOR} | |
| style={{ | |
| transform: `rotate(-${midAngle}deg)`, | |
| transformOrigin: `${needleBaseCenterX}px ${needleBaseCenterY}px`, | |
| }} | |
| /> | |
| </g> | |
| ); | |
| }; | |
| const Needle = ({ cx, cy, midAngle, innerRadius, outerRadius }: PieSectorDataItem): JSX.Element | null => { | |
| if (cx == null || cy == null || midAngle == null || innerRadius == null || outerRadius == null) { | |
| return null; | |
| } | |
| const needleBaseCenterX = cx; | |
| const needleBaseCenterY = cy; | |
| const needleLength = innerRadius + (outerRadius - innerRadius) / 2; | |
| return ( | |
| <g> | |
| <circle | |
| cx={needleBaseCenterX} | |
| cy={needleBaseCenterY} | |
| r={NEEDLE_BASE_RADIUS_PX} | |
| fill={NEEDLE_COLOR} | |
| stroke="none" | |
| /> | |
| <path | |
| d={`M${needleBaseCenterX},${needleBaseCenterY}l${needleLength},0`} | |
| strokeWidth={2} | |
| stroke={NEEDLE_COLOR} | |
| fill={NEEDLE_COLOR} | |
| style={{ | |
| transform: `rotate(-${midAngle}deg)`, | |
| transformOrigin: `${needleBaseCenterX}px ${needleBaseCenterY}px`, | |
| }} | |
| /> | |
| </g> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
In www/src/docs/exampleComponents/PieChart/PieChartWithNeedle.tsx around lines
14 to 40, the Needle component destructures fields that may be undefined and
lacks an explicit return type; update the function signature to include an
explicit return type (e.g., JSX.Element | null) and add null-safety guards for
cx, cy, midAngle, innerRadius, and outerRadius (return null if required numeric
values are missing or coerce with safe defaults) before computing
needleBaseCenterX/needleBaseCenterY/needleLength; ensure transform and
transformOrigin only use valid numbers (e.g., derive strings after validation)
so the component never operates on undefined values.
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 (3)
src/polar/Pie.tsx(2 hunks)test/polar/Pie.spec.tsx(2 hunks)test/state/selectors/pieSelectors.spec.tsx(15 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
{test,www/test}/**
📄 CodeRabbit inference engine (DEVELOPING.md)
Place unit tests in the test directory; some tests may also live in www/test
Files:
test/polar/Pie.spec.tsxtest/state/selectors/pieSelectors.spec.tsx
test/**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
test/**/*.spec.{ts,tsx}: When writing new code, aim for 100% unit test code coverage
When adding or changing functionality, add appropriate tests (prefer unit tests; use RTL if rendering is involved)
Files:
test/polar/Pie.spec.tsxtest/state/selectors/pieSelectors.spec.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{ts,tsx}: Ensure code lints cleanly before submitting PRs (npm run lint)
Never use the TypeScript any type (implicit or explicit)
Prefer unknown over any and refine types appropriately
Explicitly type all function parameters and return values; do not rely on implicit any or inference
Do not use as type assertions; the only exception is as const
Files:
test/polar/Pie.spec.tsxsrc/polar/Pie.tsxtest/state/selectors/pieSelectors.spec.tsx
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/polar/Pie.tsx
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
PR: recharts/recharts#0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.180Z
Learning: Applies to test/**/*.test.{ts,tsx} : Mock getBoundingClientRect using test/helper/MockGetBoundingClientRect.ts when rendering Recharts components (e.g., Tooltip, Legend, charts)
📚 Learning: 2025-10-25T07:35:14.559Z
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Applies to test/component/**/*.spec.tsx : Test rendering behavior and component interactions with React Testing Library
Applied to files:
test/polar/Pie.spec.tsxtest/state/selectors/pieSelectors.spec.tsx
📚 Learning: 2025-10-25T07:35:14.559Z
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Applies to test/**/*.spec.{ts,tsx} : When adding or changing functionality, add appropriate tests (prefer unit tests; use RTL if rendering is involved)
Applied to files:
test/polar/Pie.spec.tsx
📚 Learning: 2025-10-25T07:35:46.180Z
Learnt from: CR
PR: recharts/recharts#0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.180Z
Learning: Write unit tests using Vitest and React Testing Library
Applied to files:
test/polar/Pie.spec.tsx
📚 Learning: 2025-10-25T07:35:46.180Z
Learnt from: CR
PR: recharts/recharts#0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.180Z
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/polar/Pie.spec.tsxtest/state/selectors/pieSelectors.spec.tsx
📚 Learning: 2025-10-25T07:35:14.559Z
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Prefer the simplest tests (unit tests); use React Testing Library or Storybook tests only when needed
Applied to files:
test/polar/Pie.spec.tsx
📚 Learning: 2025-10-25T07:35:46.180Z
Learnt from: CR
PR: recharts/recharts#0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.180Z
Learning: Applies to test/**/*.test.{ts,tsx} : When testing selectors, verify render counts using the spy and rerenderSameComponent from createSelectorTestCase
Applied to files:
test/polar/Pie.spec.tsxtest/state/selectors/pieSelectors.spec.tsx
📚 Learning: 2025-10-25T07:35:14.559Z
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Provide a reduced, reproducible test case (e.g., via CodeSandbox or fork of recharts.github.io) when reporting issues
Applied to files:
test/polar/Pie.spec.tsxsrc/polar/Pie.tsxtest/state/selectors/pieSelectors.spec.tsx
📚 Learning: 2025-10-25T07:35:46.180Z
Learnt from: CR
PR: recharts/recharts#0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.180Z
Learning: Applies to test/**/*.test.{ts,tsx} : In tests not using createSelectorTestCase, advance timers after renders with vi.runOnlyPendingTimers()
Applied to files:
test/polar/Pie.spec.tsx
📚 Learning: 2025-10-25T07:35:14.559Z
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Applies to storybook/stories/**/*.stories.tsx : Use Storybook stories as smoke tests and add play-function tests with assertions when appropriate
Applied to files:
test/polar/Pie.spec.tsxtest/state/selectors/pieSelectors.spec.tsx
📚 Learning: 2025-10-25T07:35:14.559Z
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Applies to **/*.{ts,tsx} : Prefer unknown over any and refine types appropriately
Applied to files:
src/polar/Pie.tsx
📚 Learning: 2025-10-25T07:35:14.559Z
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Applies to **/*.{ts,tsx} : Explicitly type all function parameters and return values; do not rely on implicit any or inference
Applied to files:
src/polar/Pie.tsx
📚 Learning: 2025-10-25T07:35:14.559Z
Learnt from: CR
PR: recharts/recharts#0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-10-25T07:35:14.559Z
Learning: Applies to **/*.{ts,tsx} : Never use the TypeScript any type (implicit or explicit)
Applied to files:
src/polar/Pie.tsx
📚 Learning: 2025-10-25T07:36:02.211Z
Learnt from: CR
PR: recharts/recharts#0
File: AGENTS.md:0-0
Timestamp: 2025-10-25T07:36:02.211Z
Learning: Recharts aims for simple, declarative, and composable charts; prioritize consistency, usability, performance, and accessibility
Applied to files:
src/polar/Pie.tsx
📚 Learning: 2025-10-25T07:35:46.180Z
Learnt from: CR
PR: recharts/recharts#0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.180Z
Learning: Applies to test/**/*.test.{ts,tsx} : Prefer using the createSelectorTestCase helper when writing or modifying tests
Applied to files:
test/state/selectors/pieSelectors.spec.tsx
🧬 Code graph analysis (1)
src/polar/Pie.tsx (4)
src/util/types.ts (2)
GeometrySector(637-644)Coordinate(86-89)src/state/tooltipSlice.ts (1)
TooltipPayload(29-29)src/index.ts (1)
PieSectorDataItem(55-55)src/state/types/PieSettings.ts (1)
PiePresentationProps(8-8)
⏰ 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). (1)
- GitHub Check: Build, Test, Pack
🔇 Additional comments (7)
src/polar/Pie.tsx (3)
144-158: Pragmatic documentation of typing limitations.The documentation clearly explains the TypeScript limitation when spreading arbitrary user data. This honest approach helps consumers understand why some properties might not have perfect type inference, and the explicit
cornerRadiusfield improves the type definition where possible.
568-570: Verify data spreading order and cornerRadius parsing.The spreading order correctly ensures computed properties (
percent,cornerRadius, etc.) override any user-provided properties with the same name. However, ifcornerRadiusis a string that's not a valid number,parseFloatreturnsNaNrather thanundefined, which might cause unexpected rendering behavior.Consider adding a safeguard:
- cornerRadius: typeof cornerRadius === 'string' ? parseFloat(cornerRadius) : cornerRadius, + cornerRadius: typeof cornerRadius === 'string' ? (isNaN(parseFloat(cornerRadius)) ? undefined : parseFloat(cornerRadius)) : cornerRadius,Or verify that invalid
cornerRadiusstrings are already validated upstream.
234-234: RealPieData type acknowledged from past discussion.The
anytype here aligns with the pragmatic approach discussed in previous reviews. The TypeScript limitation with index signatures and the added documentation inPieSectorDataItem(lines 144-153) clarify why this compromise is necessary.test/state/selectors/pieSelectors.spec.tsx (2)
7-7: LGTM: Public API imports.The import now uses the public API surface (importing from
../../../src), which aligns withPieSectorDataItembeing re-exported as part of the public API.
48-49: Pragmatic handling of data spreading limitation.The
@ts-expect-errorcomments acknowledge that spreading arbitrary user data creates a "soup" where TypeScript cannot track individual properties. This is honest and aligns with the documentation added insrc/polar/Pie.tsx(lines 144-153), providing a pragmatic solution to the TypeScript limitation.Also applies to: 94-95, 140-141, 186-187, 232-233, 278-279, 324-325, 383-384, 429-430, 475-476, 521-522, 567-568, 613-614, 659-660
test/polar/Pie.spec.tsx (2)
4-16: LGTM: Public API imports.The imports now use the public API surface (importing from
../../src), consistent withPieLabelRenderPropsandPieSectorDataItembeing re-exported at the package root.
557-573: Excellent runtime verification of label prop types.The type annotations and runtime assertions effectively verify that
PieLabelRenderPropsprovides properly typed values despite the data spreading limitations. The defensive checks confirm thatinnerRadius,outerRadius, andpercentare correctly typed as numbers at both compile-time and runtime, directly addressing the concerns from issues #6375 and #6380.
| middleRadius?: number; | ||
| name?: string | number; | ||
| paddingAngle?: number; | ||
| payload?: any; |
There was a problem hiding this comment.
Replace any with unknown for payload.
The payload property uses any, which violates the coding guideline "Never use the TypeScript any type." Use unknown instead and let consumers refine the type as needed.
Apply this diff:
- payload?: any;
+ payload?: unknown;As per coding guidelines
📝 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.
| payload?: any; | |
| payload?: unknown; |
🤖 Prompt for AI Agents
In src/polar/Pie.tsx around line 137, the payload property's type is declared as
`any`; change it to `unknown` and update any local usages to narrow or assert
the type before treating it as a specific shape. Replace `payload?: any;` with
`payload?: unknown;`, and for places that read fields from payload add
appropriate type guards, instanceof checks, or casts (e.g., create a typed
interface and cast after validation) so consumers refine the type before use.
## Description Updates `PieChartWithNeedle` example to import `PieSectorDataItem` from the public API (`'recharts'`) instead of internal path (`'recharts/types/polar/Pie'`). The type was exported in PR #6533 but the example continued using the internal path temporarily until npm publish. **Before:** ```typescript import { Pie, PieChart, PieProps, Tooltip } from 'recharts'; import { PieSectorDataItem } from 'recharts/types/polar/Pie'; ``` **After:** ```typescript import { Pie, PieChart, PieProps, PieSectorDataItem, Tooltip } from 'recharts'; ``` ## Related Issue Closes #[issue_number] Follow-up to PR #6533 ## Motivation and Context The `PieSectorDataItem` type is now part of the public API (exported from `src/index.ts`). Examples should reference public APIs, not internal implementation paths. ## How Has This Been Tested? - Type-checking passes (`npm run check-types-website`) - Website tests pass (`npm run test-website`) - All unit tests pass ## Screenshots (if appropriate): N/A ## Types of changes - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) ## Checklist: - [ ] My change requires a change to the documentation. - [ ] I have updated the documentation accordingly. - [ ] I have added tests to cover my changes. - [ ] I have added a storybook story or VR test, or extended an existing story or VR test to show my changes <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>Update PieSectorDataItem import to use public API path after npm publish</issue_title> > <issue_description>## Description > > After PR #6533 is merged and published to npm, update the import path for `PieSectorDataItem` in the PieChartWithNeedle example to use the public API. > > ## Location > `www/src/docs/exampleComponents/PieChart/PieChartWithNeedle.tsx` (line 2) > > ## Current Import > ```typescript > import { PieSectorDataItem } from 'recharts/types/polar/Pie'; > ``` > > ## Desired Import > ```typescript > import { PieSectorDataItem } from 'recharts'; > ``` > > ## Context > The `PieSectorDataItem` type is being added to the public API in PR #6533 and is exported from `src/index.ts`. However, the example currently uses an internal import path because the changes haven't been published to npm yet. > > ## References > - PR: #6533 > - Discussion: #6533 (comment) > - Requested by: @PavelVanecek</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> - Fixes #6535 <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/recharts/recharts/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: PavelVanecek <1100170+PavelVanecek@users.noreply.github.com>
Description
After the zindex introduction, one of the Pie stories was broken so I fixed it. Initially I thought that it is a use-case for exporting ZIndexLayer but turns out that it's way easier to do as a custom Pie shape anyway.
Also I have removed the identical storybook because we're already over Chromatic monthly limit so let's cut down on duplicate examples a little.
Also while here I fixed the Pie types.
Related Issue
Fixes #6375
Fixes #6380
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores