Reviewing files that changed from the base of the PR and between 8bb5aa4 and 20ea880.

• test-vr/__snapshots__/tests/www/PieChartApiExamples.spec-vr.tsx-snapshots/PieChartExample-1-chromium-linux.png is excluded by !**/*.png
• test-vr/__snapshots__/tests/www/PieChartApiExamples.spec-vr.tsx-snapshots/PieChartExample-1-firefox-linux.png is excluded by !**/*.png
• test-vr/__snapshots__/tests/www/PieChartApiExamples.spec-vr.tsx-snapshots/PieChartExample-1-webkit-linux.png is excluded by !**/*.png
• test-vr/__snapshots__/tests/www/PieChartApiExamples.spec-vr.tsx-snapshots/TwoLevelPieChart-1-chromium-linux.png is excluded by !**/*.png
• test-vr/__snapshots__/tests/www/PieChartApiExamples.spec-vr.tsx-snapshots/TwoLevelPieChart-1-firefox-linux.png is excluded by !**/*.png
• test-vr/__snapshots__/tests/www/PieChartApiExamples.spec-vr.tsx-snapshots/TwoLevelPieChart-1-webkit-linux.png is excluded by !**/*.png

• src/cartesian/Area.tsx
• src/cartesian/Bar.tsx
• src/cartesian/Brush.tsx
• src/cartesian/Funnel.tsx
• src/cartesian/Line.tsx
• src/cartesian/Scatter.tsx
• src/chart/Sankey.tsx
• src/chart/SunburstChart.tsx
• src/chart/Treemap.tsx
• src/polar/Pie.tsx
• src/polar/Radar.tsx
• src/polar/RadialBar.tsx
• src/state/chartDataSlice.ts
• src/state/optionsSlice.ts
• src/state/selectors/combiners/combineCoordinateForDefaultIndex.ts
• src/state/selectors/selectTooltipPayloadSearcher.ts
• src/state/selectors/selectors.ts
• src/state/selectors/tooltipSelectors.ts
• src/state/selectors/touchSelectors.ts
• src/state/tooltipSlice.ts
• src/util/DataUtils.ts
• src/util/types.ts
• test-vr/tests/www/PieChartApiExamples.spec-vr.tsx
• test/cartesian/Area.spec.tsx
• test/cartesian/Bar.spec.tsx
• test/chart/ScatterChart.spec.tsx
• test/component/Tooltip/Tooltip.payload.spec.tsx
• test/component/Tooltip/Tooltip.visibility.spec.tsx
• test/component/Tooltip/itemSorter.spec.tsx
• test/helper/expectFunctionReturning.ts
• test/polar/Pie.spec.tsx
• test/state/selectors/axisSelectors.spec.tsx
• test/state/selectors/selectors.spec.tsx
• test/tsconfig.json
• vitest.config.mts
• www/src/docs/apiExamples/PieChart/PieChartExample.tsx
• www/src/docs/apiExamples/PieChart/index.tsx
• www/src/docs/exampleComponents/PieChart/TwoLevelPieChart.tsx

• src/state/selectors/tooltipSelectors.ts
• www/src/docs/apiExamples/PieChart/PieChartExample.tsx
• src/state/selectors/selectors.ts

• GitHub Check: Build, Test, Pack
• GitHub Check: Analyze (javascript-typescript)

test/state/selectors/axisSelectors.spec.tsx (1)

176-176: LGTM! Test structure simplification.

The direct usage of ExpectAxisDomain removes an unnecessary wrapper layer, making these tests more straightforward while maintaining identical functionality and assertions.

Also applies to: 241-241, 1452-1452

test/tsconfig.json (1)

11-11: LGTM!

The new helper file is correctly added to the TypeScript include list, following the same pattern as the existing toBeRechartsScale.ts entry.

src/state/tooltipSlice.ts (2)

46-51: LGTM - Type simplification.

Removing the unused generic R from TooltipPayloadSearcher simplifies the type while maintaining the same functionality. The return type T | undefined is appropriate for the searcher pattern.

---

76-80: LGTM - Good API improvement.

Replacing the optional positions array/map with a required getPosition function is a clean design choice that:

1. Eliminates the type ambiguity between array and map
2. Makes the positioning logic lazy and dynamic
3. Uses NonNullable<TooltipIndex> to ensure callers handle null indices before calling

The added documentation clearly explains the expected behavior.

test/cartesian/Area.spec.tsx (2)

25-25: LGTM!

The import of noop from the internal path is appropriate for test files. Based on learnings, test files are allowed to import from internal paths.

---

869-887: LGTM - Test expectation updated correctly.

The test expectation is properly updated to use getPosition: noop instead of the previous positions: undefined, aligning with the new tooltip positioning API.

src/util/types.ts (2)

36-36: LGTM - Good type import addition.

Adding the ChartData import centralizes the data type definition, ensuring consistency across the codebase.

---

1332-1332: Type improvement from any[] to ChartData is sound and beneficial.

Changing data to ChartData (ReadonlyArray<unknown>) in the BaseChartProps interface is a positive change that:

1. Eliminates any type per coding guidelines
2. Makes the data array readonly, preventing accidental mutations
3. Uses unknown for better type safety

As a public API change in component props, consumers who attempt direct array mutations will see TypeScript errors—the intended behavior. No evidence of such mutations exists in the codebase.

test/cartesian/Bar.spec.tsx (3)

46-46: LGTM!

The import of noop from the internal path is appropriate for test files per coding guidelines.

---

1616-1651: LGTM - Test expectations updated correctly.

The tooltip item payload expectations are properly updated to use getPosition: noop instead of the previous positions: undefined, correctly reflecting the new API structure.

---

1689-1724: LGTM - Consistent updates across test cases.

The post-hover tooltip state expectations maintain consistency with the pre-hover state, using getPosition: noop for both tooltip item payloads.

src/state/optionsSlice.ts (2)

21-21: LGTM - Good type tightening.

Making tooltipPayloadSearcher required instead of optional eliminates the need for null checks when accessing this property throughout the codebase.

---

43-48: LGTM - Appropriate default value.

The default () => undefined is a valid implementation of TooltipPayloadSearcher that safely returns undefined for any input. In TypeScript, functions with fewer parameters are assignable to function types with more parameters, so this is type-safe.

src/util/DataUtils.ts (1)

210-210: LGTM!

The return type change from void to undefined is appropriate. This aligns with the function's usage as a getPosition placeholder in tooltip configurations, where getPosition: (index) => Coordinate | undefined is expected, and noop serves as a no-op implementation returning undefined.

test/component/Tooltip/Tooltip.payload.spec.tsx (2)

83-83: LGTM!

The import of noop from DataUtils is appropriate for updating test expectations to match the new getPosition-based tooltip payload configuration API.

---

974-974: LGTM!

The test expectations correctly updated from positions: undefined to getPosition: noop, aligning with the refactored tooltip positioning API. The changes are consistent across all three payload configuration expectations.

Also applies to: 1028-1028, 1082-1082

test/component/Tooltip/itemSorter.spec.tsx (3)

39-39: LGTM!

The import of noop from DataUtils supports the updated test expectations for the new getPosition-based tooltip configuration API.

---

86-86: LGTM!

The test expectations correctly updated from positions: undefined to getPosition: noop for Area, Bar, and Line components. This aligns with the refactored tooltip positioning API.

Also applies to: 103-103, 120-120

---

306-313: The expect.functionReturning helper is properly available. The custom matcher is defined in test/helper/expectFunctionReturning.ts, extended to the expect object, and has proper TypeScript type declarations for AsymmetricMatchersContaining. The test code at these lines correctly uses this available helper.

src/cartesian/Line.tsx (5)

25-25: LGTM!

The import of noop supports the new getPosition-based tooltip configuration pattern.

---

69-69: LGTM!

The import of ChartData enables stronger typing for the component's data props, replacing the previous any[] type.

---

170-170: LGTM! Improved type safety.

Changing the data prop type from any[] to ChartData (which is ReadonlyArray<unknown>) improves type safety while maintaining flexibility. The readonly constraint is appropriate since the component should not mutate input data.

---

363-363: LGTM!

The change from positions: undefined to getPosition: noop aligns with the refactored tooltip positioning API. Line chart doesn't provide positional data per point (unlike Scatter), so noop is the appropriate placeholder.

---

927-927: LGTM!

The parameter type change from any[] to ChartData improves type safety. This function signature now matches the improved typing established throughout the component.

src/cartesian/Scatter.tsx (1)

399-399: LGTM! The index parameter type is correct.

The refactor from a precomputed positions array to a dynamic getPosition accessor function is correct and more memory-efficient. The index parameter is TooltipIndex (a string), and using Number(index) for array access is the appropriate pattern. All callers properly pass valid string indices through the tooltip state selectors, consistent with implementations in Pie and Funnel charts.

src/polar/RadialBar.tsx (2)

17-17: LGTM!

The import of noop is consistent with the PR's refactoring pattern across other chart components.

---

443-443: LGTM!

Replacing positions: undefined with getPosition: noop aligns with the PR objective to standardize tooltip positioning with a function-based API. Using noop is appropriate for RadialBar since it doesn't provide position data.

www/src/docs/apiExamples/PieChart/index.tsx (1)

2-8: LGTM!

The consolidation of duplicate Pie examples by replacing PieChartExample with TwoLevelPieChart is consistent and well-structured. The imports and references are updated correctly.

test-vr/tests/www/PieChartApiExamples.spec-vr.tsx (1)

56-56: LGTM!

Adding defaultIndex="1" exercises the tooltip positioning functionality as intended by the PR, providing valuable visual regression coverage for the new getPosition API.

src/state/selectors/touchSelectors.ts (2)

27-29: LGTM!

The null check for tooltipIndex is a proper guard before attempting to call getPosition, preventing potential runtime errors.

---

36-40: LGTM!

The refactoring from array-based positions to function-based getPosition(tooltipIndex) simplifies the tooltip positioning API and improves type safety. The null check and function call pattern are implemented correctly.

src/state/selectors/selectTooltipPayloadSearcher.ts (1)

4-5: Verification confirms the type change is correct. tooltipPayloadSearcher is always defined: it's a required field in ChartOptions, initialized to () => undefined in the initial state, and no reducer can mutate it to undefined. The return type change from optional to required is safe.

test/component/Tooltip/Tooltip.visibility.spec.tsx (2)

94-94: LGTM! Test expectations updated correctly.

The test now correctly expects getPosition: noop in tooltip payload configurations instead of the previous positions: undefined, aligning with the PR's shift to function-based position resolution.

Also applies to: 1125-1125, 1217-1217

---

987-993: LGTM! Dynamic position validation using new test helper.

The test correctly validates the getPosition function using expect.functionReturning, which verifies that each sector index ('0' to '4') maps to its corresponding coordinate. This properly exercises the new function-based position API.

src/state/selectors/combiners/combineCoordinateForDefaultIndex.ts (1)

2-2: LGTM! Simplified position lookup using configuration's getPosition method.

The refactor correctly removes the tooltipPayloadSearcher parameter and instead accesses getPosition directly from the first tooltip configuration. The optional chaining handles empty configurations safely, and the fallback to tooltipTicks preserves existing behavior when getPosition returns undefined.

Also applies to: 11-25

src/state/chartDataSlice.ts (1)

2-2: LGTM! Proper use of readonly types with Immer.

The type change from unknown[] to ReadonlyArray<unknown> improves immutability guarantees, and castDraft is correctly used to handle the assignment of readonly payload into mutable draft state.

Also applies to: 15-15, 59-59

src/cartesian/Area.tsx (1)

23-23: LGTM! Improved typing and tooltip configuration.

The changes properly:

• Replace any[] with the typed ChartData (aligning with the "no any" coding guideline)
• Use getPosition: noop instead of positions: undefined to match the new function-based position API
• Import noop from the utilities module

Based on learnings, this follows the established pattern for tooltip position resolution across chart components.

Also applies to: 99-99, 354-354

src/polar/Radar.tsx (2)

6-6: LGTM!

The noop import is necessary for the tooltip configuration change at line 218, aligning with the PR's pattern of replacing static positions with dynamic getPosition functions.

---

218-218: LGTM!

The change from positions: undefined to getPosition: noop is consistent with the PR's objective to unify tooltip positioning with a function-based API. The existing comment (lines 211-216) appropriately documents why returning undefined is correct for Radar charts.

src/chart/Sankey.tsx (1)

30-30: LGTM!

The changes align with the PR's objective to replace static positions with a dynamic getPosition function. Using noop (which returns undefined) is appropriate for Sankey charts where tooltip positioning is not defined per item.

Also applies to: 551-551

src/polar/Pie.tsx (1)

444-444: Efficient lazy position lookup!

The change from a static positions array to a dynamic getPosition function improves efficiency by avoiding the creation of an intermediate array. The use of Number(index) conversion and optional chaining (?.) provides appropriate safety for index lookups.

src/chart/SunburstChart.tsx (2)

144-144: LGTM!

The change from convertMapToRecord(positions) to a direct getPosition function using Map.get() is more efficient and aligns with the PR's pattern of using dynamic position lookups.

---

180-185: No action required—function is internal.

addToSunburstNodeIndex was never exported through the public API (src/index.ts) and has no usage outside SunburstChart.tsx. It is purely an internal utility function, so removing the export (if previously present) is not a breaking change and appropriately maintains a minimal public API surface.

test/chart/ScatterChart.spec.tsx (1)

1733-1740: LGTM!

The test updates correctly reflect the API change from static positions arrays to dynamic getPosition functions. The use of expect.functionReturning([...]) appropriately validates both that getPosition is a function and that it returns the expected coordinate values for each index.

Also applies to: 1935-1942, 1972-1979, 2315-2319

src/cartesian/Brush.tsx (2)

161-172: Good helper extraction for aria label generation.

The getNameFromUnknown and getAriaLabel helper functions properly encapsulate the logic for extracting string names from unknown data types and composing aria labels. The type guards in getNameFromUnknown are correct.

However, consider what happens when data[startIndex] or data[endIndex] don't have a name property - the aria label will display "Min value: undefined, Max value: undefined". While this isn't critical, you might want to handle this edge case more gracefully (e.g., fallback to index numbers).

---

127-127: Excellent type safety improvement from any[] to ChartData.

The systematic replacement of any[] with ChartData (which is ReadonlyArray<unknown>) across PropertiesFromContext, TextOfTickProps, getIndex parameter, BrushText, Panorama, and State.prevData improves type safety while maintaining flexibility for various data shapes. This aligns well with the PR's objective to modernize type definitions.

Also applies to: 239-239, 287-287, 339-339, 425-425, 464-464

www/src/docs/exampleComponents/PieChart/TwoLevelPieChart.tsx (1)

1-1: LGTM! Clean addition of defaultIndex support.

The changes properly add Tooltip with defaultIndex support to the example component. The typing is correct, using TooltipIndex from the public API, and the component signature is cleanly extended.

Also applies to: 26-32, 58-58

test/state/selectors/selectors.spec.tsx (1)

38-38: LGTM! Test updates properly align with the new API.

The test updates consistently replace positions arrays with getPosition: noop, which is appropriate for tests that don't need actual tooltip positioning logic. The import of noop from DataUtils and removal of the Customized wrapper simplify the test setup.

Also applies to: 53-53, 90-90, 128-128, 265-265, 278-278, 323-323, 336-336, 370-370, 416-416, 510-510

src/cartesian/Bar.tsx (2)

25-25: LGTM! Consistent use of noop for tooltip positioning.

The import and usage of noop for getPosition is consistent with the refactoring pattern across other components. Since Bar tooltips compute positions dynamically elsewhere, using noop here is appropriate.

Also applies to: 414-414

---

88-88: Good type safety improvement with ChartData.

Changing the displayedData parameter from implicit any[] to ChartData (ReadonlyArray<unknown>) in computeBarRectangles improves type safety and consistency with the broader refactoring across the codebase.

Also applies to: 951-951

src/chart/Treemap.tsx (5)

76-89: LGTM: Enhanced type guard with explicit runtime checks.

The explicit typeof checks on lines 84-87 improve the robustness of the isTreemapNode type guard by verifying that x, y, width, and height are not only present but are also numbers at runtime.

---

91-99: LGTM: Explicit function signature improves type safety.

The updated treemapPayloadSearcher signature with explicit parameters and return type TreemapNode | undefined (instead of unknown) provides better type safety and clarity.

---

625-655: LGTM: Tooltip configuration updated to use new getPosition API.

The change from positions: undefined to getPosition: noop aligns with the PR objective to replace static position arrays with dynamic functions. The noop placeholder is appropriate given the TODO comment indicating that proper position computation is not yet implemented for Treemap.

---

964-996: LGTM: Proper use of unknown type with type guard.

Line 974 correctly uses unknown instead of any, followed by the isTreemapNode type guard on line 975 to safely narrow the type before accessing properties. This follows the coding guideline to prefer unknown over any and refine the type.

Based on coding guidelines.

---

483-483: Null-to-undefined migration is complete and safe.

The change to currentRoot: TreemapNode | undefined (lines 483, 508) is correctly paired with the default initialization, and the codebase uses loose equality checks (== null) which safely handle both null and undefined values. The migration is scoped appropriately—only currentRoot was changed to undefined while formatRoot correctly remains null.

test/polar/Pie.spec.tsx (1)

987-993: LGTM! Correctly implements the new function-based tooltip positioning API.

The test now validates that getPosition is a function returning position coordinates keyed by sector index, replacing the previous static positions array. The use of expect.functionReturning appropriately tests the function's return value without prematurely calling it during state validation.