motiondivision / motiondivision/motion
[BUG] Motion component is using incorrect motion value in animation after updating injected motion value reference
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 33.7k
- Forks
- 1.4k
- Avg merge
- 1d 10h
- Merged PRs (30d)
- 14
Description
1. Read the FAQs 👇
Done, not applicable to this issue.
2. Describe the bug
- I have a scenario where I am injecting a motion value
x1into one motion componentbox1, and a transformed motion valuex2into another motion componentbox2 - At a certain condition I would like to swap which of the components that receives which of the motion values. E.g.
x1will be injected intobox2, whilex2is injected intobox1. - The problem that now arises is that
box2is seemingly still subscribing to and using values fromx1. Which in turn causesbox2to be positioned incorrectly. - This issue seems to only appear when the motion values are animated.
3. IMPORTANT: Provide a CodeSandbox reproduction of the bug
Example in CodeSandbox
4. Steps to reproduce
Steps to reproduce the behavior:
- Go to Example in CodeSandbox
- Wait approximately 3 seconds for the steps to complete
- See that
box2is not transformed and positioned as expected in the example with animation, which should be at x = 0 - See the expected positions in the example that is without animation
5. Expected behavior
I expect the boxes in the animated example to position themselves, along the x axis, in the same manner as the example without animation.
6. Video or screenshots
Not applicable, see the example.
7. Environment details
Windows 11, Chrome 114.0.5735.199 (Official Build) (64-bit)
Potential issue and potential solution
To me it seems that this issue happens because of the following events:
VisualElement.addValueis first called, which callsVisualElement.bindToMotionValue, which is creating a subscription to the motion value'schangeevent;- Afterwards the
VisualElement.mountis called which also callsbindToMotionValueand creates a second subscription to the motion value'schangeevent.
Problem now is that there is two subscriptions for the same key, and it is not necessarily that both subscriptions are to the same motion value.
addValue avoids this issue by checking if there is an existing value for the same key and removes it, but mount does not have a similar check.
I was planning on submitting a pull request with the following changes, but the changes caused some of the tests to fail, so I concluded that I am likely missing something in the bigger picture:
diff --git a/packages/framer-motion/src/render/VisualElement.ts b/packages/framer-motion/src/render/VisualElement.ts
index 32485e2c..5a1ca2ff 100644
--- a/packages/framer-motion/src/render/VisualElement.ts
+++ b/packages/framer-motion/src/render/VisualElement.ts
@@ -403,7 +403,11 @@ export abstract class VisualElement<
this.removeFromVariantTree = this.parent.addVariantChild(this)
}
- this.values.forEach((value, key) => this.bindToMotionValue(key, value))
+ this.values.forEach((value, key) => {
+ // Remove existing value subscription before adding a new binding.
+ this.unbindFromMotionValue(key)
+ this.bindToMotionValue(key, value)
+ })
if (!hasReducedMotionListener.current) {
initPrefersReducedMotion()
@@ -471,6 +475,14 @@ export abstract class VisualElement<
})
}
+ private unbindFromMotionValue(key: string) {
+ const unsubscribe = this.valueSubscriptions.get(key)
+ if (unsubscribe) {
+ unsubscribe()
+ this.valueSubscriptions.delete(key)
+ }
+ }
+
sortNodePosition(other: VisualElement<Instance>) {
/**
* If these nodes aren't even of the same type we can't compare their depth.
@@ -779,11 +791,7 @@ export abstract class VisualElement<
*/
removeValue(key: string) {
this.values.delete(key)
- const unsubscribe = this.valueSubscriptions.get(key)
- if (unsubscribe) {
- unsubscribe()
- this.valueSubscriptions.delete(key)
- }
+ this.unbindFromMotionValue(key)
delete this.latestValues[key]
this.removeValueFromRenderState(key, this.renderState)
}
Potential workaround
Update the key in motion.div when swapping the motion values, to destroy the existing motion component with the subscription to the old value, and create a fresh new one that only has a subscription to the newly injected motion value.
See the commented out key property the motion.div at SetupExample.tsx in the CodeSandbox example.
This workaround may work for some, but is not a good workaround in my case as it will recreate all child elements and cause a "flash of content" that is visible to the user.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the linked CodeSandbox reproduction, then read packages/framer-motion/src/render/VisualElement.ts around mount, addValue, bindToMotionValue, and removeValue. Compare the animated and non-animated examples, and use SetupExample.tsx to understand the motion.div value swap. Done means the animated boxes reach the same x-axis positions without recreating the component, with the relevant tests passing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react, typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100