From 36cbd62689fc62cf038080908b0f447cf12f9426 Mon Sep 17 00:00:00 2001 From: Matt Perry Date: Fri, 12 Jun 2026 11:08:28 +0200 Subject: [PATCH 1/3] Add failing tests for spring NaN with polygon points (#2791) Document that the spring generator emits NaN when a physics parameter (stiffness/mass) is 0 or an explicit undefined, and that this NaN propagates through the complex-value mixer into an SVG polygon's points list ("NaN,NaN NaN,NaN ..."). Co-Authored-By: Claude Opus 4.8 --- .../animation/__tests__/JSAnimation.test.ts | 18 ++++++++++ .../generators/__tests__/spring.test.ts | 33 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts b/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts index 7130d0ca6e..ab1b65bc5e 100644 --- a/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts +++ b/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts @@ -1422,4 +1422,22 @@ 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 animation = animateValue({ + keyframes: ["150,5 75,200 225,200", "150,5 50,180 250,180"], + type: "spring", + stiffness: undefined, + autoplay: false, + }) + + for (let t = 0; t <= 2000; t += 50) { + expect(animation.sample(t).value).not.toContain("NaN") + } + }) }) 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..63558edc85 100644 --- a/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts +++ b/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts @@ -285,3 +285,36 @@ describe("toString", () => { ) }) }) + +// https://github.com/motiondivision/motion/issues/2791 +describe("spring NaN guards", () => { + const sample = (options: ValueAnimationOptions) => { + const generator = spring(options) + return [0, 100, 300, 600, 1000].map((t) => generator.next(t).value) + } + + test("stiffness of 0 does not produce NaN", () => { + const values = sample({ keyframes: [0, 100], stiffness: 0 }) + values.forEach((v) => expect(v).not.toBeNaN()) + }) + + test("mass of 0 does not produce NaN", () => { + const values = sample({ keyframes: [0, 100], mass: 0 }) + values.forEach((v) => expect(v).not.toBeNaN()) + }) + + /** + * An explicit `stiffness: undefined` (e.g. a forwarded prop that resolves + * to undefined) clobbers the default via the options spread in + * getSpringOptions, which previously produced NaN spring values. + */ + test("explicit undefined stiffness does not produce NaN", () => { + const values = sample({ keyframes: [0, 100], stiffness: undefined }) + values.forEach((v) => expect(v).not.toBeNaN()) + }) + + test("explicit undefined mass does not produce NaN", () => { + const values = sample({ keyframes: [0, 100], mass: undefined }) + values.forEach((v) => expect(v).not.toBeNaN()) + }) +}) From 0997630a0f69ff1e72dd57d069f45e69b6ecd019 Mon Sep 17 00:00:00 2001 From: Matt Perry Date: Fri, 12 Jun 2026 11:08:39 +0200 Subject: [PATCH 2/3] Fix NaN in spring animations with falsy/undefined stiffness or mass (#2791) getSpringOptions builds its parameters with `{ ...defaults, ...options }`, so an explicit `stiffness: undefined` (e.g. an optional prop forwarded into `transition`) overrides the default, and `0`/negative values pass straight through. Those then divide and feed Math.sqrt() during spring resolution, producing NaN spring output. For complex values such as an SVG 's `points` attribute this surfaces as an invalid "NaN,NaN NaN,NaN ..." point list and console errors. Guard stiffness and mass to a positive, finite value, falling back to the defaults otherwise. The guard runs once at resolution time, not in the per-frame hot path. Fixes #2791 Co-Authored-By: Claude Opus 4.8 --- .../motion-dom/src/animation/generators/spring.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/motion-dom/src/animation/generators/spring.ts b/packages/motion-dom/src/animation/generators/spring.ts index 92647be5fa..491526d667 100644 --- a/packages/motion-dom/src/animation/generators/spring.ts +++ b/packages/motion-dom/src/animation/generators/spring.ts @@ -215,6 +215,20 @@ function getSpringOptions(options: SpringOptions) { } } + /** + * Guard against non-positive or non-finite stiffness/mass. These divide + * and feed Math.sqrt() during resolution, so a 0 (or an explicit + * `undefined` that clobbers the default via the spread above) produces NaN + * spring values — which corrupt any animated value, e.g. an SVG polygon's + * points list. See https://github.com/motiondivision/motion/issues/2791 + */ + if (!(springOptions.stiffness > 0)) { + springOptions.stiffness = springDefaults.stiffness + } + if (!(springOptions.mass > 0)) { + springOptions.mass = springDefaults.mass + } + return springOptions } From 02d3fe4f061a2e007600dd0941e872948a0a0309 Mon Sep 17 00:00:00 2001 From: Matt Perry Date: Tue, 28 Jul 2026 09:58:14 +0200 Subject: [PATCH 3/3] Address code review on spring NaN guard (#2791) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reworks the guard added in the previous commit: - Cover `damping` as well as `stiffness`/`mass`. An explicit `damping: undefined` reproduced the original bug in full — NaN points and a spring that never reports done, so the frameloop spins forever. - Reject non-finite values. `!(x > 0)` let `Infinity` through, which still resolved to NaN despite the comment claiming otherwise. - Resolve physics *before* choosing between physics- and duration-based resolution. `stiffness: 0` counted as "physics specified" via `isSpringType`, so a provided `duration`/`bounce` was silently discarded. - Replace stiffness and damping as a pair when duration resolution degenerates, so the relationship between them is never left half-overwritten. - Treat `visualDuration` with `!== undefined` rather than truthiness. - Warn on invalid physics rather than silently substituting defaults. `damping: 0` remains valid — it's a perpetually oscillating spring, and only stiffness and mass are divisors. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + .../animation/__tests__/JSAnimation.test.ts | 13 ++- .../generators/__tests__/spring.test.ts | 98 ++++++++++++++++--- .../src/animation/generators/spring.ts | 88 ++++++++++++----- 4 files changed, 158 insertions(+), 42 deletions(-) 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 ab1b65bc5e..430a5bff82 100644 --- a/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts +++ b/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts @@ -1429,15 +1429,24 @@ describe("JSAnimation", () => { // 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", "150,5 50,180 250,180"], + keyframes: ["150,5 75,200 225,200", target], type: "spring", stiffness: undefined, autoplay: false, }) for (let t = 0; t <= 2000; t += 50) { - expect(animation.sample(t).value).not.toContain("NaN") + 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 63558edc85..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 ", () => { @@ -288,33 +291,96 @@ 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) } - test("stiffness of 0 does not produce NaN", () => { - const values = sample({ keyframes: [0, 100], stiffness: 0 }) + /** + * 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("mass of 0 does not produce NaN", () => { - const values = sample({ keyframes: [0, 100], mass: 0 }) - values.forEach((v) => expect(v).not.toBeNaN()) + 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 }) + ) }) - /** - * An explicit `stiffness: undefined` (e.g. a forwarded prop that resolves - * to undefined) clobbers the default via the options spread in - * getSpringOptions, which previously produced NaN spring values. - */ - test("explicit undefined stiffness does not produce NaN", () => { - const values = sample({ keyframes: [0, 100], stiffness: undefined }) - values.forEach((v) => expect(v).not.toBeNaN()) + 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("explicit undefined mass does not produce NaN", () => { - const values = sample({ keyframes: [0, 100], mass: undefined }) + 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 491526d667..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,20 +252,21 @@ function getSpringOptions(options: SpringOptions) { } springOptions.isResolvedFromDuration = true } - } - /** - * Guard against non-positive or non-finite stiffness/mass. These divide - * and feed Math.sqrt() during resolution, so a 0 (or an explicit - * `undefined` that clobbers the default via the spread above) produces NaN - * spring values — which corrupt any animated value, e.g. an SVG polygon's - * points list. See https://github.com/motiondivision/motion/issues/2791 - */ - if (!(springOptions.stiffness > 0)) { - springOptions.stiffness = springDefaults.stiffness - } - if (!(springOptions.mass > 0)) { - springOptions.mass = springDefaults.mass + /** + * 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