Handle null or undefined elements

[mattgperry]

Previously, passing null elements to animateSequence (e.g., from querySelector returning null) would throw an error. This was a regression from Motion 10's permissive behavior.

Changes:

• Update resolveElements in motion-dom to return empty array for null/undefined input and filter null elements from arrays
• Update resolveSubjects in framer-motion to handle null/undefined subjects gracefully
• Remove throwing invariant checks for empty element arrays, instead return empty animations array gracefully
• Add tests for null element handling in sequences

Fixes #3390

[claude]

PR Review: Handle null or undefined elements

Summary

This PR successfully addresses issue #3390 by restoring Motion 10's permissive behavior for null/undefined elements in animation sequences. The changes are well-structured and appropriately handle edge cases.

Strengths

1. Comprehensive null handling: The PR correctly handles null/undefined at multiple levels - resolveElements (motion-dom), resolveSubjects (framer-motion), and animateSubject/animateElements all gracefully handle empty arrays instead of throwing

2. Test coverage: Excellent test coverage with three distinct scenarios - null element in sequence, null elements in array of targets, and sequence with only null elements

3. Type safety: Properly updated TypeScript types to include null and undefined in ElementOrSelector type definition

4. Consistent approach: Removes throwing behavior (invariants) and consistently returns empty arrays/animations, which is more graceful

Code Quality Observations

1. Type predicate in filter (packages/motion-dom/src/utils/resolve-elements.ts:47-49) correctly narrows the type to Element, ensuring type safety after filtering

2. Good use of loose equality (==) to catch both null and undefined consistently

3. Clear comments explain the graceful handling behavior, which is helpful for maintainability

Minor Considerations

1. NodeList handling: In resolveSubjects, NodeList items are not filtered for null. While querySelectorAll results never contain null, this is likely acceptable

2. Breaking change communication: While this restores Motion 10 behavior, developers who added defensive null checks for Motion 11 won't be negatively affected

3. Performance: The additional filtering operations have minimal performance impact

Security & Performance

No security concerns. The changes are purely about graceful degradation when elements don't exist. The filtering operations are efficient and appropriate.

Best Practices Alignment

• Uses interface for type definitions (per CLAUDE.md)
• No default exports
• Arrow functions used consistently
• Strict equality used (except appropriate loose equality for null check)
• No var declarations
• Tests written to verify the fix

Recommendations

1. Consider running the full test suite to ensure no regressions
2. Optional: Add a test case for the specific user scenario from issue [BUG] animateSequence does not support null/undefined #3390 (querySelector returning null)
3. Documentation: Consider updating migration guides to mention this restored behavior

Conclusion

This PR is ready to merge. The implementation is clean, type-safe, well-tested, and successfully restores the expected permissive behavior. The graceful handling approach (returning empty arrays instead of throwing) is superior to the previous invariant checks. Great work on the comprehensive test coverage!

[claude]

Pull Request Review

Summary

This PR successfully addresses issue #3390 by restoring Motion 10's permissive behavior for null/undefined elements in animation sequences. The changes are well-implemented and include comprehensive test coverage.

Positive Aspects

✅ Clear Problem Definition: The PR directly addresses a regression from Motion 10 where null elements (e.g., from querySelector returning null) would throw errors instead of being gracefully skipped.

✅ Comprehensive Coverage: Changes span the entire call stack from low-level resolveElements to high-level animateSubject, ensuring consistent null handling throughout.

✅ Good Test Coverage: Three new test cases cover the main scenarios:

• Null element in sequence
• Array with null elements
• Sequence with only null elements

✅ Type Safety: Properly extends type signatures to include null | undefined in ElementOrSelector and function parameters.

✅ Consistent Pattern: Uses the same null-check pattern (== null) consistently across all modified files.

Issues & Concerns

🔴 Critical: Inconsistent Behavior in resolveSubjects

Location: packages/framer-motion/src/animation/animate/resolve-subjects.ts:30-31

The NodeList branch doesn't filter null elements, creating inconsistent behavior:

} else if (subject instanceof NodeList) {
    return Array.from(subject)  // ❌ No null filtering
} else if (Array.isArray(subject)) {
    return subject.filter((s) => s \!= null)  // ✅ Filters nulls
}

Issue: If someone passes a NodeList containing null nodes (possible in some edge cases), it won't be filtered like arrays are.

Recommendation: Apply consistent filtering:

} else if (subject instanceof NodeList) {
    return Array.from(subject).filter((s) => s \!= null)
}

🟡 Minor: Redundant Null Check in animateSubject

Location: packages/framer-motion/src/animation/animate/subject.ts:110-113

// Gracefully handle null/undefined subjects
if (subject == null) {
    return animations
}

const subjects = resolveSubjects(subject, ...) // Already handles null

Issue: This check is redundant since resolveSubjects already handles null at line 24. However, the early return here avoids the invariant check on line 123, which is actually the desired behavior.

Verdict: Keep as-is for defensive programming, but this reveals a deeper issue (see next point).

🟡 Code Smell: Invariant Check After Graceful Handling

Location: packages/framer-motion/src/animation/animate/subject.ts:122-127 and animate-elements.ts:46-50

Both files check for null, return empty arrays gracefully, but then immediately throw an invariant if no elements exist:

if (elementOrSelector == null) {
    return []  // Graceful
}
const elements = resolveElements(elementOrSelector, scope)
invariant(Boolean(numElements), "No valid elements provided.") // ❌ Throws anyway

Issue: This creates confusing behavior:

• animate(null, {...}) → returns [] (no error)
• animate([], {...}) → throws "No valid elements provided"
• animate([null], {...}) → throws "No valid elements provided" (after filtering)

Recommendation: Either:

1. Remove invariants to be fully permissive (consistent with Motion 10), OR
2. Remove the null checks and let invariants handle all cases consistently

The current implementation is inconsistent: it treats null specially but still throws for empty results from other sources.

🟢 Observation: Type Casting in Tests

Location: packages/framer-motion/src/animation/sequence/__tests__/index.test.ts:776

[null as unknown as Element, { opacity: 0.5 }, { duration: 1 }],

Verdict: This is acceptable for testing purposes. The double cast correctly bypasses TypeScript's type checking to simulate runtime scenarios where null might be encountered.

Performance Considerations

✅ No performance concerns. The additional null checks are:

• Minimal overhead (simple equality checks)
• Short-circuit early, avoiding unnecessary processing
• Applied at appropriate boundaries (input validation layer)

Security Considerations

✅ No security concerns identified. The changes:

• Don't introduce new attack vectors
• Improve robustness by handling invalid input gracefully
• Don't bypass existing validation (invariants still check for empty arrays)

Recommendations

Must Fix

1. Add null filtering to NodeList branch in resolveSubjects for consistency

Should Consider

2. Clarify invariant strategy: Decide whether empty element arrays should throw or return empty animations. Current behavior is inconsistent:
   • Document this behavior in code comments if intentional
   • Or remove invariants to be fully permissive like Motion 10

Nice to Have

3. Add test for NodeList with nulls (if you fix point Initial API (WIP) #1)
4. Add JSDoc comments explaining the null-handling behavior for these functions

Overall Assessment

Verdict: ✅ Approve with minor fixes

This is a solid PR that successfully restores backward compatibility with Motion 10. The implementation is clean, well-tested, and solves the reported issue. The critical issue with NodeList inconsistency should be addressed, and the invariant behavior should be clarified, but these don't block the core functionality.

The PR improves user experience by preventing crashes when DOM elements are conditionally present, which is a common real-world scenario.

---

Review generated by Claude Code