New hooks#6960
Conversation
WalkthroughAdds coordinate-systems features: new axis-scale/inverse hooks and types, categorical inverse utilities and combiner, selector exports, tests, snapshots/types, website guide/examples, omnidoc link handling changes, and mouse-pointer typing updates. Changes
Sequence Diagram(s)sequenceDiagram
actor Component
participant Hook as useXAxisScale
participant State as ReduxState
participant Selector as selectAxisScale
participant Scale as ScaleFunction
Component->>Hook: request scale (xAxisId)
Hook->>State: read chart slice
State->>Selector: selectAxisScale(state, 'x', id)
Selector->>Scale: obtain configured scale function
Selector-->>Hook: ScaleFunction
Hook-->>Component: provide ScaleFunction
Component->>Scale: scale(dataValue)
Scale-->>Component: pixel coordinate
sequenceDiagram
actor Component
participant InvHook as useXAxisInverseScale
participant State as ReduxState
participant Combiner as combineInverseScaleFunction
participant CatInv as createCategoricalInverse
participant Bisect as bisect
Component->>InvHook: request inverse scale
InvHook->>State: read configured scale
State->>Combiner: pass configuredScale
alt configuredScale has invert
Combiner-->>InvHook: bound invert function
else
Combiner->>CatInv: build categorical inverse (scale, dataPoints)
CatInv->>Bisect: compute nearest index for pixel
Bisect-->>CatInv: index
CatInv-->>InvHook: InverseScaleFunction
end
InvHook-->>Component: provide InverseScaleFunction
Component->>InvHook: inverse(pixel)
InvHook-->>Component: data value
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
This is AI slop, does not show the hooks at all. I will rewrite it in next PR, before publishing.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6960 +/- ##
==========================================
- Coverage 91.13% 90.38% -0.75%
==========================================
Files 509 515 +6
Lines 37967 38497 +530
Branches 5287 5338 +51
==========================================
+ Hits 34601 34797 +196
- Misses 3357 3691 +334
Partials 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
omnidoc/generateApiDoc.ts (1)
598-635:⚠️ Potential issue | 🟠 MajorImport detection misses tags in
returnDesc.
returnDescis processed throughprocessInlineLinks, butwriteApiDocFileonly scansdesc/prop desc. If a@returnJSDoc includes component references, the generated file will useLinkToApiwithout importing it.✅ Suggested fix
-function hasTag(obj: ApiDoc | ApiProps, tagName: string): boolean { - if (!obj.desc) return false; - return Object.values(obj.desc).some(desc => typeof desc === 'string' && desc.includes(`</${tagName}>`)); -} +function hasTagInDesc( + desc: ApiDoc['desc'] | ApiDoc['returnDesc'] | ApiProps['desc'] | undefined, + tagName: string, +): boolean { + if (!desc) return false; + return Object.values(desc).some(d => typeof d === 'string' && d.includes(`</${tagName}>`)); +} + +function hasTag(obj: ApiDoc | ApiProps, tagName: string): boolean { + return hasTagInDesc(obj.desc, tagName); +} @@ - const hasLinkInDesc = hasTag(apiDoc, 'Link'); + const hasLinkInDesc = hasTagInDesc(apiDoc.desc, 'Link') || hasTagInDesc(apiDoc.returnDesc, 'Link'); @@ - const hasLinkToApiInDesc = hasTag(apiDoc, 'LinkToApi'); + const hasLinkToApiInDesc = + hasTagInDesc(apiDoc.desc, 'LinkToApi') || hasTagInDesc(apiDoc.returnDesc, 'LinkToApi');test/chart/BarChart.spec.tsx (1)
102-104:⚠️ Potential issue | 🟠 MajorEnable fake timers in beforeEach and cleanup in afterEach.
The test file uses
vi.runOnlyPendingTimers()andvi.advanceTimersByTime()(lines 75, 79, 86, 173, 243, 3732, 3741, 3752, 3761, 3793, 3805) but never enables fake timers, causing these calls to fail or behave unexpectedly. Addvi.useFakeTimers()in the beforeEach and cleanup withvi.useRealTimers()in afterEach.Suggested fix
beforeEach(() => { + vi.useFakeTimers(); mockGetBoundingClientRect({ width: 100, height: 100 }); }); + +afterEach(() => { + vi.runOnlyPendingTimers(); + vi.useRealTimers(); +});Also applies to lines 725-953 and any other nested describe blocks.
🤖 Fix all issues with AI agents
In `@src/state/selectors/axisSelectors.ts`:
- Around line 92-94: Change the runtime import of the type to a type-only import
to avoid the circular dependency: replace the current import of
InverseScaleFunction with a type-only import (i.e., import type {
InverseScaleFunction } from '...') in axisSelectors.ts where
InverseScaleFunction is referenced; ensure all usages in return type annotations
(the selectors that return InverseScaleFunction) remain unchanged so the symbol
is only used at type level and does not introduce a runtime import that would
trigger the circular dependency with hooks.
- Around line 504-526: selectSortedDataPoints currently sorts domain values
numerically via sortBy which breaks categorical or reversed axes because bisect
later expects pixels ordered by the scale; instead compute and sort by pixel
positions using the axis scale before bisecting. Update selectSortedDataPoints
(and similarly selectAxisInverseDataSnapScale) to map each domain value to its
pixel via the scale (e.g., {value, pixel: scale(value) ?? 0}), sort that array
by pixel, then expose both sorted pixelPositions and the correspondingly-ordered
domain values (or return domain ordered by pixel) so createCategoricalInverse
can use the pixel-sorted arrays for bisecting rather than the numeric sortBy
order.
In `@src/util/scale/createCategoricalInverse.ts`:
- Around line 14-28: The bisect function can crash or behave unpredictably for
empty or single-element arrays because it dereferences haystack[0] and
haystack[haystack.length - 1]; change bisect (the function named bisect) to
first handle haystack.length === 0 by returning 0, and treat arrays with length
< 2 as trivially ascending (e.g., set const ascending = haystack.length < 2 ?
true : haystack[0] < haystack[haystack.length - 1]) and remove the non-null
assertions; keep the existing binary search logic otherwise so bisect safely
handles empty and single-element inputs called from createCategoricalInverse.
- Around line 1-71: Add comprehensive unit tests for the bisect helper and
createCategoricalInverse function: write tests that call bisect directly on
ascending and descending numeric arrays and verify insertion indices; and tests
for createCategoricalInverse covering these cases: undefined scale returns
undefined; empty domain and range length < 2 return undefined; single-element
domain returns that element for any pixel; normal categorical mapping where
scale.domain() and scale.range() produce pixelPositions (use a fake/mock
CustomScaleDefinition with scale(d), domain(), range()); verify clamping to
first/last domain when pixel is outside bounds; verify correct neighbor
selection (tie-breaking uses left when equidistant); and verify that passing
allDataPointsOnAxis overrides scale.domain(). Use assertions on returned values
to exercise every branch in createCategoricalInverse and bisect.
In `@test/hooks/useAxisTicks.spec.tsx`:
- Around line 1-18: The test suite "new axis hooks" fails because
createSelectorTestCase calls vi.runOnlyPendingTimers() but no fake timers are
enabled; enable Vitest fake timers for this suite by calling vi.useFakeTimers()
before tests (e.g., in a beforeEach or at the top of the describe) and restore
them with vi.useRealTimers() after tests (e.g., in an afterEach), so functions
like createSelectorTestCase that invoke vi.runOnlyPendingTimers() (and any
timer-based Redux autobatching) operate correctly.
In `@www/src/components/GuideView/CoordinateSystems/index.tsx`:
- Around line 189-201: Replace the non-exported hook reference useDataToPixels
with the actual exported API: update the list item to reference
useCartesianScale (or export an alias named useDataToPixels that re-exports
useCartesianScale) and ensure the LinkToApi target points to the real exported
symbol; specifically modify the docs in the CoordinateSystems component to use
useCartesianScale (or add an export alias in the module that exports
useCartesianScale) so the LinkToApi link is valid.
In `@www/src/views/GuideView.tsx`:
- Line 3: The CoordinateSystems component signature doesn't match the typed
guideMap; change the component export from "export function CoordinateSystems()"
to accept the locale prop like "export function CoordinateSystems({ locale }: {
locale: SupportedLocale })" so it conforms to Record<string, ComponentType<{
locale: SupportedLocale }>> (even if locale is unused), and update any internal
references or imports as needed to keep types consistent with SupportedLocale
and the other guide components (e.g., Installation).
🧹 Nitpick comments (4)
www/src/components/GuideView/CoordinateSystems/ScaleConversionExample.tsx (1)
12-24: JSDoc mentions hooks but example uses ReferenceDot instead.The documentation comment mentions
useXAxisScale(),useYAxisScale(),useXAxisInverseScale(),useYAxisInverseScale(), anduseDataToPixels()hooks, but the example solely demonstratesReferenceDotwhich handles conversion internally. Consider adding a brief example showing actual hook usage, or adjusting the comment to clarify this is a simpler approach that doesn't require hooks.src/util/scale/createCategoricalInverse.ts (1)
30-50: Good structure with appropriate JSDoc, but consider adding return type documentation.The function signature and behavior are clear. The
@ts-expect-erroron Line 43 is acceptable given the challenge of typing generic domain values.Consider expanding the JSDoc to document the return value behavior:
📝 Enhanced JSDoc suggestion
/** * Computes an inverse scale function for categorical/ordinal scales. * Uses bisect to find the closest domain value for a given pixel coordinate. + * + * `@param` scale - The scale definition to invert + * `@param` allDataPointsOnAxis - Optional override for the domain values + * `@returns` An inverse function, or undefined if scale is missing or domain is empty */test/state/mouseEventsMiddleware.spec.ts (1)
1-25: Ensure fake timers are enabled for the timer-count assertions.
If not already set in global test setup, addvi.useFakeTimers()here to keepvi.getTimerCount()/runOnlyPendingTimers()reliable.As per coding guidelines: Use `vi.useFakeTimers()` in all tests due to Redux autoBatchEnhancer dependency on timers and `requestAnimationFrame`.Suggested setup (if not already global)
beforeEach(() => { + vi.useFakeTimers(); store = createRechartsStore(); }); + +afterEach(() => { + vi.useRealTimers(); +});www/src/docs/exampleComponents/AreaChart/AreaChartFillByValue.tsx (1)
52-66: Guard against zero/invalid height when computing gradient offset.
Clamping avoids invalid offsets when height is 0 or scale output falls outside the expected range.Suggested tweak
const scale = useYAxisScale(); const height = useChartHeight(); const scaledZero = scale?.(0); - if (scaledZero == null || height == null) { + if (scaledZero == null || height == null || height <= 0) { return null; } - const ratio = scaledZero / height; + const ratio = Math.min(1, Math.max(0, scaledZero / height));
| function sortBy(a: unknown, b: unknown): number { | ||
| const aNum = makeNumber(a); | ||
| const bNum = makeNumber(b); | ||
| if (aNum == null && bNum == null) { | ||
| return 0; | ||
| } | ||
| if (aNum == null) { | ||
| return -1; | ||
| } | ||
| if (bNum == null) { | ||
| return 1; | ||
| } | ||
| return aNum - bNum; | ||
| } | ||
|
|
||
| export const selectSortedDataPoints: ( | ||
| state: RechartsRootState, | ||
| axisType: AllAxisTypes, | ||
| axisId: AxisId, | ||
| isPanorama: boolean, | ||
| ) => ReadonlyArray<unknown> | undefined = createSelector([selectAllAppliedValues], appliedData => { | ||
| return appliedData?.map(item => item.value).sort(sortBy); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file structure and read the relevant sections
wc -l src/state/selectors/axisSelectors.tsRepository: recharts/recharts
Length of output: 102
🏁 Script executed:
# Read lines 504-526 and surrounding context
sed -n '490,530p' src/state/selectors/axisSelectors.ts | cat -nRepository: recharts/recharts
Length of output: 1326
🏁 Script executed:
# Check lines 1495-1510 (mentioned as "Also applies to")
sed -n '1485,1520p' src/state/selectors/axisSelectors.ts | cat -nRepository: recharts/recharts
Length of output: 1394
🏁 Script executed:
# Search for createCategoricalInverse definition
rg -A 30 "function createCategoricalInverse|const createCategoricalInverse" src/state/selectors/axisSelectors.tsRepository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Search for bisect usage in the file
rg -B 3 -A 3 "bisect" src/state/selectors/axisSelectors.tsRepository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Search for createCategoricalInverse across all files
rg "createCategoricalInverse" --type ts --type tsxRepository: recharts/recharts
Length of output: 88
🏁 Script executed:
# Search for bisect function
rg "bisect" --type ts --type tsxRepository: recharts/recharts
Length of output: 88
🏁 Script executed:
# Look at imports in axisSelectors.ts
head -50 src/state/selectors/axisSelectors.tsRepository: recharts/recharts
Length of output: 1440
🏁 Script executed:
# Continue reading imports to find createCategoricalInverse
head -100 src/state/selectors/axisSelectors.ts | tail -50Repository: recharts/recharts
Length of output: 3018
🏁 Script executed:
# Search for createCategoricalInverse in all typescript files
rg "createCategoricalInverse"Repository: recharts/recharts
Length of output: 850
🏁 Script executed:
# Search for bisect in all files
rg "bisect"Repository: recharts/recharts
Length of output: 619
🏁 Script executed:
# Read the createCategoricalInverse implementation
cat -n src/util/scale/createCategoricalInverse.tsRepository: recharts/recharts
Length of output: 2823
🏁 Script executed:
# Now let me check how selectSortedDataPoints is actually used
# Look at the selector definition again and trace the issue
sed -n '1495,1520p' src/state/selectors/axisSelectors.tsRepository: recharts/recharts
Length of output: 880
🏁 Script executed:
# Let's also check if there are any tests for this behavior
fd "axisSelectors" -e test -e specRepository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Check what makeNumber does to categorical values
rg -A 10 "function makeNumber|export.*makeNumber" src/util/DataUtils.tsRepository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Let me check what the scale.domain() returns and understand better
# Search for scale usage in categorical contexts
rg -B 5 -A 5 "isCategoricalAxis" src/state/selectors/axisSelectors.ts | head -50Repository: recharts/recharts
Length of output: 1568
🏁 Script executed:
# Check for categorical scale types and how they work
rg "ordinal|categorical" src/util/scale/RechartsScale.ts | head -20Repository: recharts/recharts
Length of output: 124
🏁 Script executed:
# Let's trace what happens with the sorted data points
# by looking at the actual scale implementation
fd "RechartsScale" -e tsRepository: recharts/recharts
Length of output: 125
🏁 Script executed:
# Find and read the makeNumber function
rg -B 3 -A 15 "export.*function makeNumber|function makeNumber" --type tsRepository: recharts/recharts
Length of output: 1230
🏁 Script executed:
# Let's also check what selectAllAppliedValues returns
rg -B 5 -A 10 "selectAllAppliedValues" src/state/selectors/axisSelectors.ts | head -40Repository: recharts/recharts
Length of output: 1394
🏁 Script executed:
# Let's understand how categorical scales and domains work
# Check how the domain is built for categorical axes
rg -B 5 -A 15 "function combineCheckedDomain" src/state/selectors/combiners/combineCheckedDomain.tsRepository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Also check if there are any tests showing the issue
fd "test" -type f -name "*axisSelectors*" -o -name "*categorical*"Repository: recharts/recharts
Length of output: 231
🏁 Script executed:
# Check how selectConfiguredScale determines the domain
rg -B 5 -A 15 "selectConfiguredScale\b" src/state/selectors/axisSelectors.ts | head -40Repository: recharts/recharts
Length of output: 1345
🏁 Script executed:
# Let me check what selectAllAppliedValues actually contains
# and when selectSortedDataPoints is actually used
rg -B 10 -A 5 "selectAxisInverseDataSnapScale" src/state/selectors/axisSelectors.tsRepository: recharts/recharts
Length of output: 635
🏁 Script executed:
# Check when selectSortedDataPoints is used
rg "selectSortedDataPoints" src/Repository: recharts/recharts
Length of output: 224
🏁 Script executed:
# Let me trace through how the domain is actually used
# Check if there's any special handling in combineConfiguredScale
rg -B 5 -A 20 "combineConfiguredScale\(" src/state/selectors/combiners/combineConfiguredScale.tsRepository: recharts/recharts
Length of output: 672
🏁 Script executed:
# Look for ordinal or band scale usage
rg "band|ordinal" src/util/scale/Repository: recharts/recharts
Length of output: 1586
🏁 Script executed:
# Let me understand when selectSortedDataPoints is actually passed
# Check combineAppliedValues to see what values are in selectAllAppliedValues
rg -B 5 -A 30 "export.*combineAppliedValues" src/state/selectors/combiners/combineAppliedValues.tsRepository: recharts/recharts
Length of output: 154
🏁 Script executed:
# Check if there's special handling for categorical vs numeric axes
rg -B 10 -A 10 "selectSortedDataPoints\|selectAllAppliedValues" src/state/selectors/axisSelectors.ts | grep -A 40 "selectAxisInverseDataSnapScale"Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Find where combineAppliedValues is defined
rg "export.*combineAppliedValues|function combineAppliedValues"Repository: recharts/recharts
Length of output: 135
🏁 Script executed:
# Let me understand the actual problem better by creating a concrete example
# First, let's see if there are any tests that would show the issue
find . -name "*.test.ts" -o -name "*.spec.ts" | grep -i "categorical\|axis" | head -10Repository: recharts/recharts
Length of output: 148
🏁 Script executed:
# Check the usage pattern - is selectSortedDataPoints actually used for categorical axes?
rg -B 20 "createCategoricalInverse" src/state/selectors/axisSelectors.ts | grep -A 20 "selectAxisInverseDataSnapScale"Repository: recharts/recharts
Length of output: 343
🏁 Script executed:
# Let me check how the domain is actually set on the scale
# Look at the scale function more carefully
rg -B 5 -A 30 "export const selectAxisInverseDataSnapScale" src/state/selectors/axisSelectors.tsRepository: recharts/recharts
Length of output: 1076
🏁 Script executed:
# Check if selectSortedDataPoints could have non-monotonic pixel positions
# by understanding what makeNumber does with categorical values
rg -B 10 -A 20 "export const combineAppliedValues" src/state/selectors/axisSelectors.tsRepository: recharts/recharts
Length of output: 1134
🏁 Script executed:
# Let me create a test scenario to understand the issue better
# Check if there are any categorical axis tests
cat test/state/selectors/cartesianAxisSlice.spec.ts | head -100Repository: recharts/recharts
Length of output: 3202
🏁 Script executed:
# Let me verify the actual issue by checking when selectSortedDataPoints is used
# and what happens with categorical numeric values
rg -B 30 "selectAxisInverseDataSnapScale" src/state/selectors/axisSelectors.ts | grep -A 30 "selectSortedDataPoints"Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Check if there's a case where the domain order differs from numeric sort order
# Let's look at what happens with reversed or categorical numeric axes
rg -B 5 -A 15 "reversed\|type.*category" src/util/scale/combineConfiguredScale.tsRepository: recharts/recharts
Length of output: 141
🏁 Script executed:
# The key issue: does sortBy produce a different order than scale.map?
# Let me check a concrete example by looking for reversed axis handling
rg -B 5 -A 15 "reversed" src/state/selectors/combiners/combineAxisRangeWithReverse.ts | head -50Repository: recharts/recharts
Length of output: 289
🏁 Script executed:
# Let me trace through the actual flow more carefully
# The key issue is: does selectSortedDataPoints override the domain order?
# Let's check what domain() returns on a categorical scale
rg -B 10 -A 10 "function domain\|\.domain\(" src/util/scale/RechartsScale.ts | head -60Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# More importantly - when allDataPointsOnAxis is provided to createCategoricalInverse,
# it replaces scale.domain(). Let's verify this logic:
sed -n '30,50p' src/util/scale/createCategoricalInverse.tsRepository: recharts/recharts
Length of output: 828
🏁 Script executed:
# The critical question: can the sorted data points be in a different order than scale.domain()?
# This would happen if:
# 1. Scale domain is categorical with custom order (e.g., ["C", "A", "B"])
# 2. selectSortedDataPoints sorts them numerically (e.g., ["A", "B", "C"])
# Let me check if there are categorical axes with numeric/string values that could be sorted differently
rg -B 5 -A 15 "isCategorical.*type.*=.*category" src/state/selectors/axisSelectors.ts | head -50Repository: recharts/recharts
Length of output: 719
🏁 Script executed:
# Let me trace the actual issue step by step
# Check what type of scale is used and when selectSortedDataPoints would be problematic
# First, understand: is selectSortedDataPoints only for specific axis types?
rg -B 15 "selectAxisInverseDataSnapScale" src/state/selectors/axisSelectors.ts | grep -B 15 "createCategoricalInverse"Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
# Check if there are any guards or conditions on when this selector is used
rg "selectAxisInverseDataSnapScale" src/Repository: recharts/recharts
Length of output: 423
🏁 Script executed:
# The critical test: can a categorical numeric axis have values that sort differently than the domain?
# Check how categorical numeric axes work in tests
find . -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "category\|categorical" | head -5Repository: recharts/recharts
Length of output: 291
Data-snap ordering can be wrong for categorical or reversed axes.
selectSortedDataPoints sorts values numerically via sortBy, but when passed to createCategoricalInverse as allDataPointsOnAxis, it overrides the scale's domain order. For categorical axes with numeric categories, custom ordering, or reversed ranges, this causes pixelPositions to be mapped in the wrong order. Since bisect then searches based on this incorrectly-ordered pixel array, it can return the wrong nearest value.
The scale itself was constructed with the original categorical domain order, but bisect operates on pixel positions calculated from the numerically-sorted domain—creating a mismatch.
Consider sorting by scale.map output (pixel positions) instead, or reordering the domain inside createCategoricalInverse before bisecting:
Example fix
const domainWithPixels = domain
.map(value => ({ value, pixel: scale(value) ?? 0 }))
.sort((a, b) => a.pixel - b.pixel);
const pixelPositions = domainWithPixels.map(d => d.pixel);
const sortedDomain = domainWithPixels.map(d => d.value);
// Use sortedDomain + pixelPositions in bisectAlso applies to: selectAxisInverseDataSnapScale (lines 1495–1510)
🤖 Prompt for AI Agents
In `@src/state/selectors/axisSelectors.ts` around lines 504 - 526,
selectSortedDataPoints currently sorts domain values numerically via sortBy
which breaks categorical or reversed axes because bisect later expects pixels
ordered by the scale; instead compute and sort by pixel positions using the axis
scale before bisecting. Update selectSortedDataPoints (and similarly
selectAxisInverseDataSnapScale) to map each domain value to its pixel via the
scale (e.g., {value, pixel: scale(value) ?? 0}), sort that array by pixel, then
expose both sorted pixelPositions and the correspondingly-ordered domain values
(or return domain ordered by pixel) so createCategoricalInverse can use the
pixel-sorted arrays for bisecting rather than the numeric sortBy order.
| import { InverseScaleFunction } from '../../hooks'; | ||
| import { CustomScaleDefinition } from './CustomScaleDefinition'; | ||
|
|
||
| /** | ||
| * Binary search to find the index where x would fit in array a. | ||
| * Works for arrays that are sorted both ascending and descending. | ||
| * | ||
| * Unlike d3.bisect, this implementation handles both ascending and descending arrays. | ||
| * | ||
| * @param haystack Sorted array of numbers | ||
| * @param needle Number to find the insertion index for | ||
| * @returns Index where x would fit in array a | ||
| */ | ||
| function bisect(haystack: ReadonlyArray<number>, needle: number): number { | ||
| let lo = 0; | ||
| let hi = haystack.length; | ||
| const ascending = haystack[0]! < haystack[haystack.length - 1]!; | ||
|
|
||
| while (lo < hi) { | ||
| const mid = Math.floor((lo + hi) / 2); | ||
| if (ascending ? haystack[mid]! < needle : haystack[mid]! > needle) { | ||
| lo = mid + 1; | ||
| } else { | ||
| hi = mid; | ||
| } | ||
| } | ||
| return lo; | ||
| } | ||
|
|
||
| /** | ||
| * Computes an inverse scale function for categorical/ordinal scales. | ||
| * Uses bisect to find the closest domain value for a given pixel coordinate. | ||
| */ | ||
| export function createCategoricalInverse( | ||
| scale: CustomScaleDefinition | undefined, | ||
| allDataPointsOnAxis?: ReadonlyArray<unknown>, | ||
| ): InverseScaleFunction | undefined { | ||
| if (!scale) { | ||
| return undefined; | ||
| } | ||
| const domain = allDataPointsOnAxis ?? scale.domain(); | ||
| // Build an array of pixel positions for each domain value | ||
| // @ts-expect-error we're attempting to scale unknown without having guarantee that it is a Domain type | ||
| const pixelPositions: number[] = domain.map(d => scale(d) ?? 0); | ||
| const range = scale.range(); | ||
|
|
||
| if (domain.length === 0 || range.length < 2) { | ||
| return undefined; | ||
| } | ||
|
|
||
| return (pixelValue: number): unknown => { | ||
| // Find the closest domain value using bisect | ||
| const index = bisect(pixelPositions, pixelValue); | ||
|
|
||
| // Clamp to valid range | ||
| if (index <= 0) { | ||
| return domain[0]; | ||
| } | ||
| if (index >= domain.length) { | ||
| return domain[domain.length - 1]; | ||
| } | ||
|
|
||
| // Check which neighbor is closer | ||
| const leftPixel = pixelPositions[index - 1] ?? 0; | ||
| const rightPixel = pixelPositions[index] ?? 0; | ||
| if (Math.abs(pixelValue - leftPixel) <= Math.abs(pixelValue - rightPixel)) { | ||
| return domain[index - 1]; | ||
| } | ||
| return domain[index]; | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for test files covering createCategoricalInverse
rg 'createCategoricalInverse' --type ts --type tsx --type js --type jsx | grep -E '\.(test|spec)\.(ts|tsx|js|jsx):'Repository: recharts/recharts
Length of output: 88
🏁 Script executed:
#!/bin/bash
# Also search more broadly for test files
find . -type f \( -name "*.test.ts" -o -name "*.test.tsx" -o -name "*.spec.ts" -o -name "*.spec.tsx" \) | xargs rg -l 'createCategoricalInverse' 2>/dev/nullRepository: recharts/recharts
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if there's a test directory and what test files exist related to scale
fd -e test.ts -e spec.ts 'scale' | head -20Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Search for any test file that mentions createCategoricalInverse
find . -name "*.test.ts" -o -name "*.test.js" -o -name "*.spec.ts" -o -name "*.spec.js" | xargs grep -l "createCategoricalInverse" 2>/dev/nullRepository: recharts/recharts
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Look at the directory structure around the source file
ls -la src/util/scale/Repository: recharts/recharts
Length of output: 644
🏁 Script executed:
#!/bin/bash
# Search more broadly for createCategoricalInverse mentions anywhere in the repo
grep -r "createCategoricalInverse" --include="*.ts" --include="*.tsx" --include="*.js" --include="*.jsx" . 2>/dev/null | head -30Repository: recharts/recharts
Length of output: 617
🏁 Script executed:
#!/bin/bash
# Search for test files in the entire src directory
find src -type f \( -name "*.test.ts" -o -name "*.test.js" \) | head -20Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check if there's a __tests__ or test directory structure
find . -type d -name "__tests__" -o -type d -name "tests" -o -type d -name "test" | grep -v node_modules | head -20Repository: recharts/recharts
Length of output: 124
🏁 Script executed:
#!/bin/bash
# Look for any test files related to scale utilities
find . -path ./node_modules -prune -o -type f \( -name "*scale*.test.ts" -o -name "*scale*.spec.ts" \) -printRepository: recharts/recharts
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check what's in the test directory
ls -la test/ | head -30Repository: recharts/recharts
Length of output: 1315
🏁 Script executed:
#!/bin/bash
# Search for createCategoricalInverse in test directory
grep -r "createCategoricalInverse" test/ 2>/dev/nullRepository: recharts/recharts
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Look for scale-related tests
find test -type f -name "*scale*" 2>/dev/nullRepository: recharts/recharts
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Search for any tests that might indirectly test this utility
find test -type f -name "*axis*" -o -name "*inverse*" 2>/dev/nullRepository: recharts/recharts
Length of output: 186
🏁 Script executed:
#!/bin/bash
# Check the axisSelectors test file
wc -l test/state/selectors/axisSelectors.spec.tsxRepository: recharts/recharts
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Look at the axisSelectors test file
cat test/state/selectors/axisSelectors.spec.tsxRepository: recharts/recharts
Length of output: 50373
🏁 Script executed:
#!/bin/bash
# Search for createCategoricalInverse in the test file
grep -n "createCategoricalInverse" test/state/selectors/axisSelectors.spec.tsxRepository: recharts/recharts
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check if there's a test file for combineInverseScaleFunction
find test -name "*combineInverseScaleFunction*" -o -name "*inverse*"Repository: recharts/recharts
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Search for any test that mentions the inverse scale function
find test -type f -name "*.tsx" -o -name "*.ts" | xargs grep -l "InverseScaleFunction" 2>/dev/nullRepository: recharts/recharts
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if there's a dedicated test directory for scale utilities
find test -type d -name "*scale*" 2>/dev/nullRepository: recharts/recharts
Length of output: 76
🏁 Script executed:
#!/bin/bash
# Check what test files exist in test/util/scale directory
ls -la test/util/scale/Repository: recharts/recharts
Length of output: 402
🏁 Script executed:
#!/bin/bash
# Search for any test file that might test createCategoricalInverse
find test/util/scale -type f -name "*.test.*" -o -name "*.spec.*"Repository: recharts/recharts
Length of output: 186
🏁 Script executed:
#!/bin/bash
# List all files in test/util/scale with details
find test/util/scale -type fRepository: recharts/recharts
Length of output: 186
Add comprehensive unit tests for createCategoricalInverse utility.
This utility exports a public function with complex logic including binary search and multiple edge cases (empty domain, single element, ascending vs descending ranges) but currently has no test coverage. Per coding guidelines, source files in src/ must aim for 100% unit test coverage. Add unit tests covering the bisect helper function and all branches of createCategoricalInverse, including the edge cases mentioned.
🤖 Prompt for AI Agents
In `@src/util/scale/createCategoricalInverse.ts` around lines 1 - 71, Add
comprehensive unit tests for the bisect helper and createCategoricalInverse
function: write tests that call bisect directly on ascending and descending
numeric arrays and verify insertion indices; and tests for
createCategoricalInverse covering these cases: undefined scale returns
undefined; empty domain and range length < 2 return undefined; single-element
domain returns that element for any pixel; normal categorical mapping where
scale.domain() and scale.range() produce pixelPositions (use a fake/mock
CustomScaleDefinition with scale(d), domain(), range()); verify clamping to
first/last domain when pixel is outside bounds; verify correct neighbor
selection (tie-breaking uses left when equidistant); and verify that passing
allDataPointsOnAxis overrides scale.domain(). Use assertions on returned values
to exercise every branch in createCategoricalInverse and bisect.
| function bisect(haystack: ReadonlyArray<number>, needle: number): number { | ||
| let lo = 0; | ||
| let hi = haystack.length; | ||
| const ascending = haystack[0]! < haystack[haystack.length - 1]!; | ||
|
|
||
| while (lo < hi) { | ||
| const mid = Math.floor((lo + hi) / 2); | ||
| if (ascending ? haystack[mid]! < needle : haystack[mid]! > needle) { | ||
| lo = mid + 1; | ||
| } else { | ||
| hi = mid; | ||
| } | ||
| } | ||
| return lo; | ||
| } |
There was a problem hiding this comment.
Potential crash on empty or single-element arrays in bisect.
The ascending calculation on Line 17 accesses haystack[0] and haystack[haystack.length - 1] with non-null assertions. If haystack is empty, this will throw. Additionally, for a single-element array, both indices reference the same element, making the ascending check unreliable.
The function is called from createCategoricalInverse which does check for empty domain on Line 47, but pixelPositions is built before that check, so bisect could theoretically be called with an empty array if the flow changes in the future.
🛡️ Proposed defensive fix
function bisect(haystack: ReadonlyArray<number>, needle: number): number {
+ if (haystack.length === 0) {
+ return 0;
+ }
let lo = 0;
let hi = haystack.length;
- const ascending = haystack[0]! < haystack[haystack.length - 1]!;
+ const ascending = haystack.length < 2 || haystack[0]! < haystack[haystack.length - 1]!;🤖 Prompt for AI Agents
In `@src/util/scale/createCategoricalInverse.ts` around lines 14 - 28, The bisect
function can crash or behave unpredictably for empty or single-element arrays
because it dereferences haystack[0] and haystack[haystack.length - 1]; change
bisect (the function named bisect) to first handle haystack.length === 0 by
returning 0, and treat arrays with length < 2 as trivially ascending (e.g., set
const ascending = haystack.length < 2 ? true : haystack[0] <
haystack[haystack.length - 1]) and remove the non-null assertions; keep the
existing binary search logic otherwise so bisect safely handles empty and
single-element inputs called from createCategoricalInverse.
|
Staging Deployment Details
These deployments will remain available for 30 days. To update snapshots: Comment |
8fff91c to
c848d88
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/hooks/useAxisScale.spec.tsx`:
- Around line 1-13: This test file renders charts with timers and layout
measurements; add local fake-timer setup and bounding-rect mocks to prevent
flakiness by calling vi.useFakeTimers() at test start and vi.useRealTimers() in
teardown, and apply the provided MockGetBoundingClientRect helper to mock
element.getBoundingClientRect during the test run; update useAxisScale.spec.tsx
(around the top-level describe or beforeEach/afterEach) to import and invoke
MockGetBoundingClientRect and ensure timers are restored after tests so symbols
like vi.useFakeTimers, vi.useRealTimers, and MockGetBoundingClientRect are used
to wrap the existing createSelectorTestCase/chart-rendering tests.
🧹 Nitpick comments (1)
test/hooks/useAxisScale.spec.tsx (1)
24-268: Add selector call-count assertions for createSelectorTestCase spies.This keeps selector re-renders in check and aligns with existing test conventions.
As per coding guidelines, Verify the number of selector calls using the spy object from
createSelectorTestCaseto spot unnecessary re-renders and improve performance.
| import React from 'react'; | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { createSelectorTestCase } from '../helper/createSelectorTestCase'; | ||
| import { Line, LineChart, XAxis, YAxis, Brush } from '../../src'; | ||
| import { | ||
| useXAxisScale, | ||
| useYAxisScale, | ||
| useXAxisInverseScale, | ||
| useYAxisInverseScale, | ||
| useCartesianScale, | ||
| } from '../../src/hooks'; | ||
| import { PageData } from '../_data'; | ||
|
|
There was a problem hiding this comment.
Add test setup for fake timers and bounding-rect mocks.
These tests render charts and rely on timers; add local setup (or ensure a shared setup does it) to avoid flakiness.
🧪 Suggested setup
-import { describe, it, expect } from 'vitest';
+import { beforeEach, afterEach, describe, it, expect, vi } from 'vitest';
+import { mockGetBoundingClientRect } from '../helper/mockGetBoundingClientRect';
+
+beforeEach(() => {
+ vi.useFakeTimers();
+ mockGetBoundingClientRect({ width: 500, height: 300 });
+});
+
+afterEach(() => {
+ vi.useRealTimers();
+});As per coding guidelines, Use vi.useFakeTimers() in all tests due to Redux autoBatchEnhancer dependency on timers and requestAnimationFrame; Mock getBoundingClientRect in tests using the helper function provided in test/helper/MockGetBoundingClientRect.ts.
🤖 Prompt for AI Agents
In `@test/hooks/useAxisScale.spec.tsx` around lines 1 - 13, This test file renders
charts with timers and layout measurements; add local fake-timer setup and
bounding-rect mocks to prevent flakiness by calling vi.useFakeTimers() at test
start and vi.useRealTimers() in teardown, and apply the provided
MockGetBoundingClientRect helper to mock element.getBoundingClientRect during
the test run; update useAxisScale.spec.tsx (around the top-level describe or
beforeEach/afterEach) to import and invoke MockGetBoundingClientRect and ensure
timers are restored after tests so symbols like vi.useFakeTimers,
vi.useRealTimers, and MockGetBoundingClientRect are used to wrap the existing
createSelectorTestCase/chart-rendering tests.
Bundle ReportChanges will increase total bundle size by 42.74kB (1.51%) ⬆️. 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-umdAssets Changed:
view changes for bundle: recharts/bundle-es6Assets Changed:
|
|
Staging Deployment Details
These deployments will remain available for 30 days. To update snapshots: Comment |
|
Staging Deployment Details
These deployments will remain available for 30 days. To update snapshots: Comment |
2 participants
Description
Adds couple of new hooks with tests and documentation.
The examples are lacking, I will add more in next PR.
Related Issue
#1678
Closes #6021
Motivation and Context
We still don't get over-the-shelf annotations but with these hooks it's fairly easy to build your own.
How Has This Been Tested?
I added annotations support to the devtools package but that I can only merge after we have released 3.8 to npm
Screenshots (if appropriate):
Screen.Recording.2026-02-03.at.17.53.13.mov
Types of changes
Checklist:
Summary by CodeRabbit
New Features
API Additions
Documentation