Replace VisualElement value management and renderer with effects#3749
Replace VisualElement value management and renderer with effects#3749mattgperry wants to merge 4 commits into
Conversation
Slots (transform, transformOrigin, stroke-dasharray) are now composed from ordered contributor chains via state.contribute()/state.build(), giving projection, transformTemplate and pathRotation a single seam to compose rendered output instead of competing render sites. - MotionValueState.latest now stores raw values; value-type coercion moves to render/build sites so latest matches VisualElement's latestValues semantics - Canonical buildTransform now lives in effects/style/transform, shared by both the effects and VisualElement render pipelines; the render-side copy is reduced to a transformTemplate wrapper that preserves the public 3-arg signature - buildTransformOrigin extracted as a shared builder Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
remove() unsubscribes a value, deletes it from latest and re-renders any slot it contributed to - the semantics VisualElement.removeValue needs. The unsubscribe returned from set() deliberately keeps its existing behavior (latest persists for shared slots). Pins the transformTemplate pattern: the framework layer swaps the transform slot's base contributor for one that builds via the template-aware buildTransform wrapper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
VisualElement now binds motion values through the effects system (addStyleValue/addSVGValue) on a shared MotionValueState whose latest object is latestValues. The build()/renderState/renderInstance pipeline is removed, along with buildHTMLStyles, buildSVGAttrs, buildSVGPath, renderHTML and renderSVG. - Granular rendering: non-layout elements render only changed values/slots per frame instead of rebuilding every style - Projection-driven elements (layout/layoutId/drag) bind values as latest-tracking only and schedule full renders per change - projection output must compose after styles in a deterministic order, and the renderScheduledAt dedupe means granular renders could otherwise land after the projection render within a frame. Re-binds if a projection node arrives post-mount (lazy features) - No imperative render at bind: React owns initial styles; standalone styleEffect/svgEffect render initial values via state.scheduleRender(). This keeps deferred appear-handoff animations in control of transform - React/SSR initial styles built by shared buildStyles/buildSVGProps, replacing the duplicated render-side builders - renderValues()/bindValueToState() replace build()/renderInstance() per renderer; full sync renders flush every value in latest, including those set via setStaticValue during measurement - transformTemplate registers a template-aware base contributor on the transform slot - Projection writes corrected CSS vars via style.setProperty instead of renderState.vars - SVG: fill/opacity etc now render as styles rather than attributes (faster, attribute writes pass through the CSS cascade anyway, and inline styles win specificity deterministically). attr-prefixed state keys no longer collide with same-named transform values Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Greptile SummaryThis PR unifies Motion's two rendering pipelines by replacing
Confidence Score: 3/5Two functional regressions should be resolved before merging: SVG layout animations silently stop working, and a subscription leak occurs when layout features are lazy-loaded onto existing elements. The HTML rendering path and the MotionValueState slot model are well-constructed and thoroughly tested. Two issues exist in edge paths: SVGVisualElement.renderValues drops the projection parameter that renderSVG previously forwarded to applyProjectionStyles, which breaks layout/layoutId on SVG elements; and the lazy-load re-bind loop overwrites valueSubscriptions entries without calling the old cleanup, leaving stale value.on change closures that fire on every subsequent value change. SVGVisualElement.ts (missing projection call) and VisualElement.ts (re-bind loop around line 770) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Value Change] --> B{isProjectionDriven?}
B -- No --> C[bindValueToState per renderer]
B -- Yes --> D[state.set track-only]
C --> E[granular render per-value onChange]
D --> F[scheduleRender full render on change]
F --> G[renderValues]
E --> H[frame.render per slot]
G --> I{renderer type}
I -- HTML --> J[renderStyles + applyProjectionStyles]
I -- SVG --> K[renderSVGValues projection ignored]
I -- Object --> L[Object.assign state.latest]
H --> M{slot type}
M -- transform --> N[state.build transform]
M -- transformOrigin --> O[buildTransformOrigin]
M -- plain style --> P[element.style key = value]
Reviews (1): Last reviewed commit: "Replace VisualElement value management a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return addSVGValue(instance, state, key, value) | ||
| } | ||
|
|
||
| mount(instance: SVGElement) { | ||
| this.isSVGTag = isSVGTag(instance.tagName) | ||
| super.mount(instance) | ||
| renderValues( | ||
| instance: SVGElement, | ||
| state: MotionValueState, | ||
| _styleProp?: MotionStyle, | ||
| _projection?: any | ||
| ) { | ||
| renderSVGValues(instance, state) | ||
| } |
There was a problem hiding this comment.
projection?.applyProjectionStyles for SVG layout animations
The old renderSVG called renderHTML(..., projection) which forwarded the projection node to applyProjectionStyles. The new renderValues ignores _projection entirely, so SVG elements with layout or layoutId props will never have their projection transforms applied. Any layout animation on <motion.circle layout /> or similar SVG shapes will fail silently — the element won't animate into position.
| this.scheduleRender() | ||
| } | ||
|
|
||
| /** | ||
| * If this element has become projection-driven since values were | ||
| * bound (e.g. lazy-loaded layout features, or layout props toggled | ||
| * on), re-bind them so rendering defers to full renders that | ||
| * compose projection output. | ||
| */ | ||
| if ( | ||
| this.current && | ||
| !this.hasProjectionBindings && | ||
| this.isProjectionDriven() | ||
| ) { | ||
| this.hasProjectionBindings = true | ||
| this.values.forEach((value, key) => |
There was a problem hiding this comment.
removeOnChange subscriptions during lazy-load re-bind
When an element transitions from non-projection-driven to projection-driven (e.g., lazy-loading dom-animation or dynamically adding a layout prop), bindToMotionValue is called for every existing value. Inside, state.set() correctly cleans up the old state subscription, but the value.on("change", ...) subscription registered in the previous bindToMotionValue call is never removed — its cleanup is stored in valueSubscriptions but that map entry is silently overwritten rather than called before re-binding. After this transition, each motion value has two active "change" listeners (scheduleRender fires twice per change), and the old closure over this is permanently retained by the MotionValue, preventing GC until the MotionValue itself is destroyed. The fix is to call this.valueSubscriptions.get(key)?.() and delete the key before re-binding.
| // If this is an SVG element, we need to set the transform-box to fill-box | ||
| // to normalise the transform relative to the element's bounding box | ||
| if (!isHTML && !state.get("transformBox")) { |
There was a problem hiding this comment.
!isHTML targets SVG elements — the condition and its comment are inverted.
| // If this is an SVG element, we need to set the transform-box to fill-box | |
| // to normalise the transform relative to the element's bounding box | |
| if (!isHTML && !state.get("transformBox")) { | |
| // If this is an SVG element, we need to set transform-box to fill-box | |
| // to normalise the transform relative to the element's bounding box | |
| if (!isHTML && !state.get("transformBox")) { |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…functions
The SVG pipeline shipped two routing implementations: addSVGValue (granular
bind-time closures) and renderSVGValues (full renders for projection-driven
elements and measurement flows), with a third partial copy in the transform
slot closures. Routing, attribute writing, transform/fill-box/origin
defaults and dasharray building are now single implementations
(renderSVGValue, renderAttrValue, renderTransform, renderTransformOrigin,
buildDasharray, renderPathOffset) dispatched from both paths.
Merging surfaced two latent routing bugs in the granular path, both fixed
with regression tests:
- pathRotation (a transform prop) was captured by the path* check and wired
into stroke-dasharray, setting pathLength="1" and dropping its rotation
from the transform. Transform/origin checks now run before path checks in
both renderers.
- CSS variables on SVG elements fell through to setAttribute("--foo"),
an invalid XML attribute name. They now route to style.setProperty,
matching the old pipeline and the initial-render builders.
SVG-ness is passed explicitly through addSVGValue/renderSVGValues/
addTransformSlot rather than duck-typed per render: renderer choice is by
tag, and an SVG tag rendered outside an <svg> root lives in the HTML
namespace where instance checks misreport it. The bind-time transformBox
MotionValue is gone; fill-box now renders with the transform slot, gated on
a user-provided transformBox in latest, making it order-independent.
Bundle size (gzipped): motion -158 B, animate -155 B, dom-animation -166 B,
dom-max -155 B (incl. assets chunks); m unchanged; standalone style-effect
+39 B from the shared fallback path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Summary
Unifies Motion's two rendering pipelines.
VisualElement(used by<motion.div />andanimate()) now manages and renders motion values through the effects system (addStyleValue/addSVGValue/MotionValueState) instead of its ownbuild()/renderState/renderInstance()pipeline. Anyone mixing motion components withstyleEffect/svgEffectno longer ships both implementations, and non-layout elements gain granular rendering: animating a subset of values only writes that subset per frame, instead of rebuilding every style.Stage 1 — slot/contributor model (
MotionValueState)transform,transformOrigin,stroke-dasharray) are built from ordered contributor chains viastate.contribute(name, index, builder)/state.build(name). Contributors can wrap or override previous output (base builder → projection override;transformTemplatehandled inside base builders). Signals-shaped, but explicit and ordered — no auto-tracking runtime, batching/glitch-freedom comes from the existing frameloop.state.latestnow stores raw values; value-type coercion happens at write sites, matchinglatestValuessemantics.buildTransform/buildTransformOriginare now single canonical builders shared by every consumer; pathRotation composition is down from 3 sites to 2 (the remaining one inbuildProjectionTransformis genuinely different maths). The public 3-argbuildTransformsignature is preserved via a thin template wrapper.Stage 2 — removal semantics
state.remove(name)unsubscribes, deletes fromlatest, and re-renders the owning slot — the contractVisualElement.removeValuerequires. The unsubscribe returned fromset()deliberately keeps its existing semantics for standalone effects.Stage 3 — the swap
bindValueToState()/renderValues()per renderer replacebuild()/renderInstance(). Full synchronous renders flush everything inlatest, including values set viasetStaticValueduring measurement flows.buildHTMLStyles,buildSVGAttrs,buildSVGPath,renderHTML,renderSVG, render-sidebuild-transform, and allrenderStateusage.buildStyles(HTML) andbuildSVGProps(SVG) builders. SSR output is byte-identical to before (vars-first property ordering preserved).style.setPropertyinstead ofrenderState.vars.Rendering semantics
layout,layoutId, or drag'salwaysMeasureLayout): values bind as latest-tracking only and schedule full renders per change — exactly the old pipeline's behavior. Projection output must compose after styles in deterministic order; therenderScheduledAtdedupe means a granular render could otherwise land after the projection render within a frame (caught by thedefer-handoff-layoutappear fixtures). Note: a projection node exists for every element once layout features load, so this is gated on projection options, not existence. Elements re-bind if they become projection-driven post-mount (lazy-loaded features).styleEffect/svgEffectrender initial values via an explicitstate.scheduleRender()increateEffect.Behavior changes
fill/opacity/widthnow render as styles rather than attributes where the browser supports them (key in element.style). Equal-or-faster (presentation attributes pass through the CSS cascade anyway, plus attribute-mutation overhead) and inline styles win specificity deterministically. Initial React render still emits attributes viabuildSVGProps; styles take priority once animating.attrX) are now tracked under their prefixed key, fixing a latent collision with same-named transform values insvgEffect.buildHTMLStyles,buildSVGAttrs,buildSVGPath,renderHTML,renderSVG(replaced bybuildStyles,buildSVGProps).VisualElement:build(),renderInstance(),triggerBuild(),removeValueFromRenderState()removed;state,bindValueToState(),renderValues()added.Bundle size (gzipped, vs main)
The adds are the slot mechanism plus SVG temporarily carrying both granular and full renderers — best follow-up trim is merging those. The structural win is the single pipeline (no double-ship with effects) and granular rendering.
Test plan
buildStyles/buildSVGProps(ported + extended), SVG transformBox granular path🤖 Generated with Claude Code