fix(YAxis): render explicit ticks when a non-literal domain can't resolve on empty data (#7362)#7393
Conversation
…olve on empty data (recharts#7362) Follow-up to recharts#7384 (which added the failing repro). When a numeric axis specifies an explicit `ticks` array but its `domain` cannot be resolved without data — a function bound, or 'auto'/'dataMin'/'dataMax' — the scale never built on empty/all-null data, so the axis rendered zero ticks. A literal numeric domain already works; this extends that to derived domains, as discussed on recharts#7362. combineNumericalDomain now falls back to the [min, max] extent of the explicit numeric ticks when: the user domain could not be resolved without data, there is no data domain (no data and no domain-extending reference elements), allowDataOverflow is set, and the axis is type='number' with numeric ticks. The fallback never overrides a resolvable literal domain, never engages when data is present, and leaves category/polar/tooltip paths untouched. - add combineNumericTicksDomain + selectNumericTicksDomain (ticks [min,max] extent) - thread it as an optional input to combineNumericalDomain (polar/tooltip unaffected) - does NOT alter parseNumericalUserDomain / numericalDomainSpecifiedWithoutRequiringData - converts the recharts#7384 it.fails repro into passing tests + adds edge-case coverage Refs recharts#7362. Signed-off-by: Nicolas <[email protected]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR implements a numeric ticks-derived domain fallback for axis resolution on empty/all-null data. It exports new helpers to extract ChangesNumeric ticks domain fallback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Bundle ReportChanges will increase total bundle size by 7.84kB (0.14%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: recharts/bundle-treeshaking-cartesianAssets Changed:
view changes for bundle: recharts/bundle-cjsAssets Changed:
view changes for bundle: recharts/bundle-es6Assets Changed:
view changes for bundle: recharts/bundle-umdAssets Changed:
view changes for bundle: recharts/bundle-treeshaking-polarAssets Changed:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7393 +/- ##
==========================================
+ Coverage 89.01% 89.07% +0.05%
==========================================
Files 579 579
Lines 13562 13579 +17
Branches 3426 3433 +7
==========================================
+ Hits 12072 12095 +23
+ Misses 1328 1324 -4
+ Partials 162 160 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
PavelVanecek
left a comment
There was a problem hiding this comment.
I am not quite certain if allowDataOverflow should make a difference here. We're talking about a chart that has all null data anyway, the overflow makes no sense. Maybe let's have the new code only when it's true so that we affect fewer people? Someone else might rely on the old behaviour.
|
Agreed — let's keep it gated on |
What
Follow-up to #7384 (the failing repro). When a numeric axis supplies an explicit
ticksarray but itsdomaincannot be resolved without data — a function bound, or'auto'/'dataMin'/'dataMax'— the scale never builds on empty/all-null data, so the axis renders zero ticks. A literal numeric domain already works; this extends that to derived domains.This avoids the typing problem raised on the issue (the
AxisDomainfunction form is typed(NumberDomain, boolean) => NumberDomain, so feeding itundefined/NaNwould be a breaking change): instead of calling the user's function without data, we fall back to the explicit ticks' extent.How
combineNumericalDomainfalls back to[Math.min(...ticks), Math.max(...ticks)]of the explicit numeric ticks only when all of:numericalDomainSpecifiedWithoutRequiringData→undefined), andparseNumericalUserDomainalso returnedundefined(no data domain — empty/all-null data and no domain-extending reference elements), andallowDataOverflowis set, andtype="number"with ≥1 numeric tick.So it never overrides a resolvable literal domain, never engages when data is present, and leaves category / polar / tooltip paths untouched.
combineNumericTicksDomain+selectNumericTicksDomain(the ticks[min,max]extent)combineNumericalDomain(polar/tooltip pass nothing → unchanged)parseNumericalUserDomain/numericalDomainSpecifiedWithoutRequiringData— their "undefined without data" contract is preservedit.failsrepro into passing tests + adds edge-case coverage (unsorted/non-numeric/single ticks, tickCount-only, literal-wins, no-overflow, data-present guards)Test plan
YAxis.7362.derivedDomain.spec.tsx— 13 cases (positive + negative guards) passunit:libsuite green locally (vitest 4)allowDataOverflow? (kept it required, for parity with the literal-domain path)Refs #7362. Follows #7384.
Summary by CodeRabbit
Bug Fixes
allowDataOverflowis enabled.Tests