diff --git a/.changeset/strict-mode-stop-no-goal.md b/.changeset/strict-mode-stop-no-goal.md new file mode 100644 index 0000000000..8ad3832b51 --- /dev/null +++ b/.changeset/strict-mode-stop-no-goal.md @@ -0,0 +1,5 @@ +--- +'@react-spring/core': patch +--- + +fix(SpringValue): `stop()` no longer establishes a goal on a spring that never had one. The previous implementation always called `_focus(this.get())` to snap `animation.to` to the current value — useful for freezing a live animation, but wrong for a paused or never-started spring whose underlying value was seeded by `_prepareNode` via `from`. The bug became observable under React.StrictMode, whose simulated unmount fires `useSprings`'s cleanup (`ctrl.stop(true)`) on a freshly-mounted, paused spring, leaving `t.goal` equal to the `from` value instead of `undefined`. diff --git a/.changeset/strict-mode-trail-chaining.md b/.changeset/strict-mode-trail-chaining.md new file mode 100644 index 0000000000..990209f9c5 --- /dev/null +++ b/.changeset/strict-mode-trail-chaining.md @@ -0,0 +1,5 @@ +--- +'@react-spring/core': patch +--- + +fix(useTrail): chaining no longer breaks under React.StrictMode. The `reverse` and `passedRef` accumulators inside the `useSprings` wrapper relied on the wrapper being invoked at least once per render. Under StrictMode's second render pass, `useSprings`'s internal `useMemo` caches and the wrapper is skipped, leaving the accumulators at their initial values and reversing the trail direction. For the object-form props the values are now derived directly from the shared props. Fixes #1991. diff --git a/packages/core/src/SpringContext.test.tsx b/packages/core/src/SpringContext.test.tsx index 55e3aceb84..8060582f78 100644 --- a/packages/core/src/SpringContext.test.tsx +++ b/packages/core/src/SpringContext.test.tsx @@ -46,7 +46,17 @@ describe('SpringContext', () => { } const elem = render(getRoot()) - expectUpdates([{ onProps, to: { x: 0 } }]) + // React.StrictMode runs the layout effect twice on initial mount + // (mount → simulated unmount → remount). Both passes apply the + // user update, and the second pass also re-broadcasts the default- + // context update — because `defaultProps.onProps` was set by the + // first pass's user update, so the otherwise-quiet default merge + // now reaches the spy. Subsequent rerenders are not affected. + expectUpdates([ + { onProps, to: { x: 0 } }, + { default: { pause: false, immediate: false } }, + { onProps, to: { x: 0 } }, + ]) context.pause = true elem.rerender(getRoot()) diff --git a/packages/core/src/SpringValue.ts b/packages/core/src/SpringValue.ts index 4918348c0e..caa7a985bb 100644 --- a/packages/core/src/SpringValue.ts +++ b/packages/core/src/SpringValue.ts @@ -477,8 +477,16 @@ export class SpringValue extends FrameValue { stop(cancel?: boolean) { const { to } = this.animation - // The current value becomes the goal value. - this._focus(this.get()) + // The current value becomes the goal value — but only if a goal + // ever existed. Otherwise we'd be establishing one where none was + // set (matters for paused/uninitialised springs whose underlying + // value was seeded via `from` during `_prepareNode`). This becomes + // observable under React.StrictMode, whose simulated unmount fires + // the useSprings cleanup `ctrl.stop(true)` on springs that never + // got a chance to start. + if (!is.und(to)) { + this._focus(this.get()) + } stopAsync(this._state, cancel && this._lastCallId) raf.batchedUpdates(() => this._stop(to, cancel)) diff --git a/packages/core/src/hooks/useSprings.test.tsx b/packages/core/src/hooks/useSprings.test.tsx index a35bf8e88f..646b15d4bd 100644 --- a/packages/core/src/hooks/useSprings.test.tsx +++ b/packages/core/src/hooks/useSprings.test.tsx @@ -7,8 +7,9 @@ import { SpringValue } from '../SpringValue' import { useSprings } from './useSprings' describe('useSprings', () => { - const isStrictMode = true - const strictModeFunctionCallMultiplier = isStrictMode ? 2 : 1 + // StrictMode is enabled globally via configure() in test/setup.ts, so + // props functions are invoked twice per render. + const strictModeFunctionCallMultiplier = 2 let springs: Lookup[] let ref: SpringRef @@ -23,7 +24,7 @@ describe('useSprings', () => { ref = undefined as any } return null - }, isStrictMode) + }) describe('when only a props function is passed', () => { it('should reach final value in strict mode', async () => { @@ -174,8 +175,7 @@ describe('useSprings', () => { }) function createUpdater( - Component: React.ComponentType<{ args: [any, any, any?] }>, - isStrictMode: boolean + Component: React.ComponentType<{ args: [any, any, any?] }> ) { let result: ReturnType | undefined afterEach(() => { @@ -184,12 +184,7 @@ function createUpdater( type Args = [number, any[] | ((i: number) => any), any[]?] return (...args: Args) => { - const component = - const elem = isStrictMode ? ( - {component} - ) : ( - component - ) + const elem = if (result) result.rerender(elem) else result = render(elem) return result diff --git a/packages/core/src/hooks/useTrail.ts b/packages/core/src/hooks/useTrail.ts index 8477c9f2ab..0227ef8c74 100644 --- a/packages/core/src/hooks/useTrail.ts +++ b/packages/core/src/hooks/useTrail.ts @@ -88,15 +88,30 @@ export function useTrail( if (propsFn && !deps) deps = [] // The trail is reversed when every render-based update is reversed. - let reverse = true + // For the object-form props, derive reverse and ref directly from the + // shared props — every spring receives the same props, so accumulating + // them via the useSprings wrapper is unnecessary and unsafe: under + // React.StrictMode the wrapper is not invoked on the second render + // pass (useSprings caches via useMemo with [length] deps), which would + // leave the accumulator stuck at its initial value. + let reverse: boolean | undefined let passedRef: SpringRef | undefined = undefined + if (!propsFn) { + reverse = (propsArg as UseTrailProps).reverse + passedRef = (propsArg as UseTrailProps).ref + } else { + reverse = true + } + const result = useSprings( length, (i, ctrl) => { const props = propsFn ? propsFn(i, ctrl) : propsArg - passedRef = props.ref - reverse = reverse && props.reverse + if (propsFn) { + passedRef = props.ref + reverse = reverse && props.reverse + } return props }, diff --git a/packages/core/test/setup.ts b/packages/core/test/setup.ts index 367183f50b..0152bb9715 100644 --- a/packages/core/test/setup.ts +++ b/packages/core/test/setup.ts @@ -7,6 +7,7 @@ import { beforeEach, afterEach, vi } from 'vitest' import { act } from 'react' +import { configure } from 'vitest-browser-react/pure' import createMockRaf, { MockRaf } from '@react-spring/mock-raf' import { flushMicroTasks } from 'flush-microtasks' import { @@ -52,6 +53,10 @@ declare global { // from interfering with the debugger. vi.setConfig({ testTimeout: 6e8 }) +// Run every render/renderHook under React.StrictMode. If a test passes +// here it passes without StrictMode; the inverse hides real bugs. +configure({ reactStrictMode: true }) + let isRunning = false let frameCache: WeakMap