Conversation
WalkthroughRefactors 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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 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: Addvi.useFakeTimers()to thebeforeEachblockThe test expectations for
includeHidden: falseare correct and align with production defaults. However, the test file is missingvi.useFakeTimers()setup required per the coding guidelines. Add it tobeforeEach:beforeEach(() => { vi.useFakeTimers(); mockGetBoundingClientRect({ width: 100, height: 100 }); spy.mockClear(); });This ensures that Redux autoBatching timers and
requestAnimationFramecalls used byvi.runOnlyPendingTimers()increateSelectorTestCaseandshowTooltipwork correctly.www/src/docs/api/BarChart.ts (1)
140-150: UpdatestackOffsetdocs to include the'positive'typeThe
stackOffsettype union here lists 5 values ('expand' | 'none' | 'wiggle' | 'silhouette' | 'sign'), but the description incorrectly claims "four types". More importantly, the actualStackOffsetTypeinsrc/util/types.tsincludes'positive'as a sixth supported value, which is already tested and used with BarChart (seetest/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 newzIndexprop look consistent; consider avoiding hard‑coded 600The switch to
@defaultValueand the addedzIndexprop fit the existingreferenceDotDefaultProps/ZIndexLayerpattern. One minor maintainability nit: the comment hard‑codes600while the runtime default comes fromDefaultZIndexes.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 newAxisIdaliasUsing
AxisIdforBarProps.xAxisId/yAxisIdis a nice centralization around the shared axis ID type. For consistency, you could also switchInternalBarProps.xAxisId/yAxisIdfromstring | numbertoAxisId; 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: PublicAreaPropsnow useAxisId; consider mirroring that in internal propsSwitching
AreaProps.xAxisId/yAxisIdtoAxisIdclarifies the contract and matches other components. For consistency,InternalAreaProps.xAxisId/yAxisIdcould also be typed asAxisIdinstead ofstring | 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 forgapThe updated descriptions for
dataKey,x,y,width,height, and defaults all look aligned with howBrushworks in the cartesian components, which should help omnidoc produce more accurate docs. The only minor nit is the English sentence forgap("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@defaultValueinstead of@default.The
xAxisIdandyAxisIdprops use@default 0while other props in this file use@defaultValue(e.g., lines 96, 102, 109, 116, 122). For consistency, use@defaultValuethroughout./** * 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
📒 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.tsxsrc/cartesian/ZAxis.tsxsrc/cartesian/Bar.tsxsrc/cartesian/ReferenceDot.tsxsrc/cartesian/ReferenceLine.tsxsrc/util/svgPropertiesAndEvents.tssrc/polar/defaultPolarRadiusAxisProps.tsxsrc/polar/RadialBar.tsxsrc/polar/PolarRadiusAxis.tsxsrc/zIndex/ZIndexLayer.tsxsrc/cartesian/Brush.tsxsrc/util/types.tssrc/cartesian/ReferenceArea.tsxsrc/component/Tooltip.tsxsrc/cartesian/CartesianGrid.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (DEVELOPING.md)
Run type checking on the codebase using
npm run check-types
**/*.{ts,tsx}: Never useanytype (implicit or explicit) in TypeScript code
Preferunknownoveranyand refine the type in TypeScript
Type function parameters and return values explicitly in TypeScript, do not rely on implicit any or inference; exceptions are React components and trivial functions
Do not useastype assertions in TypeScript; the only exception isas const
Files:
src/cartesian/Area.tsxomnidoc/readProject.spec.tssrc/cartesian/ZAxis.tsxsrc/cartesian/Bar.tsxsrc/cartesian/ReferenceDot.tsxtest/component/Tooltip/Tooltip.content.spec.tsxsrc/cartesian/ReferenceLine.tsxsrc/util/svgPropertiesAndEvents.tssrc/polar/defaultPolarRadiusAxisProps.tsxwww/src/docs/api/BarChart.tsomnidoc/omnidoc.spec.tswww/src/docs/api/Brush.tssrc/polar/RadialBar.tsxsrc/polar/PolarRadiusAxis.tsxsrc/zIndex/ZIndexLayer.tsxomnidoc/readProject.tssrc/cartesian/Brush.tsxsrc/util/types.tssrc/cartesian/ReferenceArea.tsxsrc/component/Tooltip.tsxwww/src/docs/api/AreaChart.tswww/src/docs/api/CartesianGrid.tssrc/cartesian/CartesianGrid.tsx
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Ensure code lints by running
npm run lintand follows Airbnb's Style Guide
Files:
src/cartesian/Area.tsxomnidoc/readProject.spec.tssrc/cartesian/ZAxis.tsxsrc/cartesian/Bar.tsxsrc/cartesian/ReferenceDot.tsxtest/component/Tooltip/Tooltip.content.spec.tsxsrc/cartesian/ReferenceLine.tsxsrc/util/svgPropertiesAndEvents.tssrc/polar/defaultPolarRadiusAxisProps.tsxwww/src/docs/api/BarChart.tsomnidoc/omnidoc.spec.tswww/src/docs/api/Brush.tssrc/polar/RadialBar.tsxsrc/polar/PolarRadiusAxis.tsxsrc/zIndex/ZIndexLayer.tsxomnidoc/readProject.tssrc/cartesian/Brush.tsxsrc/util/types.tssrc/cartesian/ReferenceArea.tsxsrc/component/Tooltip.tsxwww/src/docs/api/AreaChart.tswww/src/docs/api/CartesianGrid.tssrc/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.tsxsrc/cartesian/ZAxis.tsxsrc/cartesian/Bar.tsxsrc/cartesian/ReferenceDot.tsxsrc/cartesian/ReferenceLine.tsxsrc/util/svgPropertiesAndEvents.tssrc/polar/defaultPolarRadiusAxisProps.tsxsrc/polar/RadialBar.tsxsrc/polar/PolarRadiusAxis.tsxsrc/zIndex/ZIndexLayer.tsxsrc/cartesian/Brush.tsxsrc/util/types.tssrc/cartesian/ReferenceArea.tsxsrc/component/Tooltip.tsxsrc/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.tsxrather than running all tests withnpm test
Files:
omnidoc/readProject.spec.tstest/component/Tooltip/Tooltip.content.spec.tsxomnidoc/omnidoc.spec.ts
{test,www/test}/**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (DEVELOPING.md)
Write unit tests in the
testorwww/testdirectories with.spec.tsxfile 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 thecreateSelectorTestCasehelper function when writing or modifying tests
Use theexpectLastCalledWithhelper function instead ofexpect(spy).toHaveBeenLastCalledWith(...)for better typing and autocompletion
Verify the number of selector calls using the spy object fromcreateSelectorTestCaseto spot unnecessary re-renders and improve performance
MockgetBoundingClientRectin tests using the helper function provided intest/helper/MockGetBoundingClientRect.ts
Usevi.useFakeTimers()in all tests due to Redux autoBatchEnhancer dependency on timers andrequestAnimationFrame
Callvi.runOnlyPendingTimers()to advance timers after renders when not usingcreateSelectorTestCasehelper, and avoidvi.runAllTimers()to prevent infinite loops
UseuserEvent.setup({ advanceTimers: vi.runOnlyPendingTimers })or theuserEventSetuphelper function fromtest/helper/userEventSetup.tswhen creating userEvent instances
When testing tooltips on hover, usevi.runOnlyPendingTimers()after eachuserEvent.hover()call or use theshowTooltiphelper function fromtooltipTestHelpers.tsto account for requestAnimationFrame delays
Files:
test/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.tsxsrc/cartesian/Bar.tsxtest/component/Tooltip/Tooltip.content.spec.tsxsrc/util/svgPropertiesAndEvents.tssrc/polar/RadialBar.tsxsrc/util/types.tssrc/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.tstest/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.tsxsrc/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:activeJSDoc now correctly documents controlled vs uncontrolled behaviorThe added sentence on Line 74 about
activebeingundefinedmatches the runtime behavior in Line 247 (const finalIsActive: boolean = activeFromProps ?? isActive ?? false;). Tooltip is controlled whenactiveis set, and otherwise driven by Redux state from mouse/keyboard activity. No changes needed here.Also applies to: 242-248
112-117:includeHidden: falseis correctly set indefaultTooltipPropsThe change preserves filtering semantics: Line 265 filters entries with
entry.hide !== true || props.includeHidden, meaning hidden entries are excluded by default but included whenincludeHidden={true}. This aligns with the@defaultValue falsedocumentation on Lines 112–117. The implementation makes the default explicit to customcontentrenderers 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: falseas a default is appropriate and maintains backward compatibility.src/polar/PolarRadiusAxis.tsx (2)
32-37: LGTM: Well-documented prop addition.The
allowDecimalsprop is properly documented and the default value offalseis consistent with the corresponding entry indefaultPolarRadiusAxisProps(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 isDefaultZIndexes.line. Ensure thatDefaultZIndexes.lineequals 400 to maintain documentation accuracy.www/src/docs/api/BarChart.ts (1)
12-14: BarChart API doc clarifications look goodThe updated descriptions for
layout,syncId,syncMethod, containerwidth/height,data,margin,stackOffset, andreverseStackOrderare clearer and align with the corresponding JSDoc onCartesianChartProps. 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 APIThe added/expanded comments for
barCategoryGap,barGap,barSize,height,maxBarSize,reverseStackOrder, andwidthline 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: IncludingBrushin the expected public components is correctUpdating
expectedComponentsto includeBrushaligns 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: ExportingSVGPropsAndEventsis a safe and useful API surface additionMaking
SVGPropsAndEvents<T>public reuses the same filtering logic thatsvgPropertiesAndEventsalready 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: falsewithdefaultVal: 0may confuse documentation consumers.The
xAxisIdandyAxisIdprops are marked asisOptional: falsewhile still having adefaultVal: 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
Propstype correctly omitswidth,height,x, andyfromSVGPropsAndEvents<RectangleProps>since ReferenceArea usesx1, x2, y1, y2for positioning instead. The explanatory comment clarifies this design decision. Adding the optionalchildrenprop 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.eachtodescribe.eachwith 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
fillandfillOpacityprops is clear and matches the existing API documentation inwww/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.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
omnidoc/util/coloredDiff.ts (1)
28-31: Redundant difference check.The
hasDifferencescheck on line 28 is redundant because line 24 already confirmsnormalizedExpected !== 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.tsfor centralized maintenance.Also applies to: 378-421
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 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 useanytype (implicit or explicit) in TypeScript code
Preferunknownoveranyand refine the type in TypeScript
Type function parameters and return values explicitly in TypeScript, do not rely on implicit any or inference; exceptions are React components and trivial functions
Do not useastype assertions in TypeScript; the only exception isas const
Files:
omnidoc/util/coloredDiff.tsomnidoc/util/normalizeText.tsomnidoc/componentsWithInconsistentCommentsInApiDoc.tsomnidoc/util/calculateSimilarityScore.tsomnidoc/omnidoc.spec.tsomnidoc/confusing-defaults.spec.tsomnidoc/cross-component-prop-comments.spec.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Ensure code lints by running
npm run lintand follows Airbnb's Style Guide
Files:
omnidoc/util/coloredDiff.tsomnidoc/util/normalizeText.tsomnidoc/componentsWithInconsistentCommentsInApiDoc.tsomnidoc/util/calculateSimilarityScore.tsomnidoc/omnidoc.spec.tsomnidoc/confusing-defaults.spec.tsomnidoc/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.tsxrather than running all tests withnpm test
Files:
omnidoc/omnidoc.spec.tsomnidoc/confusing-defaults.spec.tsomnidoc/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.tsomnidoc/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.tsomnidoc/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.tsomnidoc/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.tsomnidoc/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
areCommentsSimilarfunction correctly usesnormalizeTextfor fuzzy comparison and handles edge cases (both empty, one empty) appropriately.
267-290: LGTM!The test correctly uses
coloredDiffto detect and report comment inconsistencies between API docs and project, with proper filtering of known inconsistent components.
| if (expected === undefined) { | ||
| return `${actual}`; | ||
| } | ||
| if (actual === undefined) { | ||
| return `${expected}`; | ||
| } |
There was a problem hiding this comment.
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.
| 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 Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will increase total bundle size by 810 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-umdAssets Changed:
view changes for bundle: recharts/bundle-es6Assets Changed:
|
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
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.