Omnidoc: Correcting documented default values for Area#6615
Conversation
WalkthroughThis pull request refactors the Area component to support automatic animation detection and derive layout from context, while standardizing default value handling across the codebase. Changes include introducing useCartesianChartLayout hook, updating Area's isAnimationActive to support 'auto' mode, removing the layout prop, and adjusting default value representations to preserve original types instead of coercing to strings. Changes
Sequence Diagram(s)sequenceDiagram
participant Area as Area Component
participant Hook as useCartesianChartLayout()
participant ChartContext as Chart Context
participant Layout as CartesianLayout
Area->>Hook: Request layout
Hook->>ChartContext: Read chart layout via useChartLayout()
ChartContext-->>Hook: Return current layout
alt Layout is 'horizontal' or 'vertical'
Hook-->>Area: Return CartesianLayout
Area->>Area: Render with derived layout
else Layout is other value or undefined
Hook-->>Area: Return undefined
Area->>Area: Return null (guard)
end
Note over Area: If isAnimationActive is 'auto':<br/>Resolve to !Global.isSsr
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
omnidoc/omnidoc.spec.ts(4 hunks)omnidoc/readProject.spec.ts(1 hunks)omnidoc/readProject.ts(1 hunks)omnidoc/readStorybookDoc.spec.ts(1 hunks)src/cartesian/Area.tsx(9 hunks)src/context/chartLayoutContext.tsx(2 hunks)storybook/stories/API/cartesian/Area.stories.tsx(1 hunks)storybook/stories/API/props/AnimationProps.ts(1 hunks)storybook/stories/API/props/CartesianComponentShared.ts(1 hunks)storybook/stories/API/props/Styles.ts(1 hunks)test/cartesian/Area.spec.tsx(7 hunks)test/component/Legend.spec.tsx(1 hunks)www/src/docs/api/Area.ts(11 hunks)www/src/docs/api/types.ts(1 hunks)www/src/views/APIViewNew.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not hardcode any user-facing strings or formatting choices in the library code; leave internationalization to library users
Files:
src/context/chartLayoutContext.tsxsrc/cartesian/Area.tsx
🧠 Learnings (3)
📚 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:
test/cartesian/Area.spec.tsxsrc/context/chartLayoutContext.tsxsrc/cartesian/Area.tsx
📚 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:
src/context/chartLayoutContext.tsxsrc/cartesian/Area.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} : When testing selectors, verify render counts using the spy and rerenderSameComponent from createSelectorTestCase
Applied to files:
omnidoc/omnidoc.spec.ts
🧬 Code graph analysis (6)
src/context/chartLayoutContext.tsx (1)
src/util/types.ts (1)
CartesianLayout(49-49)
storybook/stories/API/props/CartesianComponentShared.ts (1)
storybook/StorybookArgs.ts (1)
StorybookArg(13-48)
omnidoc/omnidoc.spec.ts (1)
omnidoc/DocReader.ts (1)
DefaultValue(1-1)
src/cartesian/Area.tsx (3)
src/util/types.ts (1)
CartesianLayout(49-49)src/context/chartLayoutContext.tsx (1)
useCartesianChartLayout(137-143)storybook/stories/API/props/AnimationProps.ts (1)
isAnimationActive(29-36)
storybook/stories/API/props/AnimationProps.ts (1)
storybook/StorybookArgs.ts (1)
StorybookArg(13-48)
www/src/docs/api/Area.ts (1)
www/src/docs/api/types.ts (1)
ApiDoc(20-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build, Test, Pack
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (14)
storybook/stories/API/props/Styles.ts (1)
128-128: LGTM! Standardizing property name for consistency.The change from
defaulttodefaultValuecorrectly aligns with Storybook's Args interface and matches the naming convention used throughout the rest of the file (e.g., lines 9, 40, 50, 72).storybook/stories/API/props/CartesianComponentShared.ts (1)
22-22: Verified ✓ Changes are correct.Confirmed that
0is indeed the default value for bothxAxisIdandyAxisIdin the Area component (seedefaultAreaPropsinsrc/cartesian/Area.tsx, lines 1083-1084). The Storybook documentation updates correctly reflect these actual component defaults, and using the numeric type aligns with the PR's goal to preserve original types.www/src/views/APIViewNew.tsx (1)
67-67: LGTM! Proper handling of expanded default value types.The use of
JSON.stringifycorrectly handles the expanded type ofdefaultVal(which now includes boolean). Wrapping in a<code>element improves visual presentation.omnidoc/readProject.ts (1)
134-144: Excellent improvement to preserve default value types.Removing the
String(value)coercion preserves the original type of default values (numbers, booleans, etc.), which improves type fidelity throughout the documentation pipeline.omnidoc/readProject.spec.ts (1)
861-863: LGTM! Test validates numeric default value preservation.The test correctly verifies that numeric defaults remain as numbers rather than being coerced to strings.
test/component/Legend.spec.tsx (1)
1852-1885: LGTM! Test expectations updated for corrected Area defaults.The updated assertions reflect the new default values for Area:
isAnimationActive: 'auto'enables automatic animation detectionlabel: falseandtype: 'linear'are now explicitly documented defaultsomnidoc/readStorybookDoc.spec.ts (1)
196-198: LGTM! Test ensures Storybook docs preserve numeric types.Validates that the Storybook documentation reader maintains numeric default values, ensuring consistency with the project-based documentation reader.
www/src/docs/api/types.ts (1)
12-12: LGTM! Type expansion supports boolean default values.Adding
booleanto thedefaultValtype enables accurate documentation of boolean props, supporting the broader effort to preserve original default value types.src/context/chartLayoutContext.tsx (1)
137-143: LGTM! Well-designed public hook for Cartesian layout.The
useCartesianChartLayouthook provides type-safe access to Cartesian-specific layout information, following the established patterns in this file. The implementation is clean and properly filters non-Cartesian layouts.test/cartesian/Area.spec.tsx (1)
790-795: Tests cover the new defaults.Nice to see the legend payload now asserts
'auto','linear', andlabel: false, so we’ll catch any drift from the new runtime defaults.omnidoc/omnidoc.spec.ts (1)
199-206: Stringifying project defaults avoids false negatives.Converting project-side defaults to strings before comparing with JSDoc keeps booleans and numbers from triggering spurious failures—nice tightening of the omnidoc guardrail.
storybook/stories/API/props/AnimationProps.ts (1)
17-34: Docs reflect runtime defaults.Surfacing 1500 / 'ease' / 'auto' directly through the arg defaults keeps Storybook perfectly in step with the component behaviour.
src/cartesian/Area.tsx (1)
752-755: SSR-aware animation toggle.Routing
'auto'through!Global.isSsrgives callers a sane default—animations stay off during SSR but flip on in the browser without extra props.www/src/docs/api/Area.ts (1)
58-249: API defaults now share real types.Switching to boolean/number defaults (and documenting
'auto') keeps the published API in lockstep with what Area actually ships—great for omnidoc parity.
| dot: { | ||
| description: `If false set, dots will not be drawn. If true set, dots will be drawn which have the props | ||
| calculated internally. If object set, dots will be drawn which have the props merged by the internal | ||
| calculated props and the option. If ReactElement set, the option can be the custom dot element.If set | ||
| a function, the function will be called to render customized dot.`, | ||
| defaultValue: false, | ||
| table: { | ||
| type: { | ||
| summary: 'Boolean | Object | ReactElement | Function', | ||
| category: 'Style', | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Fix argType structure: category should be at table level.
The category property should be directly under table, not nested under table.type. Compare with the correct structure in stackId (lines 21-26).
Apply this diff to fix the structure:
dot: {
description: `If false set, dots will not be drawn. If true set, dots will be drawn which have the props
calculated internally. If object set, dots will be drawn which have the props merged by the internal
calculated props and the option. If ReactElement set, the option can be the custom dot element.If set
a function, the function will be called to render customized dot.`,
defaultValue: false,
table: {
type: {
summary: 'Boolean | Object | ReactElement | Function',
- category: 'Style',
},
+ category: 'Style',
},
},📝 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.
| dot: { | |
| description: `If false set, dots will not be drawn. If true set, dots will be drawn which have the props | |
| calculated internally. If object set, dots will be drawn which have the props merged by the internal | |
| calculated props and the option. If ReactElement set, the option can be the custom dot element.If set | |
| a function, the function will be called to render customized dot.`, | |
| defaultValue: false, | |
| table: { | |
| type: { | |
| summary: 'Boolean | Object | ReactElement | Function', | |
| category: 'Style', | |
| }, | |
| }, | |
| }, | |
| dot: { | |
| description: `If false set, dots will not be drawn. If true set, dots will be drawn which have the props | |
| calculated internally. If object set, dots will be drawn which have the props merged by the internal | |
| calculated props and the option. If ReactElement set, the option can be the custom dot element.If set | |
| a function, the function will be called to render customized dot.`, | |
| defaultValue: false, | |
| table: { | |
| type: { | |
| summary: 'Boolean | Object | ReactElement | Function', | |
| }, | |
| category: 'Style', | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In storybook/stories/API/cartesian/Area.stories.tsx around lines 35 to 47, the
argType for `dot` incorrectly places `category` under `table.type`; move
`category: 'Style'` out one level to be directly under `table` (alongside `type`
and `defaultValue`/`description`) so the structure matches `stackId` (lines
21-26) — i.e., keep `table.type.summary` as the type and add `table.category =
'Style'`.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6615 +/- ##
==========================================
- Coverage 94.15% 94.14% -0.01%
==========================================
Files 493 493
Lines 41081 41080 -1
Branches 4773 4778 +5
==========================================
- Hits 38681 38676 -5
- Misses 2395 2399 +4
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bundle ReportChanges will increase total bundle size by 939 bytes (0.04%) ⬆️. 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:
view changes for bundle: recharts/bundle-umdAssets Changed:
|
| fillOpacity: 0.6, | ||
| hide: false, | ||
| isAnimationActive: !Global.isSsr, | ||
| isAnimationActive: 'auto', |
There was a problem hiding this comment.
What is this? Has this always been available? lol
There was a problem hiding this comment.
I added it now. Just a serializable name for what is going on.
There was a problem hiding this comment.
Ah duh, I missed the check below this
Description
Also removed
layoutprop from Area that we never read - this should have been done in 3.0Related Issue
#6069
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation