From 1654c3ddc77ec5a41560a19245257e76b6310b9c Mon Sep 17 00:00:00 2001 From: Matt Perry Date: Fri, 12 Jun 2026 11:08:28 +0200 Subject: [PATCH 1/8] 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 433a85fc0b..b5874e5607 100644 --- a/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts +++ b/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts @@ -308,3 +308,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 913eef906c8e6a31598e711741eeb176060a5904 Mon Sep 17 00:00:00 2001 From: Matt Perry Date: Fri, 12 Jun 2026 11:08:39 +0200 Subject: [PATCH 2/8] 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. Related to #2791 Co-Authored-By: Claude Opus 4.8 Co-authored-by: Matt Perry --- .../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 455af9595b..ef42a8703d 100644 --- a/packages/motion-dom/src/animation/generators/spring.ts +++ b/packages/motion-dom/src/animation/generators/spring.ts @@ -219,6 +219,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 4b3fb441bb2f874f6124a19e1a1ea551dee52c47 Mon Sep 17 00:00:00 2001 From: Matt Perry Date: Tue, 28 Jul 2026 09:58:14 +0200 Subject: [PATCH 3/8] 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 38817146c8..13051204ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -104,6 +104,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 b5874e5607..121d8306cb 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.each([ @@ -311,33 +314,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 ef42a8703d..7d4f0a8bc9 100644 --- a/packages/motion-dom/src/animation/generators/spring.ts +++ b/packages/motion-dom/src/animation/generators/spring.ts @@ -166,33 +166,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 @@ -217,20 +256,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 From d2729b430067f8c66bf458e7ef482add065b8136 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 25 Sep 2026 08:02:55 +0000 Subject: [PATCH 4/8] Drop CHANGELOG entry for spring physics guard After rebasing onto main the entry landed under the already released 12.43.0. The changelog is written at release time, so leave it to that. Co-authored-by: Matt Perry --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13051204ad..38817146c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -104,7 +104,6 @@ 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 From f8514942914d3807d160221fc6535f78a5019cd5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 25 Sep 2026 08:03:26 +0000 Subject: [PATCH 5/8] Fix spring guard after rebase: share time-defined decision with retarget main added retarget(), which decided whether to ignore inherited velocity with its own isSpringType(options, physicsKeys) check. This branch removed physicsKeys, so the rebased code no longer compiled, and restoring the constant alone would let retarget() disagree with initial resolution: { duration, stiffness: 0 } resolves as time-defined (velocity ignored) but retargeted with velocity. Decide once in getSpringOptions and reuse it in retarget(). Also renames the resolved physics to fix no-shadow against the visualDuration branch. Co-authored-by: Matt Perry --- .../generators/__tests__/spring.test.ts | 23 ++++++++++-- .../src/animation/generators/spring.ts | 37 +++++++++---------- 2 files changed, 37 insertions(+), 23 deletions(-) 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 121d8306cb..1e57f79a25 100644 --- a/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts +++ b/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts @@ -362,9 +362,9 @@ describe("spring NaN guards", () => { 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 }) - ) + 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", () => { @@ -389,6 +389,23 @@ describe("spring NaN guards", () => { values.forEach((v) => expect(v).not.toBeNaN()) }) + test("retargeting treats invalid physics as absent", () => { + // Initial resolution treats this as a time-defined spring, which + // ignores inherited velocity, so retargeting must too + const options = { duration: 600, stiffness: 0 } + const generator = spring({ ...options, keyframes: [0, 100] }) + generator.next(16) + generator.retarget!([20, -50], 300) + const fresh = spring({ + ...options, + keyframes: [20, -50], + velocity: 300, + }) + for (const t of [0, 16, 100, 300, 1000]) { + expect(generator.next(t)).toEqual(fresh.next(t)) + } + }) + test("invalid stiffness still resolves to a spring that completes", () => { const generator = spring({ keyframes: [0, 100], stiffness: undefined }) expect(calcGeneratorDuration(generator)).toBeLessThan( diff --git a/packages/motion-dom/src/animation/generators/spring.ts b/packages/motion-dom/src/animation/generators/spring.ts index 7d4f0a8bc9..7f774094c9 100644 --- a/packages/motion-dom/src/animation/generators/spring.ts +++ b/packages/motion-dom/src/animation/generators/spring.ts @@ -209,22 +209,26 @@ function getSpringOptions(options: SpringOptions) { * 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 + const validStiffness = resolvePhysics(options.stiffness) + const validDamping = resolvePhysics(options.damping, true) + const validMass = resolvePhysics(options.mass) let springOptions = { ...options, velocity: options.velocity ?? springDefaults.velocity, - stiffness: stiffness ?? springDefaults.stiffness, - damping: damping ?? springDefaults.damping, - mass: mass ?? springDefaults.mass, + stiffness: validStiffness ?? springDefaults.stiffness, + damping: validDamping ?? springDefaults.damping, + mass: validMass ?? springDefaults.mass, isResolvedFromDuration: false, + // stiffness/damping/mass overrides duration/bounce + isTimeDefined: + validStiffness === undefined && + validDamping === undefined && + validMass === undefined && + isSpringType(options, durationKeys), } - // stiffness/damping/mass overrides duration/bounce - if (!hasPhysics && isSpringType(options, durationKeys)) { + + if (springOptions.isTimeDefined) { // Time-defined springs should ignore inherited velocity. // Velocity from interrupted animations can cause findSpring() // to compute wildly different spring parameters, leading to @@ -307,6 +311,7 @@ function spring( duration, velocity, isResolvedFromDuration, + isTimeDefined, } = getSpringOptions({ ...options, velocity: -millisecondsToSeconds(options.velocity || 0), @@ -455,13 +460,6 @@ function spring( update() - /** - * Time-defined springs ignore inherited velocity, see getSpringOptions. - */ - const ignoreVelocity = - !isSpringType(options, physicsKeys) && - isSpringType(options, durationKeys) - const calculatedDuration = isResolvedFromDuration ? duration || null : null const generator = { @@ -473,9 +471,8 @@ function spring( retarget: (keyframes: number[], newVelocity: number) => { s.target = keyframes[keyframes.length - 1] s.delta = s.target - keyframes[0] - s.velocity = ignoreVelocity - ? 0 - : -millisecondsToSeconds(newVelocity) + // Time-defined springs ignore inherited velocity, see getSpringOptions + s.velocity = isTimeDefined ? 0 : -millisecondsToSeconds(newVelocity) // Default thresholds depend on the scale of the new delta if (!(options.restSpeed && options.restDelta)) setRestThresholds() // Invalidate any duration lazily cached by JSAnimation From a9fc30a5ec99a7ad454c2591801f4556f3d21452 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 25 Sep 2026 08:03:49 +0000 Subject: [PATCH 6/8] Keep existing spring behaviour for numeric strings and visualDuration: 0 - isValidPhysics used Number.isFinite, which rejects numeric strings that spring physics have always coerced (stiffness: "300" silently became 100). Use relational checks, which also reject NaN/undefined/Infinity. - visualDuration: 0 was switched from a truthiness check to !== undefined, which made it produce an infinite stiffness that the guard then had to catch. Restore main's check; nothing about this bug needed it changed. - Correct the degenerate-guard comment: findSpring() doesn't collapse to 0/0 for any finite duration/bounce; the real cases are a NaN bounce and an infinite visualDuration. Test those instead of visualDuration: 0. - Drop the velocity default, which spring() always overrides. Co-authored-by: Matt Perry --- .../generators/__tests__/spring.test.ts | 32 +++++++++++++++---- .../src/animation/generators/spring.ts | 26 +++++++-------- 2 files changed, 36 insertions(+), 22 deletions(-) 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 1e57f79a25..c3c8ec87ff 100644 --- a/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts +++ b/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts @@ -359,6 +359,24 @@ describe("spring NaN guards", () => { expect(values[values.length - 1]).not.toBeCloseTo(100) }) + test("numeric string physics still coerce", () => { + expect( + sample({ + keyframes: [0, 100], + stiffness: "300", + damping: "20", + mass: "2", + } as any) + ).toEqual( + sample({ + keyframes: [0, 100], + stiffness: 300, + damping: 20, + mass: 2, + }) + ) + }) + 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. @@ -380,13 +398,13 @@ describe("spring NaN guards", () => { ) }) - 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.each([ + { duration: 500, bounce: NaN }, + { visualDuration: Infinity, bounce: 0.2 }, + { visualDuration: 0, bounce: 0.2 }, + ])("non-finite time options don't produce NaN: %o", (options) => { + const values = sample({ keyframes: [0, 100], ...options }) + values.forEach((v) => expect(Number.isFinite(v)).toBe(true)) }) test("retargeting treats invalid physics as absent", () => { diff --git a/packages/motion-dom/src/animation/generators/spring.ts b/packages/motion-dom/src/animation/generators/spring.ts index 7f774094c9..2c11922d62 100644 --- a/packages/motion-dom/src/animation/generators/spring.ts +++ b/packages/motion-dom/src/animation/generators/spring.ts @@ -174,22 +174,20 @@ function isSpringType(options: SpringOptions, keys: string[]) { /** * 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. + * spring. Relational rather than Number.isFinite so numeric strings still + * coerce. */ const isValidPhysics = (value: number | undefined, canBeZero?: boolean) => - Number.isFinite(value) && (canBeZero ? value! >= 0 : value! > 0) + (canBeZero ? value! >= 0 : value! > 0) && value! < Infinity /** * Returns value if it's usable spring physics, otherwise undefined so callers - * can fall back to a default. + * fall back to the default. An explicit `undefined`, e.g. from a forwarded + * optional prop, must fall back too. * - * 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 + * Invalid physics resolve to NaN spring values, which corrupt every animated + * value downstream (an SVG polygon's points become "NaN,NaN NaN,NaN") and + * never report done. */ function resolvePhysics(value: number | undefined, canBeZero?: boolean) { if (isValidPhysics(value, canBeZero)) return value @@ -215,7 +213,6 @@ function getSpringOptions(options: SpringOptions) { let springOptions = { ...options, - velocity: options.velocity ?? springDefaults.velocity, stiffness: validStiffness ?? springDefaults.stiffness, damping: validDamping ?? springDefaults.damping, mass: validMass ?? springDefaults.mass, @@ -235,7 +232,7 @@ function getSpringOptions(options: SpringOptions) { // massive oscillation on small-range animations. springOptions.velocity = 0 - if (options.visualDuration !== undefined) { + if (options.visualDuration) { const visualDuration = options.visualDuration const root = (2 * Math.PI) / (visualDuration * 1.2) const stiffness = root * root @@ -262,9 +259,8 @@ function getSpringOptions(options: SpringOptions) { } /** - * 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 + * Non-finite time options degenerate: a NaN bounce gives a NaN + * damping, an infinite visualDuration a 0 stiffness. Replace the two * together, so the relationship duration resolution establishes * between them is never left half-overwritten. */ From ff716aa7bdccc34a43e6c90f81266ed050fce349 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 25 Sep 2026 08:03:57 +0000 Subject: [PATCH 7/8] Tighten spring NaN guard tests and cover the React path - Assert invalid physics resolve to exactly the default spring, rather than not.toBeNaN(), which also passes for Infinity. - Assert an explicit undefined (a forwarded optional prop) doesn't warn. - The polygon JSAnimation test threw a TypeError on failure (match() returns null for an all-NaN string); split and check every coordinate. - Add a motion.polygon test with an unset stiffness prop forwarded into the transition, which renders NaN points without the guard. - Drop the #2791 references. The reporter's sandbox uses { type: "spring", duration: 3, delay }, with no physics options; its NaN came from the delay phase and was fixed by f11e74d17 (#3351). Co-authored-by: Matt Perry --- .../motion/__tests__/component-svg.test.tsx | 40 +++++++++++++++++++ .../animation/__tests__/JSAnimation.test.ts | 20 +++++----- .../generators/__tests__/spring.test.ts | 23 ++++++----- 3 files changed, 63 insertions(+), 20 deletions(-) diff --git a/packages/framer-motion/src/motion/__tests__/component-svg.test.tsx b/packages/framer-motion/src/motion/__tests__/component-svg.test.tsx index 10c6f5d6f9..b8b4af5351 100644 --- a/packages/framer-motion/src/motion/__tests__/component-svg.test.tsx +++ b/packages/framer-motion/src/motion/__tests__/component-svg.test.tsx @@ -144,6 +144,46 @@ describe("SVG", () => { expect(gElement).toHaveStyle("transform: translate(50, 50)") }) + test("spring with an unset stiffness prop never renders NaN points", async () => { + const target = "150,5 50,180 250,180" + let onComplete: VoidFunction + const complete = new Promise((resolve) => (onComplete = resolve)) + + // An unset optional prop forwarded into the transition + const Component = ({ stiffness }: { stiffness?: number }) => ( + + onComplete()} + /> + + ) + const { container } = render() + const polygon = container.querySelector("polygon")! + + const rendered: string[] = [] + new MutationObserver(() => + rendered.push(polygon.getAttribute("points")!) + ).observe(polygon, { attributeFilter: ["points"] }) + + await Promise.race([ + complete, + new Promise((resolve) => setTimeout(resolve, 3000)), + ]) + + expect(rendered.length).toBeGreaterThan(1) + rendered.forEach((points) => + points + .split(/[ ,]/u) + .forEach((coord) => + expect(Number.isFinite(Number(coord))).toBe(true) + ) + ) + expect(polygon.getAttribute("points")).toBe(target) + }) + test("animates viewBox", async () => { const Component = () => { return ( diff --git a/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts b/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts index 430a5bff82..ed6bd0ae7b 100644 --- a/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts +++ b/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts @@ -1423,12 +1423,10 @@ describe("JSAnimation", () => { 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. + test("Spring with undefined stiffness over polygon points never produces NaN", () => { + // An explicit `undefined` stiffness, as forwarded from an optional + // prop, must fall back to the default rather than emit NaN progress + // that the complex-value mixer turns into "NaN,NaN NaN,NaN" const target = "150,5 50,180 250,180" const animation = animateValue({ keyframes: ["150,5 75,200 225,200", target], @@ -1438,11 +1436,11 @@ describe("JSAnimation", () => { }) 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) + const coords = animation.sample(t).value.split(/[ ,]/u) + expect(coords).toHaveLength(6) + coords.forEach((coord) => + expect(Number.isFinite(Number(coord))).toBe(true) + ) } // ...and the spring must still settle on the target, so this doesn't 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 c3c8ec87ff..1284c37900 100644 --- a/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts +++ b/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts @@ -312,7 +312,6 @@ describe("toString", () => { }) }) -// https://github.com/motiondivision/motion/issues/2791 describe("spring NaN guards", () => { // These deliberately pass invalid physics, which warns let warn: jest.SpyInstance @@ -333,9 +332,8 @@ describe("spring NaN guards", () => { /** * 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. + * explicit `undefined`, e.g. a forwarded optional prop, is the most likely + * in practice. */ const physicsKeys = ["stiffness", "damping", "mass"] as const const invalidValues = [0, -1, NaN, Infinity, -Infinity, undefined] @@ -345,16 +343,17 @@ describe("spring NaN guards", () => { // 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(`${key} of ${String(value)} falls back to the default`, () => { + expect(sample({ keyframes: [0, 100], [key]: value })).toEqual( + sample({ keyframes: [0, 100] }) + ) }) } } 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()) + values.forEach((v) => expect(Number.isFinite(v)).toBe(true)) // An undamped spring oscillates rather than settling on the target expect(values[values.length - 1]).not.toBeCloseTo(100) }) @@ -437,8 +436,14 @@ describe("spring NaN guards", () => { expect(warn.mock.calls[0][0]).toContain("spring-invalid-physics") }) - test("valid physics does not warn", () => { + test("valid or explicitly undefined physics does not warn", () => { spring({ keyframes: [0, 100], stiffness: 200, damping: 0, mass: 2 }) + spring({ + keyframes: [0, 100], + stiffness: undefined, + damping: undefined, + mass: undefined, + }) expect(warn).not.toHaveBeenCalled() }) }) From 88d58cc8f1e92a643bc4a526278dec36e72d96ce Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 25 Sep 2026 08:26:54 +0000 Subject: [PATCH 8/8] Pass resolved options to findSpring so invalid mass keeps the duration findSpring() was given the raw options, so an invalid mass alongside a duration (e.g. { duration: 500, mass: 0 }) derived a degenerate stiffness that the guard replaced with default physics, discarding the duration. springOptions already carries the defaulted mass and zeroed velocity. Also dev-gates the invalid-physics warning so its strings are stripped from production builds, and tightens the tests: visualDuration: 0 and retargeting now compare against specs that fail on main, and the React polygon test samples 10 frames instead of waiting up to 3s. Co-authored-by: Matt Perry --- .../motion/__tests__/component-svg.test.tsx | 20 ++++-------- .../animation/__tests__/JSAnimation.test.ts | 2 -- .../generators/__tests__/spring.test.ts | 31 ++++++++++++------- .../src/animation/generators/spring.ts | 25 +++++++-------- 4 files changed, 37 insertions(+), 41 deletions(-) diff --git a/packages/framer-motion/src/motion/__tests__/component-svg.test.tsx b/packages/framer-motion/src/motion/__tests__/component-svg.test.tsx index b8b4af5351..ce4b381965 100644 --- a/packages/framer-motion/src/motion/__tests__/component-svg.test.tsx +++ b/packages/framer-motion/src/motion/__tests__/component-svg.test.tsx @@ -145,18 +145,13 @@ describe("SVG", () => { }) test("spring with an unset stiffness prop never renders NaN points", async () => { - const target = "150,5 50,180 250,180" - let onComplete: VoidFunction - const complete = new Promise((resolve) => (onComplete = resolve)) - // An unset optional prop forwarded into the transition const Component = ({ stiffness }: { stiffness?: number }) => ( onComplete()} /> ) @@ -164,14 +159,12 @@ describe("SVG", () => { const polygon = container.querySelector("polygon")! const rendered: string[] = [] - new MutationObserver(() => + const observer = new MutationObserver(() => rendered.push(polygon.getAttribute("points")!) - ).observe(polygon, { attributeFilter: ["points"] }) - - await Promise.race([ - complete, - new Promise((resolve) => setTimeout(resolve, 3000)), - ]) + ) + observer.observe(polygon, { attributeFilter: ["points"] }) + for (let i = 0; i < 10; i++) await nextFrame() + observer.disconnect() expect(rendered.length).toBeGreaterThan(1) rendered.forEach((points) => @@ -181,7 +174,6 @@ describe("SVG", () => { expect(Number.isFinite(Number(coord))).toBe(true) ) ) - expect(polygon.getAttribute("points")).toBe(target) }) test("animates viewBox", async () => { diff --git a/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts b/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts index ed6bd0ae7b..bfd0f5b247 100644 --- a/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts +++ b/packages/motion-dom/src/animation/__tests__/JSAnimation.test.ts @@ -1443,8 +1443,6 @@ describe("JSAnimation", () => { ) } - // ...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 1284c37900..53df43bfe1 100644 --- a/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts +++ b/packages/motion-dom/src/animation/generators/__tests__/spring.test.ts @@ -376,13 +376,14 @@ describe("spring NaN guards", () => { ) }) - 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.each(physicsKeys)( + "invalid %s does not discard a provided duration", + (key) => { + expect( + sample({ keyframes: [0, 100], duration: 500, [key]: NaN }) + ).toEqual(sample({ keyframes: [0, 100], duration: 500 })) + } + ) test("invalid physics does not discard a provided visualDuration", () => { expect( @@ -400,22 +401,30 @@ describe("spring NaN guards", () => { test.each([ { duration: 500, bounce: NaN }, { visualDuration: Infinity, bounce: 0.2 }, - { visualDuration: 0, bounce: 0.2 }, ])("non-finite time options don't produce NaN: %o", (options) => { const values = sample({ keyframes: [0, 100], ...options }) values.forEach((v) => expect(Number.isFinite(v)).toBe(true)) }) + test("visualDuration of 0 falls back to duration-based resolution", () => { + expect( + sample({ keyframes: [0, 100], visualDuration: 0, bounce: 0.2 }) + ).toEqual(sample({ keyframes: [0, 100], bounce: 0.2 })) + }) + test("retargeting treats invalid physics as absent", () => { // Initial resolution treats this as a time-defined spring, which // ignores inherited velocity, so retargeting must too - const options = { duration: 600, stiffness: 0 } - const generator = spring({ ...options, keyframes: [0, 100] }) + const generator = spring({ + keyframes: [0, 100], + duration: 600, + stiffness: 0, + }) generator.next(16) generator.retarget!([20, -50], 300) const fresh = spring({ - ...options, keyframes: [20, -50], + duration: 600, velocity: 300, }) for (const t of [0, 16, 100, 300, 1000]) { diff --git a/packages/motion-dom/src/animation/generators/spring.ts b/packages/motion-dom/src/animation/generators/spring.ts index 2c11922d62..93be21b2a5 100644 --- a/packages/motion-dom/src/animation/generators/spring.ts +++ b/packages/motion-dom/src/animation/generators/spring.ts @@ -183,20 +183,19 @@ const isValidPhysics = (value: number | undefined, canBeZero?: boolean) => /** * Returns value if it's usable spring physics, otherwise undefined so callers * fall back to the default. An explicit `undefined`, e.g. from a forwarded - * optional prop, must fall back too. - * - * Invalid physics resolve to NaN spring values, which corrupt every animated - * value downstream (an SVG polygon's points become "NaN,NaN NaN,NaN") and - * never report done. + * optional prop, must fall back too. Invalid physics would otherwise resolve + * to NaN spring values that never report done. */ 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" - ) + if (process.env.NODE_ENV !== "production") { + warning( + value === undefined, + "Spring stiffness and mass must be positive, damping 0 or greater", + "spring-invalid-physics" + ) + } return undefined } @@ -219,9 +218,7 @@ function getSpringOptions(options: SpringOptions) { isResolvedFromDuration: false, // stiffness/damping/mass overrides duration/bounce isTimeDefined: - validStiffness === undefined && - validDamping === undefined && - validMass === undefined && + (validStiffness ?? validDamping ?? validMass) === undefined && isSpringType(options, durationKeys), } @@ -248,7 +245,7 @@ function getSpringOptions(options: SpringOptions) { damping, } } else { - const derived = findSpring({ ...options, velocity: 0 }) + const derived = findSpring(springOptions) springOptions = { ...springOptions,