Skip to content

[Omnidoc]: Fixing prop comments in Brush, CartesianGrid #6729

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

[Omnidoc]: Fixing prop comments in Brush, CartesianGrid #6729
ckifer merged 8 commits intomainfrom
omnidoc

Conversation

@PavelVanecek
Copy link
Collaborator

@PavelVanecek PavelVanecek commented Dec 3, 2025

Description

Also I found that the project reader was ignoring components that were exported as a function so I fixed that too.

Summary by CodeRabbit

  • New Features

    • Added and expanded many public props (Brush sizing/positioning, CartesianGrid fill/stroke, ReferenceArea children, AxisId usage, zIndex on multiple components, polar/radial props, and chart bar sizing/width controls).
  • Documentation

    • Clarified and tightened numerous prop descriptions and defaults across API docs; removed one ReferenceArea API entry.
  • Tests

    • Updated test expectations for new/default props and introduced/refactored tests around defaults and cross-component doc consistency.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

Walkthrough

Refactors omnidoc tests by extracting/adding normalization and diff utilities, simplifies some omnidoc suites, and reworks public-component discovery; expands and documents multiple cartesian and polar component props (including AxisId and zIndex additions), exports an SVG props type, and updates related docs and tests.

Changes

Cohort / File(s) Summary
Omnidoc tests & utilities
omnidoc/omnidoc.spec.ts, omnidoc/confusing-defaults.spec.ts, omnidoc/cross-component-prop-comments.spec.ts, omnidoc/componentsWithInconsistentCommentsInApiDoc.ts, omnidoc/util/*, omnidoc/readProject.spec.ts, omnidoc/readProject.ts
Reworked omnidoc tests: replaced in-file normalization/similarity code with imported normalizeText, coloredDiff, and calculateSimilarityScore; added componentsWithInconsistentCommentsInApiDoc module; added new test files (confusing-defaults, cross-component-prop-comments); broadened public symbol discovery to include Function symbols; adjusted expectations to include Brush.
Axis identifier typing
src/cartesian/Area.tsx, src/cartesian/Bar.tsx
Replaced xAxisId/yAxisId types from `string
Brush public API additions
src/cartesian/Brush.tsx, www/src/docs/api/Brush.ts
Added many optional Brush props (x, y, width, height, travellerWidth, gap, dataKey, startIndex, endIndex, tickFormatter, onChange) and updated Brush docs.
CartesianGrid props & docs
src/cartesian/CartesianGrid.tsx, www/src/docs/api/CartesianGrid.ts
Added fill, fillOpacity, strokeDasharray to CartesianGridProps; clarified docs and made xAxisId/yAxisId non-optional in generated API doc.
Reference primitives & z-index props
src/cartesian/ReferenceArea.tsx, src/cartesian/ReferenceDot.tsx, src/cartesian/ReferenceLine.tsx, www/src/docs/api/ReferenceArea.ts
ReferenceArea: added children? and changed exported Props to omit positional SVG props; ReferenceDot/ReferenceLine: added zIndex? props; removed viewBox doc entry for ReferenceArea.
Polar components & defaults
src/polar/PolarRadiusAxis.tsx, src/polar/RadialBar.tsx, src/polar/defaultPolarRadiusAxisProps.tsx
Added allowDecimals? and zIndex? to PolarRadiusAxisProps; added zIndex? to RadialBar internals; added hide: false to defaultPolarRadiusAxisProps.
Utility types & chart props
src/util/svgPropertiesAndEvents.ts, src/util/types.ts
Exported SVGPropsAndEvents type; added barCategoryGap, barGap, barSize, maxBarSize, and width to CartesianChartProps.
Tooltip defaults & tests
src/component/Tooltip.tsx, test/component/Tooltip/Tooltip.content.spec.tsx
Set includeHidden: false in defaultTooltipProps; updated tests/snapshots to include includeHidden: false.
Docs wording updates
www/src/docs/api/AreaChart.ts, www/src/docs/api/BarChart.ts, www/src/docs/api/Brush.ts
Clarified width/height accept percent strings and refined multiple prop descriptions across AreaChart and BarChart docs; minor wording tweaks in Brush docs.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas needing focused review:
    • omnidoc test changes: verify imported utilities (normalizeText, coloredDiff, calculateSimilarityScore) are used correctly and exceptions list is applied as intended.
    • readProject.ts change to include Function symbols: confirm no unintended exports appear and downstream consumers still behave.
    • AxisId typing in Area/Bar: check call sites and public API compatibility where string | number was previously accepted.
    • New Brush and CartesianGrid props: ensure runtime ignores/adds props consistently with docs and that defaults behaviour (width/height) matches implementations.
    • Docs vs types mismatch (e.g., CartesianGrid xAxisId/yAxisId non-optional in docs): verify types/runtime align with documentation.

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • ckifer

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title is partially related to the changeset. It mentions fixing prop comments in Brush and CartesianGrid, which are indeed part of the changes, but the PR scope is significantly broader, including changes to many other components, utility functions, test files, and a fix to the project reader that now includes function exports. Revise the title to reflect the full scope of changes, such as: '[Omnidoc]: Refactor tests and fix prop comments across components' or provide a more comprehensive title capturing the broader changes.
Description check ⚠️ Warning The PR description is incomplete and lacks required sections. It mentions fixing prop comments and an issue with function exports, but is missing key sections from the template: Related Issue, Motivation and Context, How Has This Been Tested, Types of changes, and the Checklist. The description is too brief and vague to fully understand the scope and rationale. Complete the PR description by filling out all required template sections, including Related Issue link, detailed Motivation and Context, comprehensive testing description, Types of changes checkboxes, and the Checklist items.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ 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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
test/component/Tooltip/Tooltip.content.spec.tsx (1)

34-75: Add vi.useFakeTimers() to the beforeEach block

The test expectations for includeHidden: false are correct and align with production defaults. However, the test file is missing vi.useFakeTimers() setup required per the coding guidelines. Add it to beforeEach:

  beforeEach(() => {
    vi.useFakeTimers();
    mockGetBoundingClientRect({ width: 100, height: 100 });
    spy.mockClear();
  });

This ensures that Redux autoBatching timers and requestAnimationFrame calls used by vi.runOnlyPendingTimers() in createSelectorTestCase and showTooltip work correctly.

www/src/docs/api/BarChart.ts (1)

140-150: Update stackOffset docs to include the 'positive' type

The stackOffset type union here lists 5 values ('expand' | 'none' | 'wiggle' | 'silhouette' | 'sign'), but the description incorrectly claims "four types". More importantly, the actual StackOffsetType in src/util/types.ts includes 'positive' as a sixth supported value, which is already tested and used with BarChart (see test/cartesian/YAxis/YAxis.spec.tsx).

Extend the type union to 'expand' | 'none' | 'wiggle' | 'silhouette' | 'sign' | 'positive' and update the description to reflect all six supported types.

🧹 Nitpick comments (5)
src/cartesian/ReferenceDot.tsx (1)

22-61: JSDoc defaults and new zIndex prop look consistent; consider avoiding hard‑coded 600

The switch to @defaultValue and the added zIndex prop fit the existing referenceDotDefaultProps/ZIndexLayer pattern. One minor maintainability nit: the comment hard‑codes 600 while the runtime default comes from DefaultZIndexes.scatter. If that constant ever changes, the documented default will drift. Consider either referencing the constant name in the comment or omitting the numeric value.

src/cartesian/Bar.tsx (1)

86-87: Align internal axis ID types with the new AxisId alias

Using AxisId for BarProps.xAxisId/yAxisId is a nice centralization around the shared axis ID type. For consistency, you could also switch InternalBarProps.xAxisId/yAxisId from string | number to AxisId; it’s purely a type‑alias change and keeps internal and external props in sync.

Also applies to: 121-127

src/cartesian/Area.tsx (1)

73-74: Public AreaProps now use AxisId; consider mirroring that in internal props

Switching AreaProps.xAxisId/yAxisId to AxisId clarifies the contract and matches other components. For consistency, InternalAreaProps.xAxisId/yAxisId could also be typed as AxisId instead of string | number, which remains a no‑op at runtime but keeps the types aligned.

Also applies to: 277-283

www/src/docs/api/Brush.ts (1)

9-13: Brush prop docs match the implementation; tiny wording nit for gap

The updated descriptions for dataKey, x, y, width, height, and defaults all look aligned with how Brush works in the cartesian components, which should help omnidoc produce more accurate docs. The only minor nit is the English sentence for gap ("The data with gap of refreshing chart..."), which is a bit hard to parse; if you touch this again, consider a phrasing like “Number of data points to skip between chart refreshes.”

Also applies to: 22-24, 32-34, 42-43, 52-53, 72-75

src/cartesian/CartesianGrid.tsx (1)

181-189: Inconsistent JSDoc tag: use @defaultValue instead of @default.

The xAxisId and yAxisId props use @default 0 while other props in this file use @defaultValue (e.g., lines 96, 102, 109, 116, 122). For consistency, use @defaultValue throughout.

   /**
    * The id of XAxis which is corresponding to the data. Required when there are multiple XAxes.
-   * @default 0
+   * @defaultValue 0
    */
   xAxisId?: AxisId;
   /**
    * The id of YAxis which is corresponding to the data. Required when there are multiple YAxes.
-   * @default 0
+   * @defaultValue 0
    */
   yAxisId?: AxisId;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c3bd1a0 and f1fc5f8.

📒 Files selected for processing (24)
  • omnidoc/omnidoc.spec.ts (3 hunks)
  • omnidoc/readProject.spec.ts (1 hunks)
  • omnidoc/readProject.ts (1 hunks)
  • src/cartesian/Area.tsx (2 hunks)
  • src/cartesian/Bar.tsx (2 hunks)
  • src/cartesian/Brush.tsx (1 hunks)
  • src/cartesian/CartesianGrid.tsx (4 hunks)
  • src/cartesian/ReferenceArea.tsx (3 hunks)
  • src/cartesian/ReferenceDot.tsx (2 hunks)
  • src/cartesian/ReferenceLine.tsx (1 hunks)
  • src/cartesian/ZAxis.tsx (1 hunks)
  • src/component/Tooltip.tsx (2 hunks)
  • src/polar/PolarRadiusAxis.tsx (2 hunks)
  • src/polar/RadialBar.tsx (1 hunks)
  • src/polar/defaultPolarRadiusAxisProps.tsx (1 hunks)
  • src/util/svgPropertiesAndEvents.ts (1 hunks)
  • src/util/types.ts (4 hunks)
  • src/zIndex/ZIndexLayer.tsx (2 hunks)
  • test/component/Tooltip/Tooltip.content.spec.tsx (2 hunks)
  • www/src/docs/api/AreaChart.ts (2 hunks)
  • www/src/docs/api/BarChart.ts (7 hunks)
  • www/src/docs/api/Brush.ts (6 hunks)
  • www/src/docs/api/CartesianGrid.ts (7 hunks)
  • www/src/docs/api/ReferenceArea.ts (0 hunks)
💤 Files with no reviewable changes (1)
  • www/src/docs/api/ReferenceArea.ts
🧰 Additional context used
📓 Path-based instructions (9)
src/**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (DEVELOPING.md)

Run ESLint and Prettier on the codebase using npm run lint

Files:

  • src/cartesian/Area.tsx
  • src/cartesian/ZAxis.tsx
  • src/cartesian/Bar.tsx
  • src/cartesian/ReferenceDot.tsx
  • src/cartesian/ReferenceLine.tsx
  • src/util/svgPropertiesAndEvents.ts
  • src/polar/defaultPolarRadiusAxisProps.tsx
  • src/polar/RadialBar.tsx
  • src/polar/PolarRadiusAxis.tsx
  • src/zIndex/ZIndexLayer.tsx
  • src/cartesian/Brush.tsx
  • src/util/types.ts
  • src/cartesian/ReferenceArea.tsx
  • src/component/Tooltip.tsx
  • src/cartesian/CartesianGrid.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/cartesian/Area.tsx
  • omnidoc/readProject.spec.ts
  • src/cartesian/ZAxis.tsx
  • src/cartesian/Bar.tsx
  • src/cartesian/ReferenceDot.tsx
  • test/component/Tooltip/Tooltip.content.spec.tsx
  • src/cartesian/ReferenceLine.tsx
  • src/util/svgPropertiesAndEvents.ts
  • src/polar/defaultPolarRadiusAxisProps.tsx
  • www/src/docs/api/BarChart.ts
  • omnidoc/omnidoc.spec.ts
  • www/src/docs/api/Brush.ts
  • src/polar/RadialBar.tsx
  • src/polar/PolarRadiusAxis.tsx
  • src/zIndex/ZIndexLayer.tsx
  • omnidoc/readProject.ts
  • src/cartesian/Brush.tsx
  • src/util/types.ts
  • src/cartesian/ReferenceArea.tsx
  • src/component/Tooltip.tsx
  • www/src/docs/api/AreaChart.ts
  • www/src/docs/api/CartesianGrid.ts
  • src/cartesian/CartesianGrid.tsx
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

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

Files:

  • src/cartesian/Area.tsx
  • omnidoc/readProject.spec.ts
  • src/cartesian/ZAxis.tsx
  • src/cartesian/Bar.tsx
  • src/cartesian/ReferenceDot.tsx
  • test/component/Tooltip/Tooltip.content.spec.tsx
  • src/cartesian/ReferenceLine.tsx
  • src/util/svgPropertiesAndEvents.ts
  • src/polar/defaultPolarRadiusAxisProps.tsx
  • www/src/docs/api/BarChart.ts
  • omnidoc/omnidoc.spec.ts
  • www/src/docs/api/Brush.ts
  • src/polar/RadialBar.tsx
  • src/polar/PolarRadiusAxis.tsx
  • src/zIndex/ZIndexLayer.tsx
  • omnidoc/readProject.ts
  • src/cartesian/Brush.tsx
  • src/util/types.ts
  • src/cartesian/ReferenceArea.tsx
  • src/component/Tooltip.tsx
  • www/src/docs/api/AreaChart.ts
  • www/src/docs/api/CartesianGrid.ts
  • src/cartesian/CartesianGrid.tsx
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/cartesian/Area.tsx
  • src/cartesian/ZAxis.tsx
  • src/cartesian/Bar.tsx
  • src/cartesian/ReferenceDot.tsx
  • src/cartesian/ReferenceLine.tsx
  • src/util/svgPropertiesAndEvents.ts
  • src/polar/defaultPolarRadiusAxisProps.tsx
  • src/polar/RadialBar.tsx
  • src/polar/PolarRadiusAxis.tsx
  • src/zIndex/ZIndexLayer.tsx
  • src/cartesian/Brush.tsx
  • src/util/types.ts
  • src/cartesian/ReferenceArea.tsx
  • src/component/Tooltip.tsx
  • src/cartesian/CartesianGrid.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/readProject.spec.ts
  • test/component/Tooltip/Tooltip.content.spec.tsx
  • omnidoc/omnidoc.spec.ts
{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/component/Tooltip/Tooltip.content.spec.tsx
test/**/*.spec.{ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Aim for 100% unit test code coverage when writing new code

Files:

  • test/component/Tooltip/Tooltip.content.spec.tsx
test/component/**/*.spec.tsx

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use React Testing Library for testing component interactions and behavior upon rendering

Files:

  • test/component/Tooltip/Tooltip.content.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/component/Tooltip/Tooltip.content.spec.tsx
🧠 Learnings (9)
📚 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/cartesian/Area.tsx
  • src/cartesian/Bar.tsx
  • test/component/Tooltip/Tooltip.content.spec.tsx
  • src/util/svgPropertiesAndEvents.ts
  • src/polar/RadialBar.tsx
  • src/util/types.ts
  • src/component/Tooltip.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,www/test}/**/*.spec.{ts,tsx} : Write unit tests in the `test` or `www/test` directories with `.spec.tsx` file extension

Applied to files:

  • omnidoc/readProject.spec.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} : Verify the number of selector calls using the spy object from `createSelectorTestCase` to spot unnecessary re-renders and improve performance

Applied to files:

  • omnidoc/readProject.spec.ts
  • test/component/Tooltip/Tooltip.content.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:

  • omnidoc/readProject.spec.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} : 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/component/Tooltip/Tooltip.content.spec.tsx
  • src/component/Tooltip.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/component/Tooltip/Tooltip.content.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 : Update Storybook stories when APIs have been changed to ensure they work as expected

Applied to files:

  • omnidoc/omnidoc.spec.ts
📚 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:

  • omnidoc/omnidoc.spec.ts
📚 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:

  • src/component/Tooltip.tsx
🧬 Code graph analysis (6)
src/cartesian/Area.tsx (1)
src/state/cartesianAxisSlice.ts (1)
  • AxisId (8-8)
src/cartesian/Bar.tsx (1)
src/state/cartesianAxisSlice.ts (1)
  • AxisId (8-8)
src/cartesian/ReferenceDot.tsx (1)
src/component/Label.tsx (1)
  • ImplicitLabelType (107-114)
src/cartesian/Brush.tsx (1)
src/util/types.ts (2)
  • Padding (632-637)
  • DataKey (60-60)
src/cartesian/ReferenceArea.tsx (4)
src/cartesian/ReferenceLine.tsx (1)
  • Props (119-119)
src/cartesian/XAxis.tsx (1)
  • Props (84-84)
src/component/Text.tsx (1)
  • Props (171-171)
src/util/svgPropertiesAndEvents.ts (1)
  • SVGPropsAndEvents (12-12)
src/cartesian/CartesianGrid.tsx (1)
src/state/cartesianAxisSlice.ts (1)
  • AxisId (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 (23)
src/component/Tooltip.tsx (2)

71-77: active JSDoc now correctly documents controlled vs uncontrolled behavior

The added sentence on Line 74 about active being undefined matches the runtime behavior in Line 247 (const finalIsActive: boolean = activeFromProps ?? isActive ?? false;). Tooltip is controlled when active is set, and otherwise driven by Redux state from mouse/keyboard activity. No changes needed here.

Also applies to: 242-248


112-117: includeHidden: false is correctly set in defaultTooltipProps

The change preserves filtering semantics: Line 265 filters entries with entry.hide !== true || props.includeHidden, meaning hidden entries are excluded by default but included when includeHidden={true}. This aligns with the @defaultValue false documentation on Lines 112–117. The implementation makes the default explicit to custom content renderers and tests.

Note: Line 112 has a minor grammar issue ("will information" should be "will show information about").

src/cartesian/ZAxis.tsx (1)

55-55: LGTM: Documentation formatting update.

Minor spacing adjustment in the @DefaultValue annotation with no functional impact.

omnidoc/readProject.ts (1)

66-66: LGTM: Core fix for function-exported components.

This change correctly addresses the PR objective by including both Variable and Function exports in the component detection logic, ensuring that function-exported components are no longer ignored.

www/src/docs/api/AreaChart.ts (2)

58-58: LGTM: Documentation improvement.

Adding the percent string example clarifies the acceptable value formats for the width prop.


67-67: LGTM: Documentation improvement.

Adding the percent string example clarifies the acceptable value formats for the height prop.

src/zIndex/ZIndexLayer.tsx (2)

19-19: LGTM: Documentation clarification.

The updated JSDoc clearly explains that undefined or 0 renders in the default layer without portals, improving developer understanding.


29-32: LGTM: Documentation clarification.

The expanded JSDoc provides better clarity on rendering behavior and portal usage for different zIndex values.

src/polar/defaultPolarRadiusAxisProps.tsx (1)

11-11: LGTM: Sensible default prop addition.

Adding hide: false as a default is appropriate and maintains backward compatibility.

src/polar/PolarRadiusAxis.tsx (2)

32-37: LGTM: Well-documented prop addition.

The allowDecimals prop is properly documented and the default value of false is consistent with the corresponding entry in defaultPolarRadiusAxisProps (line 6).


55-58: JSDoc @DefaultValue is correct. DefaultZIndexes.axis equals 500, confirming the documented default value in PolarRadiusAxis.tsx is accurate.

src/cartesian/ReferenceLine.tsx (1)

107-110: Verify JSDoc @DefaultValue matches the actual default.

The JSDoc comment states @defaultValue 400, but the actual default on line 305 is DefaultZIndexes.line. Ensure that DefaultZIndexes.line equals 400 to maintain documentation accuracy.

www/src/docs/api/BarChart.ts (1)

12-14: BarChart API doc clarifications look good

The updated descriptions for layout, syncId, syncMethod, container width/height, data, margin, stackOffset, and reverseStackOrder are clearer and align with the corresponding JSDoc on CartesianChartProps. No functional concerns from these text‑only changes.

Also applies to: 21-23, 38-47, 54-54, 63-64, 72-75, 145-149, 169-171

src/util/types.ts (1)

1097-1107: CartesianChartProps JSDoc updates are consistent with the public API

The added/expanded comments for barCategoryGap, barGap, barSize, height, maxBarSize, reverseStackOrder, and width line up with the BarChart docs and the existing types (number | string / number | Percent). This should make omnidoc output and API references clearer without affecting behavior.

Also applies to: 1109-1112, 1127-1131, 1145-1147, 1149-1154, 1191-1194

omnidoc/readProject.spec.ts (1)

17-22: Including Brush in the expected public components is correct

Updating expectedComponents to include Brush aligns the test with the new reader behavior that treats function exports as public components. The negative cases list still covers hooks/helpers, so the assertion set stays focused.

src/util/svgPropertiesAndEvents.ts (1)

10-13: Exporting SVGPropsAndEvents is a safe and useful API surface addition

Making SVGPropsAndEvents<T> public reuses the same filtering logic that svgPropertiesAndEvents already enforces and enables consumers to type their own helpers consistently. Since it’s a pure type alias, there’s no behavioral change.

src/cartesian/Brush.tsx (1)

39-107: JSDoc comments for Brush props look good.

The added documentation clearly describes each prop's purpose, default behavior, and how undefined values are handled. The comments are consistent with the component's implementation and align with the PR objective of fixing prop comments.

www/src/docs/api/CartesianGrid.ts (1)

165-183: Verify intent: isOptional: false with defaultVal: 0 may confuse documentation consumers.

The xAxisId and yAxisId props are marked as isOptional: false while still having a defaultVal: 0. This combination could be confusing in generated documentation. The description clarifies these are "Required when there are multiple XAxes/YAxes", but consider if the API doc structure accurately conveys this conditional requirement.

src/cartesian/ReferenceArea.tsx (1)

49-56: Type refinements and children prop addition look good.

The updated Props type correctly omits width, height, x, and y from SVGPropsAndEvents<RectangleProps> since ReferenceArea uses x1, x2, y1, y2 for positioning instead. The explanatory comment clarifies this design decision. Adding the optional children prop enables nested content within the reference area.

omnidoc/omnidoc.spec.ts (2)

729-748: Exception entries are well-documented with clear rationale.

Each exception explains why the prop comment for Brush and CartesianGrid differs from other components (pixel-based sizing with component-specific defaults). This is the correct approach for handling intentional documentation differences.


781-819: Test refactor improves readability of cross-component prop consistency checks.

The change from test.each to describe.each with a nested test that includes component names in the test description makes test output more informative when debugging failures. The pairwise comparison logic correctly identifies inconsistencies between all component pairs sharing a prop.

src/cartesian/CartesianGrid.tsx (2)

66-72: Width and height descriptions are clear and consistent.

The updated comments properly explain that undefined values result in covering the full chart plot area, matching the behavior in the implementation (lines 456-457).


119-128: New fill and fillOpacity JSDoc comments added.

The documentation for fill and fillOpacity props is clear and matches the existing API documentation in www/src/docs/api/CartesianGrid.ts.

vitest-dev/vitest#6511 suggests that vitest 3.x will timeout if a test file takes longer than 60 seconds to complete.
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: 1

🧹 Nitpick comments (2)
omnidoc/util/coloredDiff.ts (1)

28-31: Redundant difference check.

The hasDifferences check on line 28 is redundant because line 24 already confirms normalizedExpected !== normalizedActual, which guarantees at least one character differs.

Apply this diff to remove the redundant check:

   const diff = diffChars(normalizedExpected, normalizedActual);
-  const hasDifferences = diff.some(part => part.added || part.removed);
-  if (!hasDifferences) {
-    return undefined;
-  }
   let result = '';
omnidoc/omnidoc.spec.ts (1)

296-343: Consider centralizing inconsistent component lists.

Three separate lists of components with inconsistent comments exist:

  • componentsWithInconsistentCommentsInApiDoc (centralized in a separate file)
  • componentsWithInconsistentCommentsInStorybook (lines 296-343, local to this file)
  • componentsWithInconsistentCommentsInApiDocAndStorybook (lines 378-421, local to this file)

These lists have significant overlap and create maintenance burden. Consider extracting the Storybook-related lists to separate modules similar to componentsWithInconsistentCommentsInApiDoc.ts for centralized maintenance.

Also applies to: 378-421

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 044dc85 and 667b5cc.

📒 Files selected for processing (7)
  • omnidoc/componentsWithInconsistentCommentsInApiDoc.ts (1 hunks)
  • omnidoc/confusing-defaults.spec.ts (1 hunks)
  • omnidoc/cross-component-prop-comments.spec.ts (1 hunks)
  • omnidoc/omnidoc.spec.ts (1 hunks)
  • omnidoc/util/calculateSimilarityScore.ts (1 hunks)
  • omnidoc/util/coloredDiff.ts (1 hunks)
  • omnidoc/util/normalizeText.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{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:

  • omnidoc/util/coloredDiff.ts
  • omnidoc/util/normalizeText.ts
  • omnidoc/componentsWithInconsistentCommentsInApiDoc.ts
  • omnidoc/util/calculateSimilarityScore.ts
  • omnidoc/omnidoc.spec.ts
  • omnidoc/confusing-defaults.spec.ts
  • omnidoc/cross-component-prop-comments.spec.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

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

Files:

  • omnidoc/util/coloredDiff.ts
  • omnidoc/util/normalizeText.ts
  • omnidoc/componentsWithInconsistentCommentsInApiDoc.ts
  • omnidoc/util/calculateSimilarityScore.ts
  • omnidoc/omnidoc.spec.ts
  • omnidoc/confusing-defaults.spec.ts
  • omnidoc/cross-component-prop-comments.spec.ts
**/*.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/omnidoc.spec.ts
  • omnidoc/confusing-defaults.spec.ts
  • omnidoc/cross-component-prop-comments.spec.ts
🧠 Learnings (10)
📚 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 : Update Storybook stories when APIs have been changed to ensure they work as expected

Applied to files:

  • omnidoc/omnidoc.spec.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} : Verify the number of selector calls using the spy object from `createSelectorTestCase` to spot unnecessary re-renders and improve performance

Applied to files:

  • omnidoc/omnidoc.spec.ts
  • omnidoc/cross-component-prop-comments.spec.ts
📚 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:

  • omnidoc/omnidoc.spec.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} : Aim for 100% unit test code coverage when writing new code

Applied to files:

  • omnidoc/omnidoc.spec.ts
  • omnidoc/cross-component-prop-comments.spec.ts
📚 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:

  • omnidoc/omnidoc.spec.ts
  • omnidoc/confusing-defaults.spec.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} : 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:

  • omnidoc/omnidoc.spec.ts
📚 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/**/*.spec.{ts,tsx} : Aim for 100% unit test code coverage when writing new code

Applied to files:

  • omnidoc/omnidoc.spec.ts
  • omnidoc/cross-component-prop-comments.spec.ts
📚 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,www/test}/**/*.spec.{ts,tsx} : Write unit tests in the `test` or `www/test` directories with `.spec.tsx` file extension

Applied to files:

  • omnidoc/confusing-defaults.spec.ts
📚 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 storybook/**/*.stories.{ts,tsx} : Use Storybook stories in the `storybook` directory for high-fidelity component examples that will be published and used for visual regression testing

Applied to files:

  • omnidoc/confusing-defaults.spec.ts
📚 Learning: 2025-11-25T01:23:14.977Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T01:23:14.977Z
Learning: Applies to src/**/*.{ts,tsx} : Do not hardcode any strings or formatting choices in library code; users should provide localized strings as desired

Applied to files:

  • omnidoc/confusing-defaults.spec.ts
🧬 Code graph analysis (3)
omnidoc/util/coloredDiff.ts (1)
omnidoc/util/normalizeText.ts (1)
  • normalizeText (7-12)
omnidoc/util/calculateSimilarityScore.ts (1)
omnidoc/util/normalizeText.ts (1)
  • normalizeText (7-12)
omnidoc/cross-component-prop-comments.spec.ts (4)
omnidoc/readProject.ts (1)
  • ProjectDocReader (24-444)
omnidoc/componentsWithInconsistentCommentsInApiDoc.ts (1)
  • componentsWithInconsistentCommentsInApiDoc (5-47)
omnidoc/util/calculateSimilarityScore.ts (1)
  • calculateSimilarityScore (9-55)
omnidoc/util/coloredDiff.ts (1)
  • coloredDiff (12-46)
⏰ 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 (8)
omnidoc/componentsWithInconsistentCommentsInApiDoc.ts (1)

1-47: LGTM!

The centralized list of components with inconsistent comments is well-documented as temporary and provides clear intent for test exclusions.

omnidoc/util/normalizeText.ts (1)

7-12: LGTM!

The normalization logic correctly handles edge cases (undefined, empty strings) and applies appropriate transformations for fuzzy comparison.

omnidoc/util/calculateSimilarityScore.ts (1)

9-55: LGTM!

The Levenshtein distance implementation is correct, handles edge cases properly (undefined inputs, empty strings), and returns an appropriate similarity score in the [0,1] range.

omnidoc/confusing-defaults.spec.ts (1)

1-77: LGTM!

The test suite correctly validates that default values don't use confusing string representations of primitives across all three documentation sources (project, API docs, Storybook). The parameterized tests provide good coverage.

omnidoc/cross-component-prop-comments.spec.ts (1)

7-146: LGTM!

The test suite provides comprehensive cross-component prop comment consistency validation with:

  • Configurable similarity threshold
  • Well-documented exception handling
  • Clear error reporting with diffs and similarity scores
  • Efficient pair-wise comparison logic
omnidoc/omnidoc.spec.ts (3)

1-10: LGTM!

The import reorganization correctly prioritizes vitest imports and adds the new utility imports (normalizeText, coloredDiff, componentsWithInconsistentCommentsInApiDoc).


244-264: LGTM!

The areCommentsSimilar function correctly uses normalizeText for fuzzy comparison and handles edge cases (both empty, one empty) appropriately.


267-290: LGTM!

The test correctly uses coloredDiff to detect and report comment inconsistencies between API docs and project, with proper filtering of known inconsistent components.

Comment on lines +16 to +21
if (expected === undefined) {
return `${actual}`;
}
if (actual === undefined) {
return `${expected}`;
}
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 | 🟠 Major

Early returns lack diff markers.

When one input is undefined, the function returns the non-undefined value without diff markers (+++/---/<<<), which breaks the function's contract of providing a "colored diff". For example, if expected is undefined and actual is "foo", the function returns "foo" instead of "+++foo<<<" to indicate an addition.

Apply this diff to fix the markers:

   if (expected === undefined) {
-    return `${actual}`;
+    return `+++${actual}<<<`;
   }
   if (actual === undefined) {
-    return `${expected}`;
+    return `---${expected}<<<`;
   }
📝 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.

Suggested change
if (expected === undefined) {
return `${actual}`;
}
if (actual === undefined) {
return `${expected}`;
}
if (expected === undefined) {
return `+++${actual}<<<`;
}
if (actual === undefined) {
return `---${expected}<<<`;
}
🤖 Prompt for AI Agents
In omnidoc/util/coloredDiff.ts around lines 16 to 21, the early returns for when
one value is undefined return the raw non-undefined value without diff markers;
change them so when expected === undefined you return the actual wrapped as an
addition marker ("+++" + actual + "<<<") and when actual === undefined you
return the expected wrapped as a deletion marker ("---" + expected + "<<<"),
ensuring you stringify the value the same way the rest of the function does and
preserve any existing escaping or trimming logic.

@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.02%. Comparing base (c3bd1a0) to head (667b5cc).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6729   +/-   ##
=======================================
  Coverage   94.02%   94.02%           
=======================================
  Files         500      500           
  Lines       42684    42685    +1     
  Branches     4917     4917           
=======================================
+ Hits        40135    40136    +1     
  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.

@ckifer ckifer merged commit 70b1e80 into main Dec 4, 2025
38 checks passed
@ckifer ckifer deleted the omnidoc branch December 4, 2025 05:58
@codecov
Copy link

codecov bot commented Dec 4, 2025

Bundle Report

Changes will increase total bundle size by 810 bytes (0.03%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
recharts/bundle-cjs 1.15MB 368 bytes (0.03%) ⬆️
recharts/bundle-es6 997.25kB 369 bytes (0.04%) ⬆️
recharts/bundle-umd 520.17kB 73 bytes (0.01%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: recharts/bundle-cjs

Assets Changed:

Asset Name Size Change Total Size Change (%)
component/Label.js 165 bytes 20.44kB 0.81%
component/Tooltip.js 24 bytes 8.68kB 0.28%
cartesian/ReferenceArea.js 164 bytes 7.67kB 2.19%
polar/defaultPolarRadiusAxisProps.js 15 bytes 626 bytes 2.45%
view changes for bundle: recharts/bundle-umd

Assets Changed:

Asset Name Size Change Total Size Change (%)
Recharts.js 73 bytes 520.17kB 0.01%
view changes for bundle: recharts/bundle-es6

Assets Changed:

Asset Name Size Change Total Size Change (%)
component/Label.js 165 bytes 18.57kB 0.9%
component/Tooltip.js 24 bytes 7.54kB 0.32%
cartesian/ReferenceArea.js 165 bytes 6.5kB 2.6%
polar/defaultPolarRadiusAxisProps.js 15 bytes 452 bytes 3.43%

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