[Omnidoc] Treat comment consistency as groups, instead of exceptions#6834
Conversation
WalkthroughThis PR centralizes prop comment similarity exception rules into a new dedicated module, adds three components (Brush, Cross, Dot) to automated API documentation generation, migrates component API documentation to new files, and clarifies numerous JSDoc comments across source and documentation files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
www/src/docs/api/LegendAPI.tsx (1)
67-77: Fix theheightandwidthtype documentation to match the implementation.The
typefield at lines 68 and 195 documents these props as'string | number', but the actual TypeScript implementation inLegend.tsx(lines 110, 114) types them asnumberonly. Update the documentation to reflect that only numbers are accepted, not strings.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
omnidoc/commentSimilarityExceptions.tsomnidoc/cross-component-prop-comments.spec.tsomnidoc/generateApiDoc.tssrc/cartesian/Brush.tsxsrc/cartesian/ErrorBar.tsxsrc/cartesian/XAxis.tsxsrc/cartesian/YAxis.tsxsrc/cartesian/ZAxis.tsxsrc/chart/Sankey.tsxsrc/component/Label.tsxsrc/component/LabelList.tsxsrc/component/Legend.tsxsrc/shape/Cross.tsxsrc/shape/Dot.tsxsrc/shape/Rectangle.tsxwww/src/docs/api/Brush.tswww/src/docs/api/BrushAPI.tsxwww/src/docs/api/Cross.tswww/src/docs/api/CrossAPI.tsxwww/src/docs/api/Dot.tswww/src/docs/api/DotAPI.tsxwww/src/docs/api/ErrorBarAPI.tsxwww/src/docs/api/LabelAPI.tsxwww/src/docs/api/LabelListAPI.tsxwww/src/docs/api/LegendAPI.tsxwww/src/docs/api/RectangleAPI.tsxwww/src/docs/api/SankeyAPI.tsxwww/src/docs/api/XAxisAPI.tsxwww/src/docs/api/YAxisAPI.tsxwww/src/docs/api/ZAxisAPI.tsxwww/src/docs/api/index.ts
💤 Files with no reviewable changes (3)
- www/src/docs/api/Cross.ts
- www/src/docs/api/Dot.ts
- www/src/docs/api/Brush.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.{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 constAll imports from
rechartsmust use the public API entry point (e.g.,import { TooltipIndex } from 'recharts'). Imports from internal paths likerecharts/types/*orrecharts/src/*are not allowed and will fail the linter.
Files:
src/chart/Sankey.tsxwww/src/docs/api/LabelListAPI.tsxsrc/cartesian/ErrorBar.tsxsrc/component/Label.tsxsrc/component/LabelList.tsxwww/src/docs/api/LabelAPI.tsxomnidoc/commentSimilarityExceptions.tswww/src/docs/api/BrushAPI.tsxwww/src/docs/api/XAxisAPI.tsxsrc/cartesian/ZAxis.tsxsrc/component/Legend.tsxsrc/shape/Cross.tsxwww/src/docs/api/SankeyAPI.tsxwww/src/docs/api/DotAPI.tsxsrc/shape/Rectangle.tsxsrc/cartesian/Brush.tsxomnidoc/generateApiDoc.tswww/src/docs/api/index.tssrc/cartesian/XAxis.tsxwww/src/docs/api/LegendAPI.tsxwww/src/docs/api/CrossAPI.tsxomnidoc/cross-component-prop-comments.spec.tswww/src/docs/api/RectangleAPI.tsxsrc/cartesian/YAxis.tsxwww/src/docs/api/ZAxisAPI.tsxsrc/shape/Dot.tsxwww/src/docs/api/ErrorBarAPI.tsxwww/src/docs/api/YAxisAPI.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Ensure code lints by running
npm run lintand follows Airbnb's Style Guide
Files:
src/chart/Sankey.tsxwww/src/docs/api/LabelListAPI.tsxsrc/cartesian/ErrorBar.tsxsrc/component/Label.tsxsrc/component/LabelList.tsxwww/src/docs/api/LabelAPI.tsxomnidoc/commentSimilarityExceptions.tswww/src/docs/api/BrushAPI.tsxwww/src/docs/api/XAxisAPI.tsxsrc/cartesian/ZAxis.tsxsrc/component/Legend.tsxsrc/shape/Cross.tsxwww/src/docs/api/SankeyAPI.tsxwww/src/docs/api/DotAPI.tsxsrc/shape/Rectangle.tsxsrc/cartesian/Brush.tsxomnidoc/generateApiDoc.tswww/src/docs/api/index.tssrc/cartesian/XAxis.tsxwww/src/docs/api/LegendAPI.tsxwww/src/docs/api/CrossAPI.tsxomnidoc/cross-component-prop-comments.spec.tswww/src/docs/api/RectangleAPI.tsxsrc/cartesian/YAxis.tsxwww/src/docs/api/ZAxisAPI.tsxsrc/shape/Dot.tsxwww/src/docs/api/ErrorBarAPI.tsxwww/src/docs/api/YAxisAPI.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/chart/Sankey.tsxsrc/cartesian/ErrorBar.tsxsrc/component/Label.tsxsrc/component/LabelList.tsxsrc/cartesian/ZAxis.tsxsrc/component/Legend.tsxsrc/shape/Cross.tsxsrc/shape/Rectangle.tsxsrc/cartesian/Brush.tsxsrc/cartesian/XAxis.tsxsrc/cartesian/YAxis.tsxsrc/shape/Dot.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 testUnit tests should be placed in the
testdirectory, with some tests also allowed inwww/test. Test files follow the naming convention*.spec.tsx.
Files:
omnidoc/cross-component-prop-comments.spec.ts
🧠 Learnings (6)
📚 Learning: 2025-12-14T13:58:49.197Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6771
File: src/shape/Curve.tsx:39-40
Timestamp: 2025-12-14T13:58:49.197Z
Learning: In the recharts codebase, hooks like useAppSelector and other hooks (e.g., useChartLayout()) return undefined when used outside Redux Provider context, instead of throwing. This enables components that call these hooks, such as Curve, to operate in standalone mode by falling back to prop values. During code reviews, ensure TSX files gracefully handle undefined results from hooks and implement fallbacks (e.g., via default props or explicit prop-based values) rather than assuming the hook is always within Provider.
Applied to files:
src/chart/Sankey.tsxsrc/cartesian/ErrorBar.tsxsrc/component/Label.tsxsrc/component/LabelList.tsxsrc/cartesian/ZAxis.tsxsrc/component/Legend.tsxsrc/shape/Cross.tsxsrc/shape/Rectangle.tsxsrc/cartesian/Brush.tsxsrc/cartesian/XAxis.tsxsrc/cartesian/YAxis.tsxsrc/shape/Dot.tsx
📚 Learning: 2025-12-08T08:23:26.261Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6750
File: src/state/selectors/axisSelectors.ts:593-602
Timestamp: 2025-12-08T08:23:26.261Z
Learning: In the recharts codebase, `DataKey<any>` is an intentional exception to the "no any" rule while proper typing is being developed. This should not be flagged in reviews.
Applied to files:
omnidoc/commentSimilarityExceptions.tsomnidoc/cross-component-prop-comments.spec.ts
📚 Learning: 2025-12-26T15:59:11.254Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-12-26T15:59:11.254Z
Learning: Applies to **/*.{ts,tsx} : All imports from `recharts` must use the public API entry point (e.g., `import { TooltipIndex } from 'recharts'`). Imports from internal paths like `recharts/types/*` or `recharts/src/*` are not allowed and will fail the linter.
Applied to files:
www/src/docs/api/index.tsomnidoc/cross-component-prop-comments.spec.tssrc/shape/Dot.tsx
📚 Learning: 2025-12-16T08:12:13.355Z
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6783
File: test/util/ChartUtils.spec.tsx:15-16
Timestamp: 2025-12-16T08:12:13.355Z
Learning: In the recharts codebase, files in the `test` folder are allowed to import from internal paths (e.g., `../../src/state/cartesianAxisSlice`) and do not need to use the public API entry point (`src/index.ts`). The public API import restriction applies only to non-test code.
Applied to files:
www/src/docs/api/index.tssrc/cartesian/XAxis.tsxomnidoc/cross-component-prop-comments.spec.tssrc/shape/Dot.tsx
📚 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/index.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:
src/shape/Dot.tsx
🧬 Code graph analysis (4)
www/src/docs/api/BrushAPI.tsx (1)
www/src/docs/api/types.ts (1)
ApiDoc(21-29)
www/src/docs/api/DotAPI.tsx (1)
www/src/docs/api/types.ts (1)
ApiDoc(21-29)
www/src/docs/api/index.ts (3)
www/src/docs/api/BrushAPI.tsx (1)
BrushAPI(3-181)www/src/docs/api/CrossAPI.tsx (1)
CrossAPI(3-80)www/src/docs/api/DotAPI.tsx (1)
DotAPI(4-307)
www/src/docs/api/CrossAPI.tsx (1)
www/src/docs/api/types.ts (1)
ApiDoc(21-29)
⏰ 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 (30)
src/component/Legend.tsx (1)
107-114: LGTM! Documentation improvements enhance clarity.The JSDoc updates for
widthandheightprops now explicitly state "in pixels," which accurately reflects how React interprets numeric CSS property values. These changes align well with the corresponding API documentation updates inwww/src/docs/api/LegendAPI.tsx.src/shape/Dot.tsx (1)
10-17: LGTM! Documentation improvements clarify units.The JSDoc comments now explicitly state that coordinates are in pixels, improving clarity for API consumers.
www/src/docs/api/SankeyAPI.tsx (1)
195-195: LGTM! Typo corrected.Fixed "th y axis" to "the y axis" in the sort prop description.
src/chart/Sankey.tsx (1)
723-723: LGTM! Typo corrected.Fixed "th y axis" to "the y axis" in the sort prop JSDoc, consistent with the corresponding API documentation update.
src/cartesian/YAxis.tsx (1)
126-128: LGTM! Documentation wording improved.The JSDoc now reads "Width of the axis in pixels" with improved grammar (added "the" before "axis").
src/cartesian/XAxis.tsx (1)
65-65: LGTM! Documentation wording improved.The JSDoc now reads "Height of the axis in pixels" with improved grammar, consistent with the YAxis documentation update.
www/src/docs/api/XAxisAPI.tsx (1)
170-170: LGTM! Documentation wording improved.The API documentation now reads "Height of the axis in pixels", consistent with the XAxis component's JSDoc update.
www/src/docs/api/RectangleAPI.tsx (1)
18-18: LGTM! Documentation wording improved.The descriptions now read "Height of the rectangle in pixels" and "Width of the rectangle in pixels" with improved grammar (added "the" before "rectangle").
Also applies to: 49-49
src/shape/Rectangle.tsx (1)
93-93: LGTM! Documentation wording improved.The JSDoc comments now read "Width of the rectangle in pixels" and "Height of the rectangle in pixels" with improved grammar, consistent with the API documentation updates.
Also applies to: 98-98
src/cartesian/ZAxis.tsx (1)
37-46: LGTM! Clear axis type documentation.The expanded documentation effectively distinguishes between
category(discrete values with equal spacing) andnumber(continuous range with proportional spacing) axis types. This clarification will help users choose the appropriate axis type for their data.src/cartesian/ErrorBar.tsx (1)
59-65: LGTM! Excellent clarification of the width property.The updated documentation clearly distinguishes between the
widthprop (which controls the serif/end caps) and the total error bar width (which is data-driven). This addresses a common point of confusion.www/src/docs/api/ErrorBarAPI.tsx (1)
90-98: LGTM! Consistent API documentation.The API documentation properly reflects the clarified width property behavior from the source code, maintaining consistency between implementation and user-facing documentation.
www/src/docs/api/ZAxisAPI.tsx (1)
127-139: LGTM! Consistent axis type documentation.The API documentation correctly reflects the enhanced axis type descriptions from the source code, providing clear guidance on when to use
categoryversusnumberaxis types.www/src/docs/api/LabelListAPI.tsx (1)
98-102: LGTM! Clearer id property description.The updated documentation accurately describes the
idprop as a general HTML identifier, which is more straightforward than the previous SSR-specific explanation.src/component/LabelList.tsx (1)
54-56: LGTM! Consistent id documentation.The source code documentation now matches the API documentation, providing a clear and accurate description of the
idprop as a general HTML attribute.src/cartesian/Brush.tsx (3)
51-53: LGTM! Explicit pixel unit documentation.Adding "in pixels" clarifies the expected unit for the width property.
60-64: LGTM! Clear height property documentation.The explicit pixel unit specification improves clarity for the height property.
1047-1069: LGTM! Well-structured public component export.The new public
Brushexport follows the established pattern in the codebase:
- Resolves default props
- Dispatches settings to Redux state via
BrushSettingsDispatcher- Renders internal implementation via
BrushInternal- Includes helpful JSDoc with example link
This properly exposes Brush as a public API component.
src/shape/Cross.tsx (1)
12-26: LGTM! Consistent pixel unit documentation.Adding explicit "in pixels" specifications to the x, y, width, and height properties improves clarity and consistency across the component API.
src/component/Label.tsx (1)
148-151: LGTM!The updated JSDoc accurately describes the
idprop's purpose as a generic HTML id attribute, which aligns with its usage inrenderRadialLabel(line 297) where it's applied to the<path>element.www/src/docs/api/LabelAPI.tsx (1)
76-76: LGTM!The API documentation correctly reflects the updated source JSDoc for the
idprop, maintaining consistency between the component definition and its public API documentation.www/src/docs/api/YAxisAPI.tsx (1)
507-508: LGTM!The updated description is clearer and more concise. The phrase "Width of the axis in pixels" followed by the
autobehavior is well-structured.www/src/docs/api/CrossAPI.tsx (1)
1-80: LGTM!The new API documentation file follows the established pattern. The props are well-documented with clear en-US descriptions. The
ApiDoctype is correctly imported and applied.omnidoc/cross-component-prop-comments.spec.ts (1)
43-61: LGTM!The refactored exception handling is well-designed:
- The wildcard
'*'correctly skips props entirely across all components- Non-wildcard exception groups create distinct keys to segregate similarity checking
- The augmented key format
${prop} (group: ${index}: ${components})ensures uniqueness while preserving traceabilitywww/src/docs/api/DotAPI.tsx (1)
1-307: LGTM!The new API documentation file is well-structured:
- Correctly imports
Linkfromreact-routerfor internal navigation to ReferenceDot- Core props (
cx,cy,r) have clear pixel-based descriptions- The component-level description helpfully guides users to
ReferenceDotfor data-based positioningwww/src/docs/api/BrushAPI.tsx (1)
1-181: LGTM!The new API documentation is comprehensive:
- Props are well-documented with appropriate defaults (
gap: 1,height: 40,travellerWidth: 5, etc.)- The
dataKeydescription follows the established format used across other components- Component-level description clearly explains the Brush's zoom/pan functionality
- The example link to
BrushBarChartprovides helpful user guidanceomnidoc/generateApiDoc.ts (1)
28-31: LGTM!The new components are correctly added to the automation list in alphabetical order, and the corresponding mappings for Brush, Cross, and Dot in
www/src/docs/api/index.tsare in place (imports on lines 52–54, mappings on lines 77, 102, and 104).www/src/docs/api/index.ts (1)
77-77: LGTM!The
allApiDocsmappings correctly reference the imported API documentation objects and are consistent with the existing structure.Also applies to: 102-102, 104-104
omnidoc/commentSimilarityExceptions.ts (2)
1-17: LGTM!The
CommentSimilarityGrouptype is well-defined with clear documentation. Good use ofReadonlyArrayfor immutability and comprehensive JSDoc comments explaining the structure and the special'*'wildcard behavior.
19-230: LGTM!The
commentSimilarityExceptionsarray is well-structured and comprehensive. Each exception group includes clear reasoning, and the centralized approach (as described in the PR objectives) improves maintainability. The data structure correctly allows components to appear in multiple groups with different props, which is necessary for handling the various contextual differences across components.
| import { BrushAPI } from './BrushAPI.tsx'; | ||
| import { CrossAPI } from './CrossAPI.tsx'; | ||
| import { DotAPI } from './DotAPI.tsx'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove explicit .tsx extensions from imports.
These imports include .tsx extensions, which is inconsistent with all other imports in this file (lines 1-51) that omit extensions. TypeScript's module resolver should find the correct files automatically. Explicit extensions can cause issues with different build tools and linting configurations.
🔎 Proposed fix
-import { BrushAPI } from './BrushAPI.tsx';
-import { CrossAPI } from './CrossAPI.tsx';
-import { DotAPI } from './DotAPI.tsx';
+import { BrushAPI } from './BrushAPI';
+import { CrossAPI } from './CrossAPI';
+import { DotAPI } from './DotAPI';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { BrushAPI } from './BrushAPI.tsx'; | |
| import { CrossAPI } from './CrossAPI.tsx'; | |
| import { DotAPI } from './DotAPI.tsx'; | |
| import { BrushAPI } from './BrushAPI'; | |
| import { CrossAPI } from './CrossAPI'; | |
| import { DotAPI } from './DotAPI'; |
🤖 Prompt for AI Agents
In www/src/docs/api/index.ts around lines 52 to 54, the three import statements
explicitly include the .tsx extensions which is inconsistent with the rest of
the file and can break some build/lint setups; change those imports to omit the
.tsx extension (e.g., import { BrushAPI } from './BrushAPI'; import { CrossAPI }
from './CrossAPI'; import { DotAPI } from './DotAPI';) so TypeScript's module
resolver handles the file resolution consistently with other imports.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6834 +/- ##
==========================================
+ Coverage 93.74% 93.77% +0.03%
==========================================
Files 531 531
Lines 48713 48965 +252
Branches 5117 5117
==========================================
+ Hits 45664 45916 +252
Misses 3042 3042
Partials 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Staging Deployment Details
These deployments will remain available for 30 days. To update snapshots: Comment |
Bundle ReportChanges will increase total bundle size by 718 bytes (0.03%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-cjsAssets Changed:
view changes for bundle: recharts/bundle-es6Assets Changed:
|
1 participant
Related Issue
#6069
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.