Add manual frame rendering API for environments without requestAnimationFrame

[mattgperry]

This adds support for manually advancing animations in environments where
requestAnimationFrame is unavailable, such as WebXR immersive sessions,
Remotion video rendering, or server-side rendering of animations.

New APIs:

• renderFrame({ timestamp, frame, fps, delta }) - Manually render a frame
• setManualTiming(enabled) - Enable/disable manual timing mode globally
• isManualTiming() - Check if manual timing is enabled
• useManualFrame({ frame, fps }) - React hook for Remotion integration

Fixes #2496

https://claude.ai/code/session_01AFhy8F8ubRRHsau2GRdYAZ

[claude]

Code Review - PR #3521: Manual Frame Rendering API

I've completed a thorough review of this pull request. Overall, this is a well-designed and well-tested addition that addresses a legitimate need for manual frame control in non-browser environments. Here's my detailed feedback:

---

✅ Strengths

1. Well-Designed API

The API is intuitive and flexible with multiple input modes:

• Direct timestamp control
• Frame number + FPS for video rendering frameworks
• Optional delta override

The three-tier API (renderFrame, setManualTiming, isManualTiming) provides good separation of concerns.

2. Excellent Test Coverage

The test file (render-frame.test.ts) is comprehensive with 188 lines covering:

• All frame processing steps in correct order
• Frame-to-timestamp conversion
• Delta calculations
• Manual timing state management
• Incremental rendering scenarios

This follows the CLAUDE.md guideline of writing tests first.

3. Good Integration with Existing Systems

• Properly integrates with sync-time.ts using time.set()
• Respects MotionGlobalConfig.useManualTiming
• Restores previous state after frame processing
• Minimal invasive changes to batcher.ts

4. Documentation

Excellent JSDoc comments with multiple usage examples for Remotion, WebXR, and custom loops.

---

🐛 Issues & Concerns

1. Potential Race Condition in useManualFrame (packages/framer-motion/src/utils/use-manual-frame.ts:69-76)

The cleanup effect that disables manual timing could cause issues in multi-component scenarios:

useEffect(() => {
    if (isStatic) return
    
    setManualTiming(true)
    
    return () => {
        setManualTiming(false)  // ⚠️ Problem: What if multiple components use this hook?
    }
}, [isStatic])

Problem: If Component A and Component B both use useManualFrame, when Component A unmounts, it will disable manual timing globally, breaking Component B's animations.

Recommendation: Implement reference counting:

// In render-frame.ts
let manualTimingRefCount = 0

export function enableManualTiming(): () => void {
    manualTimingRefCount++
    MotionGlobalConfig.useManualTiming = true
    
    return () => {
        manualTimingRefCount--
        if (manualTimingRefCount === 0) {
            MotionGlobalConfig.useManualTiming = false
        }
    }
}

// In use-manual-frame.ts
useEffect(() => {
    if (isStatic) return
    return enableManualTiming()
}, [isStatic])

2. Missing Dependency in useManualFrame Effect (packages/framer-motion/src/utils/use-manual-frame.ts:83-94)

The render effect is missing callback as a dependency:

useEffect(() => {
    // ... uses fps in calculations
}, [frame, fps, isStatic])  // ⚠️ Missing fps as dependency in delta calculation

While fps is included, the effect correctly depends on all values it uses. However, consider if the logic should handle fps changes mid-animation differently.

3. No Validation for Invalid Input (packages/motion-dom/src/frameloop/render-frame.ts:66-82)

The function doesn't validate its inputs:

renderFrame({ timestamp: -1000 })  // Negative timestamp - what happens?
renderFrame({ frame: -10, fps: 30 })  // Negative frame
renderFrame({ fps: 0 })  // Division by zero potential
renderFrame({})  // No arguments - uses frameData.timestamp + delta

Recommendation: Add validation:

if (fps !== undefined && fps <= 0) {
    console.warn('[Motion] Invalid fps value:', fps, 'defaulting to 30')
    fps = 30
}

if (frame !== undefined && frame < 0) {
    console.warn('[Motion] Negative frame number:', frame)
}

4. Potential Timestamp Discontinuity (packages/motion-dom/src/frameloop/render-frame.ts:80-82)

When no timing info is provided, the function uses frameData.timestamp + frameDelta. This could cause unexpected jumps if mixing manual and automatic timing.

// If frameData.timestamp is stale from a previous auto render:
renderFrame({})  // Uses old timestamp + delta = potentially wrong time

Recommendation: Either require at least one timing parameter or document this fallback behavior clearly.

5. Missing callback Dependency Warning (packages/framer-motion/src/utils/use-manual-frame.ts:10)

React's exhaustive-deps rule would flag this. Consider if fps changing should reset the prevFrame reference:

useEffect(() => {
    // If fps changes mid-animation, delta calculations might be wrong
    // Should we reset prevFrame.current?
}, [frame, fps, isStatic])

---

🔍 Code Quality Observations

1. Code Duplication in processFrame and processBatch (packages/motion-dom/src/frameloop/batcher.ts:89-107)

Lines 97-104 in processFrame are identical to lines 58-65 in processBatch. Consider extracting to a shared function:

const runRenderPipeline = () => {
    setup.process(state)
    read.process(state)
    resolveKeyframes.process(state)
    preUpdate.process(state)
    update.process(state)
    preRender.process(state)
    render.process(state)
    postRender.process(state)
}

However, the comment mentions "unrolled for better per-frame performance", so this might be intentional. If so, add a comment explaining why duplication is preferred.

2. Inconsistent fps Default

The default FPS is 30 in renderFrame but the fallback delta is 1000/60 (60fps):

// render-frame.ts:66
fps = 30  // default

// render-frame.ts:73
frameDelta = delta !== undefined ? delta : 1000 / 60  // Uses 60fps

Recommendation: Use consistent defaults or document why they differ.

3. Type Safety Enhancement

Consider making the options type safer with discriminated unions:

type RenderFrameOptions =
    | { timestamp: number; delta?: number }
    | { frame: number; fps?: number; delta?: number }
    | { delta: number }  // Only delta

This would prevent confusing calls like renderFrame({ timestamp: 100, frame: 10 }) where both are provided.

---

🎯 Performance Considerations

1. time.set() on Every Frame (packages/motion-dom/src/frameloop/render-frame.ts:89)

Calling time.set() schedules a microtask on every frame. For high-frequency rendering (60fps video), this could add up. However, this is consistent with existing patterns and likely necessary for synchronization.

2. Delta Calculation on Every Render (packages/framer-motion/src/utils/use-manual-frame.ts:85-88)

The delta calculation includes a conditional and Math.abs():

const delta =
    hasInitialized.current && prevFrame.current >= 0
        ? ((frame - prevFrame.current) / fps) * 1000
        : 1000 / fps

This is fine, but for Remotion use cases where frame always increments by 1, you could optimize to just use 1000 / fps.

---

🔒 Security Considerations

No security concerns identified. The API doesn't accept user input that could cause injection attacks or other vulnerabilities.

---

📋 Suggestions

1. Add Integration Test

While unit tests are excellent, consider adding an integration test that:

• Creates a motion component with animations
• Manually advances frames
• Verifies the animation values at specific frames

2. Document Edge Cases

Add notes about:

• What happens if you mix renderFrame() and RAF-driven frames?
• Behavior when multiple components use useManualFrame with different FPS values
• Whether animations pause when setManualTiming(false) is called mid-animation

3. Export Types

Consider exporting RenderFrameOptions and UseManualFrameOptions for TypeScript users:

export type { RenderFrameOptions, UseManualFrameOptions }

4. Add Warning for Common Mistake

Many users might forget to disable manual timing when switching back to RAF:

if (MotionGlobalConfig.useManualTiming && typeof requestAnimationFrame !== 'undefined') {
    console.warn('[Motion] Manual timing is enabled but requestAnimationFrame is available')
}

---

✨ Minor Nits

1. Line 87 in use-manual-frame.ts: The Math.abs(delta) suggests frames could go backward. Is this intentional? If so, document it. If not, consider clamping to positive values without abs.

2. Export order: The new exports follow existing patterns (alphabetical in index.ts).

3. Naming: useManualFrame is clear and follows React naming conventions.

---

📊 Summary

Approval Status: ✅ Approve with Minor Fixes

This is a solid addition to the Motion API. The core implementation is sound, tests are comprehensive, and the API design is clean. The main concern is the race condition in multi-component scenarios with useManualFrame.

Must Fix (Before Merge):

1. ✋ Reference counting for setManualTiming to handle multiple components

Should Fix:

2. Input validation in renderFrame
3. Consistent FPS defaults (30 vs 60)
4. Document the fallback timestamp behavior

Nice to Have:

5. Integration test
6. Export types
7. Edge case documentation

---

Great work on addressing issue #2496! This will unlock Motion for Remotion, WebXR, and other non-browser environments. 🎉

[claude]

Pull Request Review

Overview

This PR adds a manual frame rendering API for environments without requestAnimationFrame, solving a real pain point for WebXR, Remotion, and server-side animation rendering use cases. The implementation is well-designed and the test coverage is comprehensive.

---

✅ Strengths

1. Strong API Design

• Clear separation of concerns: low-level renderFrame() in motion-dom, high-level useManualFrame() hook in framer-motion
• Flexible frame timing: supports both explicit timestamps and frame/fps conversion
• Follows existing patterns (similar to how processBatch works)

2. Excellent Test Coverage

The test suite in render-frame.test.ts is thorough:

• Tests all execution paths (timestamp, frame/fps, delta handling)
• Verifies correct callback ordering
• Tests manual timing state management
• Includes incremental frame rendering scenarios

3. Documentation

• Comprehensive JSDoc comments with multiple usage examples
• Clear explanation of use cases (WebXR, Remotion, SSR)
• Good inline documentation in the implementation

---

🔍 Issues Found

1. Potential Race Condition in useManualFrame (Minor)

Location: packages/framer-motion/src/utils/use-manual-frame.ts:82-93

The delta calculation may produce negative values when frames go backward (scrubbing in Remotion):

const delta =
    hasInitialized.current && prevFrame.current >= 0
        ? ((frame - prevFrame.current) / fps) * 1000
        : 1000 / fps

renderFrame({ frame, fps, delta: Math.abs(delta) })

While Math.abs() handles negative delta, the frame rendering might not handle backward time travel correctly. Consider:

• Should backward scrubbing be explicitly handled or documented?
• Should the hook prevent rendering if frame < prevFrame.current?

2. Missing Type Safety for processFrame (Minor)

Location: packages/motion-dom/src/frameloop/batcher.ts:125

The return type doesn't include processFrame:

return { schedule, cancel, state, steps, processFrame }

The interface/type definition should be updated to include processFrame for proper TypeScript support.

3. Potential Memory Leak in useManualFrame (Critical)

Location: packages/framer-motion/src/utils/use-manual-frame.ts:68-76

The effect that enables manual timing has no cleanup on strict mode double-mounting:

useEffect(() => {
    if (isStatic) return

    setManualTiming(true)

    return () => {
        setManualTiming(false)
    }
}, [isStatic])

In React Strict Mode (or with multiple components using this hook), the cleanup will disable manual timing even if other components still need it. This could cause:

1. Animations to start using RAF unexpectedly
2. Timing conflicts between manual and automatic mode

Suggested fix: Use a reference counter:

let manualTimingRefCount = 0

export function enableManualTiming() {
    if (manualTimingRefCount === 0) {
        setManualTiming(true)
    }
    manualTimingRefCount++
    
    return () => {
        manualTimingRefCount--
        if (manualTimingRefCount === 0) {
            setManualTiming(false)
        }
    }
}

4. Code Duplication Between processBatch and processFrame (Moderate)

Location: packages/motion-dom/src/frameloop/batcher.ts:42-107

The frame processing logic is duplicated:

// Lines 58-65 in processBatch
setup.process(state)
read.process(state)
resolveKeyframes.process(state)
preUpdate.process(state)
update.process(state)
preRender.process(state)
render.process(state)
postRender.process(state)

// Lines 97-104 in processFrame (identical)
setup.process(state)
read.process(state)
// ... same code

Suggestion: Extract to a shared function:

const executeFrameSteps = (state: FrameData) => {
    setup.process(state)
    read.process(state)
    resolveKeyframes.process(state)
    preUpdate.process(state)
    update.process(state)
    preRender.process(state)
    render.process(state)
    postRender.process(state)
}

However, the comment "Unrolled render loop for better per-frame performance" suggests this duplication might be intentional for performance. If so, add a comment explaining why extraction would hurt performance.

5. Missing Edge Case: No Timestamp or Frame Provided (Low)

Location: packages/motion-dom/src/frameloop/render-frame.ts:73-82

When neither timestamp nor frame is provided, the fallback uses frameData.timestamp:

frameTimestamp = frameData.timestamp + frameDelta

On the very first call, frameData.timestamp is 0, which is correct. But the behavior should be explicitly tested and documented.

6. time.set() Side Effect Not Documented (Low)

Location: packages/motion-dom/src/frameloop/render-frame.ts:89

time.set(frameTimestamp)

This modifies global state (sync-time.ts). While necessary, it's a side effect that could affect other code using time.now(). Document this behavior in the JSDoc.

---

🛡️ Security Concerns

None identified. The API doesn't accept user input that could be exploited. Delta and timestamp values are validated/calculated safely.

---

⚡ Performance Considerations

Positive:

• Manual timing avoids unnecessary RAF scheduling
• Delta calculation is simple arithmetic
• No new allocations in hot path

Concerns:

• useManualFrame calls renderFrame() on every frame change, which processes all scheduled callbacks even if nothing changed. This is expected behavior but could be expensive in complex scenes.
• Consider documenting performance best practices (e.g., minimize keepAlive callbacks in manual mode)

---

📋 Test Coverage Assessment

Well Covered ✅

• Frame rendering with different timing inputs
• Callback execution order
• Manual timing state toggling
• Delta calculation from fps

Missing Tests ⚠️

1. React hook tests: No tests for useManualFrame itself
2. Integration tests: No E2E test showing actual animations working in manual mode
3. Edge cases:
   • Multiple concurrent useManualFrame hooks
   • Switching between manual and RAF modes mid-animation
   • Frame scrubbing backward
   • Very high/low fps values

---

📝 Recommendations

High Priority

1. Fix the ref counting issue in useManualFrame to prevent timing mode conflicts
2. Add React tests for the useManualFrame hook
3. Update TypeScript types to include processFrame in the return type

Medium Priority

4. Document backward frame handling or add validation
5. Add integration test showing a complete animation cycle in manual mode
6. Document the time.set() side effect

Low Priority

7. Add comment explaining code duplication if intentional for performance
8. Consider exporting frame/fps to timestamp conversion as a utility function for advanced users

---

Summary

This is a well-implemented feature that solves a real problem. The core logic is sound, test coverage is good, and the API design is intuitive. The main concern is the potential timing mode conflict with multiple hook instances (issue #3), which should be addressed before merging.

Recommendation: Approve with requested changes (fix issue #3, add hook tests, update types)

Great work overall! This will unblock WebXR and Remotion users. 🚀

[claude]

Pull Request Review: Manual Frame Rendering API

I've completed a thorough review of PR #3521. This is a well-designed feature that adds manual frame control for environments without requestAnimationFrame. Here's my feedback:

---

✅ Strengths

Architecture & Design

• Excellent separation of concerns: The implementation correctly places frame rendering logic in motion-dom (framework-agnostic) and React-specific hooks in framer-motion.
• Clean API design: The renderFrame function supports both timestamp-based and frame-based approaches, making it flexible for different use cases (WebXR, Remotion, etc.).
• Non-invasive implementation: The changes integrate seamlessly with existing code by extending the MotionGlobalConfig and createRenderBatcher.

Code Quality

• Comprehensive test coverage: Excellent test suite with 325+ lines of tests covering:
  • Frame-by-frame stepping
  • Remotion integration patterns
  • Backward scrubbing
  • Different FPS values
  • Manual timing mode lifecycle
• Well-documented: Clear JSDoc comments with practical examples for each API function.
• Follows conventions: Uses interface for types, named exports, and arrow callbacks as per CLAUDE.md guidelines.

---

🔍 Issues & Recommendations

1. Potential Race Condition in useManualFrame (packages/framer-motion/src/utils/use-manual-frame.ts:748-752)

Issue: The delta calculation for backward scrubbing may be incorrect.

const delta =
    hasInitialized.current && prevFrame.current >= 0
        ? ((frame - prevFrame.current) / fps) * 1000
        : 1000 / fps

Problem: When scrubbing backward (e.g., frame 60 → 30), delta becomes negative (-1000ms at 30fps), but then Math.abs(delta) is applied. This means backward scrubbing will report a positive delta, which could confuse animations that care about time direction.

Recommendation: Consider whether animations need to know about backward time travel. If so, preserve the sign. If not, document this behavior explicitly.

2. Missing time.set() call in useManualFrame (packages/framer-motion/src/utils/use-manual-frame.ts)

Issue: The hook calls renderFrame({ frame, fps, delta }), which internally calls time.set(timestamp). However, this only happens during renderFrame execution. According to CLAUDE.md:

Use time.now() from motion-dom/src/frameloop/sync-time.ts instead of performance.now() for frame-synced timestamps.

Analysis: Looking at render-frame.ts:1111, the time.set() call IS present, so this is actually correct. However, the reliance on this internal implementation detail could be fragile.

Recommendation: Document this dependency or consider making it more explicit.

3. Type Safety: Missing Return Type on processFrame (packages/motion-dom/src/frameloop/batcher.ts:89)

Issue: The new processFrame function lacks an explicit return type annotation.

const processFrame = (timestamp: number, delta?: number) => {
    // ...
}

Recommendation: Add explicit void return type for consistency with other functions in the file.

const processFrame = (timestamp: number, delta?: number): void => {

4. Potential Inconsistency in Manual Timing State (packages/motion-dom/src/frameloop/render-frame.ts:1106-1117)

Issue: renderFrame temporarily enables manual timing, processes the frame, then restores it:

const previousManualTiming = MotionGlobalConfig.useManualTiming
MotionGlobalConfig.useManualTiming = true
// ... process frame ...
MotionGlobalConfig.useManualTiming = previousManualTiming

Problem: If an error occurs during frame processing, the manual timing state won't be restored.

Recommendation: Use try-finally:

const previousManualTiming = MotionGlobalConfig.useManualTiming
try {
    MotionGlobalConfig.useManualTiming = true
    time.set(frameTimestamp)
    processFrame(frameTimestamp, frameDelta)
} finally {
    MotionGlobalConfig.useManualTiming = previousManualTiming
}

5. Edge Case: renderFrame with No Arguments (packages/motion-dom/src/frameloop/render-frame.ts:1100-1104)

Issue: When called with no arguments or with only delta, the function uses frameData.timestamp + frameDelta:

frameTimestamp = frameData.timestamp + frameDelta

Problem: On the very first call to renderFrame, frameData.timestamp is 0, so this would start at 0 + delta. This might be unexpected behavior.

Recommendation: Either document this behavior or initialize to 0 explicitly:

else {
    // Start from 0 if timestamp is uninitialized
    frameDelta = delta \!== undefined ? delta : 1000 / 60
    frameTimestamp = frameData.timestamp > 0 
        ? frameData.timestamp + frameDelta 
        : frameDelta
}

6. Missing Export in Main Package (packages/motion/src/index.ts)

Observation: The PR exports useManualFrame from framer-motion but I don't see evidence of it being re-exported from the main motion package.

Recommendation: Verify that users can import these APIs from both:

• import { useManualFrame } from 'framer-motion'
• import { useManualFrame } from 'motion/react'

7. Test Coverage: Missing Error Cases

Issue: The tests don't cover error scenarios:

• What happens if fps is 0 or negative?
• What happens if frame is negative?
• What happens if animations throw errors during manual frame processing?

Recommendation: Add tests for edge cases and error handling.

---

🎯 Performance Considerations

Positive

• ✅ Reuses the existing unrolled render loop in processFrame (lines 97-104 in batcher.ts)
• ✅ Minimal overhead when manual timing is disabled
• ✅ Efficient frame-to-timestamp conversion

Potential Concern

• The demo component (dev/react/src/tests/manual-frame-control.tsx) calls renderFrame in a useEffect that depends on currentFrame. This is fine for demos, but the docs should clarify that users shouldn't put renderFrame in effects that can trigger during normal React rendering.

---

🔒 Security Considerations

No security concerns identified. The API doesn't:

• Accept external URLs or data
• Perform DOM manipulation
• Execute user-provided code
• Handle sensitive data

---

📝 Documentation & Examples

Strengths

• ✅ Excellent JSDoc comments with practical examples
• ✅ Comprehensive demo component showing real-world usage
• ✅ Clear Remotion integration example

Suggestions

1. Add a README or migration guide explaining when to use this API vs regular animations
2. Document the behavior when scrubbing backward through animations
3. Add TypeScript example showing the full type signature
4. Consider adding a troubleshooting section for common issues

---

🧪 Test Quality

The test coverage is excellent, but consider adding:

1. Integration test: A full Remotion-style render loop that generates frames 0-60 and validates the final positions
2. Stress test: Rapid frame changes (simulating fast scrubbing)
3. Memory test: Ensure no memory leaks when switching between manual and automatic modes repeatedly
4. Edge cases: As mentioned in issue Adding useViewportScroll #7 above

---

📊 Summary

Category	Rating	Notes
Code Quality	⭐⭐⭐⭐⭐	Clean, well-structured, follows conventions
Test Coverage	⭐⭐⭐⭐☆	Comprehensive, but missing some edge cases
Documentation	⭐⭐⭐⭐⭐	Excellent examples and JSDoc
Performance	⭐⭐⭐⭐⭐	Minimal overhead, efficient implementation
Security	⭐⭐⭐⭐⭐	No concerns
Architecture	⭐⭐⭐⭐⭐	Excellent separation of concerns

Overall: This is a high-quality PR that solves a real problem (Remotion/WebXR integration) with a clean, well-tested API. The issues identified are minor and mostly about edge cases and error handling.

---

✅ Recommended Actions Before Merge

1. MUST: Add try-finally to renderFrame (issue adding circleci config #4)
2. SHOULD: Add explicit return type to processFrame (issue Add motionValue prop #3)
3. SHOULD: Add edge case tests (issue Adding useViewportScroll #7)
4. CONSIDER: Review backward scrubbing delta behavior (issue Initial API (WIP) #1)
5. CONSIDER: Document the renderFrame({}) behavior (issue Initial event setup #5)
6. VERIFY: Exports are available from motion/react (issue Feature/frame #6)

Great work on this feature! 🎉

[claude]

Pull Request Review: Manual Frame Rendering API

Summary

This PR adds comprehensive support for manual frame rendering in environments without requestAnimationFrame (WebXR, Remotion, SSR). The implementation is well-architected and includes a new motion-remotion package for seamless integration with Remotion video rendering.

Strengths

Architecture & Design

• Clean separation of concerns: The driver pattern cleanly separates timing control from animation logic
• Minimal API surface: The new APIs (renderFrame, MotionGlobalConfig.driver, MotionRemotion component) are intuitive and well-documented
• Backward compatible: Changes don't affect existing code paths when no custom driver is set
• Proper use of React hooks: useInsertionEffect for driver setup ensures animations mount with correct timing

Code Quality

• Comprehensive test coverage: 1,061 lines of tests covering edge cases, different FPS rates, springs, AnimatePresence, staggered animations, and Remotion sequences
• Well-documented: Inline comments explain the "why" behind implementation choices
• Follows repository conventions: Uses named exports, interfaces over types, arrow callbacks

Performance Considerations

• Efficient frame processing: Unrolled render loop in processFrame (batcher.ts:88-105) avoids overhead
• Smart rAF scheduling: Properly skips requestAnimationFrame when custom driver is active (batcher.ts:66, 78)
• Synchronized timing: Uses time.set() to ensure consistent timestamps within frame

Issues & Suggestions

1. Critical: Type Safety Issue in global-config.ts

Location: packages/motion-utils/src/global-config.ts:5-10

The GlobalDriver interface is duplicated from the full Driver type in motion-dom. This creates a maintenance burden and potential type drift.

Recommendation:

// Option A: Use a minimal interface that's validated at assignment time
// Option B: Move Driver type to motion-utils (better for shared types)

The current approach works but could cause issues if the Driver interface evolves.

2. Potential Bug: Missing Cleanup in renderFrame

Location: packages/motion-dom/src/frameloop/render-frame.ts:111-128

In the test at line 111, there's a pattern of scheduling keepAlive callbacks and cleaning them up. However, renderFrame doesn't provide guidance on cleanup. If users schedule keepAlive callbacks with custom drivers, they may leak memory.

Recommendation: Add documentation or helper for cleanup patterns in manual timing mode.

3. Edge Case: Negative Delta Values

Location: packages/motion-remotion/src/MotionRemotion.tsx:41-46

The code uses Math.abs(delta) to handle negative deltas (backward scrubbing), but the comment in the comprehensive test file (line 1014) says "backward scrubbing not supported". This is inconsistent.

Recommendation: Either:

• Remove Math.abs() and document that backward scrubbing is unsupported
• Properly support backward scrubbing with state resets
• Add a warning when negative delta is detected

4. Performance: Redundant Re-renders in MotionRemotion

Location: packages/motion-remotion/src/MotionRemotion.tsx:39-50

The condition frame !== prevFrame.current || !hasInitialized.current causes renderFrame to run on every frame change, but React's useEffect already guarantees this. The hasInitialized.current check adds complexity without clear benefit.

Suggestion: Simplify to:

useEffect(() => {
    const delta = prevFrame.current >= 0 
        ? ((frame - prevFrame.current) / fps) * 1000 
        : 1000 / fps
    renderFrame({ frame, fps, delta: Math.abs(delta) })
    prevFrame.current = frame
}, [frame, fps])

5. Minor: Inconsistent Default FPS

Location: Multiple files

• renderFrame.ts:55 defaults to 30 FPS
• batcher.ts:47 uses 60 FPS (1000/60) as default delta

While these serve different purposes, it could confuse users. Consider documenting why these defaults differ.

6. Documentation: Missing JSDoc for processFrame

Location: packages/motion-dom/src/frameloop/batcher.ts:83-106

The processFrame function has good inline comments but lacks JSDoc. Since it's exported and part of the public API, it should have proper TypeScript documentation.

7. Security: No Validation of FPS Values

Location: packages/motion-dom/src/frameloop/render-frame.ts:54-71

No validation for edge cases:

• Negative FPS values
• FPS = 0 (division by zero)
• Extremely high FPS values

Recommendation: Add validation:

if (fps <= 0 || !Number.isFinite(fps)) {
    throw new Error(`Invalid fps value: ${fps}. Must be a positive finite number.`)
}

Test Coverage Assessment

Excellent Coverage

• ✅ Different FPS rates (24, 30, 60)
• ✅ Spring animations with overshoot
• ✅ AnimatePresence exit animations
• ✅ Staggered children
• ✅ Sequential frame rendering
• ✅ Remotion Sequence components
• ✅ Frame callback processing order
• ✅ WAAPI disabling with custom drivers

Missing Coverage

• ❌ Error handling for invalid inputs (negative FPS, NaN values)
• ❌ Concurrent rendering scenarios (multiple MotionRemotion components)
• ❌ Driver switching at runtime
• ❌ Memory leak testing for keepAlive callbacks
• ❌ Layout animations in manual timing mode

Performance Considerations

Positive:

• Minimal overhead when custom driver is not active
• Efficient frame processing with unrolled loops
• Smart rAF scheduling prevents unnecessary work

Concern:

• Large test file (1061 lines) may slow down test suite. Consider splitting by feature area.

Security Assessment

No significant security concerns. The manual timing API:

• Doesn't introduce XSS vectors
• Doesn't handle sensitive data
• Properly isolates driver state
• No eval or dynamic code execution

Breaking Changes

None. All changes are additive:

• New optional MotionGlobalConfig.driver property
• New exported functions (renderFrame, remotionDriver)
• New package (motion-remotion)

Existing code continues to work unchanged.

Recommendations

Must Fix

1. Add FPS validation in renderFrame to prevent division by zero

Should Fix

2. Clarify backward scrubbing support (remove Math.abs or add proper support)
3. Document cleanup patterns for keepAlive callbacks in manual mode
4. Add JSDoc to processFrame function

Nice to Have

5. Simplify MotionRemotion useEffect logic
6. Document why default FPS differs between contexts
7. Add tests for error cases and concurrent scenarios
8. Consider splitting large test file for faster test runs

Final Assessment

Overall: Excellent work! ✅

This is a well-designed feature that solves a real problem (Remotion integration, WebXR support) with a clean API. The test coverage is comprehensive, and the implementation follows the repository's conventions.

The issues noted are minor and don't block merging, but addressing them would improve robustness and maintainability. The most critical item is adding FPS validation to prevent runtime errors.

Recommendation: Approve with minor suggested improvements.

---

Generated with ❤️ using Claude Code - thorough review following CLAUDE.md conventions

[claude]

Pull Request Review: Manual Frame Rendering API

Summary

This PR adds comprehensive support for manually advancing animations in environments without requestAnimationFrame (WebXR, Remotion, server-side rendering). The implementation is well-architected and thoroughly tested, with a clean separation of concerns.

✅ Strengths

1. Excellent Architecture

• Clean separation: renderFrame() in motion-dom for low-level control, MotionRemotion wrapper for React integration
• The driver pattern is elegant - global driver takes precedence over options driver over default
• Well-documented public APIs with clear JSDoc examples

2. Comprehensive Test Coverage

• 1061 lines of tests covering edge cases (packages/framer-motion/src/utils/tests/use-manual-frame-remotion.test.tsx)
• Tests multiple FPS values (24, 30, 60), spring animations, AnimatePresence, staggered children, sequences
• Excellent test scenarios for Remotion use cases

3. Performance Considerations

• Frame caching system (packages/motion-dom/src/render/frame-cache.ts) using WeakMaps for automatic GC
• Unrolled render loops in batcher for better per-frame performance
• Cache is optional and only active when currentFrame is set

4. Proper WAAPI Handling

• Disables WAAPI when custom driver is set (packages/motion-dom/src/animation/waapi/supports/waapi.ts:1517-1522)
• Prevents timing desync between JS animations and native WAAPI

🐛 Potential Issues

1. Frame Cache Memory Leak Risk (packages/motion-dom/src/render/frame-cache.ts)

Severity: Medium

The frame cache uses Map<number, WeakMap<Element, styles>>, but the outer Map is keyed by frame numbers which are primitives and never garbage collected. In long-running video exports (e.g., 10-minute video at 60fps = 36,000 frame entries), this could accumulate significant memory.

Recommendation:

// Add a maximum cache size or LRU eviction
const MAX_CACHED_FRAMES = 100
// Or document that users should call clearFrameCache() periodically

Location: packages/motion-dom/src/render/frame-cache.ts:9-14

2. Missing setCurrentFrame() Integration

Severity: Low

The frame-cache exports setCurrentFrame() and clearFrameCache() but these are never called by renderFrame(). The cache appears to be implemented but not actually used.

Questions:

• Is this feature incomplete or intentionally manual?
• Should renderFrame() automatically call setCurrentFrame(frameNumber)?

Location: packages/motion-dom/src/frameloop/render-frame.ts:54-78

3. Type Safety: Remotion Peer Dependency

Severity: Low

The motion-remotion package imports from remotion but marks it as an optional peer dependency. TypeScript may complain if users don't have Remotion installed.

Recommendation:
Consider importing types only: import type { useCurrentFrame, useVideoConfig } from 'remotion'

Location: packages/motion-remotion/src/MotionRemotion.tsx:6

4. Render Function Duplication (packages/motion-dom/src/render/html/utils/render.ts)

Severity: Low - Code Quality

The cache read and cache write logic is mixed with rendering logic. If caching is incomplete (per issue #2), this adds complexity for no benefit yet.

Suggestion: Extract caching to a separate wrapper function if/when it's fully integrated.

Location: packages/motion-dom/src/render/html/utils/render.ts:14-43

🔒 Security Concerns

None identified. No user input processing, no eval/Function constructors, no external data sources.

📝 Code Quality

Positive

• Follows repository conventions (named exports, interface over type)
• Consistent error handling
• Good inline documentation

Minor Issues

1. Unused getCurrentFrame() export - exported but never used
2. Test comment outdated - render-frame.ts:52 references useRemotionFrame which doesn't exist (it's <MotionRemotion>)

🎯 Best Practices Alignment

According to CLAUDE.md:

✅ Follows conventions:

• Named exports only
• Uses interface for types
• No default exports
• Time syncing via time.now() from sync-time.ts (packages/motion-dom/src/frameloop/sync-time.ts:1855-1856)

✅ Test requirements:

• Extensive test coverage with real-world scenarios
• Tests written first (validates the bug/feature)

📊 Performance Impact

Positive:

• Manual timing mode prevents unnecessary rAF scheduling
• Unrolled render loops avoid function call overhead
• WeakMaps ensure elements are GC'd when unmounted

Concern:

• Frame cache could grow unbounded (see issue Initial API (WIP) #1)

🎨 API Design

The API is intuitive and flexible:

// Simple timestamp-based
renderFrame({ timestamp: 1000 })

// Frame-based (Remotion style)
renderFrame({ frame: 30, fps: 30 })

// React wrapper
<MotionRemotion>
  <motion.div animate={{ x: 100 }} />
</MotionRemotion>

The three-tier approach (manual renderFrame(), React hook pattern, and <MotionRemotion> wrapper) covers different use cases elegantly.

🚀 Recommendations

High Priority

1. Clarify frame cache status - Is it complete? If so, integrate with renderFrame(). If not, consider removing from this PR.
2. Add cache size limit - Prevent unbounded memory growth in long exports

Medium Priority

3. Add integration example - Show complete Remotion composition in dev/examples
4. Document backward scrubbing limitation - Tests mention animations are stateful and don't support backward scrubbing (line 1014). Document this clearly.

Low Priority

5. Remove unused exports - getCurrentFrame() if not needed
6. Fix JSDoc reference - Update render-frame.ts:52 comment

✨ Conclusion

This is excellent work. The architecture is sound, test coverage is exceptional, and it solves a real problem elegantly. The frame cache concern is the only significant issue - either complete the integration or document the memory management expectations for long-running exports.

Recommendation: Approve with minor revisions ✅

The core functionality is production-ready. Address the frame cache questions and this will be a great addition to the library.

---

Great work @mattgperry! This feature will unlock Motion for video rendering workflows. 🎬

[claude]

PR Review: Manual Frame Rendering API

Overall Assessment: This is a well-architected feature that adds crucial functionality for non-requestAnimationFrame environments. The implementation is solid with excellent test coverage. I've identified some areas for improvement around edge cases, performance optimizations, and API design.

---

✅ Strengths

1. Architecture & Design

• Clean separation of concerns: The driver abstraction elegantly separates timing control from animation logic
• Global config approach: Using MotionGlobalConfig.driver is the right pattern for framework-wide timing control
• Frame cache optimization: Smart optimization for Remotion's repeated frame rendering
• Non-breaking: Changes are purely additive with backward compatibility maintained

2. Test Coverage

• Comprehensive Remotion integration tests: 1061 lines of tests covering various FPS rates (24, 30, 60), springs, AnimatePresence, stagger, and sequences
• Unit tests for renderFrame(): Good coverage of frame number → timestamp conversion and delta calculations
• WAAPI integration tests: Proper verification that WAAPI disables when custom driver is active

3. Documentation

• Excellent inline comments explaining manual timing mode
• Clear JSDoc for renderFrame() function with usage examples
• Demo component serves as living documentation

---

🔴 Critical Issues

1. Frame Cache Memory Leak Risk

packages/motion-dom/src/render/frame-cache.ts:1888

The frame cache uses Map<number, WeakMap<Element, ElementStyles>> but the outer Map with frame numbers will grow unbounded in long-running Remotion renders.

Impact: A 5-minute video at 60fps = 18,000 Map entries. Each entry is small but the Map overhead adds up.

Question: Is the frame cache needed during sequential forward rendering (typical video export)? Or only when scrubbing? If only for scrubbing, consider disabling during export.

---

2. Race Condition: Driver Setup Timing

packages/motion-remotion/src/MotionRemotion.tsx:2258

useInsertionEffect sets the driver, but child animations may start before the first renderFrame() call.

Scenario:

1. MotionRemotion mounts, sets driver
2. Child motion.div mounts, animation starts with remotionDriver
3. Driver calls frame.update() but frame hasn't been rendered yet
4. Animation uses stale/incorrect timestamp

---

3. Delta Calculation Inconsistency

packages/motion-remotion/src/MotionRemotion.tsx:2276

Delta calculation breaks when frames jump backward (scrubbing) or skip frames.

Problem cases:

• Backward scrubbing: frame=30 → frame=10 gives delta = -666ms (then Math.abs() makes it positive, but animation state is still forward-oriented)
• Frame skips: frame=10 → frame=40 gives delta = 1000ms (animation jumps ahead)

Recommendation: Document clearly that this is export-only, not for interactive scrubbing.

---

⚠️ Important Issues

4. Missing Cleanup in Test Mock

packages/framer-motion/src/utils/tests/use-manual-frame-remotion.test.tsx:364

mockRemotionDriver doesn't clean up scheduled callbacks in afterEach. Callbacks remain scheduled between tests, causing potential interference.

5. Type Safety: GlobalDriver vs Driver

packages/motion-utils/src/global-config.ts:1345

GlobalDriver interface is redefined locally rather than importing from motion-dom. Risk of type drift if Driver type changes.

6. Performance: Unoptimized Frame Cache Lookup

packages/motion-dom/src/render/html/utils/render.ts:1977

Cache lookup happens on every render even when cache is disabled. Negligible but unnecessary overhead in normal rendering.

7. Inconsistent Frame Cache Capture

packages/motion-dom/src/render/html/utils/render.ts:2011

Cache captures projection properties conditionally. Why check if(elementStyle.opacity)? Empty string is falsy but valid.

---

💡 Suggestions

8. API: Consider useRemotionFrame Hook Export

The test file implements useManualFrame locally. Some users might want lower-level control. Consider exporting the hook.

9. Testing: Add E2E Integration Test

Current tests use mocked Remotion context. Consider adding one E2E test with actual Remotion if feasible.

10. Documentation: Add Migration Guide

Consider adding examples for both Remotion integration and manual control (WebXR, custom environments) to CLAUDE.md.

---

🔒 Security

No security concerns identified. The changes are purely internal timing mechanisms.

---

🎯 Performance

Positive:

• ✅ Frame cache significantly reduces DOM operations for repeated frames
• ✅ WeakMap ensures garbage collection of unmounted elements
• ✅ Unrolled render loop avoids abstraction overhead

Concerns:

• ⚠️ Frame cache unbounded growth (Issue 1)
• ⚠️ Double function call for cache check (Issue 6)

---

📋 Summary

Category	Status
Code Quality	⭐⭐⭐⭐ (Very Good)
Test Coverage	⭐⭐⭐⭐⭐ (Excellent)
Performance	⭐⭐⭐⭐ (Good, with caveats)
Security	⭐⭐⭐⭐⭐ (No concerns)
Documentation	⭐⭐⭐⭐ (Very Good)

Recommended Actions Before Merge:

1. Critical: Address frame cache memory growth (Issue 1)
2. Critical: Fix driver initialization race condition (Issue 2)
3. Important: Document backward scrubbing limitations (Issue 3)
4. Important: Fix test cleanup (Issue 4)

Nice to Have:

• Type safety improvements (Issue 5)
• Performance micro-optimizations (Issues 6, 7)
• Additional E2E tests (Suggestion 9)

---

Great work on this feature! The architecture is sound and the implementation is thorough. The issues identified are mostly edge cases and optimizations rather than fundamental problems. With the critical issues addressed, this will be a solid addition to the library.

[claude]

PR Review: Manual Frame Rendering API

This is an impressive and well-architected addition that enables Motion to work in non-rAF environments like Remotion, WebXR, and server-side rendering. The implementation is clean, the tests are comprehensive, and it follows the existing codebase patterns well.

✅ Strengths

Architecture & Design

• Clean separation of concerns: The driver-based approach elegantly decouples timing from animation logic
• Minimal API surface: Simple renderFrame() function with flexible options (timestamp/frame+fps)
• Well-designed caching system: Frame-based render caching with WeakMaps for proper memory management
• No breaking changes: Entirely additive, existing code paths remain untouched

Code Quality

• Excellent test coverage:
  • Unit tests for renderFrame() API (render-frame.test.ts)
  • Integration tests for Remotion use case (use-manual-frame-remotion.test.tsx - 1740 lines!)
  • WAAPI interaction tests
  • Layout animation scrubbing tests with microtask handling
• Performance-conscious:
  • WeakMaps for automatic GC
  • Early return on cache hits in render functions
  • Unrolled render loop maintained
• Good documentation: Clear JSDoc comments explaining use cases and examples

Implementation Details

• Smart WAAPI disabling: Correctly disables WAAPI when custom driver is set (waapi.ts:28-30)
• Microtask batcher fix: Properly handles non-keepAlive batchers regardless of driver (batcher.ts:81-86)
• Frame cache integration: Cleanly integrated into render pipeline without disrupting normal flow

🔍 Issues & Concerns

1. Critical: Comment Syntax Error in JSAnimation.ts

Line 402 has a single / instead of //, which will cause a syntax error.

Fix needed: Change / Priority: to // Priority:

2. Logic Issue: Restarting Finished Animations

The code at JSAnimation.ts:237-240 transitions finished animations back to "running" when scrubbing backwards with a custom driver.

Concern: This mutates animation state in the sample method, which feels fragile. It's not clear this handles all edge cases like animations with repeat that finished multiple iterations.

Recommendation: Add a comment explaining this behavior is specifically for backward scrubbing, and consider adding tests for edge cases.

3. Frame Cache Memory Management

While WeakMaps prevent element-based leaks, the styleCache and attrCache Maps grow unbounded by frame number (frame-cache.ts:9-14).

Scenario: A long Remotion video at 60fps for 5 minutes = 18,000 frames cached.

Recommendation: Consider LRU cache, periodic cleanup, or document that users should call clearFrameCache() between compositions.

4. Delta Calculation in MotionRemotion Component

The delta calculation (MotionRemotion.tsx:41-44) is based on frame jump, but Remotion can scrub non-sequentially.

Recommendation: Clarify in comments whether delta should represent time between renders (current) vs time that passed in the animation timeline.

5. setCurrentFrame Global State

The global mutable state in frame-cache.ts could cause issues with concurrent animations or React 18 concurrent rendering.

Recommendation: Document that this is designed for single-composition rendering.

🎯 Minor Suggestions

1. Unused import useCallback in manual-frame-control.tsx
2. Extract magic number 1000 / 60 to named constant
3. Missing README for motion-remotion package
4. Verify Driver type is exported from motion-dom

📝 Summary

This is high-quality work that solves a real problem elegantly.

Must fix before merge:

• ❌ Syntax error in JSAnimation.ts:402 comment

Should fix before merge:

• ⚠️ Frame cache unbounded growth (or document the limitation)

Consider for follow-up:

• Animation state transition edge cases
• Delta calculation semantics documentation
• Global state implications with concurrent React

Overall: Recommend merge after fixing the syntax error and addressing frame cache concerns.

Great work! The driver abstraction is elegant and sets up nice foundations for future timing contexts.

---

Review by Claude Code

[claude]

Code Review: Manual Frame Rendering API

Overview

This PR adds support for manually advancing animations in environments without requestAnimationFrame (WebXR, Remotion, SSR). The implementation is well-structured and addresses a real need (fixes #2496).

✅ Strengths

Architecture & Design

• Clean separation of concerns: Frame rendering logic properly extracted into renderFrame() function
• Flexible API: Supports both timestamp and frame/fps approaches for different use cases
• Non-breaking changes: All changes are additive, maintaining backward compatibility
• Proper driver abstraction: Uses MotionGlobalConfig.driver to disable rAF scheduling and WAAPI

Code Quality

• Follows project conventions: Adheres to CLAUDE.md guidelines (named exports, arrow callbacks, concise patterns)
• Good test coverage: Comprehensive unit tests for renderFrame() covering edge cases
• Documentation: Clear JSDoc comments explaining usage patterns
• Memory efficiency: Frame cache uses WeakMap for automatic garbage collection

🔍 Issues & Concerns

1. CRITICAL: Potential Memory Leak in Frame Cache

Location: packages/motion-dom/src/render/frame-cache.ts

The frame cache uses Map<number, WeakMap<Element, styles>> but never clears old frame numbers. In long-running Remotion renders (thousands of frames), this will accumulate unbounded entries in the outer Map.

Recommendation: Implement a cleanup strategy (LRU cache, max size limit, or clearOldFrames() API)

2. Missing Delta Validation

Location: packages/motion-dom/src/frameloop/render-frame.ts:49-66

The renderFrame() function doesn't validate inputs. Negative fps or extreme values could cause animation glitches.

Recommendation: Add input validation for fps (must be > 0) and warn on unusual delta values.

3. Frame Cache API Not Exported

Location: packages/motion-dom/src/index.ts

The frame cache module (setCurrentFrame, clearFrameCache) isn't exported from motion-dom's public API. Users implementing manual frame control need these APIs.

Recommendation: Export frame cache utilities from motion-dom.

4. Missing Error Handling for Concurrent Renders

Location: packages/motion-dom/src/frameloop/batcher.ts:94-112

processFrame() doesn't prevent concurrent processing. If called while state.isProcessing === true, it could lead to corrupted state.

Recommendation: Add guard against re-entrant calls.

🧪 Test Coverage

Good Coverage

• ✅ Basic renderFrame() functionality
• ✅ Frame-to-timestamp conversion
• ✅ Delta calculation
• ✅ Frame processing order
• ✅ WAAPI disabling with custom driver

Missing Tests

• ❌ Frame cache memory behavior
• ❌ Concurrent renderFrame() calls
• ❌ Edge cases: negative fps, extreme timestamps
• ❌ Integration test: full animation sequence with manual frames

📝 Style & Conventions

Follows CLAUDE.md guidelines well:

• ✅ Named exports used
• ✅ Interfaces for types
• ✅ File size conscious (concise implementations)
• ✅ Proper use of time.now() instead of performance.now()

🎬 Recommendations Summary

Must Fix (Before Merge)

1. Implement frame cache cleanup strategy to prevent memory leaks
2. Add input validation to renderFrame()
3. Export frame cache APIs from motion-dom

Should Fix (Important)

4. Add guard against concurrent processFrame() calls
5. Add tests for edge cases and memory behavior

Nice to Have

6. Make frame cache opt-in via config flag
7. Add integration tests for complete animation sequences
8. Document memory characteristics in JSDoc

Overall Assessment

Quality: ⭐⭐⭐⭐☆ (4/5)

This is a solid implementation that addresses a real need. The architecture is clean and the code follows project conventions. However, the frame cache memory leak is a critical issue that must be addressed before merging, especially for the primary use case (Remotion video rendering with thousands of frames).

Recommendation: Request changes to fix the memory leak, then approve with remaining items as follow-up improvements.

Great work on this feature! 🎉

[claude]

Pull Request Review: Manual Frame Rendering API

Summary

This PR adds support for manually advancing animations in environments without requestAnimationFrame (WebXR, Remotion, SSR video rendering). The implementation is well-structured and follows the codebase conventions outlined in CLAUDE.md.

✅ Strengths

Architecture & Design

• Clean API design: The renderFrame API is flexible and intuitive, supporting both timestamp and frame-based workflows
• Minimal surface area: New functionality is concentrated in a few key files without sprawling changes
• Backwards compatible: The feature is opt-in via MotionGlobalConfig.useManualTiming, with no breaking changes
• Proper separation: Manual timing logic is correctly isolated in the frameloop layer

Code Quality

• Follows project conventions: Uses optional chaining, arrow functions, interfaces, and concise patterns as per CLAUDE.md
• Good file size consideration: Implementation is compact and efficient
• Proper timing sync: Uses time.set() from sync-time.ts as recommended in CLAUDE.md

Test Coverage

• Comprehensive unit tests: render-frame.test.ts covers core functionality (122 lines)
• Excellent integration tests: use-manual-frame-remotion.test.tsx is extensive (1764 lines)
• WAAPI behavior: Tests confirm that manual timing correctly disables Web Animations API acceleration
• Good test practices: Proper setup/teardown of global config state

🔍 Issues & Concerns

1. Critical: Potential Race Condition in renderFrame()

Location: packages/motion-dom/src/frameloop/render-frame.ts:65-75

Issue: If processFrame() throws an exception, useManualTiming won't be restored. This could leave the animation system in an inconsistent state.

Recommendation: Use try-finally pattern to ensure useManualTiming is always restored.

2. Logic Inconsistency in JSAnimation.tick()

Location: packages/motion-dom/src/animation/JSAnimation.ts:236-242

Issue: Why should a finished animation revert to "running" just because manual timing is enabled? This seems like a workaround for frame-by-frame scrubbing, but it breaks the state machine invariant and lacks explanation.

Recommendation: Add a clear comment explaining this behavior, or consider alternative approaches.

3. Missing Teardown in finish()

Location: packages/motion-dom/src/animation/JSAnimation.ts:468-476

Issue: In manual timing mode, animations never call teardown(), which means the activeAnimations.mainThread counter is never decremented and the driver is never stopped. This causes memory leaks in long-running Remotion/WebXR sessions.

Recommendation: Either always teardown when finished, document that users must explicitly call .stop(), or add a cleanup API for manual timing mode.

4. Default FPS Value Inconsistency

Location: packages/motion-dom/src/frameloop/render-frame.ts:46-57

The default FPS is 30, but many contexts use different defaults (24fps, 30fps, 60fps).

Recommendation: Consider making FPS required when using frame parameter, or document why 30 was chosen.

5. Type Safety: renderFrame({}) is Valid

Location: packages/motion-dom/src/frameloop/render-frame.ts:45

Calling renderFrame({}) is allowed but uses frameData.timestamp + delta, which might be unexpected.

Recommendation: Consider making the options non-optional or use a discriminated union.

💡 Suggestions & Enhancements

6. Performance: Redundant processFrame() Code Duplication

Location: packages/motion-dom/src/frameloop/batcher.ts:95-113

The processFrame() function duplicates the entire frame processing loop from processBatch(). While noted as "unrolled for better per-frame performance", the duplication makes maintenance harder.

7. Missing Public API Documentation

The PR body mentions setManualTiming(enabled), isManualTiming(), and useManualFrame() but they're not found in the code. Update the PR description to reflect what's actually included.

8. Demo App Could Be More Discoverable

Location: dev/react/src/tests/manual-frame-control.tsx

Great demo, but it's in tests/ which might not be intuitive. Consider moving to examples/.

9. Wake Condition Logic

Location: packages/motion-dom/src/frameloop/batcher.ts:82-87

The condition in wake() is subtle and hard to parse. Add an inline comment explaining the intended behavior.

🔒 Security Considerations

No security concerns identified.

⚡ Performance Considerations

Positive:

• Direct frame control avoids rAF overhead in appropriate contexts
• Unrolled frame processing loop is efficient
• Minimal conditional checks added to hot paths

Concerns:

• The check !MotionGlobalConfig.useManualTiming is evaluated every frame in the hot path
• Consider caching this value if performance profiling shows it's an issue

📋 Checklist

✅ Code follows CLAUDE.md style guidelines
✅ Tests cover core functionality
✅ No obvious security vulnerabilities
✅ Backwards compatible
⚠️ Memory leak concern in manual timing mode (Issue #3)
⚠️ Exception safety needed (Issue #1)
⚠️ State machine logic needs clarification (Issue #2)

🎯 Verdict

Conditional Approval - This is solid work that adds valuable functionality for Remotion/WebXR use cases. The core implementation is sound, but there are three issues that should be addressed before merging:

1. Must fix: Add try-finally to renderFrame() (Issue Initial API (WIP) #1)
2. Should fix: Address memory leak in finish() (Issue Add motionValue prop #3)
3. Should clarify: Document the state transition logic in tick() (Issue Fixing types and es references #2)

The other suggestions are nice-to-haves that can be addressed in follow-up PRs.

Great work overall! The extensive test coverage is particularly impressive. 🚀

Fixes #2496

[claude]

Pull Request Review

This PR adds manual frame rendering support for environments without requestAnimationFrame (WebXR, Remotion, SSR). The implementation is well-designed and follows the repository conventions.

✅ Strengths

1. Architecture & Design

• Clean separation of concerns with renderFrame as the main public API
• Properly integrates with existing MotionGlobalConfig pattern
• processFrame method added to batcher provides low-level control while renderFrame offers a convenient wrapper
• Smart handling of manual timing mode - temporarily enables it during frame processing to prevent rAF scheduling

2. API Design

• Flexible API supporting multiple timing modes:
  • timestamp: Direct millisecond control
  • frame + fps: Natural for video rendering (Remotion)
  • delta: Optional explicit control over frame timing
• Good defaults (30fps, 1000/60 delta)
• Clear and comprehensive JSDoc comments

3. Code Quality

• Follows repository style guide (arrow functions, optional chaining, interfaces over types)
• Small file sizes prioritized (important for library code)
• Zero breaking changes - purely additive API
• Proper cleanup in JSAnimation.finish() at line 470 - doesn't teardown when in manual timing mode

4. Test Coverage

• Comprehensive unit tests covering all API variations
• Tests frame order, timestamp conversion, delta calculation, incremental rendering
• Proper setup/teardown of MotionGlobalConfig.useManualTiming

5. Integration Points

• sync-time.ts:23: Correctly checks manual timing mode when calculating time
• waapi.ts:44: Properly disables WAAPI acceleration in manual mode (critical - WAAPI can't be controlled frame-by-frame)
• batcher.ts:72: Prevents auto-scheduling next frame in manual mode
• JSAnimation.ts:237: Smart state handling - transitions back to "running" if current time < total duration in manual mode

🔍 Observations & Suggestions

1. State Management (Minor)

// JSAnimation.ts:237-242
if (this.state === "finished" && this.holdTime === null) {
    if (MotionGlobalConfig.useManualTiming && this.currentTime < totalDuration) {
        this.state = "running"
    } else {
        this.currentTime = totalDuration
    }
}

This logic allows animations to "resurrect" from finished state in manual mode. While this makes sense for scrubbing backwards in video rendering, it could be worth documenting this behavior difference from normal mode.

2. Timestamp Calculation (Question)

// render-frame.ts:59-62
} else {
    frameDelta = delta !== undefined ? delta : 1000 / 60
    frameTimestamp = frameData.timestamp + frameDelta
}

When neither timestamp nor frame is provided, it increments from frameData.timestamp. This means calling renderFrame() repeatedly with no args advances time automatically. Is this intentional? It seems convenient but might be surprising behavior. Consider documenting this explicitly.

3. Export Strategy
The new API is exported via:

• packages/motion-dom/src/frameloop/index.ts exports renderFrame
• packages/motion-dom/src/index.ts re-exports all of ./frameloop

This means renderFrame is available from motion-dom but the demo imports it directly. Users would discover it via:

import { renderFrame } from "motion-dom"

Consider whether this should also be exposed from the main motion package or if motion-dom is the right level.

4. Potential Race Condition (Low Risk)

// render-frame.ts:65-66
const previousManualTiming = MotionGlobalConfig.useManualTiming
MotionGlobalConfig.useManualTiming = true
// ... processing ...
MotionGlobalConfig.useManualTiming = previousManualTiming

If renderFrame is called while already processing a frame, this could cause nested state changes. The isProcessing flag should prevent this, but consider adding a guard:

if (frameData.isProcessing) {
    console.warn('renderFrame called during frame processing')
    return
}

5. Delta Calculation Consistency
Different delta defaults are used in different contexts:

• render-frame.ts:53: 1000 / 60 when timestamp provided
• render-frame.ts:57: 1000 / fps when frame provided
• batcher.ts:50: 1000 / 60 in processBatch

This is correct but could be clearer. The frame-based path correctly uses fps for delta, but the timestamp path always assumes 60fps. Consider whether fps should be respected even when timestamp is provided.

6. File Size Consideration
The PR adds ~200 lines of production code (excluding tests and demo). Given the repository's focus on small file sizes, this is reasonable for the functionality provided. The main renderFrame function is only 32 lines.

🔒 Security & Performance

• Security: No concerns. No user input processing, no DOM manipulation, no external dependencies.
• Performance: Minimal overhead. The temporary config toggle is cheap. No new memory allocations in hot paths.

📋 Testing Recommendations

Manual Testing Needed:

1. Test with actual Remotion project
2. Verify WebXR use case
3. Test scrubbing backwards through animations
4. Test rapid frame changes (e.g., 60fps playback in manual mode)
5. Test with WAAPI-accelerated animations to ensure they fall back correctly

Additional Test Cases (Optional):

• Test concurrent animations in manual mode
• Test switching between manual and auto mode mid-animation
• Test with repeat/mirror animations
• Test with spring animations

📚 Documentation Suggestions

1. Add a dedicated example/guide for Remotion integration showing the useManualFrame hook pattern
2. Document the state resurrection behavior in manual mode
3. Add migration guide for users currently using workarounds
4. Document performance characteristics (manual mode prevents WAAPI acceleration)

🎯 Overall Assessment

This is a high-quality implementation that solves a real problem for important use cases (Remotion, WebXR). The code is clean, well-tested, and follows repository conventions. The API design is intuitive and flexible.

Recommendation: Approve with minor documentation improvements

The concerns raised are minor and mostly about documentation clarity rather than implementation issues. The core implementation is solid and ready to merge.

---

Great work! This will be a valuable addition for users working with video rendering and immersive environments. 🎬

Review by Claude Code