Deprecate CartesianAxis, stop passing scale#6774
Conversation
WalkthroughThis PR deprecates the CartesianAxis component and removes the scale prop from XAxis and YAxis. The scale prop type in CartesianAxisProps changes from RechartsScale to unknown, and deprecation notices are added indicating the component is internal-only and not intended for direct use. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 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: 0
🧹 Nitpick comments (3)
src/cartesian/XAxis.tsx (1)
130-131: Correctly swallowingscalebefore rendering; consider clearer alias namingRemoving
scale(andscalefromsynchronizedSettings) before spreading into<CartesianAxis>is the right way to stop leaking the internal scale while keeping the public prop and store wiring intact. The early null-guards above also keep this safe when selectors returnundefined.As a minor readability tweak, consider renaming
del/del2to something like_unusedScaleProp/_unusedScaleStateor adding a brief comment so it’s obvious they exist only to excludescalefrom the spread.src/cartesian/YAxis.tsx (1)
170-171: YAxisscaleswallowing matches XAxis pattern; small naming nitThis mirrors the XAxis change and correctly keeps
scaleout of<CartesianAxis>while still honoring it inSetYAxisSettings, so the public API remains intact while the internal axis implementation stops exposing its scale.As with XAxis, you might want to rename
del/del2to something like_unusedScaleProp/_unusedScaleStatefor self‑documentation.src/cartesian/CartesianAxis.tsx (1)
75-79:scaleprop deprecation andunknowntyping look good; verify version noteMarking
scaleas deprecated and documenting that it’s ignored aligns with the PR goal of removing the scale from the public API surface, and switching the type tounknownavoids tying consumers to the internalRechartsScaleshape, which is good for future refactors. As per coding guidelines, usingunknowninstead ofanyhere is appropriate.Please just double‑check that “ignored since 3.0” is historically accurate; if
scalewas still honored in any 3.x releases, it may be safer to either omit the version or tighten it to the exact minor where it stopped having effect.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/cartesian/CartesianAxis.tsx(2 hunks)src/cartesian/XAxis.tsx(1 hunks)src/cartesian/YAxis.tsx(1 hunks)
🧰 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 const
Files:
src/cartesian/XAxis.tsxsrc/cartesian/YAxis.tsxsrc/cartesian/CartesianAxis.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/XAxis.tsxsrc/cartesian/YAxis.tsxsrc/cartesian/CartesianAxis.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/XAxis.tsxsrc/cartesian/YAxis.tsxsrc/cartesian/CartesianAxis.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (DEVELOPING.md)
All imports from
rechartsmust use the public API entry point; imports from internal paths likerecharts/types/*orrecharts/src/*are not allowed
Files:
src/cartesian/XAxis.tsxsrc/cartesian/YAxis.tsxsrc/cartesian/CartesianAxis.tsx
🧠 Learnings (5)
📓 Common learnings
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.
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, `useAppSelector` and hooks like `useChartLayout()` are designed to return `undefined` when used outside Redux Provider context, rather than throwing errors. This allows components like `Curve` that call these hooks to work standalone by falling back to prop values.
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-12-06T03:36:59.377Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : All imports from `recharts` must use the public API entry point; imports from internal paths like `recharts/types/*` or `recharts/src/*` are not allowed
📚 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/cartesian/XAxis.tsxsrc/cartesian/YAxis.tsxsrc/cartesian/CartesianAxis.tsx
📚 Learning: 2025-12-06T03:36:59.377Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-12-06T03:36:59.377Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : All imports from `recharts` must use the public API entry point; imports from internal paths like `recharts/types/*` or `recharts/src/*` are not allowed
Applied to files:
src/cartesian/CartesianAxis.tsx
📚 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/cartesian/CartesianAxis.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:
src/cartesian/CartesianAxis.tsx
⏰ 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 (1)
src/cartesian/CartesianAxis.tsx (1)
541-548: Clear deprecation messaging forCartesianAxisexportThe deprecation JSDoc on the exported
CartesianAxis—stating it’s not meant for direct app use, pointing users at XAxis/YAxis, and noting planned internalization in v4—is clear and consistent with the PR’s intent. This should help steer users away from depending on this internal component ahead of the breaking change.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6774 +/- ##
==========================================
- Coverage 93.72% 93.72% -0.01%
==========================================
Files 516 516
Lines 43401 43397 -4
Branches 5008 5006 -2
==========================================
- Hits 40678 40674 -4
Misses 2717 2717
Partials 6 6 ☔ 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 74 bytes (0.0%) ⬆️. 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:
|
Description
I discovered that CartesianAxis never actually reads the scale prop. Also marked the whole component as deprecated - people should be using XAxis and YAxis instead.
Related Issue
#6645 - we don't want to expose the internal scale object in public API
#6734
Motivation and Context
I want to add more things to the internal scale representation but couldn't since it was used in CartesianAxis as public API. Turns out it's not used!
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.