Handle default transform values when strings

[mattgperry]

When passing scale: "0" or similar string values, the buildTransform function was incorrectly treating these as default values because it only checked if parseFloat(value) === 0 for string values.

For scale properties, the default is 1, not 0. This fix makes the string branch mirror the number branch logic, checking against 1 for scale properties and 0 for others.

Fixes #3465

[claude]

Code Review

Summary: This PR correctly fixes a bug where string scale values of "0" were being incorrectly treated as default values and omitted from the generated transform string. The fix is well-implemented and includes appropriate test coverage.

✅ Strengths

1. Correct Fix: The logic now properly mirrors the number branch by checking scale properties against their actual default value of 1, rather than incorrectly checking against 0.

2. Good Test Coverage: The new test at packages/framer-motion/src/render/html/utils/__tests__/build-transform.test.ts:105-110 covers the critical cases (scale, scaleX, scaleY) with string values of "0".

3. Clear Documentation: The PR description and issue reference clearly explain the problem and solution.

4. Minimal Change: The fix is surgical and doesn't introduce unnecessary complexity.

🐛 Critical Issue Found

There's a duplicate bug in another file that also needs fixing:

packages/motion-dom/src/effects/style/transform.ts:29 has the exact same bug:

} else {
    valueIsDefault = parseFloat(value) === 0  // ❌ Wrong for scale properties
}

This should be:

} else {
    const parsed = parseFloat(value)
    valueIsDefault = key.startsWith("scale") ? parsed === 1 : parsed === 0
}

This file appears to be a different implementation of buildTransform (likely for the effects system), and it will have the same issue with string scale values.

📝 Minor Suggestions

1. Test Edge Cases: Consider adding tests for:

   • scale: "0%" (percentage values, as mentioned in the issue)
   • scale: "0.0" (decimal zero)
   • Mixed scenarios like { scaleX: "0", scaleY: 1 }

2. Consider skew property: While checking the code, I noticed skew, skewX, and skewY are also in transformPropOrder. These should have default values of 0, which your fix already handles correctly, but it might be worth adding a comment or test to document this behavior.

🔒 Security & Performance

• ✅ No security concerns
• ✅ No performance regressions (same number of operations)
• ✅ Type safety is maintained

✅ Code Style

• ✅ Follows repository conventions (arrow callbacks, const over let)
• ✅ Variable naming is clear (parsed for the float value)
• ✅ Consistent with the number branch logic

Recommended Action

Please fix the duplicate bug in packages/motion-dom/src/effects/style/transform.ts:29 and add a test for that code path as well. Otherwise, the bug will still exist in that part of the system.

Files to update:

• packages/motion-dom/src/effects/style/transform.ts (line 29)
• Add corresponding test coverage for the effects transform builder

Once that's addressed, this will be ready to merge! 🚀

[claude]

Pull Request Review

Summary

This PR fixes a bug where string values of "0" for scale properties were incorrectly treated as default values, causing the transform to be omitted from the CSS output. The fix ensures that the string branch mirrors the number branch logic by checking against 1 for scale properties and 0 for others.

Code Quality & Best Practices ✅

Strengths:

• Follows DRY principle: The fix is consistently applied to both files that contain the buildTransform logic
• Clear variable naming: Using parsed makes the code more readable than inline parseFloat(value)
• Maintains consistency: The string handling now properly mirrors the number handling logic
• Good test coverage: Added tests cover the specific bug case with scale: "0", scaleX: "0", and scaleY: "0"

Suggestions: None - the code follows the repository's style conventions and is clear and maintainable.

Correctness & Potential Issues ✅

Analysis:

• The fix correctly identifies that scale properties have a default of 1 while other transforms (translate, rotate, skew) have a default of 0
• The logic properly uses key.startsWith("scale") to identify all scale-related properties
• The fix handles both the direct scale value and the parsed numeric representation

Edge Cases Covered:

• ✅ scale: "0" - now correctly produces scale(0)
• ✅ scaleX: "0" - now correctly produces scaleX(0)
• ✅ scaleY: "0" - now correctly produces scaleY(0)
• ✅ scale: "0%" - will also work (parsed to 0)
• ✅ Default values still optimized away (scale: "1" produces none)

No Issues Found - the logic is sound and handles all relevant cases.

Performance Considerations ✅

Impact:

• Minimal performance impact - only adds one variable assignment per string transform value
• The key.startsWith("scale") check is already present in the number branch, so no additional overhead
• No unnecessary loops or allocations introduced

Assessment: No performance concerns.

Security Concerns ✅

Analysis:

• The code uses parseFloat() which safely handles untrusted string input
• No potential for injection attacks or prototype pollution
• No use of eval() or similar dangerous constructs

Assessment: No security concerns.

Test Coverage ✅

Coverage Analysis:

• ✅ Tests cover the specific bug case from issue [BUG] motion.div "scale" style property does not properly parse "0" #3465
• ✅ Tests verify both the build-transform utility and the style effect integration
• ✅ Tests include verification that default values ("1") still produce none
• ✅ Tests use nextFrame() helper correctly as per CLAUDE.md guidelines

Minor Suggestion: Consider adding a test case for "0%" to ensure percentage strings are also handled correctly. However, the current test coverage is sufficient.

Additional Observations

Bug Fix Verification:

• ✅ Directly addresses the issue described in [BUG] motion.div "scale" style property does not properly parse "0" #3465
• ✅ Fixes the scroll-linked animation use case where scale animates from "100%" to "0%"
• ✅ No regression risk for existing behavior

Code Consistency:

• The fix is applied identically in both locations where the logic exists
• Both files now have consistent handling of string transform values

Recommendation

Approve and Merge ✅

This is a clean, well-tested bug fix that:

• Solves the reported issue without introducing side effects
• Maintains code quality and consistency
• Has appropriate test coverage
• Follows repository conventions

The implementation is correct and ready to merge.