Fix duplicate exit animation processing in AnimatePresence

[mattgperry]

Summary

• Fixes issue where AnimatePresence processes exit animations multiple times when receiving rapid sequential events (e.g., Radix UI dismissable layers firing multiple dismiss events)
• Adds a Set to track components currently processing exit and guards against duplicate onExit calls
• Adds tests using usePresence to verify the fix without relying on animation timing

Test plan

• Added test "Calling safeToRemove multiple times only triggers exit once" - verifies onExitComplete is called only once when safeToRemove is called multiple times
• Added test "Rapid rerenders during exit only triggers exit once" - verifies rapid re-renders don't cause duplicate exit processing
• All existing AnimatePresence tests pass
• Full test suite passes

🤖 Generated with Claude Code

[claude]

PR Review: Fix duplicate exit animation processing in AnimatePresence

Summary

This PR addresses an important edge case where AnimatePresence could process exit animations multiple times when receiving rapid sequential events. The fix adds guard logic using a Set to track components currently in the exit process.

---

✅ Strengths

1. Correct Problem Identification: The issue where rapid events (e.g., Radix UI dismissable layers) trigger multiple onExit calls is a real bug that could lead to unexpected behavior and multiple onExitComplete callbacks.

2. Clean Implementation: Using a Set to track exiting components is the right data structure for this use case - provides O(1) lookups and naturally handles uniqueness.

3. Comprehensive Test Coverage: The two new tests effectively cover both scenarios:

   • Direct multiple safeToRemove() calls
   • Rapid re-renders during exit
     Both tests correctly verify that onExitComplete is only called once.

4. Follows Repo Conventions: Code style matches existing patterns (interfaces for types, const/let usage, arrow callbacks).

5. Proper Cleanup: The fix correctly cleans up the exitingComponents Set when components re-enter (line 117), preventing memory leaks.

---

🔍 Code Quality Observations

Potential Race Condition (Minor)

Location: packages/framer-motion/src/components/AnimatePresence/index.tsx:187-191

The guard check happens before setting the exitComplete status:

const onExit = () => {
    if (exitingComponents.current.has(key)) {
        return  // Early return - exitComplete never gets set to true
    }
    exitingComponents.current.add(key)

    if (exitComplete.has(key)) {
        exitComplete.set(key, true)  // Only reached on first call
    }

Scenario: If onExit is called multiple times in rapid succession before the component is removed from renderedChildren, subsequent calls will early-return and never set exitComplete to true. However, this appears to be the intended behavior - we only want the first call to process the exit.

Verdict: This is actually correct behavior. The guard prevents duplicate processing, and only the first call should update state. Not an issue.

---

Missing Edge Case Test (Suggestion)

The tests cover the happy path well, but consider adding a test for this scenario:

• Component starts exiting (onExit called, added to exitingComponents Set)
• Component re-enters (becomes present again)
• Component exits again

Expected behavior: The Set should be cleared when the component re-enters (line 117), allowing proper exit processing on the second exit.

Suggestion: Add a test like:

test("Component can exit again after re-entering", async () => {
    // Child exits -> re-enters -> exits again
    // Verify onExitComplete is called both times (once per exit cycle)
})

This would verify the cleanup logic at line 117 works correctly.

---

🎯 Best Practices & Performance

✅ Memory Management: Proper cleanup in the effect (line 117) prevents memory leaks
✅ Performance: Set operations are O(1), minimal overhead
✅ Concurrent Mode Safety: Using useRef for the Set is correct - doesn't trigger re-renders
✅ Type Safety: Properly typed with Set<ComponentKey>

---

🔒 Security

No security concerns identified. This is internal state management with no user input or external data handling.

---

🧪 Test Coverage

Current Coverage: Good

• ✅ Multiple safeToRemove() calls
• ✅ Rapid re-renders during exit
• ✅ Assertions verify onExitComplete count and DOM removal

Suggested Addition:

• Re-entry scenario (exit → re-enter → exit again)

Test Pattern Observation: Tests use setTimeout with arbitrary delays (150ms). While this matches existing test patterns in the file, consider if there's a more deterministic way to test this without relying on timing. However, since you're using usePresence hooks rather than actual animations, this is reasonable.

---

📝 Minor Suggestions

1. Comment Clarity (line 90-92): The comment is good, but could be slightly more specific:

   /**
    * Track which components are currently processing exit to prevent duplicate 
    * onExit calls from rapid events (e.g., multiple dismiss events from UI libraries).
    */

2. Variable Naming: exitingComponents is clear and follows conventions. Good choice.

---

✨ Overall Assessment

Verdict: Approve with minor suggestions ✅

This is a solid fix that:

• Solves a real bug
• Uses the right approach (guard with Set)
• Includes proper cleanup
• Has good test coverage
• Follows project conventions

The implementation is production-ready. The suggestions above are minor enhancements that could be addressed now or in a future PR.

Recommendation: Merge after considering the re-entry edge case test. If the existing test suite already covers component re-entry scenarios elsewhere, this may not be necessary.

---

Nice work! This fix will prevent subtle bugs in applications using AnimatePresence with UI libraries that fire rapid events.

[claude]

Code Review

I've reviewed PR #3454 and have the following feedback:

✅ Strengths

1. Problem Identification & Solution

• Correctly identifies and fixes the duplicate exit animation processing issue
• The use of a Set to track exiting components is an appropriate solution for preventing duplicate processing
• The guard clause at the start of onExit (lines 188-191) is well-placed and prevents redundant work early

2. Test Coverage

• Excellent test coverage with three comprehensive test cases:
  • Multiple safeToRemove calls (simulates rapid events)
  • Rapid re-renders during exit
  • Re-entry after exit (validates cleanup works correctly)
• Tests use usePresence directly, avoiding timing dependencies - this is a robust testing approach
• All tests verify both onExitComplete call count AND DOM state

3. Code Quality

• Clean, minimal changes following the principle of least modification
• Good documentation with clear JSDoc comment explaining the purpose
• Follows existing code patterns in the file
• Properly cleans up the Set when components re-enter (line 117)

🔍 Observations & Considerations

1. Cleanup Logic ✅
The cleanup in the effect (line 117) is correct:

exitingComponents.current.delete(key)

This ensures that when a component re-enters the tree, it can exit again later. The third test validates this works correctly.

2. Concurrent React Safety
The use of useRef for exitingComponents is appropriate here since:

• It's used purely for de-duplication guard logic
• It doesn't need to trigger re-renders
• It's always accessed/modified synchronously within the same render cycle or effects

However, there's a subtle consideration: in React Concurrent Mode, multiple renders can be attempted and discarded. The current implementation should be safe because:

• The Set is only mutated in onExit (called from child effects) and in the layout effect
• The layout effect runs synchronously after commit
• The cleanup happens before the component can exit again

3. Edge Case - Component Re-mounting During Exit
Consider this scenario:

1. Component A starts exiting (added to Set)
2. Before onExit completes, component A re-enters
3. Component A exits again immediately

The current code handles this correctly via line 117, which removes the key from the Set when the component re-enters. ✅

4. Memory Leak Prevention
The Set is properly cleaned up when:

• Components re-enter (line 117) ✅
• When exit completes (implicitly, as components are removed) ✅

No memory leak concerns identified.

📊 Performance

• Using a Set for lookups is optimal (O(1) vs O(n) for an array)
• Minimal overhead - only adds one .has() and one .add() call per exit
• No performance concerns

🔒 Security

No security concerns identified. This is internal state management logic with no external inputs or potential for injection/XSS.

🧪 Test Quality

The tests are well-written but could benefit from one small improvement:

Minor Suggestion: Consider using async/await with a helper function instead of nested setTimeout callbacks in the third test for better readability:

// Instead of nested setTimeout callbacks, could use:
async function wait(ms: number) {
    return new Promise(resolve => setTimeout(resolve, ms))
}

// Then in test:
await wait(150)
expect(onExitCompleteCount).toBe(1)
// etc.

But this is a very minor style preference - the current approach is consistent with existing tests in the file.

📝 Minor Nit

Line 188-191: Consider whether this early return needs a comment explaining why we silently return vs throwing/warning. The current behavior seems correct (fail silently to prevent duplicate processing), but a brief comment could help future maintainers:

const onExit = () => {
    // Prevent duplicate exit processing (can happen with rapid events)
    if (exitingComponents.current.has(key)) {
        return
    }
    exitingComponents.current.add(key)

But this is very minor - the JSDoc on line 89-91 already explains the Set's purpose.

✅ Final Verdict

This is a well-crafted fix that:

• ✅ Solves the reported issue correctly
• ✅ Has comprehensive test coverage
• ✅ Follows project conventions and code style
• ✅ Handles edge cases properly
• ✅ No performance or security concerns
• ✅ No bugs identified

Recommendation: Approve and merge

The implementation is solid and the tests give confidence that this won't regress. The minimal, targeted change is exactly what you want for a bug fix.

---

Nice work! This is a clean fix for a tricky race condition issue. 🎯