diff --git a/CHANGELOG.md b/CHANGELOG.md index ea6491a8ff..2491204881 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Undocumented APIs should be considered internal and may change without warning. - `AnimatePresence`: Exiting children no longer interleave with entering children, which could reorder and remount children present in both renders. - `motion`: Throw error when passing a custom `motion` component an incorrect `ref` type. +- `spring`: Invalid `stiffness`, `damping` or `mass` (`0`, negative, non-finite, or an explicit `undefined` from a forwarded prop) no longer resolve to `NaN` animation values, which corrupted values like an SVG `polygon`'s `points` and left the animation running forever. ## [12.42.2] 2026-07-01 diff --git a/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts b/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts index 7130d0ca6e..430a5bff82 100644 --- a/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts +++ b/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts @@ -1422,4 +1422,31 @@ describe("JSAnimation", () => { expect(animation.sample(1000).value).toBe("90%") expect(animation.sample(1999).value).toBe("90%") }) + + // https://github.com/motiondivision/motion/issues/2791 + test("Spring over SVG polygon points never produces NaN", () => { + // An invalid spring physics value (here an explicit `undefined` + // stiffness, as forwarded from an optional prop) previously emitted + // NaN progress, which the complex-value mixer turned into an invalid + // "NaN,NaN NaN,NaN" point list written to the element. + const target = "150,5 50,180 250,180" + const animation = animateValue({ + keyframes: ["150,5 75,200 225,200", target], + type: "spring", + stiffness: undefined, + autoplay: false, + }) + + for (let t = 0; t <= 2000; t += 50) { + const coords = animation.sample(t).value.match(/-?[\d.]+/g)! + // Every coordinate must be a finite number + expect( + coords.every((coord) => Number.isFinite(Number(coord))) + ).toBe(true) + } + + // ...and the spring must still settle on the target, so this doesn't + // pass for any non-NaN corruption of the point list + expect(animation.sample(2000).value).toBe(target) + }) }) diff --git a/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts b/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts index 309b82e9d6..c2b7ab9b69 100644 --- a/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts +++ b/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts @@ -1,7 +1,10 @@ import { animateSync } from "../../__tests__/utils" import { ValueAnimationOptions } from "../../types" import { spring } from "../spring" -import { calcGeneratorDuration } from "../utils/calc-duration" +import { + calcGeneratorDuration, + maxGeneratorDuration, +} from "../utils/calc-duration" describe("spring", () => { test("Runs animations with default values ", () => { @@ -285,3 +288,99 @@ describe("toString", () => { ) }) }) + +// https://github.com/motiondivision/motion/issues/2791 +describe("spring NaN guards", () => { + // These deliberately pass invalid physics, which warns + let warn: jest.SpyInstance + beforeEach(() => { + warn = jest.spyOn(console, "warn").mockImplementation(() => {}) + }) + afterEach(() => warn.mockRestore()) + + /** + * animateSync() can't be reused here — it loops `while (!done)`, and a + * spring resolving to NaN never sets done, so it would hang rather than + * fail. + */ + const sample = (options: ValueAnimationOptions) => { + const generator = spring(options) + return [0, 100, 300, 600, 1000].map((t) => generator.next(t).value) + } + + /** + * Every physics option is covered, for each way it can be invalid. An + * explicit `undefined` (e.g. a forwarded optional prop) is the case from + * the original report — it clobbers the default via the options spread in + * getSpringOptions. + */ + const physicsKeys = ["stiffness", "damping", "mass"] as const + const invalidValues = [0, -1, NaN, Infinity, -Infinity, undefined] + + for (const key of physicsKeys) { + for (const value of invalidValues) { + // damping of 0 is a valid, perpetually oscillating spring + if (key === "damping" && value === 0) continue + + test(`${key} of ${String(value)} does not produce NaN`, () => { + const values = sample({ keyframes: [0, 100], [key]: value }) + values.forEach((v) => expect(v).not.toBeNaN()) + }) + } + } + + test("damping of 0 is honoured as an undamped spring", () => { + const values = sample({ keyframes: [0, 100], damping: 0 }) + values.forEach((v) => expect(v).not.toBeNaN()) + // An undamped spring oscillates rather than settling on the target + expect(values[values.length - 1]).not.toBeCloseTo(100) + }) + + test("invalid physics does not discard a provided duration", () => { + // `stiffness: 0` previously counted as "physics specified", so the + // duration branch was skipped and `duration` silently ignored. + expect(sample({ keyframes: [0, 100], duration: 500, stiffness: 0 })).toEqual( + sample({ keyframes: [0, 100], duration: 500 }) + ) + }) + + test("invalid physics does not discard a provided visualDuration", () => { + expect( + sample({ + keyframes: [0, 100], + visualDuration: 0.5, + bounce: 0.2, + mass: 0, + }) + ).toEqual( + sample({ keyframes: [0, 100], visualDuration: 0.5, bounce: 0.2 }) + ) + }) + + test("visualDuration of 0 does not produce NaN", () => { + const values = sample({ + keyframes: [0, 100], + visualDuration: 0, + bounce: 0.2, + }) + values.forEach((v) => expect(v).not.toBeNaN()) + }) + + test("invalid stiffness still resolves to a spring that completes", () => { + const generator = spring({ keyframes: [0, 100], stiffness: undefined }) + expect(calcGeneratorDuration(generator)).toBeLessThan( + maxGeneratorDuration + ) + }) + + test("invalid physics warns rather than failing silently", () => { + spring({ keyframes: [0, 100], stiffness: 0 }) + expect(warn).toHaveBeenCalledTimes(1) + expect(warn.mock.calls[0][0]).toContain("spring-invalid-physics") + }) + + test("valid physics does not warn", () => { + spring({ keyframes: [0, 100], stiffness: 200, damping: 0, mass: 2 }) + expect(warn).not.toHaveBeenCalled() + }) +}) diff --git a/packages/motion-dom/src/animation/generators/spring.ts b/packages/motion-dom/src/animation/generators/spring.ts index 92647be5fa..b5d93fde4e 100644 --- a/packages/motion-dom/src/animation/generators/spring.ts +++ b/packages/motion-dom/src/animation/generators/spring.ts @@ -162,33 +162,72 @@ function findSpring({ } const durationKeys = ["duration", "bounce"] -const physicsKeys = ["stiffness", "damping", "mass"] function isSpringType(options: SpringOptions, keys: string[]) { return keys.some((key) => (options as any)[key] !== undefined) } +/** + * Spring physics must be finite. stiffness and mass are also divisors so must + * be positive, whereas a damping of 0 is a valid, perpetually oscillating + * spring. + */ +const isValidPhysics = (value: number | undefined, canBeZero?: boolean) => + Number.isFinite(value) && (canBeZero ? value! >= 0 : value! > 0) + +/** + * Returns value if it's usable spring physics, otherwise undefined so callers + * can fall back to a default. + * + * Anything invalid — a 0 stiffness, a negative, Infinity, or an explicit + * `undefined` forwarded from an optional prop that clobbers the default via + * the spread below — divides or feeds Math.sqrt() during resolution and + * produces NaN spring values. Those corrupt every animated value downstream: + * an SVG polygon's points list becomes "NaN,NaN NaN,NaN", and the spring never + * reports done so the frameloop spins indefinitely. + * See https://github.com/motiondivision/motion/issues/2791 + */ +function resolvePhysics(value: number | undefined, canBeZero?: boolean) { + if (isValidPhysics(value, canBeZero)) return value + + warning( + value === undefined, + "Spring stiffness and mass must be positive, damping 0 or greater", + "spring-invalid-physics" + ) + + return undefined +} + function getSpringOptions(options: SpringOptions) { + /** + * Resolve physics before choosing between physics- and duration-based + * resolution, so an invalid stiffness doesn't also silently discard a + * valid duration/bounce. + */ + const stiffness = resolvePhysics(options.stiffness) + const damping = resolvePhysics(options.damping, true) + const mass = resolvePhysics(options.mass) + const hasPhysics = + stiffness !== undefined || damping !== undefined || mass !== undefined + let springOptions = { - velocity: springDefaults.velocity, - stiffness: springDefaults.stiffness, - damping: springDefaults.damping, - mass: springDefaults.mass, - isResolvedFromDuration: false, ...options, + velocity: options.velocity ?? springDefaults.velocity, + stiffness: stiffness ?? springDefaults.stiffness, + damping: damping ?? springDefaults.damping, + mass: mass ?? springDefaults.mass, + isResolvedFromDuration: false, } // stiffness/damping/mass overrides duration/bounce - if ( - !isSpringType(options, physicsKeys) && - isSpringType(options, durationKeys) - ) { + if (!hasPhysics && isSpringType(options, durationKeys)) { // Time-defined springs should ignore inherited velocity. // Velocity from interrupted animations can cause findSpring() // to compute wildly different spring parameters, leading to // massive oscillation on small-range animations. springOptions.velocity = 0 - if (options.visualDuration) { + if (options.visualDuration !== undefined) { const visualDuration = options.visualDuration const root = (2 * Math.PI) / (visualDuration * 1.2) const stiffness = root * root @@ -213,6 +252,21 @@ function getSpringOptions(options: SpringOptions) { } springOptions.isResolvedFromDuration = true } + + /** + * Duration-based resolution can degenerate: findSpring()'s root + * approximation can collapse to a {stiffness: 0, damping: 0} pair, and + * a visualDuration of 0 gives an infinite stiffness. Replace the two + * together, so the relationship duration resolution establishes + * between them is never left half-overwritten. + */ + if ( + !isValidPhysics(springOptions.stiffness) || + !isValidPhysics(springOptions.damping, true) + ) { + springOptions.stiffness = springDefaults.stiffness + springOptions.damping = springDefaults.damping + } } return springOptions