Skip to content

[Omnidoc] Fix prop comments for cartesian charts, Cross and Curve#6735

Merged
ckifer merged 7 commits intomainfrom
omnidoc
Dec 4, 2025
Merged

[Omnidoc] Fix prop comments for cartesian charts, Cross and Curve#6735
ckifer merged 7 commits intomainfrom
omnidoc

Conversation

@PavelVanecek
Copy link
Collaborator

@PavelVanecek PavelVanecek commented Dec 4, 2025

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

    • Area component no longer exposes mouse event handler props or the layout prop.
  • Documentation

    • Clarified Curve/Area prop descriptions (baseline, interpolation, null handling); removed implementation-specific language.
    • Consolidated and reused shared Cartesian/Curve API prop docs across chart docs.
    • Several components removed from the inconsistent-comments list.
  • Refactor

    • Improved mouse event typing and standardized event prop handling across shapes and docs.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 4, 2025

Walkthrough

Consolidates 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

Cohort / File(s) Change Summary
Omnidoc list
omnidoc/componentsWithInconsistentCommentsInApiDoc.ts
Removes six components from the inconsistent-comments list (ComposedChart, Cross, Curve, FunnelChart, LineChart, ScatterChart).
Cross‑component comment tests
omnidoc/cross-component-prop-comments.spec.ts
Exception type changed to allow components: ReadonlyArray<string>; prop‑metadata now includes propName; matching/filtering and runtime validation updated.
Mouse event types
src/util/types.ts
Adds RechartsMouseEventHandler<P, T> (replaces previous alias), expands and replaces mouse event handler props in DOMAttributesWithProps to use the new type and include additional onMouse* (and capture) handlers.
Curve event typing
src/shape/Curve.tsx
Adds CurveMouseEventHandler and CurveMouseEvents; updates Props to omit base event props then recompose with explicit CurveMouseEvents.
Area props & internal layout
src/cartesian/Area.tsx
Removes public Area mouse event props (onClick, onMouseDown, onMouseEnter, onMouseLeave, onMouseMove, onMouseOut, onMouseOver, onMouseUp); introduces layout: CartesianLayout on InternalAreaProps; adjusts AreaSvgProps to omit layout.
Minor docs/comments
src/component/Cell.tsx, src/shape/Cross.tsx
Removes a file-level header comment from Cell.tsx; adds JSDoc to CrossProps fields in Cross.tsx (documentation-only).
Shared Curve props & docs
www/src/docs/api/Curve.ts, www/src/docs/api/Area.ts
Introduces sharedCurveAPIProps; moves CurveAPI/AreaAPI to reuse shared props; expands Curve/Area prop descriptions (layout, points, baseline, connectNulls, custom interpolation, event props adjustments).
New shared Cartesian API
www/src/docs/api/CartesianChartAPI.ts
New file exporting baseCartesianChartAPIProps and stackedChartAPIProps (detailed prop descriptor arrays).
Chart API consolidation
www/src/docs/api/AreaChart.ts, www/src/docs/api/BarChart.ts, www/src/docs/api/ComposedChart.ts, www/src/docs/api/FunnelChart.ts, www/src/docs/api/LineChart.ts, www/src/docs/api/ScatterChart.ts
Replace explicit per-file prop arrays with spreads/references to baseCartesianChartAPIProps and/or stackedChartAPIProps; retain chart-specific props where applicable.
Area tests
test/cartesian/Area.spec.tsx
Test typing and spies updated to import Mock and CurveProps; wrap handlers to forward CurveProps + event to spies and update assertions accordingly.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas needing extra attention:
    • src/util/types.ts — broad, foundational typing changes for mouse events; verify type compatibility across components.
    • src/cartesian/Area.tsx — public API removal of mouse handlers; ensure internal handling/consumers and docs align.
    • www/src/docs/api/* & new CartesianChartAPI.ts — ensure prop consolidation did not drop or mis-document props; confirm examples/defaults remain correct.
    • omnidoc/cross-component-prop-comments.spec.ts — validate test logic with multi-component exceptions and new propName metadata.
    • test/cartesian/Area.spec.tsx — ensure updated test wiring still verifies intended behavior.

Possibly related PRs

Suggested reviewers

  • ckifer

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive The description is minimal but addresses the core changes. However, it lacks detail on motivation, testing, and required template sections. Add more detail on why these changes fix the issues, how they were tested, and complete the template sections (Motivation and Context, How Has This Been Tested, Types of changes, and Checklist).
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: fixing prop comments for cartesian charts, Cross, and Curve components, and corrects Area mouse props issues.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch omnidoc

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 comment

The sentence

* However. d3 types are mutable, but we can pretend that they are not, and we can pretend

reads 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 comments

The recursive relationship between CurveMouseEventHandler, Props, and CurveMouseEvents is a valid TypeScript pattern and should compile without issues. Event handler typing and the use of svgPropertiesNoEvents(props) and adaptEventHandlers(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 bookkeeping

The updated propToComponents map and guard work, but there are a couple of small simplifications you could consider:

  • The propName field stored in each entry is identical to the map key, so the filter(({ propName: p }) => p === propName) when reading back is redundant; you can likely just use the full array from propToComponents.get(propName).
  • If you ever expect dynamic changes to how sharedPropsWithComments is 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 like

The updated English descriptions for type, layout, baseLine, and connectNulls read more clearly and should better explain the behavior. One small optional polish: in the layout description, “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

📥 Commits

Reviewing files that changed from the base of the PR and between 6aec882 and a0ae71a.

📒 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.tsx
  • src/util/types.ts
  • src/cartesian/Area.tsx
  • 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 use any type (implicit or explicit) in TypeScript code
Prefer unknown over any and 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 use as type assertions in TypeScript; the only exception is as const

Files:

  • src/shape/Cross.tsx
  • www/src/docs/api/CartesianChartAPI.ts
  • www/src/docs/api/ScatterChart.ts
  • www/src/docs/api/LineChart.ts
  • www/src/docs/api/AreaChart.ts
  • www/src/docs/api/FunnelChart.ts
  • omnidoc/cross-component-prop-comments.spec.ts
  • www/src/docs/api/Area.ts
  • src/util/types.ts
  • src/cartesian/Area.tsx
  • www/src/docs/api/Curve.ts
  • www/src/docs/api/ComposedChart.ts
  • src/shape/Curve.tsx
  • test/cartesian/Area.spec.tsx
  • www/src/docs/api/BarChart.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Ensure code lints by running npm run lint and follows Airbnb's Style Guide

Files:

  • src/shape/Cross.tsx
  • www/src/docs/api/CartesianChartAPI.ts
  • www/src/docs/api/ScatterChart.ts
  • www/src/docs/api/LineChart.ts
  • www/src/docs/api/AreaChart.ts
  • www/src/docs/api/FunnelChart.ts
  • omnidoc/cross-component-prop-comments.spec.ts
  • www/src/docs/api/Area.ts
  • src/util/types.ts
  • src/cartesian/Area.tsx
  • www/src/docs/api/Curve.ts
  • www/src/docs/api/ComposedChart.ts
  • src/shape/Curve.tsx
  • test/cartesian/Area.spec.tsx
  • www/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.tsx
  • src/util/types.ts
  • src/cartesian/Area.tsx
  • src/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.tsx rather than running all tests with npm test

Files:

  • omnidoc/cross-component-prop-comments.spec.ts
  • test/cartesian/Area.spec.tsx
{test,www/test}/**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (DEVELOPING.md)

Write unit tests in the test or www/test directories with .spec.tsx file 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 the createSelectorTestCase helper function when writing or modifying tests
Use the expectLastCalledWith helper function instead of expect(spy).toHaveBeenLastCalledWith(...) for better typing and autocompletion
Verify the number of selector calls using the spy object from createSelectorTestCase to spot unnecessary re-renders and improve performance
Mock getBoundingClientRect in tests using the helper function provided in test/helper/MockGetBoundingClientRect.ts
Use vi.useFakeTimers() in all tests due to Redux autoBatchEnhancer dependency on timers and requestAnimationFrame
Call vi.runOnlyPendingTimers() to advance timers after renders when not using createSelectorTestCase helper, and avoid vi.runAllTimers() to prevent infinite loops
Use userEvent.setup({ advanceTimers: vi.runOnlyPendingTimers }) or the userEventSetup helper function from test/helper/userEventSetup.ts when creating userEvent instances
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

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.ts
  • src/util/types.ts
  • src/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.ts
  • www/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 semantics

The new comments for x, y, width, height, top, and left correctly describe how Cross interprets 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

baseCartesianChartAPIProps and stackedChartAPIProps are well-typed as ApiDoc['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 sense

Switching CommentSimilarityException to components: ReadonlyArray<string> and updating the call site to ex.components.includes(component) is a good fit for shared rules. The new exceptions for Cross (x, y, width, height) and Curve.layout also 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 base

Importing baseCartesianChartAPIProps and wiring FunnelChartAPI.props to 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 props

Using baseCartesianChartAPIProps for ScatterChartAPI.props is 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 APIs

Spreading ...baseCartesianChartAPIProps and ...stackedChartAPIProps before 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 safety

Importing CurveProps from the public ../../src entry and typing the event handlers as (curveProps: CurveProps, e: React.MouseEvent<SVGPathElement> | React.TouchEvent<SVGPathElement>) => void mocks gives you strongly‑typed assertions on the curve props passed to Area’s mouse/touch handlers. The expectLastCalledWith checks against the CurveProps payloads (including points) 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 baseCartesianChartAPIProps without 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:

  • baseCartesianChartAPIProps for common chart props
  • stackedChartAPIProps for stacking support
  • baseValue as an AreaChart-specific prop

This 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 type prop 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 MouseEventHandler to RechartsMouseEventHandler prevents confusion with React's built-in MouseEventHandler type. 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 DOMAttributesWithProps now consistently use RechartsMouseEventHandler<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 props

The current type structure passes Curve's mouse event handlers through to Area's public API:

  • AreaSvgProps = Omit<CurveProps, 'type' | 'points' | 'ref' | 'layout'> (where CurveProps is an alias for Curve.Props)
  • Curve.Props includes CurveMouseEvents, which defines handlers like onClick?: CurveMouseEventHandler
  • CurveMouseEventHandler = RechartsMouseEventHandler<Props, SVGPathElement> (where Props is Curve.Props)

This means:

  • External Area props inherit Curve's mouse event handlers (preserving the SVG/Curve-level event contract)
  • Runtime behavior is correct: events route through svgPropertiesAndEvents()StaticAreaCurve
  • TypeScript types show that <Area onClick={handler}> handlers receive Curve.Props, not Area.Props

Users calling <Area onClick={(props) => ...}> will see type hints for Curve's props (which lack dataKey, 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 AreaMouseEvents type (similar to Bar's approach) if handlers should surface Area-specific data.

Comment on lines 100 to 129
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>;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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 type prop 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
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.92%. Comparing base (6aec882) to head (14c5231).
⚠️ Report is 2 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
www/src/docs/api/Area.ts (1)

1-7: Excellent refactoring to eliminate duplication.

Using sharedCurveAPIProps from Curve.ts eliminates duplicate prop definitions for type, 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

📥 Commits

Reviewing files that changed from the base of the PR and between a0ae71a and 14c5231.

📒 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 use any type (implicit or explicit) in TypeScript code
Prefer unknown over any and 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 use as type assertions in TypeScript; the only exception is as const

Files:

  • src/shape/Curve.tsx
  • www/src/docs/api/Curve.ts
  • www/src/docs/api/Area.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Ensure code lints by running npm run lint and follows Airbnb's Style Guide

Files:

  • src/shape/Curve.tsx
  • www/src/docs/api/Curve.ts
  • www/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, and connectNulls are 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 the type prop has been added.


133-185: Mouse event handler typing pattern is well-structured.

The introduction of CurveMouseEventHandler using RechartsMouseEventHandler<Props, SVGPathElement> and the separate CurveMouseEvents interface provides proper typing for mouse events. The Props type correctly uses Omit to exclude conflicting props before unioning with the properly-typed CurveMouseEvents and CurveProps, ensuring event handlers receive the full Props including themselves. No any types 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 sharedCurveAPIProps provides a clean way for Area to inherit common Curve properties while allowing CurveAPI to 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants