Fix LazyMotion animation not firing when state changes before features load

[mattgperry]

Summary

• Fixed issue where animations would snap to final value instead of animating when state changes occur before LazyMotion features finish loading
• Added tracking for whether the component has already been through React's commit phase
• When VisualElement is created after mount, sets manuallyAnimateOnMount = true to force animation

Fixes #2759

Test plan

• New Cypress test lazy-motion-fast-state.ts verifies animation completes correctly
• Existing unit tests pass (700 tests)
• Test page lazy-motion-fast-state.tsx demonstrates the fix

🤖 Generated with Claude Code

[claude]

Code Review

Summary

This PR fixes a race condition bug where animations would snap to their final value instead of animating when state changes occur before LazyMotion features finish loading asynchronously. The fix is clean, minimal, and well-tested.

✅ Strengths

1. Minimal, Targeted Fix: The solution only adds 11 lines of core logic using a simple ref to track mount state. This is exactly the right level of complexity for the problem.

2. Excellent Test Coverage:

   • New Cypress E2E test (lazy-motion-fast-state.ts) validates the fix in a real browser environment
   • Test page (lazy-motion-fast-state.tsx) provides a reproducible demonstration
   • Tests verify both animation completion and final opacity value
   • The 50ms delay for features vs 5ms for state change effectively simulates the race condition

3. Clear Documentation: Comments explain the "why" (not just "what"), making the intent crystal clear for future maintainers.

4. Leverages Existing Architecture: The fix reuses the existing manuallyAnimateOnMount flag (originally designed for parent-child scenarios per VisualElement.ts:393) for this LazyMotion edge case. This is the right abstraction.

5. Timing Correctness: Uses useIsomorphicLayoutEffect to set hasMountedOnce synchronously during commit phase, ensuring accurate detection of post-mount feature loading.

🔍 Minor Observations

1. Potential Edge Case - Multiple Remounts:

   if (hasMountedOnce.current && visualElementRef.current) {
       visualElementRef.current.manuallyAnimateOnMount = true
   }

   This flag is set once when features load after mount, but never reset. In most scenarios this is fine since visualElementRef persists across renders. However, if the component unmounts and remounts while still in LazyMotion context, the flag remains true. This might cause an extra animation on the second mount, but given that unmount/remount cycles are rare and the behavior would still be correct (animating is better than snapping), this is acceptable.

2. Test Wait Time: The Cypress test uses cy.wait(300) which is conservative but may slow down the test suite. Consider using cy.get('#box').should('have.attr', 'data-animation-complete', 'true') with Cypress's built-in retry logic instead of a fixed wait. This would make tests both faster and more reliable.

3. Unit Test Gap: While the E2E test is excellent, consider adding a Jest unit test in packages/framer-motion/src/motion/__tests__/lazy.test.tsx to verify manuallyAnimateOnMount is set correctly when features load asynchronously. This would complement the existing "Animates after async loading" test and provide faster feedback during development.

🎯 Code Quality

• Style Compliance: ✅ Uses interface pattern, named exports, arrow callbacks
• Naming: ✅ hasMountedOnce is descriptive and follows React conventions
• Comments: ✅ Excellent inline documentation explaining the edge case
• Type Safety: ✅ Proper TypeScript usage with ref types

🔒 Security & Performance

• No Security Concerns: The changes are purely internal logic with no user input or external data
• Performance Impact: Negligible - adds one ref allocation and one boolean check per component using LazyMotion

📝 Suggestions (Optional Enhancements)

1. Improve Cypress test reliability:

   cy.visit("?test=lazy-motion-fast-state")
       .get("#box")
       .should("have.attr", "data-animation-complete", "true")
       .should(() => {
           expect(getComputedStyle([0]).opacity).to.equal("1")
       })

2. Consider adding a unit test in lazy.test.tsx:

   test("Sets manuallyAnimateOnMount when state changes before async features load", async () => {
       // Test that verifies the flag is set correctly
   })

✅ Verdict

Approve with minor suggestions. This is a well-crafted fix that solves a real user pain point (issue #2759) with minimal code changes. The test coverage is strong, and the implementation leverages existing patterns correctly. The optional suggestions above would enhance robustness but are not blockers.

Great work! 🚀

---

Review generated via Claude Code