Omnidoc: Fix default values docs for XAxis, YAxis, ZAxis#6667
Conversation
WalkthroughExports axis default-props for XAxis/YAxis/ZAxis, refactors omnidoc test helpers for safer value comparison/stringification, and normalizes default value metadata across API docs and Storybook argTypes. Changes
Sequence Diagram(s)sequenceDiagram
participant Storybook as Storybook UI
participant Omnidoc as Omnidoc test
participant MetaMap as componentMetaMap
participant Component as X/Y/Z Axis Component
participant Dispatcher as Settings Dispatcher
participant Store as Settings Store
participant Cartesian as CartesianAxis (render)
Note right of MetaMap: componentMetaMap now\ncontains exported defaultProps
Storybook->>MetaMap: read defaultProps for docs
Omnidoc->>MetaMap: compare documented vs exported defaults
MetaMap-->>Omnidoc: default props (stringified)
Component->>Dispatcher: mount / props -> build settings payload
Dispatcher->>Store: dispatch settings (includes new defaults: angle, tick, tickLine, tickSize, includeHidden, ...)
Store-->>Component: synced settings
Component->>Cartesian: render with merged props & synced settings
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
🧹 Nitpick comments (2)
storybook/stories/API/props/SharedAxisProps.ts (1)
173-183: Tick ArgType changes look right; consider the trade‑off of a boolean control for a union type.Setting
control: 'boolean'andtable.defaultValue: truefortickaligns the docs with the commonbooleanusage and makes the Arg control simple, while still documenting the full union type (Boolean | Object | ReactElement). The only trade‑off is that the Storybook control UI won't help authors explore the object/ReactElement variants. If that's acceptable for these API stories, this change is fine as‑is; otherwise, you might want to drop the control or switch to a more generic one later.omnidoc/omnidoc.spec.ts (1)
105-150: Ensurestringifyreally returns a string in all cases
stringifyis declared to returnstring, butJSON.stringifycan legitimately returnundefined(e.g. forundefined, functions, or symbols) without throwing. That value then propagates throughstringifyDefaultValue,stripQuotes, and the error message, so the actual runtime type isstring | undefinedand comparisons may end up beingundefined === undefinedinstead of string comparisons.To keep behavior predictable and match the signature, consider normalizing the JSON result:
function stringify(value: unknown): string { if (typeof value === 'string') { return value; } try { - return JSON.stringify(value); + const json = JSON.stringify(value); + return typeof json === 'string' ? json : String(value); } catch { return String(value); } }This keeps the pretty JSON output when possible, but guarantees a string fallback for edge cases like
undefinedor non-serializable values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
omnidoc/componentsAndDefaultPropsMap.ts(2 hunks)omnidoc/omnidoc.spec.ts(2 hunks)src/cartesian/XAxis.tsx(4 hunks)src/cartesian/YAxis.tsx(6 hunks)src/cartesian/ZAxis.tsx(2 hunks)src/state/selectors/axisSelectors.ts(1 hunks)src/util/types.ts(3 hunks)storybook/stories/API/cartesian/ZAxis.stories.tsx(4 hunks)storybook/stories/API/props/SharedAxisProps.ts(1 hunks)storybook/stories/API/props/utils.ts(1 hunks)www/src/docs/api/XAxis.ts(20 hunks)www/src/docs/api/YAxis.ts(18 hunks)www/src/docs/api/ZAxis.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
src/state/selectors/axisSelectors.ts (1)
src/util/types.ts (1)
AxisDomain(671-675)
storybook/stories/API/props/utils.ts (1)
storybook/StorybookArgs.ts (1)
StorybookArgs(59-61)
storybook/stories/API/cartesian/ZAxis.stories.tsx (2)
src/util/ReactUtils.ts (1)
SCALE_TYPES(9-25)storybook/stories/API/props/utils.ts (1)
getStoryArgsFromArgsTypesObject(5-16)
omnidoc/componentsAndDefaultPropsMap.ts (3)
src/cartesian/XAxis.tsx (1)
xAxisDefaultProps(152-174)src/cartesian/YAxis.tsx (1)
yAxisDefaultProps(195-217)src/cartesian/ZAxis.tsx (1)
zAxisDefaultProps(66-71)
omnidoc/omnidoc.spec.ts (1)
omnidoc/DocReader.ts (1)
DefaultValue(1-1)
src/cartesian/XAxis.tsx (4)
src/state/cartesianAxisSlice.ts (2)
XAxisOrientation(13-13)XAxisPadding(10-10)src/util/types.ts (2)
AxisTick(772-772)AxisInterval(765-765)src/state/selectors/axisSelectors.ts (1)
implicitXAxis(106-131)src/cartesian/CartesianAxis.tsx (2)
defaultCartesianAxisProps(89-112)Props(119-120)
src/cartesian/YAxis.tsx (4)
src/state/cartesianAxisSlice.ts (2)
YAxisOrientation(14-14)YAxisPadding(11-11)src/util/types.ts (1)
AxisInterval(765-765)src/state/selectors/axisSelectors.ts (1)
implicitYAxis(150-175)src/cartesian/CartesianAxis.tsx (1)
defaultCartesianAxisProps(89-112)
src/cartesian/ZAxis.tsx (4)
src/state/cartesianAxisSlice.ts (1)
AxisId(8-8)src/util/types.ts (3)
DataKey(59-59)ScaleType(147-163)AxisDomain(671-675)src/state/selectors/axisSelectors.ts (1)
AxisRange(1425-1425)src/util/ChartUtils.ts (1)
RechartsScale(152-164)
⏰ 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 (13)
storybook/stories/API/props/SharedAxisProps.ts (1)
87-101: DomaindefaultValueremoval reduces risk of misleading docs; please confirm runtime behavior.Not documenting a fixed
defaultValuefordomainis a good move if the effective domain is derived dynamically (e.g., from data or scale) rather than being[0, 'auto']in all cases. This keeps the API table from promising a constant default that the implementation can't always honor. Please just double‑check that there is no single, stable default for XAxis/YAxis/ZAxis domain that should still be surfaced here.src/state/selectors/axisSelectors.ts (1)
97-97: LGTM! Exposing the default numeric domain for external use.Exporting
defaultNumericDomainenables consistent default value documentation and usage across the codebase, particularly for the axis components' default props.storybook/stories/API/props/utils.ts (1)
1-5: LGTM! Improved type flexibility for Storybook configurations.The addition of
ArgTypessupport alongside the existingStorybookArgsenhances the utility function's flexibility while maintaining backward compatibility. The type-only import forStorybookArgsis a good practice.www/src/docs/api/ZAxis.ts (1)
16-16: LGTM! Default values now use proper types and align with code.The changes improve type consistency:
zAxisIddefault changed from string'0'to numeric0rangedefault changed from string'[10, 10]'to array[64, 64], which correctly matchesimplicitZAxis.rangein the codebaseAlso applies to: 26-26
www/src/docs/api/YAxis.ts (1)
7-7: LGTM! Standardized default values with proper primitive types.The changes improve documentation consistency by:
- Converting string-based defaults to proper primitives (e.g.,
'false'→false,'5'→5)- Using JSON string representation for object defaults (e.g., padding:
'{"top":0,"bottom":0}')- Removing
defaultValfor truly optional properties that have no defaultsThis aligns the documentation with the actual default props in the codebase.
Also applies to: 26-26, 36-36, 55-55, 65-65, 75-75, 112-112, 124-124, 142-142, 163-163, 173-173, 183-183, 196-196, 206-206, 217-217, 228-228, 256-256, 273-273, 284-284
src/util/types.ts (1)
698-702: LGTM! Enhanced API documentation with default value annotations.The JSDoc
@defaultValueannotations improve developer experience by making default values visible in IDE tooltips and generated documentation.Also applies to: 705-708, 728-730, 738-740
src/cartesian/ZAxis.tsx (2)
4-4: LGTM! Type consistency improvement with AxisId.Using the
AxisIdtype (which isstring | number) ensures consistency with XAxis and YAxis implementations.Also applies to: 49-49
66-71: LGTM! Default props export enables omnidoc integration.The exported
zAxisDefaultPropsfollows the same pattern as XAxis and YAxis, and correctly referencesimplicitZAxisfor default values. Theas const satisfies Partial<Props>typing ensures type safety while allowing the object to be deeply readonly.omnidoc/componentsAndDefaultPropsMap.ts (1)
14-16: LGTM! Registered axis components for omnidoc documentation.The addition of XAxis, YAxis, and ZAxis to the
componentMetaMapenables automated default props documentation generation, following the established pattern used by other components.Also applies to: 36-38
storybook/stories/API/cartesian/ZAxis.stories.tsx (1)
2-2: LGTM! Enhanced Storybook configuration with ArgTypes.The introduction of
ZAxisArgTypesprovides:
- Explicit type metadata for better Storybook documentation
- Consistent default value representation in the docs table
- Improved developer experience with the Storybook controls panel
The default values in the ArgTypes metadata (zAxisId: '0', range: '[64,64]', scale: 'auto') correctly match the actual component defaults.
Also applies to: 18-55, 58-58, 79-79
src/cartesian/YAxis.tsx (1)
44-80: YAxis default props and dispatcher wiring look consistentThe added JSDoc defaults on
mirror,orientation,padding,minTickGap,interval,reversed, andanglealign withimplicitYAxisandyAxisDefaultProps, andyAxisDefaultPropsitself correctly reusesimplicitYAxisanddefaultCartesianAxisProps(e.g.axisLine,tickLine,tickSize,interval,minTickGap). Passinginterval,includeHidden,angle,minTickGap, andtickthroughYAxisSettingsDispatcherintoSetYAxisSettingsmatches theYAxisSettingsshape and should keep state-derived defaults and docs in sync.No functional issues spotted here.
Also applies to: 195-217, 221-249
www/src/docs/api/XAxis.ts (1)
5-8: XAxis docs defaults now align with runtime defaultsThe updated
defaultValentries forhide,xAxisId,height,orientation,type,allowDecimals,allowDataOverflow,allowDuplicatedCategory,angle,tickCount,includeHidden,interval,padding,minTickGap,axisLine,tickLine,tickSize,tick,mirror, andreversedmatch the values exposed viaimplicitXAxis/xAxisDefaultPropsin the implementation.This should make omnidoc’s default-value consistency checks much more reliable and keeps the public docs truthful to the actual behavior.
Also applies to: 24-27, 43-46, 53-56, 63-76, 96-99, 106-109, 116-119, 153-156, 165-168, 183-186, 206-209, 216-219, 227-230, 238-241, 266-269, 289-292, 300-303
src/cartesian/XAxis.tsx (1)
31-80: XAxis defaults and wiring are consistent with selectors and docsThe added JSDoc
@defaultValueannotations forxAxisId,height,mirror,orientation,padding,minTickGap,interval,reversed, andanglematch the values inimplicitXAxisand the newxAxisDefaultProps. ThexAxisDefaultPropsobject itself correctly combinesimplicitXAxis(for behavior-related defaults like type, scale, tick, tickCount, includeHidden, etc.) withdefaultCartesianAxisProps(for shared axis visuals likeaxisLine,tickLine,tickSize).
XAxisSettingsDispatcherforwards these props intoSetXAxisSettingsin a way that matches theXAxisSettingsshape, so the Redux-backed axis settings, component defaults, omnidoc metadata, and API docs should all agree on the same default values.Looks solid.
Also applies to: 152-174, 178-207
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6667 +/- ##
==========================================
- Coverage 94.03% 94.03% -0.01%
==========================================
Files 497 497
Lines 42612 42609 -3
Branches 4881 4871 -10
==========================================
- Hits 40069 40066 -3
Misses 2538 2538
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/cartesian/YAxis.tsx (1)
44-80: JSDoc defaults align with runtime; consider documenting allAxisIntervaloptionsThe
@defaultValuetags formirror,orientation,padding,minTickGap,interval,reversed, andanglecorrectly matchyAxisDefaultProps/implicitYAxis, which should keep omnidoc and runtime in sync.For completeness, since
AxisIntervalalso allows'equidistantPreserveStart', you may want to mention it in theintervaldescription so the docs expose all supported options.- * If set 0, all the ticks will be shown. If set "preserveStart", "preserveEnd" or "preserveStartEnd", + * If set 0, all the ticks will be shown. If set "preserveStart", "preserveEnd", "preserveStartEnd" or "equidistantPreserveStart",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/cartesian/YAxis.tsx(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/cartesian/YAxis.tsx (4)
src/state/cartesianAxisSlice.ts (2)
YAxisOrientation(14-14)YAxisPadding(11-11)src/util/types.ts (1)
AxisInterval(765-765)src/state/selectors/axisSelectors.ts (1)
implicitYAxis(150-175)src/cartesian/CartesianAxis.tsx (1)
defaultCartesianAxisProps(89-112)
⏰ 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: Analyze (javascript-typescript)
- GitHub Check: Build, Test, Pack
🔇 Additional comments (2)
src/cartesian/YAxis.tsx (2)
5-5: Good consolidation of Y-axis defaults viaimplicitYAxisanddefaultCartesianAxisPropsImporting
defaultCartesianAxisPropsand wiringaxisLine,tickLine, andtickSizefrom it—while sourcing the rest fromimplicitYAxis—gives a single, consistent source of truth for Y-axis defaults. The new entries (angle,includeHidden,interval,minTickGap,tick, etc.) all match the implicit settings, so the exposedyAxisDefaultPropsnow accurately reflect the real runtime behavior and X/Y axis conventions.Also applies to: 195-217
221-250: Settings dispatcher now mirrorsYAxisSettingsshape and defaultsPassing
interval,includeHidden,angle,minTickGap, andtickdirectly from thePropsWithDefaultsintoSetYAxisSettingsmakes the store’sYAxisSettingsfully aligned withimplicitYAxisand the declared defaults, instead of relying on ad-hoc fallbacks. This should help keep omnidoc, Storybook argTypes, and actual behavior in lockstep.
Bundle ReportChanges will decrease total bundle size by 241 bytes (-0.01%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-umdAssets Changed:
view changes for bundle: recharts/bundle-es6Assets Changed:
view changes for bundle: recharts/bundle-cjsAssets Changed:
|
Related Issue
#6069
Summary by CodeRabbit
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.