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

688-718: Inconsistent use of expectLastCalledWith helper.

Line 690 correctly uses expectLastCalledWith(onClick, ...), but lines 694, 696, 700, 702, 706, 710, 714, and 718 fall back to expect(spy).toHaveBeenLastCalledWith(...). The coding guidelines say to prefer expectLastCalledWith for better typing and autocompletion.

♻️ Suggested fix (representative diff for a few lines)

-      expect(onMouseEnter).toHaveBeenLastCalledWith(expect.objectContaining({ type: 'mouseenter' }));
+      expectLastCalledWith(onMouseEnter, expect.objectContaining({ type: 'mouseenter' }));
       expect(onMouseOver).toHaveBeenCalledTimes(1);
-      expect(onMouseOver).toHaveBeenLastCalledWith(expect.objectContaining({ type: 'mouseover' }));
+      expectLastCalledWith(onMouseOver, expect.objectContaining({ type: 'mouseover' }));

Apply the same pattern to onMouseLeave, onMouseOut, onMouseMove, onTouchStart, onTouchMove, and onTouchEnd.

As per coding guidelines, test/**/*.{test,spec}.{ts,tsx}: "Use the expectLastCalledWith helper function instead of expect(spy).toHaveBeenLastCalledWith(...) for better typing and autocompletion".

---

685-686: Misleading variable name dot for a reference line element.

The selector targets .recharts-reference-line .recharts-reference-line-line, which is a line element, not a dot. Consider renaming to line or refLine for clarity.

-      const dot = container.querySelector('.recharts-reference-line .recharts-reference-line-line');
-      assertNotNull(dot);
+      const refLine = container.querySelector('.recharts-reference-line .recharts-reference-line-line');
+      assertNotNull(refLine);

Then update all subsequent references accordingly.

---

669-671: Stray r={3} prop on ReferenceLine.

r is a radius prop that belongs to ReferenceDot, not ReferenceLine. This appears to be a copy-paste artifact. While it's harmless (it'll be ignored or spread as an SVG attribute), it's misleading in a test that's meant to validate ReferenceLine event handlers.

           <ReferenceLine
             x={1}
-            r={3}
             onClick={onClick}

omnidoc/verifyExamples.spec.ts (1)

97-101: Naming concern: InternalRadarProps exported as public API.

Exporting a type prefixed with Internal as part of the public API is confusing for consumers — the name implies it's an internal implementation detail. Consider renaming it to something like RadarEventHandlerProps or RadarComposedProps to better communicate its intended public use, or keep it internal and expose a differently named alias.

This is a broader concern rooted in src/index.ts / src/polar/Radar.tsx, not this file specifically.

test/polar/Radar.typed.spec.tsx (1)

5-47: Inconsistent handler signatures across Radar event props — is this intentional?

onMouseEnter and onMouseLeave receive (data: InternalRadarProps, event) while all other mouse/touch handlers receive only (event). If this asymmetry is intentional (matching the actual runtime behavior), it may be worth a brief comment in the test explaining why, similar to the Treemap test's onClick comment. If unintentional, the underlying Radar types may need alignment.

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

561-638: Good event handler coverage for ReferenceArea.

The new events describe block comprehensively tests all mouse and touch event handlers. A few observations:

1. Misleading variable name on Line 603: dot queries .recharts-reference-area-rect — consider naming it rect or referenceAreaRect for clarity.

2. Inconsistent use of expectLastCalledWith: Line 608 correctly uses the expectLastCalledWith helper, but lines 612, 614, 618–620, and 624 fall back to expect(spy).toHaveBeenLastCalledWith(...). As per coding guidelines: "Use the expectLastCalledWith helper function instead of expect(spy).toHaveBeenLastCalledWith(...) for better typing and autocompletion."

3. Missing vi.useFakeTimers(): The test uses userEventSetup() (which internally relies on vi.advanceTimersByTime) but doesn't set up fake timers. As per coding guidelines: "Use vi.useFakeTimers() in all tests due to Redux autoBatchEnhancer dependency on timers and requestAnimationFrame."

Suggested improvements

   describe('events', () => {
+    vi.useFakeTimers();
+
     it('should fire event handlers when provided', async () => {
       const userEvent = userEventSetup();

-      const dot = container.querySelector('.recharts-reference-area-rect');
-      assertNotNull(dot);
+      const rect = container.querySelector('.recharts-reference-area-rect');
+      assertNotNull(rect);
 
-      await userEvent.click(dot);
+      await userEvent.click(rect);
       expect(onClick).toHaveBeenCalledTimes(1);
       expectLastCalledWith(onClick, expect.objectContaining({ type: 'click' }));
 
-      await userEvent.hover(dot);
+      await userEvent.hover(rect);
       expect(onMouseEnter).toHaveBeenCalledTimes(1);
-      expect(onMouseEnter).toHaveBeenLastCalledWith(expect.objectContaining({ type: 'mouseenter' }));
+      expectLastCalledWith(onMouseEnter, expect.objectContaining({ type: 'mouseenter' }));

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

373-470: Consider extracting the shared root object to reduce duplication.

The expectedTreemapNode / expectedNode objects in the onMouseEnter (Line 373), onMouseLeave (Line 518), and onClick (Line 646) tests all contain the identical root structure (~80 lines). Extracting it to a shared constant at the describe level would cut ~160 lines of duplication and make future maintenance easier.

That said, the refactor from inline literals to typed TreemapNode constants is a clear improvement over the previous approach.

src/chart/Treemap.tsx (1)

461-465: Narrowing to SVGGraphicsElement is a good improvement for type safety.

The event target correctly reflects that Treemap's Layer renders SVG <g> elements (which extend SVGGraphicsElement).

Note the asymmetry: onMouseEnter and onMouseLeave pass both (node, event) while onClick only passes (node) — no event. This is pre-existing behavior (confirmed by the test on line 644 of Treemap.spec.tsx), but it may surprise consumers. Consider adding a JSDoc note on onClick explaining this, or filing a follow-up to include the event parameter for consistency.

src/index.ts (1)

73-73: Exporting InternalRadarProps as public API — is the naming intentional?

The prefix "Internal" in a publicly exported type may confuse consumers into thinking it's not part of the stable API. Since this type describes the payload shape passed to Radar event handlers (e.g., onMouseEnter), a name like RadarEventPayload or RadarHandlerProps might better communicate its purpose.

If the name is intentional and temporary, consider adding a JSDoc note on the type definition in src/polar/Radar.tsx clarifying its role and stability.

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

414-438: Prefer expectLastCalledWith helper over expect(spy).toHaveBeenLastCalledWith(...).

Line 410 correctly uses expectLastCalledWith, but the subsequent assertions on lines 414, 416, 420, 422, 426, 430, 434, and 438 fall back to expect(spy).toHaveBeenLastCalledWith(...). For consistency and the better typing/autocompletion the helper provides, consider using expectLastCalledWith throughout.

Example fix for one assertion

-      expect(onMouseEnter).toHaveBeenLastCalledWith(expectedDotProps, expect.objectContaining({ type: 'mouseenter' }));
+      expectLastCalledWith(onMouseEnter, expectedDotProps, expect.objectContaining({ type: 'mouseenter' }));

As per coding guidelines: "Use the expectLastCalledWith helper function instead of expect(spy).toHaveBeenLastCalledWith(...) for better typing and autocompletion"