src/cartesian/Scatter.tsx (1)

605-619: Consider simplifying the key construction.

The key uses optional chaining (entry?.cx) but entry is always defined when iterating via points.map. The optional chaining is unnecessary and could be simplified.

More importantly, if cx, cy, or size are undefined, the key becomes symbol-undefined-undefined-undefined-0, which could cause React key collisions if multiple points have undefined coordinates. Since the index i is included, uniqueness is guaranteed, but consider using just the index or a more stable identifier.

Suggested simplification

       {points.map((entry: ScatterPointItem, i: number) => (
         <ScatterPoint
-          key={`symbol-${entry?.cx}-${entry?.cy}-${entry?.size}-${i}`}
+          key={`symbol-${i}`}
           entry={entry}
           index={i}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/cartesian/Scatter.tsx` around lines 605 - 619, The key for ScatterPoint
created inside points.map in Scatter.tsx unnecessarily uses optional chaining
(entry?.cx) and can produce "undefined" parts leading to less readable keys;
replace it with a stable, simple key—either use the loop index (e.g.,
key={`symbol-${i}`}) or, if each point has a stable identifier (e.g., entry.id
or entry.key), use that (e.g., key={entry.id})—update the key prop on the
ScatterPoint component instantiation and remove the optional chaining to ensure
concise, collision-resistant keys.

src/cartesian/Scatter.tsx (1)

555-559: Consider extracting selector factory for clarity and potential reuse.

The inline selector factory pattern works correctly, but could be extracted for clarity:

💡 Optional refactor to extract selector factory

+const createIsActiveSelector = (index: number, hasActiveShape: boolean) => (state: RechartsRootState): boolean =>
+  hasActiveShape && selectActiveTooltipIndex(state) === String(index);
+
 function ScatterPoint({
   // ... props
 }): React.ReactElement {
   const hasActiveShape = activeShape != null && activeShape !== false;
-  const selectIsActive = useMemo(() => {
-    const strIndex = String(index);
-    return (state: RechartsRootState): boolean => hasActiveShape && selectActiveTooltipIndex(state) === strIndex;
-  }, [hasActiveShape, index]);
+  const selectIsActive = useMemo(
+    () => createIsActiveSelector(index, hasActiveShape),
+    [hasActiveShape, index]
+  );
   const isActive: boolean = useAppSelector(selectIsActive) ?? false;

This could make the selector reusable if other components need similar per-index active state logic. However, the current inline approach is also fine.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/cartesian/Scatter.tsx` around lines 555 - 559, Extract the inline
selector factory into a named function (e.g., makeSelectIsActive) instead of
creating it inside the useMemo; the function should accept index and
hasActiveShape and return a selector that compares
selectActiveTooltipIndex(state) to String(index) and checks hasActiveShape.
Replace the current useMemo/selectIsActive creation with a call to
makeSelectIsActive(index, hasActiveShape) and keep the
useAppSelector(selectIsActive) usage to obtain isActive; this makes selector
logic reusable and clearer while preserving existing behavior.

test/cartesian/Scatter.spec.tsx (1)

703-752: Harden timer cleanup with try/finally.

If an assertion fails before Line 751, fake timers can leak into subsequent tests in this file. Wrapping the body guarantees cleanup.

Proposed hardening

 it('should only re-render the active and previously-active points when hovering, not all points', () => {
   vi.useFakeTimers();
-  const shapeSpy = vi.fn((_props: ScatterShapeProps) => <circle r={5} cx={0} cy={0} />);
-  const pointCount = 20;
-  const scatterData = Array.from({ length: pointCount }, (_, i) => ({
-    x: i * 50,
-    y: i * 50,
-    z: 100,
-  }));
+  try {
+    const shapeSpy = vi.fn((_props: ScatterShapeProps) => <circle r={5} cx={0} cy={0} />);
+    const pointCount = 20;
+    const scatterData = Array.from({ length: pointCount }, (_, i) => ({
+      x: i * 50,
+      y: i * 50,
+      z: 100,
+    }));

-  const { container } = render(
-    <ScatterChart width={1000} height={1000}>
-      <XAxis type="number" dataKey="x" />
-      <YAxis type="number" dataKey="y" />
-      <Scatter isAnimationActive={false} data={scatterData} shape={shapeSpy} activeShape={{ fill: 'red' }} />
-    </ScatterChart>,
-  );
-  vi.runOnlyPendingTimers();
+    const { container } = render(
+      <ScatterChart width={1000} height={1000}>
+        <XAxis type="number" dataKey="x" />
+        <YAxis type="number" dataKey="y" />
+        <Scatter isAnimationActive={false} data={scatterData} shape={shapeSpy} activeShape={{ fill: 'red' }} />
+      </ScatterChart>,
+    );
+    vi.runOnlyPendingTimers();

-  // ...existing assertions...
+    // ...existing assertions...
+  } finally {
+    vi.useRealTimers();
+  }
-
-  vi.useRealTimers();
 });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/cartesian/Scatter.spec.tsx` around lines 703 - 752, Wrap the test body
that calls vi.useFakeTimers() and vi.useRealTimers() in a try/finally so
vi.useRealTimers() always runs; specifically, in the "Scatter" test that sets up
vi.useFakeTimers(), move the render(...) and all assertions into a try block and
call vi.useRealTimers() in the finally block to guarantee timer cleanup even if
an assertion fails (references: vi.useFakeTimers, vi.useRealTimers, render,
shapeSpy).

src/cartesian/Scatter.tsx (1)

555-559: Scope isActive to the active graphical item, not index alone.

On Line 557, isActive is derived only from tooltip index. In multi-<Scatter /> charts, matching indexes across series can become active together. Please verify this behavior with two scatter series using activeShape, and gate by active graphical item id if single-series activation is intended.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/cartesian/Scatter.tsx` around lines 555 - 559, The selector
selectIsActive only checks selectActiveTooltipIndex against String(index), so
multiple series with the same point index can all appear active; update
selectIsActive (used by useAppSelector) to also compare the active graphical
item id (e.g., selectActiveGraphicItemId or similar state selector) against this
series' unique id/prop (use props.seriesId or the component's unique key) and
include hasActiveShape in the predicate; specifically, modify the lambda in
selectIsActive to return true only when hasActiveShape &&
selectActiveTooltipIndex(state) === String(index) &&
selectActiveGraphicItemId(state) === String(seriesId) (or the appropriate id
field) so isActive is scoped to the actual graphical item, not index alone.