src/animation/AnimationController.ts (1)

4-15: ⚡ Quick win

Update the controller JSDoc to match the function contract.

This exported type is a function, not a class, and it returns a cancel callback rather than a listener handle. Please remove the stale AnimationManager wording too. As per coding guidelines, “Update TypeScript interfaces and JSDoc comments in source code to automatically update API documentation”.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/animation/AnimationController.ts` around lines 4 - 15, Update the JSDoc
comments for AnimationController to accurately reflect the function contract.
Remove the description referring to AnimationController as a class and update
the return value documentation to correctly state that the function returns a
cancel callback rather than an animation handle for listening to updates. Remove
any stale references to AnimationManager from the comments as it is no longer
relevant to this function's responsibility.

Source: Coding guidelines

test/animation/AnimationControllerImpl.spec.ts (1)

173-173: ⚡ Quick win

Use expectLastCalledWith helper for last-call assertions.

These assertions use toHaveBeenLastCalledWith; switch to the repository helper for typed call checks.

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

Also applies to: 180-180, 204-204, 211-211, 250-250, 254-254, 258-258, 262-262

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/animation/AnimationControllerImpl.spec.ts` at line 173, Replace all
instances of `expect(spy).toHaveBeenLastCalledWith(...)` with the
`expectLastCalledWith` helper function throughout the test file. This includes
the assertions at lines 173, 180, 204, 211, 250, 254, 258, and 262. Import the
`expectLastCalledWith` helper function at the top of the file if it is not
already imported, then refactor each assertion to use the helper function
pattern instead of the direct expect syntax to improve typing and autocompletion
support.

Source: Coding guidelines

test/animation/RechartsAnimation.spec.ts (1)

470-470: ⚡ Quick win

Use expectLastCalledWith helper instead of toHaveBeenLastCalledWith.

Replace this matcher with the typed helper to align with test conventions and keep assertion typing consistent.

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

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/animation/RechartsAnimation.spec.ts` at line 470, Replace the matcher
call expect(spy).toHaveBeenLastCalledWith(0.2) with the expectLastCalledWith
helper function instead. This helper provides better typing and aligns with the
project's test conventions for consistency in assertion patterns and improved
autocompletion support.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/animation/AnimationController.ts (1)

4-15: ⚡ Quick win

Update the controller JSDoc to match the function contract.

This exported type is a function, not a class, and it returns a cancel callback rather than a listener handle. Please remove the stale AnimationManager wording too. As per coding guidelines, “Update TypeScript interfaces and JSDoc comments in source code to automatically update API documentation”.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/animation/AnimationController.ts` around lines 4 - 15, Update the JSDoc
comments for AnimationController to accurately reflect the function contract.
Remove the description referring to AnimationController as a class and update
the return value documentation to correctly state that the function returns a
cancel callback rather than an animation handle for listening to updates. Remove
any stale references to AnimationManager from the comments as it is no longer
relevant to this function's responsibility.

Source: Coding guidelines

test/animation/AnimationControllerImpl.spec.ts (1)

173-173: ⚡ Quick win

Use expectLastCalledWith helper for last-call assertions.

These assertions use toHaveBeenLastCalledWith; switch to the repository helper for typed call checks.

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

Also applies to: 180-180, 204-204, 211-211, 250-250, 254-254, 258-258, 262-262

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/animation/AnimationControllerImpl.spec.ts` at line 173, Replace all
instances of `expect(spy).toHaveBeenLastCalledWith(...)` with the
`expectLastCalledWith` helper function throughout the test file. This includes
the assertions at lines 173, 180, 204, 211, 250, 254, 258, and 262. Import the
`expectLastCalledWith` helper function at the top of the file if it is not
already imported, then refactor each assertion to use the helper function
pattern instead of the direct expect syntax to improve typing and autocompletion
support.

Source: Coding guidelines

test/animation/RechartsAnimation.spec.ts (1)

470-470: ⚡ Quick win

Use expectLastCalledWith helper instead of toHaveBeenLastCalledWith.

Replace this matcher with the typed helper to align with test conventions and keep assertion typing consistent.

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

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/animation/RechartsAnimation.spec.ts` at line 470, Replace the matcher
call expect(spy).toHaveBeenLastCalledWith(0.2) with the expectLastCalledWith
helper function instead. This helper provides better typing and aligns with the
project's test conventions for consistency in assertion patterns and improved
autocompletion support.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

package.json (1)

143-143: ⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '.nvmrc: '
cat .nvmrc

printf '\n@types/node: '
jq -r '.devDependencies["`@types/node`"] // .dependencies["`@types/node`"] // "not found"' package.json

Repository: recharts/recharts

Length of output: 93

---

Align @types/node version with Node runtime in .nvmrc.

Currently .nvmrc specifies Node 24, but @types/[email protected] targets Node 25 APIs. This mismatch allows type-checking for Node 25 features that are unavailable in the Node 24 runtime used by dev scripts and tests. Either update .nvmrc to Node 25 or downgrade @types/node to 24.x.x.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@package.json` at line 143, The `@types/node` version in package.json is set to
25.x.x but the .nvmrc file specifies Node 24 as the runtime, creating a mismatch
where TypeScript can check for Node 25 APIs that don't exist in the Node 24
runtime. Either update the `@types/node` dependency in package.json to 24.x.x to
match the Node 24 runtime specified in .nvmrc, or alternatively update .nvmrc to
specify Node 25 to match the types version. Choose whichever aligns with your
project's runtime requirements.

Source: Learnings

src/animation/AnimationControllerImpl.ts (1)

18-31: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Stop scheduling once the handle is completed.

A completed handle’s tick() returns 0; the non-active branch then reschedules forever. Guard completed before the Line 30 fallback so already-completed or custom-completing handles don’t spin.

Suggested fix

   let cancellable: CancelableTimeout | undefined;
   const nextUpdate = (now: number) => {
     const timeRemaining = animationHandle.tick(now);
+    if (animationHandle.getState() === 'completed') {
+      cancellable = undefined;
+      return;
+    }
     if (animationHandle.getState() === 'active') {
       listener(animationHandle.getInterpolated());
       if (animationHandle.getProgress() === 1) {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/animation/AnimationControllerImpl.ts` around lines 18 - 31, The
nextUpdate function reschedules indefinitely after an animation completes
because the non-active branch (the fallback setTimeout on line 33) always
reschedules regardless of whether the handle is already completed. When
animationHandle.getState() is no longer 'active' after completion, tick()
returns 0 and the fallback reschedule creates an infinite loop. Add a guard
condition before the final timeoutController.setTimeout call (line 33) to check
if the animation handle is completed, and only reschedule if it is not
completed. This prevents already-completed or custom-completing handles from
spinning with perpetual zero-timeout reschedules.

src/animation/JavascriptAnimate.tsx (1)

66-86: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reset progress when starting a new animation.

When this effect restarts after a completed animation, style can still be 1. Because the controller does not emit during the begin delay, children render stale progress until activation, then jump back to 0.

Suggested fix

     if (!isActive || !canBegin || easingFunction == null) {
       return noop;
     }
 
+    setStyle(from);
+
     const timeoutController = new RequestAnimationFrameTimeoutController();
 
     const animation = new JavascriptAnimation({

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  useEffect(() => {
    const easingFunction = createEasingFunction(easing);

    if (!isActive || !canBegin || easingFunction == null) {
      return noop;
    }

    setStyle(from);

    const timeoutController = new RequestAnimationFrameTimeoutController();

    const animation = new JavascriptAnimation({
      animationId,
      easing: easingFunction,
      animationDuration: duration,
      animationBegin: begin,
      onAnimationStart,
      onAnimationEnd,
      from,
      to,
    });

    return animationManager(timeoutController, animation, setStyle);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/animation/JavascriptAnimate.tsx` around lines 66 - 86, The useEffect hook
does not reset the animation progress when starting a new animation cycle,
causing stale style values from previous animations to persist. Before creating
the new JavascriptAnimation instance, reset the style state by calling setStyle
with the initial progress value (0) to ensure a clean slate for each new
animation that begins, preventing the jump from stale progress to the correct
value.

src/animation/RechartsAnimation.ts (3)

15-23: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align the init state docs with the synchronous start callback.

Line 18 says init has not announced onAnimationStart, but the constructor calls it synchronously on Lines 171-172. Please update the contract before this becomes public API documentation. As per coding guidelines, “Update TypeScript interfaces and JSDoc comments in source code to automatically update API documentation”.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/animation/RechartsAnimation.ts` around lines 15 - 23, The JSDoc comment
for the RechartsAnimation interface's `init` state inaccurately describes when
the `onAnimationStart` callback is called. The documentation currently states
that in the `init` state the animation "did not announce its start yet", but the
constructor actually calls `onAnimationStart` synchronously. Update the JSDoc
documentation for the `init` state (in the RechartsAnimation interface) to
accurately reflect that `onAnimationStart` is invoked synchronously during
construction, ensuring the state description aligns with the actual
implementation behavior.

Source: Coding guidelines

---

214-216: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle non-positive durations before dividing.

With animationDuration <= 0, Line 215 can produce NaN or negative progress; the controller then keeps scheduling zero-delay updates instead of completing. Treat zero/negative durations as immediately complete progress.

Suggested fix

       }
       const timeElapsed = now - this.animationStartedTime;
+      if (this.animationDuration <= 0) {
+        this.setProgress(1);
+        return 0;
+      }
       this.setProgress(timeElapsed / this.animationDuration);
       return this.nextAnimationUpdate(timeElapsed);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

      const timeElapsed = now - this.animationStartedTime;
      if (this.animationDuration <= 0) {
        this.setProgress(1);
        return 0;
      }
      this.setProgress(timeElapsed / this.animationDuration);
      return this.nextAnimationUpdate(timeElapsed);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/animation/RechartsAnimation.ts` around lines 214 - 216, In the code block
containing the setProgress call, check if animationDuration is less than or
equal to 0 before performing the division operation. If animationDuration is
non-positive, immediately set the progress to 1 (complete state) by calling
setProgress(1). Otherwise, proceed with the normal calculation of timeElapsed
divided by animationDuration. This prevents NaN or negative progress values from
being passed to setProgress, which would cause the animation controller to keep
scheduling zero-delay updates instead of completing.

---

280-282: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Apply easing to progress before interpolation.

EasingFunction operates on normalized progress. The current order only works for from=0/to=1; for other numeric ranges it eases the interpolated value itself and returns incorrect values.

Suggested fix

   public getInterpolated(): number {
-    return this.easing(interpolate(this.getFrom(), this.getTo(), this.getProgress()));
+    return interpolate(this.getFrom(), this.getTo(), this.easing(this.getProgress()));
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/animation/RechartsAnimation.ts` around lines 280 - 282, The
getInterpolated method in RechartsAnimation currently applies the easing
function to the interpolated result, but it should apply easing to the
normalized progress value before interpolation. Change the order so that the
easing function is applied to this.getProgress() first to get the eased progress
value, then pass that eased progress to the interpolate function along with
this.getFrom() and this.getTo(). This ensures the easing function operates on
the correct normalized progress range regardless of the actual from and to
numeric values.

test/animation/AnimationControllerImpl.spec.ts (1)

135-136: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Second assertion repeats spy2 and misses spy1 pre-cancel check.

Line 136 duplicates the previous assertion, so this step never verifies that animation1 also advanced before cancellation.

Suggested fix

     expect(spy2).toHaveBeenCalledTimes(2);
-    expect(spy2).toHaveBeenCalledTimes(2);
+    expect(spy1).toHaveBeenCalledTimes(2);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/animation/AnimationControllerImpl.spec.ts` around lines 135 - 136, The
second assertion on line 136 duplicates the spy2 check instead of verifying the
pre-cancel state. Replace the duplicate expect(spy2).toHaveBeenCalledTimes(2) on
line 136 with expect(spy1).toHaveBeenCalledTimes(1) to properly assert that spy1
(the callback for animation1) was called once before the cancellation occurred,
ensuring the test validates both animations advanced before cancellation.

test/animation/JavascriptAnimate.timing.spec.tsx (1)

66-68: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Duplicate assertion hides missing end-callback check.

handleAnimationStart is asserted twice, so this case never verifies that onAnimationEnd stays uncalled when canBegin=false.

Suggested fix

       expect(handleAnimationStart).not.toHaveBeenCalled();
-      expect(handleAnimationStart).not.toHaveBeenCalled();
+      expect(handleAnimationEnd).not.toHaveBeenCalled();

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/animation/JavascriptAnimate.timing.spec.tsx` around lines 66 - 68, In
the test block around the duplicate assertions, replace the second expect
statement that checks handleAnimationStart.not.toHaveBeenCalled() with an
assertion that verifies handleAnimationEnd is also not called when canBegin is
false. This ensures the test properly validates that both animation callbacks
remain unused when the animation cannot begin.

test/animation/MockProgressAnimationManager.ts (1)

108-117: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

stop() does not cancel the active controller loop.

Line 111 starts a controller but drops the cancel handle, so Line 114 only nulls fields while scheduled callbacks can still run.

Suggested fix

 export class MockProgressAnimationManager<T, E> implements MockAnimationManager {
   private readonly onStop?: () => void;
+  private cancelController?: () => void;
@@
   start(animation: RechartsAnimation<T, E>, listener: (newState: T) => void) {
+    this.cancelController?.();
     this.animation = animation;
     this.isPrimed = false; // Reset the primed state when starting a new animation
-    mockAnimationController(this.timeoutController, this.animation, listener);
+    this.cancelController = mockAnimationController(this.timeoutController, this.animation, listener);
   }
 
   stop() {
+    this.cancelController?.();
+    this.cancelController = undefined;
     this.animation = null;
     this.isPrimed = false; // Reset the primed state when stopping the queue
     this.onStop?.();
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/animation/MockProgressAnimationManager.ts` around lines 108 - 117, The
start() method calls mockAnimationController() but does not capture or store the
cancel handle it returns, so the stop() method has no way to actually cancel the
active controller loop. Store the return value of mockAnimationController() as
an instance variable (likely something like this.cancelAnimation or similar),
and then in the stop() method, call that cancel function before nulling out the
animation and isPrimed fields to ensure scheduled callbacks are properly cleaned
up.

test/animation/mockTimeoutController.ts (1)

39-45: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Check timeoutLimit before mutating the queue.

Line 39 pushes the timeout before Line 41 enforces the limit, so a rejected setTimeout still leaves a queued callback behind.

Suggested fix

-    this.timeouts.push({ callback: copy, delay });
-
-    if (this.config.timeoutLimit != null && this.timeouts.length > this.config.timeoutLimit) {
+    if (this.config.timeoutLimit != null && this.timeouts.length >= this.config.timeoutLimit) {
       throw new Error(
-        `Timeout queue has size ${this.timeouts.length} which is more than the allowed ${this.config.timeoutLimit}`,
+        `Timeout queue has size ${this.timeouts.length + 1} which is more than the allowed ${this.config.timeoutLimit}`,
       );
     }
+    this.timeouts.push({ callback: copy, delay });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/animation/mockTimeoutController.ts` around lines 39 - 45, The
timeoutLimit check in mockTimeoutController.ts occurs after the callback is
already pushed to the this.timeouts queue, allowing the queue to exceed the
limit before the error is thrown. Reorder the logic to validate the timeoutLimit
condition before pushing the callback to the queue, ensuring that if the limit
would be exceeded, the callback is never added and the queue remains in a valid
state when the error is thrown.