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.