fix(core)!: stop firing onRest when an active animation is retargeted (#2467)

Author: joshuaellis

.changeset/onrest-not-on-retarget.md

@@ -0,0 +1,19 @@
+---
+'@react-spring/core': major
+---
+
+**Breaking:** `onRest` no longer fires when an active animation is retargeted
+
+Calling `spring.start()` with a new `to` value mid-flight no longer
+invokes the previous animation's `onRest` handler. `onRest` is
+documented as called when the animation comes to a stand-still, and a
+retarget is not a stand-still — the spring keeps moving toward the new
+goal. The `start()` promise still resolves with `finished: false` on
+retarget, so callers that need the old goal-abandoned signal can read
+it there. `onRest` continues to fire as before on `reset`, on `cancel`,
+and on normal settle.
+
+**Migration:** If you relied on `onRest` running on every retarget
+(e.g. for cleanup or analytics), inspect the `start()` promise's
+`finished: false` resolution instead, or move the side effect into
+`onChange`.

packages/core/src/SpringValue.test.ts

@@ -202,7 +202,15 @@ function describeToProp() {
       expect(onStart).toBeCalledTimes(1)
     })
 
-    it.todo('avoids calling the "onRest" prop')
+    it('avoids calling the "onRest" prop', async () => {
+      const onRest = vi.fn()
+      const spring = new SpringValue(0)
+      spring.start(1, { onRest })
+      await global.advance(5)
+      spring.start(2)
+      await global.advanceUntilIdle()
+      expect(onRest).not.toBeCalled()
+    })
   })
 
   describe('when "to" prop equals current value', () => {

packages/core/src/SpringValue.ts

@@ -851,18 +851,16 @@ export class SpringValue<T = any> extends FrameValue<T> {
             // Ensure `onStart` can be called after a reset.
             anim.changed = !reset
 
-            // Call the active `onRest` handler from the interrupted animation.
-            onRest?.(result, this)
-
-            // Notify the default `onRest` of the reset, but wait for the
-            // first frame to pass before sending an `onStart` event.
             if (reset) {
+              // Notify the previous animation's `onRest` that it did not
+              // finish, then the default `onRest`, before jumping back to
+              // `from` on the next frame.
+              onRest?.(result, this)
               callProp(defaultProps.onRest, result)
-            }
-            // Call the active `onStart` handler here since the first frame
-            // has already passed, which means this is a goal update and not
-            // an entirely new animation.
-            else {
+            } else {
+              // Goal update mid-flight: notify the active `onStart` that we
+              // are animating toward a new target. The spring never came to
+              // a stand-still, so do not fire `onRest`.
               anim.onStart?.(result, this)
             }
           })