Remove more non-existent props from documentation#6590
Conversation
these are properties of the data object, not Sankey Component
we already have documented Enter+Leave which are better behaving anyway so let's promote that variant.
WalkthroughThis PR removes several documented and storybook props across multiple components while refactoring Funnel story argTypes to consolidate shape-related properties. Changes include eliminating activeIndex from ActiveShapeProps, removing activeShape from CartesianComponentShared, and deleting unused props from API documentation for RadialBar, Sankey, and ScatterChart components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
omnidoc/omnidoc.spec.ts (1)
74-74: Consider adding console.error logging before the assertion for easier debugging.The strict
toEqual([])check aligns well with the PR's goal of cleaning up non-existent props. However, unlike the Storybook props test (lines 96-101), this test no longer logs missing props before failing, making future debugging harder.Apply this diff to add back the logging:
+ if (missingProps.length > 0) { + console.error( + `Found ${missingProps.length} prop(s) documented in API docs but not exported from the project:`, + missingProps, + ); + } + expect(missingProps).toEqual([]);storybook/stories/API/chart/FunnelChart.stories.tsx (1)
14-14: Consider clarifying the Simple story's component type.The Simple story is typed as
StoryObj<FunnelProps>while the default export declarescomponent: FunnelChart. This creates a mismatch where the story is documenting Funnel props but is part of FunnelChart stories. This might confuse developers about which component's props are being demonstrated.Consider either:
- Changing the type to
StoryObj<FunnelChartProps>if this story is meant to demonstrate FunnelChart usage- Moving this story to Funnel.stories.tsx if it's primarily demonstrating Funnel props
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
omnidoc/omnidoc.spec.ts(1 hunks)storybook/stories/API/cartesian/Funnel.stories.tsx(2 hunks)storybook/stories/API/chart/FunnelChart.stories.tsx(1 hunks)storybook/stories/API/props/ActiveShapeProps.ts(0 hunks)storybook/stories/API/props/CartesianComponentShared.ts(0 hunks)www/src/docs/api/RadialBar.ts(0 hunks)www/src/docs/api/Sankey.ts(0 hunks)www/src/docs/api/ScatterChart.ts(0 hunks)
💤 Files with no reviewable changes (5)
- storybook/stories/API/props/CartesianComponentShared.ts
- www/src/docs/api/ScatterChart.ts
- www/src/docs/api/RadialBar.ts
- www/src/docs/api/Sankey.ts
- storybook/stories/API/props/ActiveShapeProps.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-25T07:36:02.229Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-25T07:36:02.229Z
Learning: Recharts aims for simple, declarative, and composable charts; prioritize consistency, usability, performance, and accessibility
Applied to files:
storybook/stories/API/chart/FunnelChart.stories.tsxstorybook/stories/API/cartesian/Funnel.stories.tsx
📚 Learning: 2025-10-25T07:35:46.188Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-10-25T07:35:46.188Z
Learning: Applies to test/**/*.test.{ts,tsx} : Mock getBoundingClientRect using test/helper/MockGetBoundingClientRect.ts when rendering Recharts components (e.g., Tooltip, Legend, charts)
Applied to files:
storybook/stories/API/cartesian/Funnel.stories.tsx
🧬 Code graph analysis (2)
storybook/stories/API/chart/FunnelChart.stories.tsx (1)
storybook/stories/API/props/ChartProps.ts (1)
CategoricalChartProps(72-220)
storybook/stories/API/cartesian/Funnel.stories.tsx (2)
storybook/stories/API/props/ActiveShapeProps.ts (1)
ActiveShapeProps(3-24)storybook/stories/API/props/CartesianComponentShared.ts (1)
General(33-57)
⏰ 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 (4)
omnidoc/omnidoc.spec.ts (1)
103-126: Note the different validation approach between API docs and Storybook tests.The API docs test (line 74) now strictly expects zero missing props, while this Storybook test continues to allow 5 known missing props via inline snapshot. This asymmetry may be intentional if different standards apply, but it's worth confirming that this is the desired behavior.
storybook/stories/API/cartesian/Funnel.stories.tsx (2)
3-3: LGTM: Proper imports for consolidated prop definitions.The addition of
dataKeyfromCartesianComponentSharedandActiveShapePropsimports correctly support the argTypes consolidation changes below.Also applies to: 24-24
30-34: LGTM: Improved argTypes organization.The changes consolidate shape-related props by spreading
ActiveShapePropsinstead of defining them individually, and makedataKey,id, andnameexplicit in the argTypes. This improves maintainability by using centralized prop definitions while ensuring these important props are clearly visible in the Storybook documentation.storybook/stories/API/chart/FunnelChart.stories.tsx (1)
10-10: The review comment is valid—verify the intent of this change with the team.The Simple story (lines 25-26, 37-38) indeed still references
args.shapeandargs.activeShape, but these props will no longer have interactive Storybook controls after removingActiveShapePropsfromargTypeson line 10. The story will render with hardcoded values, but developers won't be able to adjust these props via the Storybook UI.Additionally, the Simple story is typed as
StoryObj<FunnelProps>while rendering insideFunnelChart—this type mismatch should also be reviewed.Confirm whether this removal is intentional. If shape/activeShape should remain controllable, apply one of the suggested approaches from the original review comment.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6590 +/- ##
==========================================
- Coverage 94.53% 94.51% -0.02%
==========================================
Files 491 491
Lines 40761 40665 -96
Branches 4752 4752
==========================================
- Hits 38532 38436 -96
Misses 2224 2224
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportBundle size has no change ✅ |
2 participants
Related Issue
#6069
Summary by CodeRabbit
Documentation
Tests
Chores