Fix NaN in spring-animated SVG polygon points (#2791)

[mattgperry]

Bug

Animating an SVG <polygon>'s points with type: "spring" intermittently writes an invalid point list and floods the console:

<polygon> attribute points: Expected number, "NaN,NaN NaN,NaN …".

(With non-spring types the NaNs don't appear.) Reported in #2791.

Cause

The all-NaN point list (with an otherwise correct comma/space template) can only occur if the spring's progress output is NaN. Tracing the spring generator:

• For non-numeric keyframes (a points string), JSAnimation animates a 0 → 100 progress spring and maps it back through the complex-value mixer. The mixer is not at fault — it produces finite output even with spring overshoot (progress > 1 / < 0) and mismatched point counts.
• getSpringOptions() resolves physics parameters with { ...defaults, ...options }. An explicit stiffness: undefined — e.g. an optional prop forwarded into transition — clobbers the default, and 0/negative values pass straight through. Those values then divide and feed Math.sqrt() during resolution (dampingRatio = damping / (2 * Math.sqrt(stiffness * mass)), undampedAngularFreq = sqrt(stiffness / mass)), yielding NaN spring values for every frame.

A NaN velocity, by contrast, is harmless — it's sanitised by velocity || 0.

Fix

Guard stiffness and mass to a positive, finite value in getSpringOptions(), falling back to the defaults otherwise. The check runs once at resolution time, not in the per-frame hot path.

if (!(springOptions.stiffness > 0)) springOptions.stiffness = springDefaults.stiffness
if (!(springOptions.mass > 0)) springOptions.mass = springDefaults.mass

This catches 0, negative, NaN, and undefined while leaving valid values untouched.

Tests

Failing-first (verified to fail on the unfixed code, pass after the fix):

• packages/motion-dom/.../generators/__tests__/spring.test.ts — spring() with stiffness/mass of 0 or explicit undefined no longer returns NaN.
• packages/motion-dom/.../__tests__/JSAnimation.test.ts — a spring over a polygon points string never emits "NaN". Before the fix this produced exactly "NaN,NaN NaN,NaN NaN,NaN", matching the screenshot in the issue.

The unit tests reproduce the precise symptom and are the regression gate; the root cause is environment-independent spring math, so no Cypress E2E is required. Full motion-dom (476) and framer-motion client (799) suites pass.

Uncertainty

The original CodeSandbox reproduction is no longer reachable, so the exact transition the reporter used couldn't be confirmed. The fix targets the only input class that reproduces the reported "NaN,NaN …" output — a non-positive/undefined spring physics parameter — and the precise symptom is reproduced and gated by the new tests. The separate Expected number, "undefined" message noted in the issue is a distinct, smaller symptom (an unresolved origin keyframe rendering as undefined) and is intentionally left out of scope to keep this fix minimal.

Fixes #2791

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR fixes intermittent "NaN,NaN NaN,NaN" output when spring-animating an SVG <polygon>'s points attribute by guarding stiffness and mass against non-positive/invalid values in getSpringOptions() — the root cause being that an explicit undefined (e.g. a forwarded optional prop) clobbers the spread defaults and then feeds into Math.sqrt(), yielding NaN for every spring frame.

• spring.ts: Adds !(x > 0) guards for stiffness and mass after the options spread, falling back to defaults for 0, negative, NaN, and undefined inputs.
• spring.test.ts: New "spring NaN guards" describe block verifies each bad input class for both parameters.
• JSAnimation.test.ts: End-to-end integration test drives a spring over a polygon points string and asserts no "NaN" appears in the sampled output over 2 seconds.

Confidence Score: 3/5

The stiffness/mass guards and tests are correct, but damping is subject to the same undefined-clobber path and has no guard, leaving the same class of NaN bug reachable.

The stiffness and mass guards are sound and the new tests reproduce the exact reported failure. However, damping is subject to the identical spread-clobber mechanism: undefined overwrites the default 10, then dampingRatio = undefined / positive = NaN propagates through every spring branch. A user forwarding an optional transition prop with damping: undefined would still see NaN output after this fix.

packages/motion-dom/src/animation/generators/spring.ts needs a damping guard matching the ones added for stiffness and mass.

Important Files Changed

Filename	Overview
packages/motion-dom/src/animation/generators/spring.ts	Adds NaN guards for stiffness and mass in getSpringOptions(); the damping parameter is subject to the same undefined-clobber mechanism but is not guarded, and Infinity is not excluded despite the comment claiming non-finite values are handled.
packages/motion-dom/src/animation/generators/tests/spring.test.ts	Adds targeted NaN-guard tests for stiffness/mass = 0 and = undefined; missing a parallel damping: undefined case.
packages/motion-dom/src/animation/tests/JSAnimation.test.ts	Adds integration test covering the full SVG polygon points / spring NaN path; checks 41 sample points across the animation timeline.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["spring(options)"] --> B["getSpringOptions()"]
    B --> C["Set defaults: stiffness=100, damping=10, mass=1"]
    C --> D["...options spread\n(may clobber defaults with undefined)"]
    D --> E{Physics or\nduration spring?}
    E -->|duration/bounce| F["findSpring() derive stiffness/damping"]
    E -->|visualDuration| G["Compute stiffness from visualDuration"]
    E -->|physics| H["Use spread values as-is"]
    F --> I
    G --> I
    H --> I
    I["NaN guards (NEW)"]
    I --> J{"stiffness > 0?"}
    J -->|No: 0 / neg / NaN / undefined| K["stiffness = 100 default"]
    J -->|Yes| L["Keep stiffness"]
    K --> M{"mass > 0?"}
    L --> M
    M -->|No| N["mass = 1 default"]
    M -->|Yes| O["Keep mass"]
    N --> P["Compute dampingRatio, undampedAngularFreq"]
    O --> P
    P --> Q["Select spring regime underdamped / critical / overdamped"]
    Q --> R["next(t) finite spring value"]

Loading

_{Reviews (1): Last reviewed commit: "Fix NaN in spring animations with falsy/..." | Re-trigger Greptile}

[greptile-apps]

damping: undefined not guarded — same NaN path as the fixed bug

stiffness and mass are guarded, but damping goes through the exact same spread-clobber mechanism and isn't protected. If a caller passes transition={{ damping: optionalProp }} where optionalProp is undefined, the ...options spread clobbers springDefaults.damping (10), leaving springOptions.damping = undefined. Then dampingRatio = undefined / (2 * Math.sqrt(stiffness * mass)) evaluates to NaN, which makes every branch comparison (NaN < 1, NaN === 1) false, so the overdamped path runs with a NaN dampedAngularFreq — producing NaN outputs for every frame.

A guard like if (!(springOptions.damping >= 0)) springOptions.damping = springDefaults.damping would cover undefined, NaN, and negative damping (note: >= 0 rather than > 0 because zero damping is a valid underdamped spring). The new test suite would benefit from a damping: undefined case as well.

[greptile-apps]

The comment says "non-positive or non-finite" but Infinity satisfies Infinity > 0, so an Infinity stiffness or mass slips through the guard. With stiffness = Infinity, undampedAngularFreq = Math.sqrt(Infinity / mass) = Infinity, and A = (velocity + 0 * Infinity * delta) / Infinity = NaN, still producing NaN. Using isFinite alongside the positivity check closes this gap.

Suggested change

	if (!(springOptions.stiffness > 0)) {
	springOptions.stiffness = springDefaults.stiffness
	}
	if (!(springOptions.mass > 0)) {
	springOptions.mass = springDefaults.mass
	}
	if (!(springOptions.stiffness > 0) || !isFinite(springOptions.stiffness)) {
	springOptions.stiffness = springDefaults.stiffness
	}
	if (!(springOptions.mass > 0) || !isFinite(springOptions.mass)) {
	springOptions.mass = springDefaults.mass
	}

[mattgperry]

Addressed code review feedback and rebased onto main (was 98 commits behind).

Fixed

The original guard was incomplete — it ran too late and covered the wrong set of values:

• damping was omitted. spring({ keyframes: [0, 100], damping: undefined }) still returned [NaN, NaN, NaN, NaN, NaN], i.e. the reported bug reproduced in full through a third prop. It also never sets done, so the frameloop spins indefinitely.
• !(x > 0) let Infinity through, despite the comment claiming to guard non-finite values. stiffness: Infinity still resolved to NaN. Now uses Number.isFinite.
• The guard ran after the physics-vs-duration branch decision. isSpringType tests !== undefined, so stiffness: 0 counted as "physics specified" and silently discarded a provided duration — { duration: 500, stiffness: 0 } produced output byte-identical to a bare default spring. Physics are now resolved before the branch decision.
• Stiffness was replaced without its paired damping. Duration resolution derives damping from stiffness; overwriting one alone yields dampingRatio: 0, an undamped spring. The degenerate cases are now replaced as a pair.
• if (options.visualDuration) → !== undefined.
• Invalid physics now warns (spring-invalid-physics) instead of silently substituting defaults.

damping: 0 is deliberately still honoured — it's a valid, perpetually oscillating spring, and only stiffness/mass are divisors.

Tests

The NaN-guard suite now tables all three physics keys against every way each can be invalid (0, negative, NaN, ±Infinity, explicit undefined), plus the duration-not-discarded cases and the warning. The JSAnimation polygon test asserted .not.toContain("NaN"), which passes for any non-NaN corruption; it now checks every coordinate is finite and that the spring settles on the target.

Verified these are real gates: reverting spring.ts to the previous commit fails 9 tests (the damping, Infinity, discarded-duration and warning cases); reverting to main fails 21.

Other findings, not changed

• restSpeed ||= / restDelta ||= were flagged as the same falsy-zero bug class. They aren't — springs approach their target asymptotically, so restSpeed: 0 means the spring can only rest once the exponential underflows to exactly zero, which is ~149s for a default spring and would just hit the generator duration cap. The ||= is protective here, so I left it.
• inertia's bounceStiffness now falls back to spring's default of 100 rather than inertia's 500 when explicitly passed 0. Honouring inertia's default would mean inventing a policy for a value the caller explicitly set to zero, so I left the invalid-input path pointing at spring's own default.

Checks

• motion-dom: 517 passed
• framer-motion: 802 passed
• Bundle size: +~130 B gzip. The size gates that currently fail (size-rollup-motion, -m, -animate, -scroll, -waapi-animate, style-effect) fail identically on main at the same numbers — pre-existing, not from this change. The two gates with headroom still pass.

[MILLERMARRU]

The spring physics logic itself is solid. Tracing through resolvePhysics, the object-spread ordering was the real bug: the old code spread ...options after the property defaults, and in JS an explicitly present stiffness: undefined key (which is exactly what happens when a component forwards an optional prop that wasn't set) survives a spread and overwrites the default rather than falling through to it. Moving the physics resolution to compute stiffness ?? springDefaults.stiffness etc. after the spread, and doing it for all three physics keys together before deciding between physics- and duration-based resolution, closes both the original NaN report and the more subtle case the tests call out, where stiffness: 0 used to be treated as "physics was specified" and silently discarded a valid duration/visualDuration.

One thing that looks like it'll break the build though: spring.ts imports warning from motion-utils, but that package's public entry only re-exports warn-once.ts, which exports warnOnce and hasWarned, no warning. I checked the full file listing under packages/motion-utils/src and the git history on warn-once.ts, there's never been a plain warning export there. This PR also doesn't touch anything under motion-utils per the file list, so as it stands import { warning } from "motion-utils" has nothing to resolve to.

It also looks like warnOnce's actual behavior wouldn't give the test at the bottom (invalid physics warns rather than failing silently) what it expects anyway, even if the name were swapped in as a quick fix. warnOnce dedupes by the exact message string process-wide via a module-level Set, and every resolvePhysics call in this file uses the same message text regardless of which key or value triggered it. The parameterized "does not produce NaN" tests earlier in the same file already exercise stiffness: 0-equivalent invalid values multiple times before that later test runs, so under real warnOnce semantics the message would already be marked as warned by the time that assertion runs, and console.warn wouldn't fire a second time for it. That reads like the intent here is a warning that fires on every occurrence, which isn't what either existing export in motion-utils does today.