Fix path animations with Safari zoom

[mattgperry]

Safari incorrectly scales stroke-dasharray and stroke-dashoffset values when using px units during page zoom. This causes SVG path animations using pathLength to shrink progressively as the zoom level decreases.

The fix removes the px unit conversion and uses unitless numeric values instead, which Safari does not scale during zoom operations.

Fixes #3301
Fixes #2544

[claude]

Code Review Summary

This PR fixes a Safari-specific bug where SVG path animations shrink when zooming the page. The fix is clean, well-tested, and addresses the root cause correctly.

✅ Code Quality & Best Practices

Excellent:

• Clean, focused change that addresses exactly one issue
• Follows the repository's conventions (no default exports, named exports, proper TypeScript types)
• Comprehensive test coverage updates across all affected test files
• Helpful inline comments explaining the Safari zoom bug context
• Updated type annotations to match new behavior (number → string)

Consistency:

• Comments added consistently across all test files explaining the unitless value rationale
• All affected code paths updated (both motion-dom and framer-motion packages)

✅ Bug Fix Quality

Root Cause Analysis:
The fix correctly identifies that Safari incorrectly scales stroke-dasharray and stroke-dashoffset values when using px units during page zoom operations. Using unitless numeric values bypasses this Safari quirk.

Correctness:

• SVG spec explicitly allows unitless values for stroke-dasharray and stroke-dashoffset (they're interpreted as user units)
• The change maintains functional equivalence across all browsers while fixing Safari
• No breaking changes to the public API

✅ Test Coverage

Comprehensive:

• Unit tests updated: use-props.test.ts, path.test.ts, svg-effect.test.ts
• E2E tests updated: svg.spec.ts (Playwright)
• All test expectations now verify unitless output format
• Tests cover: initial values, updates, cleanup, and edge cases (looping animations)

Suggestion: Consider adding a Safari-specific regression test that verifies the zoom behavior doesn't break in the future (though this may require browser automation with zoom controls, which could be complex).

✅ Performance Considerations

Improved:

• Removes px.transform!() calls, eliminating function call overhead
• Uses direct string interpolation instead
• Slightly more efficient runtime performance (minor, but a nice side effect)

No Regressions:

• Change only affects SVG path rendering code path
• No impact on non-SVG animations

✅ Security

No Concerns:

• Simple string interpolation with numeric values
• No user-controlled input injection risk
• Values come from controlled MotionValue instances

📝 Minor Observations

1. Type Changes: The PR correctly updates types in tests from number to string for stroke-dash attributes (lines in path.test.ts:6-7). This matches the new string-based implementation.

2. Documentation: The inline comments are excellent. Consider also mentioning this behavior in:

   • The main package documentation (if there's a pathLength animation guide)
   • The changelog/release notes

3. Code Location Consistency: Changes span both motion-dom (packages/motion-dom/src/effects/svg/index.ts:18-31, packages/motion-dom/src/render/svg/utils/path.ts:35-40) and framer-motion (packages/framer-motion/src/render/svg/utils/tests/path.test.ts). The changes are properly coordinated.

🎯 Verdict

✅ Approve - This is a high-quality fix that:

• Solves the reported issue correctly
• Maintains backward compatibility
• Includes comprehensive test updates
• Improves performance slightly
• Follows repository conventions

The fix is minimal, targeted, and well-reasoned. Ready to merge.

---

Related Issue: Fixes #3301
Impact: Safari users will no longer experience shrinking path animations when zooming

[claude]

Pull Request Review: Fix path animations with Safari zoom

Summary: This PR fixes a Safari-specific bug where SVG path animations using pathLength would progressively shrink when the browser zoom level changed. The fix removes px unit conversion and uses unitless numeric values for stroke-dasharray and stroke-dashoffset.

Code Quality - EXCELLENT

• Clean, focused change targeting only problematic code paths
• Well-documented with inline comments explaining the Safari zoom bug
• Consistent application across effect system and render utility
• Follows repo conventions; removed unused px.transform calls

Potential Issues - MINOR

1. String concatenation on every frame render (acceptable - modern engines optimize this)
2. No validation for NaN/Infinity (may be handled upstream in MotionValue system)

Performance - POSITIVE

• Removed unnecessary function calls
• Simplified string building
• No regressions expected

Security - CLEAN

No issues identified. Values are safely converted and DOM APIs handle escaping properly.

Test Coverage - EXCELLENT

• All unit tests updated (ssr.test.tsx, use-props.test.ts, path.test.ts, svg-effect.test.ts)
• E2E tests updated (svg.spec.ts)
• Good test descriptions with bug context

Browser Compatibility - SAFE

• SVG spec allows unitless values
• Works across all browsers
• More standards-compliant

Architecture - SOUND

• Fix at correct level (attribute serialization)
• Preserves public API
• No breaking changes

Final Verdict: APPROVE ✅

Strengths:

• Solves real Safari bug affecting users
• Clean minimal implementation
• Comprehensive test coverage
• Well-documented
• Standards-compliant

Minor suggestions (non-blocking):

1. Consider input validation for NaN/Infinity edge cases if not handled upstream
2. Add Safari-specific E2E test with zoom if feasible
3. Document in release notes as it affects rendered HTML/SVG output

This PR is ready to merge. Elegant fix for a longstanding Safari compatibility issue. Great work!

[claude]

Code Review: Fix path animations with Safari zoom

Summary

This PR fixes a Safari-specific bug where SVG path animations using pathLength would incorrectly shrink when users zoom the page. The fix removes px unit conversion and uses unitless numeric values instead, which Safari doesn't scale during zoom operations.

✅ Strengths

1. Excellent Root Cause Analysis

• The fix correctly identifies that Safari incorrectly scales px values in stroke-dasharray and stroke-dashoffset during zoom
• Using unitless values is the proper solution per SVG spec (these properties accept both <length> and unitless numbers)

2. Comprehensive Test Coverage

• All affected tests updated (unit tests, integration tests, E2E tests)
• Tests now validate unitless values with helpful comments explaining the Safari zoom bug
• Maintains behavioral correctness while changing implementation details

3. Clean Implementation

• Removed dependency on px.transform! utility, simplifying code
• Clear inline comments explain the rationale in packages/motion-dom/src/effects/svg/index.ts:19 and packages/motion-dom/src/render/svg/utils/path.ts:20-21
• Consistent change pattern across both affected files

4. Well-Documented

• PR description clearly links to issues [BUG] Zooming in / out in Safari shortens pathLength, breaking SVG stroke animations #3301 and [BUG] motion.{svg element} line lengths too short in zoomed out Safari #2544
• Code comments explain the Safari zoom bug context

🔍 Observations

1. Type Safety
In packages/motion-dom/src/render/svg/utils/path.ts:38-41, the values are coerced to strings using template literals. This is correct since DOM attributes are strings, and the test file at packages/framer-motion/src/render/svg/utils/__tests__/path.test.ts:6-7 properly updates type annotations from number to string.

2. SVG Spec Compliance
According to the SVG spec, both stroke-dasharray and stroke-dashoffset accept <length> or <number>. Unitless numbers are interpreted as user units (equivalent to px in most contexts). This change maintains semantic equivalence while avoiding the Safari bug.

3. String Coercion of Numbers
In packages/motion-dom/src/effects/svg/index.ts:21, when negating the offset value, if it's already a string, JavaScript will convert it to a number first, then the template literal converts it back to string. This is correct behavior.

4. Potential Edge Case: Non-numeric Values
If someone passes a non-numeric string to pathOffset, this would result in -NaN being set as the attribute. However, this would have been an issue with the previous code too (and would be a user error). The code assumes valid numeric inputs from the motion value system.

⚠️ Minor Suggestions

1. Consider Explicit Number Coercion (Optional)
For extra safety, you could explicitly convert to numbers, though this might be unnecessary since the motion value system likely ensures numeric values already.

2. Verify WAAPI Compatibility
The WAAPI polyfill at packages/framer-motion/src/animation/animators/waapi/__tests__/polyfill.js references stroke-dasharray and stroke-dashoffset. While the polyfill tests weren't updated, this is likely fine since WAAPI should handle unitless values correctly. Worth verifying this doesn't cause issues with animated path properties.

🎯 Performance Considerations

• Positive impact: Removed three px.transform! function calls per path update
• Minimal GC impact: String template literals vs function call + string concatenation is roughly equivalent
• Overall: Slight performance improvement due to fewer function calls

🔒 Security Considerations

No security concerns. The change is purely presentational and doesn't introduce any injection vectors.

✅ Final Recommendation

LGTM - Approve with confidence

This is a well-researched, properly implemented fix for a real Safari bug. The change:

• ✅ Fixes the reported issues ([BUG] Zooming in / out in Safari shortens pathLength, breaking SVG stroke animations #3301, [BUG] motion.{svg element} line lengths too short in zoomed out Safari #2544)
• ✅ Maintains cross-browser compatibility
• ✅ Follows SVG spec correctly
• ✅ Has comprehensive test coverage
• ✅ Improves code simplicity
• ✅ Includes clear documentation

The only minor consideration is verifying WAAPI animation compatibility, but this should work correctly since unitless values are valid per SVG spec.

---

Review performed by Claude Code following CLAUDE.md conventions