Skip to content

Conversation

@mattgperry
Copy link
Collaborator

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

…ionFrame

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
Copy link

claude bot commented Feb 1, 2026

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:

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

Nice to Have:

  1. Integration test
  2. Export types
  3. Edge case documentation

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

@claude
Copy link

claude bot commented Feb 2, 2026

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

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

Low Priority

  1. Add comment explaining code duplication if intentional for performance
  2. 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. 🚀

- Add unit tests for useManualFrame hook demonstrating Remotion-style integration
- Add tests for direct renderFrame usage with frame-by-frame stepping
- Add tests for scrubbing backwards through animations
- Add interactive dev demo with frame slider and step buttons

https://claude.ai/code/session_01AFhy8F8ubRRHsau2GRdYAZ
@claude
Copy link

claude bot commented Feb 2, 2026

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! 🎉

mattgperry and others added 2 commits February 2, 2026 21:22
- Create motion-remotion package with remotionDriver and useRemotionFrame hook
- Add MotionGlobalConfig.driver for custom animation drivers
- Disable WAAPI when custom driver or useManualTiming is active
- Update batcher to skip rAF scheduling with custom driver
- Export Driver type and frameloopDriver from motion-dom
- Remove useManualFrame from framer-motion (moved to motion-remotion)
- Add waapi supports tests for driver/timing checks

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove useManualTiming from MotionGlobalConfig
- Remove setManualTiming and isManualTiming functions
- Update batcher, waapi, and sync-time to only check for driver
- Update all tests to use driver instead of useManualTiming
- Use useLayoutEffect in useRemotionFrame for proper timing

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Manually update animations without requestAnimationFrame

3 participants