Refactor VisualElement to motion-dom package

[mattgperry]

Summary

• Move core VisualElement classes from framer-motion to motion-dom, making them framework-agnostic
• Add VisualElement, DOMVisualElement, HTMLVisualElement, SVGVisualElement, ObjectVisualElement to motion-dom
• Move Feature base class, visualElementStore, and animation state utilities
• Move projection geometry, styles, and animation utilities
• Keep React-specific code (features, contexts, hooks) in framer-motion

Test plan

• yarn build passes for all packages
• make test - 703 unit tests pass
• yarn test-playwright - 166 e2e tests pass

🤖 Generated with Claude Code

[claude]

Code Review Summary

This PR successfully refactors the VisualElement architecture from framer-motion to motion-dom, making it framework-agnostic. The changes are well-structured and comprehensive.

Strengths

1. Clean Architecture: The refactor properly separates framework-agnostic rendering logic (motion-dom) from React-specific features (framer-motion).

2. Comprehensive Migration: Successfully moved core VisualElement classes, Feature base class, animation state utilities, projection geometry and styling logic.

3. Good API Design: The setFeatureDefinitions() and getFeatureDefinitions() pattern allows framework layer to inject behavior without tight coupling.

4. Test Coverage: All tests passing (703 unit tests, 166 e2e tests).

Potential Issues

1. Import Path in mix-values.ts Line 10: Imports ResolvedValues from ../../node/types but should use ../../render/types within motion-dom

2. Type Safety: visualElement parameter typed as any in animation-state.ts - consider using minimal interface instead

3. Missing JSDoc Comments: Some newly exposed public APIs lack documentation

Code Quality

Naming, structure, dependencies all follow conventions correctly. No security or performance concerns.

Recommendation

High-quality refactor. Issues noted are minor. Approve with minor suggestions.

---

Review by Claude Code

[claude]

Pull Request Review - Refactor VisualElement to motion-dom package

Overview

This is a well-structured architectural refactoring that successfully moves the core VisualElement classes and related infrastructure from framer-motion to motion-dom, making them framework-agnostic. The changes align with the stated goal of separating framework-independent DOM animation logic from React-specific code.

Code Quality ✅

Strengths:

• Clean separation of concerns: The refactoring properly separates framework-agnostic VisualElement logic (motion-dom) from React-specific code (framer-motion)
• Consistent import structure: All 130+ files that now import from motion-dom follow a consistent pattern
• Type safety maintained: Type definitions have been properly moved and re-exported where needed
• No deprecated patterns: Clean removal of unused files (e.g., empty transform-origin.ts)

Minor observations:

• The Feature class in motion-dom/src/render/Feature.ts:13 uses any type for the node property to avoid circular dependencies. While pragmatic, consider adding a type comment explaining this is a VisualElement at runtime
• Some functions in animation-state.ts accept any for visualElement parameter - this is acceptable given the framework-agnostic nature but could benefit from a generic type parameter in future iterations

Architecture ✅

Excellent dependency flow:

motion-utils (base utilities)
    ↓
motion-dom (DOM animation engine + VisualElement) ← New location
    ↓
framer-motion (React integration)
    ↓
motion (public API)

Key improvements:

1. Proper abstraction: VisualElement is now truly framework-agnostic
2. Feature injection pattern: The setFeatureDefinitions() function (motion-dom/src/render/VisualElement.ts:64) allows the React layer to inject features cleanly
3. Clean boundaries: React-specific contexts and hooks remain in framer-motion while core rendering logic moved to motion-dom

Potential Issues 🟡

1. Type Compatibility (Low risk)

The moved types like ScaleCorrectorDefinition and ScaleCorrectorMap now reside in motion-dom/src/projection/styles/types.ts. Verify that all consuming code in framer-motion properly imports these types.

Check: Run yarn build and ensure no type resolution errors occur.

2. Circular Dependency Risk (Low risk)

The Feature class and VisualElement have a circular relationship that's resolved using any. While this works, ensure:

• Runtime initialization order doesn't cause issues
• TypeScript's --noCheck isn't hiding problems

Recommendation: Consider adding integration tests that verify feature mounting/unmounting works correctly with the new structure.

3. Animation State Injection (Low risk)

The createAnimationState function uses a default no-op animator (motion-dom/src/render/utils/animation-state.ts:57-59). Confirm that framer-motion always provides its implementation before animations are triggered.

Performance Considerations ✅

Positive impacts:

• Tree-shaking: Better separation allows consumers to import only what they need from motion-dom
• No runtime overhead: The refactoring is purely structural - no new abstraction layers that add runtime cost
• Maintained optimizations: Critical optimizations like projection transforms and scale correction remain intact

No concerns identified in terms of performance degradation.

Security Concerns ✅

No security issues identified. This is a refactoring PR with:

• No new external dependencies introduced in motion-dom
• No changes to security-sensitive operations (CSP nonces, sanitization, etc.)
• Proper preservation of existing security patterns

Test Coverage ✅

Excellent test coverage:

• ✅ 703 unit tests pass (Jest)
• ✅ 166 E2E tests pass (Playwright)
• ✅ All test imports updated correctly (verified by passing builds)
• ✅ Tests for moved code (geometry, projection, scale correction) updated with new import paths

Test migration quality:
All test files properly updated their imports:

• projection/geometry/__tests__/*.test.ts now import from motion-dom
• projection/styles/__tests__/*.test.ts updated correctly
• No test logic changes - purely import updates

Recommendations

Critical (None) ✅

High Priority

1. Export validation: Verify that all public APIs from the old framer-motion exports are still accessible. The index.ts changes show careful re-exports, but a manual API audit would be prudent.

Medium Priority

2. Documentation: Update package READMEs to reflect the new architecture:

   • Document that motion-dom now contains VisualElement
   • Update contribution guides if developers need to know where to add features

3. Type improvements: Consider adding JSDoc comments to the Feature and VisualElement classes in motion-dom explaining their framework-agnostic nature and how they're extended by React.

Low Priority

4. Bundle size analysis: Run yarn measure to verify the refactoring didn't accidentally increase bundle sizes due to different tree-shaking behavior.

Specific Code Notes

Excellent Patterns Found:

// packages/motion-dom/src/render/VisualElement.ts:64-66
export function setFeatureDefinitions(definitions: Partial<FeatureDefinitions>) {
    featureDefinitions = definitions
}

This injection pattern is clean and avoids tight coupling.

Areas to Watch:

// packages/motion-dom/src/render/Feature.ts:13
node: any // Typed as any to avoid circular dependencies

While pragmatic, ensure this doesn't mask type errors in consuming code.

Summary

This is a high-quality refactoring that achieves its goals effectively:

✅ Approve with confidence - The code quality is excellent, test coverage is comprehensive, and the architecture is sound.

✅ Low risk - This is a structural refactoring with no behavior changes. The passing CI and 869 passing tests provide strong confidence.

✅ Future-proof - This change sets up the codebase well for future framework-agnostic uses of motion-dom.

Final Score: 9/10 - Excellent work. The only minor deduction is for the any types which, while justified, could be improved in future iterations.

---

Review generated with attention to the repository's CLAUDE.md conventions and architectural patterns.

[claude]

Code Review - PR #3470: Refactor VisualElement to motion-dom package

Summary

This PR successfully refactors the core VisualElement class hierarchy from framer-motion to motion-dom, making it framework-agnostic. This is a significant architectural improvement that will enable better code reuse and separation of concerns.

✅ Strengths

1. Excellent Architecture - The separation between framework-agnostic DOM manipulation (motion-dom) and React-specific features (framer-motion) is well-executed.

2. Clean Dependency Flow - The refactoring maintains the proper dependency hierarchy:

   motion-utils → motion-dom → framer-motion → motion
   

3. Feature Injection Pattern - The use of setFeatureDefinitions() and getFeatureDefinitions() allows motion-dom to be framework-agnostic while framer-motion provides React-specific implementations. This is a solid design pattern.

4. Animation State Separation - Moving createAnimationState to motion-dom with a makeAnimateFunction parameter is clever. It keeps the state machine logic framework-agnostic while allowing framer-motion to inject its animation implementation (packages/framer-motion/src/motion/features/animation/index.ts:10-17).

5. Comprehensive Test Coverage - According to the PR description, all 703 unit tests and 166 E2E tests pass, which is excellent.

6. Backward Compatibility - Re-exports are properly maintained in framer-motion (packages/framer-motion/src/index.ts:90-94).

⚠️ Potential Issues

1. Duplicate Feature Classes

There are now TWO Feature base classes:

• packages/motion-dom/src/render/Feature.ts - Uses any for node type
• packages/framer-motion/src/motion/features/Feature.ts - Uses VisualElement<T> type

Impact: This could cause confusion and type safety issues. Framer-motion features import from their local Feature.ts, but motion-dom exports its own Feature class.

Recommendation: Consider either:

• Making framer-motion re-export motion-dom's Feature class and remove the duplicate
• Or clearly document why both exist and ensure they're not mixed

Location: packages/framer-motion/src/motion/features/Feature.ts:1 vs packages/motion-dom/src/render/Feature.ts:1

2. Export Naming Inconsistency

// motion-dom/src/render/utils/is-forced-motion-value.ts:10
export { addScaleCorrector as addScaleCorrectors }  // plural

// But motion-dom/src/projection/styles/scale-correction.ts:23
export function addScaleCorrector(...)  // singular

Impact: The function is named addScaleCorrector (singular) but re-exported as addScaleCorrectors (plural). This is fine for backward compatibility but could be confusing.

Recommendation: Add a comment explaining this is for backward compatibility.

3. Feature Definition Initialization Timing

The initFeatureDefinitions() function uses a module-level flag (isInitialized) to prevent re-initialization (packages/framer-motion/src/motion/features/definitions.ts:26-49).

Potential Issue: If someone tries to set custom feature definitions before any motion component renders, they might get overwritten by the lazy initialization.

Recommendation: Document the initialization behavior or consider initializing at module load time instead of lazily.

4. Type Safety in VisualElement

// packages/motion-dom/src/render/VisualElement.ts:58
let featureDefinitions: Partial<FeatureDefinitions> = {}

The featureDefinitions is module-level mutable state that gets modified via setFeatureDefinitions().

Potential Issue: This could lead to unexpected behavior if multiple versions of motion-dom are loaded, or in testing scenarios.

Recommendation: Consider making this more explicit by documenting the singleton pattern or adding guards.

📝 Code Quality Observations

1. Good: Import organization is clean, using named exports consistently
2. Good: Type definitions are properly migrated with the code
3. Good: Comments are preserved where helpful
4. Good: Test files are updated to import from the correct packages

🔒 Security Considerations

No security concerns identified. The refactoring doesn't introduce any new attack surfaces.

⚡ Performance Considerations

This refactoring should have minimal performance impact:

• No changes to runtime algorithms
• The feature injection pattern adds negligible overhead
• Bundle size might improve slightly due to better tree-shaking potential

🧪 Testing

According to the PR description:

• ✅ 703 unit tests pass
• ✅ 166 E2E tests pass
• ✅ Build succeeds

Recommendation: Consider adding integration tests that specifically verify the framework-agnostic nature of motion-dom (e.g., using it without React).

📚 Documentation

Recommendation: Update the main README or add a migration guide if this is a breaking change for anyone importing internal APIs.

Final Verdict: ✅ APPROVE with Minor Suggestions

This is a well-executed refactoring that significantly improves the architecture. The identified issues are minor and mostly about documentation and consistency. The code is production-ready.

Priority fixes before merge:

1. Address the duplicate Feature class situation
2. Document the feature definition initialization behavior

Nice-to-have improvements:

1. Add comments for naming inconsistencies (addScaleCorrector/addScaleCorrectors)
2. Consider integration tests for motion-dom standalone usage

Great work on this refactoring! 🎉

---

Reviewed with Claude Code

[claude]

Code Review: Refactor VisualElement to motion-dom Package

Summary

This is an excellent architectural refactoring that successfully moves core VisualElement classes from framer-motion to motion-dom, making them framework-agnostic. The PR demonstrates strong software engineering principles and maintains backward compatibility while improving the codebase structure.

✅ Strengths

1. Clean Architectural Separation

• Successfully extracted framework-agnostic DOM animation primitives to motion-dom
• React-specific code (features, contexts, hooks) properly remains in framer-motion
• Clear dependency flow: motion-utils → motion-dom → framer-motion → motion

2. Well-Designed Dependency Injection

The introduction of makeAnimateFunction as a parameter to createAnimationState is particularly elegant:

// motion-dom provides the animation state management
export function createAnimationState(
    visualElement: any,
    makeAnimateFunction: MakeAnimateFunction = defaultAnimateList
): AnimationState

// framer-motion injects its specific animation implementation
node.animationState ||= createAnimationState(node, makeAnimateFunction)

This allows motion-dom to manage state while framer-motion provides the actual animation logic, avoiding circular dependencies.

3. Feature Definition System

The feature definition initialization pattern is well thought out:

// motion-dom exposes setters/getters
export function setFeatureDefinitions(definitions: Partial<FeatureDefinitions>)
export function getFeatureDefinitions(): Partial<FeatureDefinitions>

// framer-motion initializes with React-specific logic
initFeatureDefinitions() // Called lazily

4. Comprehensive Testing

• ✅ 703 unit tests pass
• ✅ 166 E2E tests pass
• ✅ All builds successful

5. No Circular Dependencies

Verified that motion-dom has no imports from framer-motion - excellent dependency hygiene.

🔍 Areas for Consideration

1. Type Safety in Animation State (Minor)

// packages/framer-motion/src/motion/features/animation/index.ts:14
return (animations: Array<{ animation: any; options?: any }>) => {

The use of any types here reduces type safety. Consider defining proper interfaces:

interface AnimationWithOptions {
    animation: AnimationDefinition
    options?: VisualElementAnimationOptions
}

Location: packages/framer-motion/src/motion/features/animation/index.ts:14

2. Feature Definition Initialization Timing (Minor)

The lazy initialization pattern for feature definitions is good, but it's worth documenting when initFeatureDefinitions() gets called:

• Called in getProjectionFunctionality() (motion/index.tsx:206)
• Called in loadFeatures() (motion/features/load-features.ts:6)

Consider adding a comment or documentation about the initialization contract to prevent future issues.

3. Export Organization (Nit)

The motion-dom/src/index.ts file has grown to 234 lines. While well-organized, consider grouping exports by domain in the future:

// Visual Element Core
export { VisualElement, ... } from "./render/VisualElement"

// Animation System  
export { animateMotionValue, ... } from "./animation/interfaces/motion-value"

// etc.

4. Documentation Gap

The CLAUDE.md package architecture section should be updated to reflect this refactoring. The current dependency flow description is accurate but could mention the new framework-agnostic nature of motion-dom.

🎯 Best Practices Observed

1. Backward Compatibility: All public APIs maintained
2. Single Responsibility: Each package now has a clearer purpose
3. Testability: Animation logic can be mocked via dependency injection
4. Build Hygiene: All TypeScript builds pass without errors
5. Consistent Patterns: Import reorganization follows consistent patterns throughout

🔒 Security & Performance

• ✅ No security concerns identified
• ✅ No performance regressions expected (pure refactoring)
• ✅ No changes to runtime behavior
• ✅ Tree-shaking capabilities preserved

📝 Minor Suggestions

1. Consider adding JSDoc comments to the new public APIs in motion-dom, especially:

   • setFeatureDefinitions()
   • getFeatureDefinitions()
   • The MakeAnimateFunction type

2. Future consideration: The VisualElement class is now 886 lines. While this refactoring successfully extracts it to the right package, consider breaking it into mixins or composition patterns in a future refactoring if it continues to grow.

✅ Verdict

APPROVE - This is excellent architectural work that significantly improves the codebase structure. The refactoring is well-executed, thoroughly tested, and maintains backward compatibility. The minor suggestions above are not blockers and can be addressed in follow-up PRs if desired.

Great work on maintaining such clean architectural boundaries while performing a complex refactoring! 🎉

---

Review performed by Claude Code following repository conventions from CLAUDE.md

[claude]

PR Review: Refactor VisualElement to motion-dom package

Summary

This is a significant architectural refactor that moves core VisualElement classes and related infrastructure from framer-motion to motion-dom, making them framework-agnostic. The changes affect 137 files with 981 additions and 524 deletions.

✅ Strengths

1. Architecture & Design

• Excellent separation of concerns: Moving framework-agnostic visual element code to motion-dom while keeping React-specific code in framer-motion is the right architectural decision
• Clear dependency injection pattern: The new makeAnimateFunction callback in createAnimationState (motion-dom/src/render/utils/animation-state.ts:56-59) allows the framework layer to provide animation implementation without creating circular dependencies
• Well-structured feature system: The setFeatureDefinitions/getFeatureDefinitions API in VisualElement.ts:64-73 provides a clean way for framework layers to inject features

2. Code Quality

• Import statements are properly updated throughout the codebase
• Type safety is maintained with proper TypeScript typing
• The code follows the repository's style guide (named exports, interfaces over types)

3. Testing

• PR description confirms all tests pass: 703 unit tests and 166 E2E tests
• Existing test coverage should catch regressions
• Test files properly updated to import from new locations

🔍 Issues & Concerns

1. Initialization Timing (Medium Priority)

Location: packages/framer-motion/src/motion/features/definitions.ts:26-50

The initFeatureDefinitions() function uses a module-level isInitialized flag to ensure features are only initialized once. However, there's a potential race condition:

let isInitialized = false

export function initFeatureDefinitions() {
    if (isInitialized) return
    // ...initialization code...
    setFeatureDefinitions(initialFeatureDefinitions)
    isInitialized = true
}

Issue: If initFeatureDefinitions() is called from multiple components during initial render (especially with concurrent React features), there could be a brief window where multiple calls pass the isInitialized check before it's set to true.

Recommendation: Consider using a more robust initialization pattern:

let featureDefinitionsPromise: Promise<void> | null = null

export async function initFeatureDefinitions() {
    if (featureDefinitionsPromise) return featureDefinitionsPromise
    
    featureDefinitionsPromise = Promise.resolve().then(() => {
        // initialization code
    })
    
    return featureDefinitionsPromise
}

Or simply use a once/memoization pattern if synchronous initialization is required.

2. Circular Dependency Risk (Low Priority)

Location: Multiple files in motion-dom/src/index.ts

The motion-dom package now exports a large number of utilities, types, and classes (234 lines of exports). While this isn't inherently problematic, it increases the risk of circular dependencies as the codebase evolves.

Recommendation:

• Consider using barrel exports more judiciously
• Monitor bundle size to ensure tree-shaking works correctly
• Document which exports are "public API" vs "internal but exported for framework layer"

3. Missing Type Export (Low Priority)

Location: packages/framer-motion/src/motion/features/animation/index.ts:13-21

The makeAnimateFunction returns an anonymous function type that matches AnimateFunction from motion-dom, but this relationship isn't explicitly typed:

function makeAnimateFunction(visualElement: VisualElement) {
    return (animations: Array<{ animation: any; options?: any }>) => {
        return Promise.all(
            animations.map(({ animation, options }) =>
                animateVisualElement(visualElement, animation, options)
            )
        )
    }
}

Recommendation: Explicitly type the return type to ensure type safety:

function makeAnimateFunction(visualElement: VisualElement): AnimateFunction {
    // implementation
}

4. Feature Definition Mutation (Low Priority)

Location: packages/framer-motion/src/motion/features/load-features.ts:5-13

The loadFeatures function mutates the featureDefinitions object and then calls setFeatureDefinitions:

export function loadFeatures(features: FeaturePackages) {
    const featureDefinitions = getInitializedFeatureDefinitions()

    for (const key in features) {
        featureDefinitions[key] = {
            ...featureDefinitions[key],
            ...features[key],
        } as any
    }

    setFeatureDefinitions(featureDefinitions)
}

Issue: The as any cast suppresses type checking, and mutating the retrieved object before setting it back could lead to subtle bugs.

Recommendation: Create a new object instead of mutating:

export function loadFeatures(features: FeaturePackages) {
    const currentDefinitions = getInitializedFeatureDefinitions()
    const newDefinitions: Partial<FeatureDefinitions> = {}

    for (const key in currentDefinitions) {
        newDefinitions[key] = {
            ...currentDefinitions[key],
            ...features[key],
        }
    }

    setFeatureDefinitions(newDefinitions)
}

📝 Minor Observations

1. Deleted Test File

The test file packages/framer-motion/src/animation/animators/waapi/utils/__tests__/get-final-keyframes.test.ts was deleted. This is fine since the functionality likely moved to motion-dom and has equivalent test coverage there, but worth noting.

2. Code Comments

The makeAnimateFunction in AnimationFeature has good documentation explaining its purpose (lines 9-12). This is excellent for maintainability.

3. Empty File Touches

Several files show no actual changes (e.g., projection/geometry/conversion.ts). These are likely just formatting changes or side effects of the refactor.

🔒 Security

No security concerns identified. The refactor is purely architectural and doesn't introduce new attack vectors.

⚡ Performance

Positive Impact:

• The lazy initialization of feature definitions should have minimal performance impact
• The dependency injection pattern allows better code splitting
• Framework-agnostic code in motion-dom can potentially be used by other frameworks without pulling in React-specific code

Potential Concerns:

• The initialization check in getInitializedFeatureDefinitions() adds a small overhead to every call, but this is negligible

📦 Bundle Size

The refactor should have a neutral or positive impact on bundle size:

• Better separation of concerns enables more effective tree-shaking
• Motion-dom can be used by non-React frameworks without React dependencies

🧪 Test Coverage

According to the PR description:

• ✅ yarn build passes for all packages
• ✅ 703 unit tests pass
• ✅ 166 E2E tests pass

The existing test coverage should adequately cover the refactored code. However, consider adding specific tests for:

1. Feature definition initialization timing
2. The new makeAnimateFunction injection pattern
3. Multiple calls to initFeatureDefinitions()

📋 Recommendations Summary

Priority	Issue	Recommendation
Medium	Initialization race condition	Add race-condition-safe initialization pattern
Low	Type safety in makeAnimateFunction	Add explicit return type annotation
Low	Feature definition mutation	Create new object instead of mutating
Low	Export organization	Document public vs internal exports

✅ Conclusion

This is a well-executed refactor that significantly improves the architecture of the codebase. The changes are logical, well-tested, and maintain backward compatibility. The issues identified are minor and can be addressed in follow-up PRs if desired.

Recommendation: ✅ APPROVE with minor suggestions

The core refactor is solid, and the identified issues are low-risk quality-of-life improvements rather than blockers.

---

Great work on this architectural improvement! 🎉

[claude]

Code Review: Refactor VisualElement to motion-dom Package

This is a well-executed refactoring that successfully moves the core VisualElement classes and related utilities from framer-motion to motion-dom, making them framework-agnostic. The PR accomplishes its stated goals with comprehensive test coverage (703 unit tests + 166 E2E tests passing).

✅ Strengths

1. Clean Architecture Separation: The refactoring properly maintains the package dependency flow (motion-utils → motion-dom → framer-motion → motion), with motion-dom now owning the framework-agnostic core.

2. Comprehensive Migration: Successfully moved all key abstractions:

   • VisualElement class hierarchy (Base, DOM, HTML, SVG, Object)
   • Feature system and visual element store
   • Animation state management
   • Variant resolution utilities
   • Projection geometry and styles
   • Reduced motion utilities

3. Good Backward Compatibility: framer-motion properly re-exports types and utilities from motion-dom, maintaining the public API surface.

4. Test Coverage: All existing tests pass, demonstrating that the refactoring doesn't break functionality.

5. Proper Encapsulation: React-specific code (contexts, hooks, features) correctly remains in framer-motion.

🐛 Issues Found

Critical: Export Naming Inconsistency

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

The export uses addScaleCorrector (singular), but the actual function name in projection/styles/scale-correction.ts:23 is also addScaleCorrector. However, I noticed the framer-motion index may have referenced addScaleCorrectors (plural) in the past. Please verify all consumers use the correct singular form.

Actually, reviewing further, the exports are consistent - this is not an issue. The naming is correct throughout.

⚠️ Considerations

1. Type Import in motion-dom from node/types

Location: packages/motion-dom/src/node/types.ts:197

Contains a comment with example code:

* import { AnimatePresence, motion } from 'framer-motion'

This is just documentation in a comment, but it's worth noting that motion-dom now contains types that reference framer-motion in comments. This is fine for documentation but confirms the architectural relationship.

2. Feature Definition Injection Pattern

Locations:

• packages/motion-dom/src/render/VisualElement.ts:58-73
• packages/framer-motion/src/motion/features/definitions.ts:32-50

The injectable feature definitions pattern using setFeatureDefinitions() is elegant and allows framer-motion to provide React-specific feature detection while keeping VisualElement framework-agnostic. However:

• Consideration: If setFeatureDefinitions() isn't called before VisualElements are created, they won't have feature detection. This appears to be handled by initFeatureDefinitions() in framer-motion, but it's a footgun for future framework integrations.
• Suggestion: Consider adding a runtime warning in VisualElement if features are accessed before definitions are set, or document this requirement clearly in motion-dom's README.

3. Scrape Motion Values Implementation

Location: packages/motion-dom/src/render/html/utils/scrape-motion-values.ts and render/svg/utils/scrape-motion-values.ts

The scraping utilities are now in motion-dom, which makes sense. The SVG version correctly delegates to the HTML version and extends it. Clean implementation.

4. Export Surface Expansion

The motion-dom package now exports 100+ new symbols (lines 149-231 in index.ts). This significantly expands the public API surface of motion-dom.

• Question: Are all these exports intended to be public API, or are some internal to framer-motion's use? If some are internal, consider using a separate entry point or marking them clearly.
• The exports look intentional and well-organized by category, so this is likely fine.

🎯 Code Quality

1. Import Hygiene: ✅ Excellent - no circular dependencies detected between motion-dom and framer-motion.
2. Type Safety: ✅ TypeScript types properly migrated and re-exported where needed.
3. Code Style: ✅ Follows existing patterns, uses interface over type, proper arrow functions.
4. Documentation: ⚠️ JSDoc comments maintained but no migration guide for potential external consumers of internal APIs.

🚀 Performance Considerations

This refactoring shouldn't introduce performance regressions since it's primarily moving code between packages without changing logic. The feature definition injection adds minimal overhead (single object lookup at initialization).

🔒 Security Concerns

No security issues identified. The refactoring is purely structural.

📋 Suggestions

1. Documentation: Add a comment or doc explaining the feature definition injection pattern and when setFeatureDefinitions must be called.

2. Future Consideration: The large export surface of motion-dom could benefit from organized entry points (e.g., motion-dom/render, motion-dom/projection) in a future PR, though the current flat structure is acceptable.

3. Migration Guide: If there are any third-party libraries depending on internal framer-motion APIs that moved to motion-dom, consider adding a migration guide (though this may not be relevant if those APIs weren't documented as public).

✨ Conclusion

This is a solid, well-tested refactoring that achieves its architectural goals. The code quality is high, tests pass, and the separation of concerns is clean. The injectable feature definitions pattern is a smart solution for maintaining framework-agnosticism while allowing framework-specific feature detection.

Recommendation: ✅ Approve with minor suggestions - The issues noted are either non-existent (export naming) or minor considerations that don't block merging. Great work on maintaining test coverage throughout such a large structural change!

---

🤖 Review generated by Claude Code

[claude]

Pull Request Review: Refactor VisualElement to motion-dom

Summary

This is a well-executed architectural refactoring that successfully moves the core VisualElement class hierarchy from framer-motion to motion-dom, making it framework-agnostic. The changes align with the goal of creating cleaner package boundaries and enabling framework-independent animation capabilities.

✅ Strengths

Architecture & Design

• Clean separation of concerns: Framework-agnostic DOM animation logic now lives in motion-dom, while React-specific features remain in framer-motion
• Dependency injection pattern: The setFeatureDefinitions() / getFeatureDefinitions() pattern elegantly handles circular dependencies and allows framework-specific features to be injected
• Consistent abstraction: The VisualElement → DOMVisualElement → HTMLVisualElement/SVGVisualElement hierarchy is well-designed and maintainable
• Type safety: Proper use of TypeScript generics and interfaces throughout the refactored code

Code Quality

• Good documentation: Comments like "Uses any type for visualElement to avoid circular dependencies" at packages/motion-dom/src/render/types.ts:78 show awareness of trade-offs
• Test coverage: 703 unit tests + 166 E2E tests passing demonstrates thorough validation
• Consistent patterns: Export structure in motion-dom/src/index.ts is comprehensive and well-organized

Migration Quality

• Backwards compatibility: The refactor maintains existing public APIs by re-exporting from new locations
• Minimal breaking changes: Import paths updated correctly across the codebase

🔍 Observations & Minor Concerns

1. Circular Dependency Handling

Location: packages/motion-dom/src/render/types.ts:78

parent?: any // VisualElement<unknown> - circular reference handled at runtime

While the any types are documented as necessary to avoid circular dependencies, consider future refactoring to use proper forward declarations or interface segregation. The comments appropriately explain the rationale.

Files affected:

• packages/motion-dom/src/render/types.ts:78-79
• packages/motion-dom/src/render/utils/get-variant-context.ts:18
• packages/motion-dom/src/projection/geometry/delta-apply.ts:79

Recommendation: Document a future task to explore interface segregation to reduce any usage, but this is acceptable for now given the complexity of the refactor.

2. Type Assertion in Animation State

Location: packages/motion-dom/src/render/utils/animation-state.ts:401

// @ts-expect-error - @mattgperry to figure if we should do something here

This indicates unresolved type issues that should be addressed in a follow-up.

Recommendation: Create a follow-up issue to resolve this type error properly rather than suppressing it.

3. Security - Attribute/Style Setting

Location: packages/motion-dom/src/render/svg/utils/render.ts:16

The SVG rendering uses setAttribute() with values from animation state. While the values go through the animation pipeline which provides some sanitization, ensure that:

• Values are always properly typed and validated
• No user input can directly inject into these paths without sanitization

Current assessment: ✅ Appears safe as values flow through the motion value system and type checking, but worth monitoring.

4. Feature Definition Initialization

Location: packages/framer-motion/src/motion/features/definitions.ts:26-50

The initFeatureDefinitions() uses a guard flag isInitialized to prevent double initialization. This pattern is solid, but ensure it's called early enough in all entry points.

Recommendation: Verify this is called during framer-motion package initialization and not just lazily when first component mounts.

📊 Test Coverage Assessment

• ✅ 703 unit tests passing - Excellent coverage
• ✅ 166 E2E tests passing - Comprehensive integration testing
• ✅ 138 files changed - Large refactor with no test failures indicates careful migration
• The test count of 1190 occurrences of test cases across 161 files shows thorough testing practices

⚡ Performance Considerations

• Positive: Moving to motion-dom reduces bundle size for non-React frameworks
• Neutral: Feature definition lookup happens via getter function - should be cached after first call (appears to be)
• Positive: No additional runtime overhead detected; refactor is primarily structural

🔒 Security Assessment

• ✅ No obvious security vulnerabilities introduced
• ✅ DOM manipulation goes through established patterns
• ✅ No new external dependencies added
• ⚠️ Ensure values passed to setAttribute() and setProperty() remain validated through the type system

📝 Code Style & Conventions

All changes follow the repository's code style per CLAUDE.md:

• ✅ Uses interface for type definitions
• ✅ Named exports (no default exports)
• ✅ Arrow callbacks where appropriate
• ✅ Proper TypeScript strict mode compliance

🎯 Recommendations

High Priority

None - the refactor is production-ready as-is.

Medium Priority

1. Follow-up on type error: Resolve the @ts-expect-error at animation-state.ts:401
2. Document breaking changes: If any import paths changed, ensure migration guide exists

Low Priority

1. Reduce any usage: Plan future refactor to eliminate documented any types through interface segregation
2. Add JSDoc: Consider adding JSDoc comments to exported functions in motion-dom/src/index.ts for better DX

✨ Conclusion

Recommendation: ✅ APPROVE

This is a high-quality refactoring that achieves its architectural goals without compromising stability or introducing significant technical debt. The comprehensive test coverage (869 total tests passing) gives strong confidence in the changes. The few minor concerns noted are manageable and don't block merging.

Great work on this refactor! The dependency injection pattern for feature definitions is particularly elegant and sets up the codebase well for future framework integrations beyond React.

---

Review generated by Claude Code

[claude]

Code Review: Refactor VisualElement to motion-dom package

Summary

This PR successfully moves core VisualElement classes and related utilities from framer-motion to motion-dom, making them framework-agnostic. This is a solid architectural improvement that better separates concerns between DOM-level abstractions and React-specific code.

✅ Strengths

1. Clean Architecture: The separation between framework-agnostic DOM code (motion-dom) and React-specific code (framer-motion) is well-executed. The dependency flow is logical: motion-utils → motion-dom → framer-motion → motion.

2. Comprehensive Test Coverage: According to the PR description, all tests pass (703 unit tests, 166 E2E tests), which is excellent for a refactor of this magnitude.

3. Proper Re-exports: The framer-motion/src/index.ts correctly re-exports moved types from motion-dom, maintaining backwards compatibility for consumers.

4. Good Use of Feature Definitions Pattern: The setFeatureDefinitions()/getFeatureDefinitions() pattern allows motion-dom to remain framework-agnostic while still supporting framework-specific features.

🔍 Potential Issues & Recommendations

1. Feature Definitions Initialization Timing (Medium Priority)

Location: packages/framer-motion/src/motion/features/definitions.ts:26-50

The initFeatureDefinitions() function uses a module-level isInitialized flag to prevent double-initialization. However, there's a potential issue:

let isInitialized = false

export function initFeatureDefinitions() {
    if (isInitialized) return
    // ...
    setFeatureDefinitions(initialFeatureDefinitions)
    isInitialized = true
}

Concern: If a motion component is rendered before features are loaded via loadFeatures(), the feature definitions will be initialized with only isEnabled checks but no actual feature implementations. Subsequent calls to loadFeatures() will work, but there's no guarantee about initialization order.

Recommendation: Consider adding a warning or explicit check to ensure loadFeatures() is called in the expected order, especially for LazyMotion scenarios.

2. Deleted Test File Without Replacement (Low Priority)

Location: packages/framer-motion/src/animation/animators/waapi/utils/__tests__/get-final-keyframes.test.ts (deleted)

The test file was deleted, but I don't see a corresponding test in motion-dom. The getFinalKeyframe utility is now in motion-dom/src/animation/utils/get-final-keyframe.ts.

Recommendation: Consider whether tests should be migrated to motion-dom for utilities that moved there, or if the test coverage is redundant with other tests.

3. Type Safety for Feature Definitions (Low Priority)

Location: packages/motion-dom/src/render/VisualElement.ts:58-73

The feature definitions are stored in a module-level variable that can be mutated:

let featureDefinitions: Partial<FeatureDefinitions> = {}

export function setFeatureDefinitions(definitions: Partial<FeatureDefinitions>) {
    featureDefinitions = definitions
}

Recommendation: Consider making this immutable after initialization or adding defensive checks to prevent accidental multiple calls that could override features unexpectedly. For example:

export function setFeatureDefinitions(definitions: Partial<FeatureDefinitions>) {
    if (Object.keys(featureDefinitions).length > 0) {
        console.warn('Feature definitions already set')
    }
    featureDefinitions = definitions
}

4. Import Organization (Code Style)

Location: Multiple files, e.g., packages/framer-motion/src/gestures/drag/VisualElementDragControls.ts:1-19

Some imports from motion-dom are quite long and could benefit from grouping:

import {
    addValueToWillChange,
    animateMotionValue,
    calcLength,
    convertBoundingBoxToBox,
    // ... 15 more imports
} from "motion-dom"

Recommendation: This is acceptable but consider whether some commonly used groups of utilities could be re-exported together from a subpath like motion-dom/projection or motion-dom/animation for better organization.

5. Missing Documentation for Breaking Changes (Low Priority)

While the PR re-exports types to maintain compatibility, internal APIs have changed significantly. If any consumers import from deep paths (e.g., framer-motion/dist/render/VisualElement), they'll break.

Recommendation: Consider adding a note in the PR description or changelog about avoiding deep imports.

🎯 Security & Performance

• Security: No security concerns identified. The refactor doesn't introduce any new external inputs or change security boundaries.
• Performance: No performance concerns. The code is moving between packages but not changing execution logic.

📝 Test Coverage Assessment

Strengths:

• All existing tests pass (703 unit tests, 166 E2E)
• Tests properly updated to import from new locations

Gaps:

• Tests for utilities moved to motion-dom/src/render/utils/ don't appear to have corresponding test files in motion-dom
• The deleted get-final-keyframes.test.ts has no obvious replacement

Recommendation: Consider adding a test file in packages/motion-dom/src/render/utils/__tests__/ to cover core utilities like animation-state.ts, is-forced-motion-value.ts, etc.

✅ Overall Assessment

This is a well-executed refactor that improves the architecture of the codebase. The changes are logical, comprehensive, and maintain backwards compatibility. The concerns raised are mostly minor and don't block merging.

Recommendation: ✅ Approve with minor suggestions

The feature definition initialization pattern is the only item I'd recommend addressing before merge, and even that is optional if the team is confident about initialization order in practice.

Great work on this significant refactor! 🎉

[claude]

PR Review: Refactor VisualElement to motion-dom

This is a significant architectural refactoring that moves the core VisualElement class hierarchy from framer-motion to motion-dom, making it framework-agnostic. Overall, this is a well-executed refactoring with comprehensive test coverage. Here are my findings:

---

✅ Strengths

1. Clear Architectural Separation: The refactoring successfully separates framework-agnostic animation engine code (motion-dom) from React-specific code (framer-motion). This is a solid architectural decision that enables future framework integrations.

2. Comprehensive Test Coverage: All 703 unit tests and 166 e2e tests pass, demonstrating that the refactoring maintains backward compatibility.

3. Thorough Code Cleanup: Duplicate code has been properly removed, and barrel files have been consolidated. The changes reduce code duplication across the codebase.

4. Good Commit Structure: The commits are well-organized and incremental, making the refactoring easier to review and understand.

---

⚠️ Potential Issues & Concerns

1. Race Condition Risk with Feature Definitions (High Priority)

Location: packages/framer-motion/src/motion/features/definitions.ts:26-49

The featureDefinitions system now uses a lazy initialization pattern with a module-level isInitialized flag. This could cause issues in several scenarios:

let isInitialized = false

export function initFeatureDefinitions() {
    if (isInitialized) return
    // ... initialization
    setFeatureDefinitions(initialFeatureDefinitions)
    isInitialized = true
}

Problems:

• If VisualElement is instantiated before initFeatureDefinitions() is called, features won't be available
• No explicit initialization call in the main entry points
• In SSR scenarios, the initialization state persists across requests in the same Node.js process
• The isInitialized flag prevents re-initialization even if needed (e.g., testing scenarios)

Recommendation:

• Add explicit initFeatureDefinitions() call in main entry points (framer-motion index.ts)
• Consider adding a reset function for testing: export function resetFeatureDefinitions() { isInitialized = false }
• Add defensive checks in VisualElement.updateFeatures() to warn if definitions are empty
• Document the initialization requirement in CLAUDE.md

2. Circular Dependency Potential

Location: Multiple files importing from both motion-dom and local modules

The refactoring creates bidirectional dependencies between packages:

• motion-dom exports VisualElement and core utilities
• framer-motion imports from motion-dom and calls setFeatureDefinitions()
• Feature classes in framer-motion extend Feature from motion-dom

Current flow:

motion-dom exports VisualElement with empty featureDefinitions
→ framer-motion imports VisualElement
→ framer-motion calls setFeatureDefinitions()
→ VisualElement.updateFeatures() uses the definitions

Risk: If the initialization order changes or modules are loaded out of order, features may not work correctly.

Recommendation:

• Add runtime validation in VisualElement.updateFeatures():

updateFeatures() {
    if (process.env.NODE_ENV === 'development' && Object.keys(featureDefinitions).length === 0) {
        console.warn('Feature definitions not initialized. Call initFeatureDefinitions() first.')
    }
    // ... rest of method
}

3. Missing Type Safety in Feature Definition Mutation

Location: packages/framer-motion/src/motion/features/load-features.ts:9-12

featureDefinitions[key as keyof typeof featureDefinitions] = {
    ...featureDefinitions[key as keyof typeof featureDefinitions],
    ...features[key as keyof typeof features],
} as any  // ⚠️ Type safety bypassed with 'as any'

The as any cast bypasses TypeScript's type checking, which could hide type mismatches.

Recommendation:

• Use proper typing or at least add a comment explaining why as any is necessary
• Consider using a more specific type assertion if possible

4. Projection Node System Complexity

Location: packages/motion-dom/src/projection/node/*

The entire projection node system (9 commits worth of changes) has been moved to motion-dom. This system has complex React dependencies that may not be fully abstracted:

• Uses React context concepts (PresenceContext, LayoutGroupContext)
• Has lifecycle hooks tied to React rendering
• The comment in the first commit mentions "complex React deps" for projection nodes

Concern: While the types have been abstracted, the projection system may still have implicit assumptions about React's rendering model. This could cause issues when used with other frameworks.

Recommendation:

• Add integration tests for projection animations in non-React scenarios
• Document any React-specific assumptions in the projection system
• Consider whether the projection system needs a framework adapter pattern

5. Performance: Repeated Feature Definition Lookups

Location: packages/motion-dom/src/render/VisualElement.ts:551-585

updateFeatures() {
    let key: keyof typeof featureDefinitions = "animation"
    
    for (key in featureDefinitions) {
        const featureDefinition = featureDefinitions[key]
        // ... checks and instantiation
    }
}

updateFeatures() is called on every prop update. With the new architecture, this iterates through the featureDefinitions object repeatedly. For components with frequent updates, this could add overhead.

Recommendation:

• Profile performance in high-frequency update scenarios (e.g., drag, scroll)
• Consider caching the list of active features if performance degrades
• Document the performance characteristics in CLAUDE.md

6. Inconsistent Export Pattern

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

Some exports use individual named exports while others use barrel exports:

export { VisualElement, setFeatureDefinitions, getFeatureDefinitions } from "./render/VisualElement"
export type { MotionStyle } from "./render/VisualElement"
export { Feature } from "./render/Feature"
export * from "./render/types"  // barrel export

Recommendation:

• Be consistent with export patterns for better tree-shaking
• Prefer named exports over export * where possible

---

🔍 Code Quality

1. ✅ Good: Follows the repository's code style (interfaces, arrow functions, named exports)
2. ✅ Good: Proper use of TypeScript types throughout
3. ✅ Good: No var declarations or loose equality
4. ✅ Good: Deleted test files were properly moved to motion-dom

---

🔒 Security

No security concerns identified. This is primarily an internal refactoring without changes to external-facing APIs or security-sensitive code.

---

📝 Documentation

Missing:

• No updates to CLAUDE.md explaining the new architecture or initialization requirements
• No migration guide for users who may have been importing internal APIs
• No documentation of the feature definition system in motion-dom

Recommendation:

• Add a section to CLAUDE.md explaining:
  • The motion-dom/framer-motion separation
  • How the feature definition system works
  • When/how initialization happens
  • How to extend VisualElement in other frameworks

---

🧪 Testing

✅ Strengths:

• All existing tests pass (703 unit tests, 166 e2e tests)
• Tests properly updated to import from motion-dom

⚠️ Gaps:

• No new tests for the feature definition initialization system
• No tests for error cases (e.g., using VisualElement before initialization)
• Missing tests for motion-dom as a standalone package (without framer-motion)

Recommendation:

• Add test in framer-motion that verifies feature definitions are initialized
• Add test in motion-dom that verifies VisualElement works with manually set feature definitions
• Consider adding integration tests that import only motion-dom

---

📊 Summary

Category	Rating	Notes
Architecture	⭐⭐⭐⭐	Good separation of concerns, but initialization pattern needs work
Code Quality	⭐⭐⭐⭐⭐	Clean, well-structured code following conventions
Testing	⭐⭐⭐⭐	Comprehensive existing tests, but missing tests for new initialization pattern
Performance	⭐⭐⭐⭐	No obvious issues, but should monitor feature lookup overhead
Security	⭐⭐⭐⭐⭐	No concerns
Documentation	⭐⭐⭐	Needs documentation of new architecture

Overall: ✅ Approve with recommendations

This is a solid refactoring that achieves its goal of making VisualElement framework-agnostic. The main concern is the feature definition initialization pattern, which should be hardened before merging. Consider addressing the high-priority issue around race conditions and adding explicit initialization calls.

---

🎯 Action Items

Before Merge (High Priority):

1. Add explicit initFeatureDefinitions() call in framer-motion entry point
2. Add runtime validation in VisualElement.updateFeatures() for empty definitions
3. Add tests for feature definition initialization
4. Update CLAUDE.md with new architecture documentation

After Merge (Medium Priority):

1. Add integration tests for motion-dom standalone usage
2. Profile performance of feature lookups in high-frequency scenarios
3. Create migration guide if any internal APIs have changed
4. Consider adding a framework adapter pattern for projection system

Future Considerations:

1. Evaluate if the projection system needs further abstraction for non-React frameworks
2. Consider exposing more granular exports from motion-dom for tree-shaking
3. Document the plugin/feature architecture for third-party extensions

---

Great work on this significant refactoring! The code is well-structured and the approach is sound. With the initialization concerns addressed, this will be a solid foundation for framework-agnostic animations. 🚀