Conversation
WalkthroughConsolidates Cartesian/stacked chart API docs into shared definitions, tightens omnidoc comment-exception logic to support multi-component exceptions, introduces RechartsMouseEventHandler and adjusts mouse-event typings, removes public mouse event props from Area, adjusts Curve/Area typing/docs, and updates tests and small doc/comment edits. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
www/src/docs/api/Area.ts (1)
299-369: Fix double spaces in event handler descriptions.Multiple event handler descriptions have double spaces before "on the curve":
- Line 299:
"click on the curve"→"click on the curve"- Line 314:
"mousedown on the curve"→"mousedown on the curve"- Line 323:
"mouseup on the curve"→"mouseup on the curve"- Line 332:
"mousemove on the curve"→"mousemove on the curve"- Line 341:
"mouseover on the curve"→"mouseover on the curve"- Line 350:
"mouseout on the curve"→"mouseout on the curve"- Line 359:
"mouseenter on the curve"→"mouseenter on the curve"- Line 368:
"mouseleave on the curve"→"mouseleave on the curve"- 'en-US': 'The customized event handler of click on the curve', + 'en-US': 'The customized event handler of click on the curve',Apply the same fix to all affected lines (314, 323, 332, 341, 350, 359, 368).
src/shape/Curve.tsx (2)
223-231: Fix punctuation in d3 types explanatory commentThe sentence
* However. d3 types are mutable, but we can pretend that they are not, and we can pretendreads oddly with “However.” as a standalone fragment. Switching to a comma improves readability:
- * However. d3 types are mutable, but we can pretend that they are not, and we can pretend + * However, d3 types are mutable, but we can pretend that they are not, and we can pretend
27-35: Event typing structure is correct; fix minor grammar and punctuation issues in commentsThe recursive relationship between
CurveMouseEventHandler,Props, andCurveMouseEventsis a valid TypeScript pattern and should compile without issues. Event handler typing and the use ofsvgPropertiesNoEvents(props)andadaptEventHandlers(props)are properly implemented.Fix these documentation issues:
- Line 107: "can effect" → "can affect"
- Line 110: "will specified" → "will specify"
- Line 228: "However." → "However," (period should be comma)
🧹 Nitpick comments (2)
omnidoc/cross-component-prop-comments.spec.ts (1)
79-82: Minor cleanups possible in prop-to-components bookkeepingThe updated
propToComponentsmap and guard work, but there are a couple of small simplifications you could consider:
- The
propNamefield stored in each entry is identical to the map key, so thefilter(({ propName: p }) => p === propName)when reading back is redundant; you can likely just use the full array frompropToComponents.get(propName).- If you ever expect dynamic changes to how
sharedPropsWithCommentsis built, you might future‑proof the guard with an empty‑array check as well (e.g. throw when!components?.length), though it’s not required by the current logic.Also applies to: 91-99, 111-114
www/src/docs/api/Curve.ts (1)
11-12: Curve JSDoc improvements are helpful; tweak grammar if you likeThe updated English descriptions for
type,layout,baseLine, andconnectNullsread more clearly and should better explain the behavior. One small optional polish: in thelayoutdescription, “this option will specified the type of baseline” should be “will specify the type of baseline”, and “effect” could be “affect”.Also applies to: 40-43, 50-51, 61-62
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
omnidoc/componentsWithInconsistentCommentsInApiDoc.ts(0 hunks)omnidoc/cross-component-prop-comments.spec.ts(4 hunks)src/cartesian/Area.tsx(4 hunks)src/component/Cell.tsx(0 hunks)src/shape/Cross.tsx(1 hunks)src/shape/Curve.tsx(3 hunks)src/util/types.ts(3 hunks)test/cartesian/Area.spec.tsx(5 hunks)www/src/docs/api/Area.ts(11 hunks)www/src/docs/api/AreaChart.ts(2 hunks)www/src/docs/api/BarChart.ts(1 hunks)www/src/docs/api/CartesianChartAPI.ts(1 hunks)www/src/docs/api/ComposedChart.ts(1 hunks)www/src/docs/api/Curve.ts(4 hunks)www/src/docs/api/FunnelChart.ts(1 hunks)www/src/docs/api/LineChart.ts(1 hunks)www/src/docs/api/ScatterChart.ts(1 hunks)
💤 Files with no reviewable changes (2)
- src/component/Cell.tsx
- omnidoc/componentsWithInconsistentCommentsInApiDoc.ts
🧰 Additional context used
📓 Path-based instructions (8)
src/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (DEVELOPING.md)
Run ESLint and Prettier on the codebase using
npm run lint
Files:
src/shape/Cross.tsxsrc/util/types.tssrc/cartesian/Area.tsxsrc/shape/Curve.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (DEVELOPING.md)
Run type checking on the codebase using
npm run check-types
**/*.{ts,tsx}: Never useanytype (implicit or explicit) in TypeScript code
Preferunknownoveranyand refine the type in TypeScript
Type function parameters and return values explicitly in TypeScript, do not rely on implicit any or inference; exceptions are React components and trivial functions
Do not useastype assertions in TypeScript; the only exception isas const
Files:
src/shape/Cross.tsxwww/src/docs/api/CartesianChartAPI.tswww/src/docs/api/ScatterChart.tswww/src/docs/api/LineChart.tswww/src/docs/api/AreaChart.tswww/src/docs/api/FunnelChart.tsomnidoc/cross-component-prop-comments.spec.tswww/src/docs/api/Area.tssrc/util/types.tssrc/cartesian/Area.tsxwww/src/docs/api/Curve.tswww/src/docs/api/ComposedChart.tssrc/shape/Curve.tsxtest/cartesian/Area.spec.tsxwww/src/docs/api/BarChart.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Ensure code lints by running
npm run lintand follows Airbnb's Style Guide
Files:
src/shape/Cross.tsxwww/src/docs/api/CartesianChartAPI.tswww/src/docs/api/ScatterChart.tswww/src/docs/api/LineChart.tswww/src/docs/api/AreaChart.tswww/src/docs/api/FunnelChart.tsomnidoc/cross-component-prop-comments.spec.tswww/src/docs/api/Area.tssrc/util/types.tssrc/cartesian/Area.tsxwww/src/docs/api/Curve.tswww/src/docs/api/ComposedChart.tssrc/shape/Curve.tsxtest/cartesian/Area.spec.tsxwww/src/docs/api/BarChart.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not hardcode any strings or formatting choices in library code; users should provide localized strings as desired
Files:
src/shape/Cross.tsxsrc/util/types.tssrc/cartesian/Area.tsxsrc/shape/Curve.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
When running unit tests, prefer to run a single test file using
npm run test -- path/to/TestFile.spec.tsxrather than running all tests withnpm test
Files:
omnidoc/cross-component-prop-comments.spec.tstest/cartesian/Area.spec.tsx
{test,www/test}/**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (DEVELOPING.md)
Write unit tests in the
testorwww/testdirectories with.spec.tsxfile extension
Files:
test/cartesian/Area.spec.tsx
test/**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Aim for 100% unit test code coverage when writing new code
Files:
test/cartesian/Area.spec.tsx
test/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (test/README.md)
test/**/*.{test,spec}.{ts,tsx}: Aim for 100% unit test code coverage when writing new code
Prefer to use thecreateSelectorTestCasehelper function when writing or modifying tests
Use theexpectLastCalledWithhelper function instead ofexpect(spy).toHaveBeenLastCalledWith(...)for better typing and autocompletion
Verify the number of selector calls using the spy object fromcreateSelectorTestCaseto spot unnecessary re-renders and improve performance
MockgetBoundingClientRectin tests using the helper function provided intest/helper/MockGetBoundingClientRect.ts
Usevi.useFakeTimers()in all tests due to Redux autoBatchEnhancer dependency on timers andrequestAnimationFrame
Callvi.runOnlyPendingTimers()to advance timers after renders when not usingcreateSelectorTestCasehelper, and avoidvi.runAllTimers()to prevent infinite loops
UseuserEvent.setup({ advanceTimers: vi.runOnlyPendingTimers })or theuserEventSetuphelper function fromtest/helper/userEventSetup.tswhen creating userEvent instances
When testing tooltips on hover, usevi.runOnlyPendingTimers()after eachuserEvent.hover()call or use theshowTooltiphelper function fromtooltipTestHelpers.tsto account for requestAnimationFrame delays
Files:
test/cartesian/Area.spec.tsx
🧠 Learnings (14)
📓 Common learnings
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6659
File: www/src/components/GuideView/Performance/index.tsx:232-250
Timestamp: 2025-11-19T14:08:01.728Z
Learning: In Recharts version 3.4.2, object-as-prop optimizations were introduced to reduce unnecessary re-renders when new object references are passed as props. This affects the recommendation for the `react-perf/jsx-no-new-object-as-prop` ESLint rule.
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6669
File: www/src/docs/exampleComponents/ScatterChart/ScatterChartWithLabels.tsx:2-2
Timestamp: 2025-11-23T13:30:10.395Z
Learning: The `TooltipIndex` type from recharts is defined in `src/state/tooltipSlice.ts` but is not currently exported from the public API entry points. It should not be imported from `recharts/types/state/tooltipSlice` as this is an internal implementation path. An ESLint rule is needed to prevent regressions.
📚 Learning: 2025-11-23T13:30:10.395Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6669
File: www/src/docs/exampleComponents/ScatterChart/ScatterChartWithLabels.tsx:2-2
Timestamp: 2025-11-23T13:30:10.395Z
Learning: The `TooltipIndex` type from recharts is defined in `src/state/tooltipSlice.ts` but is not currently exported from the public API entry points. It should not be imported from `recharts/types/state/tooltipSlice` as this is an internal implementation path. An ESLint rule is needed to prevent regressions.
Applied to files:
www/src/docs/api/CartesianChartAPI.tssrc/util/types.tssrc/shape/Curve.tsx
📚 Learning: 2025-11-19T14:08:01.728Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6659
File: www/src/components/GuideView/Performance/index.tsx:232-250
Timestamp: 2025-11-19T14:08:01.728Z
Learning: In Recharts version 3.4.2, object-as-prop optimizations were introduced to reduce unnecessary re-renders when new object references are passed as props. This affects the recommendation for the `react-perf/jsx-no-new-object-as-prop` ESLint rule.
Applied to files:
www/src/docs/api/LineChart.tswww/src/docs/api/AreaChart.ts
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Mock `getBoundingClientRect` in tests using the helper function provided in `test/helper/MockGetBoundingClientRect.ts`
Applied to files:
test/cartesian/Area.spec.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Use the `expectLastCalledWith` helper function instead of `expect(spy).toHaveBeenLastCalledWith(...)` for better typing and autocompletion
Applied to files:
test/cartesian/Area.spec.tsx
📚 Learning: 2025-11-25T01:22:59.729Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to test/component/**/*.spec.tsx : Use React Testing Library for testing component interactions and behavior upon rendering
Applied to files:
test/cartesian/Area.spec.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Verify the number of selector calls using the spy object from `createSelectorTestCase` to spot unnecessary re-renders and improve performance
Applied to files:
test/cartesian/Area.spec.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Use `vi.useFakeTimers()` in all tests due to Redux autoBatchEnhancer dependency on timers and `requestAnimationFrame`
Applied to files:
test/cartesian/Area.spec.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : When testing tooltips on hover, use `vi.runOnlyPendingTimers()` after each `userEvent.hover()` call or use the `showTooltip` helper function from `tooltipTestHelpers.ts` to account for requestAnimationFrame delays
Applied to files:
test/cartesian/Area.spec.tsx
📚 Learning: 2025-11-25T01:22:59.729Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.729Z
Learning: Applies to storybook/stories/**/*.stories.tsx : Use Storybook for smoke tests and add play functions with assertions for actual tests
Applied to files:
test/cartesian/Area.spec.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Call `vi.runOnlyPendingTimers()` to advance timers after renders when not using `createSelectorTestCase` helper, and avoid `vi.runAllTimers()` to prevent infinite loops
Applied to files:
test/cartesian/Area.spec.tsx
📚 Learning: 2025-11-25T01:22:48.289Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-11-25T01:22:48.289Z
Learning: Applies to test-vr/**/*.spec.{ts,tsx} : Visual regression tests should be placed in the `test-vr` directory and use Playwright for testing
Applied to files:
test/cartesian/Area.spec.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Prefer to use the `createSelectorTestCase` helper function when writing or modifying tests
Applied to files:
test/cartesian/Area.spec.tsx
📚 Learning: 2025-11-25T01:23:08.250Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.250Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Use `userEvent.setup({ advanceTimers: vi.runOnlyPendingTimers })` or the `userEventSetup` helper function from `test/helper/userEventSetup.ts` when creating userEvent instances
Applied to files:
test/cartesian/Area.spec.tsx
🧬 Code graph analysis (9)
www/src/docs/api/CartesianChartAPI.ts (1)
www/src/docs/api/types.ts (1)
ApiDoc(20-26)
www/src/docs/api/ScatterChart.ts (1)
www/src/docs/api/CartesianChartAPI.ts (1)
baseCartesianChartAPIProps(3-138)
www/src/docs/api/LineChart.ts (1)
www/src/docs/api/CartesianChartAPI.ts (1)
baseCartesianChartAPIProps(3-138)
www/src/docs/api/AreaChart.ts (1)
www/src/docs/api/CartesianChartAPI.ts (2)
baseCartesianChartAPIProps(3-138)stackedChartAPIProps(140-177)
www/src/docs/api/FunnelChart.ts (1)
www/src/docs/api/CartesianChartAPI.ts (1)
baseCartesianChartAPIProps(3-138)
src/cartesian/Area.tsx (1)
src/util/types.ts (2)
CartesianLayout(50-50)NullableCoordinate(93-96)
www/src/docs/api/ComposedChart.ts (2)
www/src/docs/api/types.ts (1)
ApiDoc(20-26)www/src/docs/api/CartesianChartAPI.ts (2)
baseCartesianChartAPIProps(3-138)stackedChartAPIProps(140-177)
src/shape/Curve.tsx (1)
src/util/types.ts (4)
LayoutType(57-57)NullableCoordinate(93-96)RechartsMouseEventHandler(181-181)PresentationAttributesWithProps(61-63)
test/cartesian/Area.spec.tsx (2)
test/helper/expectLastCalledWith.ts (1)
expectLastCalledWith(14-34)test/helper/userEventSetup.ts (1)
userEventSetup(13-17)
⏰ 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 (14)
src/shape/Cross.tsx (1)
11-34: Cross props JSDoc matches component semanticsThe new comments for
x,y,width,height,top, andleftcorrectly describe howCrossinterprets these values (center lines vs bounding box top‑left), which should make the API clearer in generated docs. If you ever touch this again, you might consider “bounding box” instead of “boundary box” for familiarity, but it’s not essential.www/src/docs/api/CartesianChartAPI.ts (1)
3-138: Shared Cartesian/stacked chart prop definitions look solid
baseCartesianChartAPIPropsandstackedChartAPIPropsare well-typed asApiDoc['props']and nicely centralize layout/sync/size/data/margin and stacking props. This should reduce drift across individual chart API files and keep omnidoc checks simpler.omnidoc/cross-component-prop-comments.spec.ts (1)
33-75: Multi‑component exceptions and new Cross/Curve cases make senseSwitching
CommentSimilarityExceptiontocomponents: ReadonlyArray<string>and updating the call site toex.components.includes(component)is a good fit for shared rules. The new exceptions forCross(x,y,width,height) andCurve.layoutalso reflect their genuinely different semantics, so skipping them from the similarity check is justified.www/src/docs/api/FunnelChart.ts (1)
1-6: FunnelChart props correctly delegate to shared Cartesian baseImporting
baseCartesianChartAPIPropsand wiringFunnelChartAPI.propsto it keeps the FunnelChart docs aligned with other cartesian charts and removes duplicated prop metadata.www/src/docs/api/ScatterChart.ts (1)
1-6: ScatterChartAPI now correctly reuses shared Cartesian propsUsing
baseCartesianChartAPIPropsforScatterChartAPI.propsis consistent with the other cartesian charts and will help keep the ScatterChart docs in sync with the shared chart surface.www/src/docs/api/ComposedChart.ts (1)
2-10: ComposedChart props composition aligns with shared Cartesian/stacked APIsSpreading
...baseCartesianChartAPIPropsand...stackedChartAPIPropsbefore the ComposedChart‑specific props (barCategoryGap,barGap,barSize,baseValue) keeps this API doc DRY and consistent with the other cartesian/stacked charts, while preserving chart‑specific options.test/cartesian/Area.spec.tsx (1)
2-4: Typed Area event tests with CurveProps improve safetyImporting
CurvePropsfrom the public../../srcentry and typing the event handlers as(curveProps: CurveProps, e: React.MouseEvent<SVGPathElement> | React.TouchEvent<SVGPathElement>) => voidmocks gives you strongly‑typed assertions on the curve props passed to Area’s mouse/touch handlers. TheexpectLastCalledWithchecks against theCurvePropspayloads (includingpoints) will make regressions in the Area–Curve integration much easier to spot. This also aligns with the prior learning about avoiding imports from internal type paths by using public exports instead. Based on learnings, this is a good direction.Also applies to: 191-261, 263-304, 326-360
www/src/docs/api/LineChart.ts (1)
1-5: LGTM! Clean refactor to use shared base props.LineChart correctly uses only
baseCartesianChartAPIPropswithout the stacked chart props, which aligns with LineChart's behavior (lines don't stack like Area or Bar).www/src/docs/api/AreaChart.ts (1)
1-22: LGTM! Properly composed props for AreaChart.AreaChart correctly combines:
baseCartesianChartAPIPropsfor common chart propsstackedChartAPIPropsfor stacking supportbaseValueas an AreaChart-specific propThis reduces duplication while maintaining the complete API surface.
www/src/docs/api/Area.ts (1)
13-13: Good documentation improvement.The updated description for the
typeprop now explicitly mentions custom interpolation function support, which better reflects the actual capability.src/util/types.ts (2)
181-181: LGTM! Good rename to avoid naming conflicts.Renaming from
MouseEventHandlertoRechartsMouseEventHandlerprevents confusion with React's built-inMouseEventHandlertype. The export makes this type available for consumers who need to type their event handlers.
292-327: Consistent type updates across all mouse event handlers.All mouse events in
DOMAttributesWithPropsnow consistently useRechartsMouseEventHandler<P, T>, maintaining type safety while using the renamed type.www/src/docs/api/BarChart.ts (1)
1-8: LGTM! Properly composed props for BarChart.BarChart correctly combines shared Cartesian and stacked chart props while retaining its specific props (
barCategoryGap,barGap,barSize,maxBarSize). This aligns with the refactoring pattern used across other chart API docs.src/cartesian/Area.tsx (1)
80-122: Confirm whether Area's mouse event handlers should receive Curve props or Area-specific propsThe current type structure passes Curve's mouse event handlers through to Area's public API:
AreaSvgProps = Omit<CurveProps, 'type' | 'points' | 'ref' | 'layout'>(whereCurvePropsis an alias forCurve.Props)Curve.PropsincludesCurveMouseEvents, which defines handlers likeonClick?: CurveMouseEventHandlerCurveMouseEventHandler = RechartsMouseEventHandler<Props, SVGPathElement>(wherePropsisCurve.Props)This means:
- External
Areaprops inherit Curve's mouse event handlers (preserving the SVG/Curve-level event contract)- Runtime behavior is correct: events route through
svgPropertiesAndEvents()→StaticArea→Curve- TypeScript types show that
<Area onClick={handler}>handlers receiveCurve.Props, notArea.PropsUsers calling
<Area onClick={(props) => ...}>will see type hints for Curve's props (which lackdataKey,stackId, etc.), even though the runtime component supports Area-specific props in other contexts. This is intentional (binding handlers to the underlying rendered path), but differs from Bar's pattern of defining separate event types.Verify this matches the intended public API, or consider defining an
AreaMouseEventstype (similar to Bar's approach) if handlers should surface Area-specific data.
| interface CurveProps { | ||
| className?: string; | ||
| /** | ||
| * The interpolation type of curve. Allows custom interpolation function. | ||
| * | ||
| * @defaultValue linear | ||
| */ | ||
| type?: CurveType; | ||
| /** | ||
| * The option can effect the interpolation algorithm when the props type is | ||
| * set to be 'monotone'. Meanwhile, this option will specified the type of baseline when the curve is closed. | ||
| */ | ||
| layout?: LayoutType; | ||
| /** | ||
| * The value which can describe the line. | ||
| */ | ||
| baseLine?: number | ReadonlyArray<NullableCoordinate>; | ||
| /** | ||
| * The coordinates of all the points in the curve. | ||
| */ | ||
| points?: ReadonlyArray<NullableCoordinate>; | ||
| /** | ||
| * Whether to connect the curve across null points. | ||
| * | ||
| * @defaultValue false | ||
| */ | ||
| connectNulls?: boolean; | ||
| path?: string; | ||
| pathRef?: Ref<SVGPathElement>; | ||
| } |
There was a problem hiding this comment.
Tighten Curve prop JSDoc wording for layout, baseLine, points, and connectNulls
The updated comments are clearer overall, but there are a couple of small grammar issues that are worth fixing since this PR focuses on docs:
- “The option can effect the interpolation algorithm…” → “This option can affect the interpolation algorithm…”
- “Meanwhile, this option will specified the type of baseline…” → “…will specify the type of baseline…”
- Consider explicitly referencing the
typeprop instead of “the props type”.
A more polished version could be:
- /**
- * The option can effect the interpolation algorithm when the props type is
- * set to be 'monotone'. Meanwhile, this option will specified the type of baseline when the curve is closed.
- */
+ /**
+ * This option affects the interpolation algorithm when the `type` prop is
+ * set to 'monotone'. It also specifies the type of baseline when the curve is closed.
+ */🤖 Prompt for AI Agents
In src/shape/Curve.tsx around lines 100 to 129, the JSDoc for props layout,
baseLine, points, and connectNulls contains minor grammar and clarity issues;
update wording to: use "This option can affect the interpolation algorithm"
instead of "The option can effect...", change "will specified" to "will
specify", explicitly reference the type prop as "`type`" instead of "the props
type", and tighten descriptions for baseLine and points to be concise and
grammatical (e.g., "The baseline value or array of coordinates" and "The
coordinates of the curve points") while keeping connectNulls default note
intact.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6735 +/- ##
==========================================
- Coverage 94.02% 93.92% -0.11%
==========================================
Files 500 501 +1
Lines 42685 41981 -704
Branches 4917 4917
==========================================
- Hits 40136 39432 -704
Misses 2544 2544
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
www/src/docs/api/Area.ts (1)
1-7: Excellent refactoring to eliminate duplication.Using
sharedCurveAPIPropsfromCurve.tseliminates duplicate prop definitions fortype,baseLine,connectNulls, and all mouse event handlers. This follows the DRY principle and ensures consistency between Area and Curve API documentation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/cartesian/Area.tsx(4 hunks)src/shape/Curve.tsx(3 hunks)www/src/docs/api/Area.ts(1 hunks)www/src/docs/api/CartesianChartAPI.ts(1 hunks)www/src/docs/api/Curve.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- www/src/docs/api/CartesianChartAPI.ts
- src/cartesian/Area.tsx
🧰 Additional context used
📓 Path-based instructions (4)
src/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (DEVELOPING.md)
Run ESLint and Prettier on the codebase using
npm run lint
Files:
src/shape/Curve.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (DEVELOPING.md)
Run type checking on the codebase using
npm run check-types
**/*.{ts,tsx}: Never useanytype (implicit or explicit) in TypeScript code
Preferunknownoveranyand refine the type in TypeScript
Type function parameters and return values explicitly in TypeScript, do not rely on implicit any or inference; exceptions are React components and trivial functions
Do not useastype assertions in TypeScript; the only exception isas const
Files:
src/shape/Curve.tsxwww/src/docs/api/Curve.tswww/src/docs/api/Area.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Ensure code lints by running
npm run lintand follows Airbnb's Style Guide
Files:
src/shape/Curve.tsxwww/src/docs/api/Curve.tswww/src/docs/api/Area.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not hardcode any strings or formatting choices in library code; users should provide localized strings as desired
Files:
src/shape/Curve.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6659
File: www/src/components/GuideView/Performance/index.tsx:232-250
Timestamp: 2025-11-19T14:08:01.728Z
Learning: In Recharts version 3.4.2, object-as-prop optimizations were introduced to reduce unnecessary re-renders when new object references are passed as props. This affects the recommendation for the `react-perf/jsx-no-new-object-as-prop` ESLint rule.
📚 Learning: 2025-11-23T13:30:10.395Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6669
File: www/src/docs/exampleComponents/ScatterChart/ScatterChartWithLabels.tsx:2-2
Timestamp: 2025-11-23T13:30:10.395Z
Learning: The `TooltipIndex` type from recharts is defined in `src/state/tooltipSlice.ts` but is not currently exported from the public API entry points. It should not be imported from `recharts/types/state/tooltipSlice` as this is an internal implementation path. An ESLint rule is needed to prevent regressions.
Applied to files:
src/shape/Curve.tsx
🧬 Code graph analysis (3)
src/shape/Curve.tsx (1)
src/util/types.ts (3)
NullableCoordinate(93-96)RechartsMouseEventHandler(181-181)PresentationAttributesWithProps(61-63)
www/src/docs/api/Curve.ts (1)
www/src/docs/api/types.ts (1)
ApiDoc(20-26)
www/src/docs/api/Area.ts (2)
www/src/docs/api/types.ts (1)
ApiDoc(20-26)www/src/docs/api/Curve.ts (1)
sharedCurveAPIProps(4-132)
⏰ 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 (3)
src/shape/Curve.tsx (2)
100-131: JSDoc improvements look good; past review feedback addressed.The JSDoc comments for
type,layout,baseLine,points, andconnectNullsare now clear and grammatically correct. The past review's grammar concerns ("can effect" → "can affect", "will specified" → "will specify") have been resolved, and the explicit reference to thetypeprop has been added.
133-185: Mouse event handler typing pattern is well-structured.The introduction of
CurveMouseEventHandlerusingRechartsMouseEventHandler<Props, SVGPathElement>and the separateCurveMouseEventsinterface provides proper typing for mouse events. ThePropstype correctly usesOmitto exclude conflicting props before unioning with the properly-typedCurveMouseEventsandCurveProps, ensuring event handlers receive the full Props including themselves. Noanytypes are present, types are explicit and properly named, and the circular reference pattern is handled correctly.www/src/docs/api/Curve.ts (1)
1-159: Well-designed shared props pattern.Extracting
sharedCurveAPIPropsprovides a clean way for Area to inherit common Curve properties while allowingCurveAPIto add Curve-specific props (layout,points). The comment on Line 3 clearly documents the intent. This pattern improves maintainability and ensures consistency across related components.
Description
Fixing comments. Plus I discovered that in previous PR I made a mistake in Area mouse props so I fixed that too.
Related Issue
#6069
Summary by CodeRabbit
Breaking Changes
Documentation
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.