framer / motion

Open source, production-ready animation and gesture library for React
https://framer.com/motion
MIT License
22.57k stars 749 forks source link

[BUG] React.memo breaks layout projection #1187

Closed rijk closed 2 years ago

rijk commented 3 years ago

CodeSandbox reproduction of the bug https://codesandbox.io/s/framer-motion-layout-transform-reactmemo-childs-shifting-issue-5b99t?file=/src/Test.tsx

Steps to reproduce

Steps to reproduce are in the Sandbox.

Expected behavior

Blue squares stay in place relative to the white div; it actually makes no sense (to me) they get a transform as the whole <Project /> element never changes layout, only its parents (main and main > div) do.

Actual behavior

When expanding the menu after an internal state change in <Project>, the blue squares shift to the right, outside of their parent.

The central problem seems to be that after a part tree render (a state change inside a React.memo() component), the projection gets messed up.

Maybe layout just can't work in combination with React.memo(), but this would be a quite serious limitation. It would be great if we could find a way to solve this in this case; as I said the childs just don't need any transforms so if framer-motion just leaves this part of the tree alone that would solve the issue.

If not possible, then I think it would be good to at least document this requirement of not using React.memo() in your tree.

rijk commented 2 years ago

Thinking about this some more, I do think it can be considered a bug. The whole point of motion is that it works outside of the React render loop, right? So it shouldn't matter that a component is not rerendered by React. Maybe something to consider with the projection rewrite @mattgperry ?

rijk commented 2 years ago

It seems better in 5.0.0: https://codesandbox.io/s/framer-motion-layout-transform-reactmemo-childs-shifting-issue-on-v500-bqpuw?file=/src/Project.tsx

Only issue now is layout animation completely breaks for the boxes when a state update happens while opening the menu (from then on, they always jump to their end position, regardless of state updates):

https://user-images.githubusercontent.com/159500/125606533-a932f2ae-7a89-46b5-8e19-e9653c79d2b5.mov

mattgperry commented 2 years ago

Hmm that's odd - the shared layout stuff is WIP, but I wonder if memo is involved here still. There's a component called LayoutGroup that I haven't exported yet, I wonder if that will help notify these memo'd children that there's been a layout update.

mattgperry commented 2 years ago

Ah yeah that seemed to fix it although the transitions might need syncing https://codesandbox.io/s/framer-motion-layout-transform-reactmemo-childs-shifting-issue-on-v500-forked-11yjx?file=/src/Test.tsx

mattgperry commented 2 years ago

I'm not sure what the opacity dips are about but we can look into that

rijk commented 2 years ago

Ah yeah that seemed to fix it although the transitions might need syncing https://codesandbox.io/s/framer-motion-layout-transform-reactmemo-childs-shifting-issue-on-v500-forked-11yjx?file=/src/Test.tsx

Very cool. I am using a workaround for now, where I just force update components when I know their position has changed. Would be nice if this happens automatically. What does the LayoutGroup do exactly, is it something like I suggested in #1027?

Also, ideally the blue squares don't transform at all; just transforming the parent <main> in this case should be enough. Not sure if this is possible, but should be more performant too I would guess. (The blue squares don't have layout activated by the way, but have a layoutId for a shared transition into a modal, which activates an implicit layout projection which I talked about in #845.)

I'm not sure what the opacity dips are about but we can look into that

Thanks, yeah I've seen the opacity dips in other tests as well, but didn't want to bother with it as I know you are still working on it and there is no PR yet.

Here is an example: https://codesandbox.io/s/framer-motion-nested-layout-transform-border-radius-flicker-on-v500-j88qo. Issues:

  1. Opacity dip
  2. No crossfade (exiting item disappears immediately) -> maybe this is the reason for the opacity dip by the way. When I tried using AnimatePresence the whole shared layout animation stopped working altogether
  3. MotionConfig transition ignored for layout animations

Unrelated:

  1. TypeError: undefined is not an object (evaluating 'box.x') when editing the Sandbox
rijk commented 2 years ago

Also, when you slow down the transition you see the border radius is animating from 0 as well:

image

https://user-images.githubusercontent.com/159500/125622907-6a3916e3-4d7f-469e-bc04-6e64f5942e4f.mov

This happens regardless of state updates: https://codesandbox.io/s/framer-motion-layout-transform-reactmemo-childs-shifting-issue-on-v500-forked-fr1qd?file=/src/Test.tsx

mattgperry commented 2 years ago

The LayoutGroup component does a similar job to part of AnimateSharedLayout's previous role - marking a collection of elements that may affect each other's layout. But now instead of force-rendering all those elements they will just all get marked for layout diffing if any one of them renders.

rijk commented 2 years ago

Ok, got it, but to be clear adding the LayoutGroup did not fix the issue, but rather introduced a new one.

The behaviour without LayoutGroup was correct (no transform applied to the .step DOM elements), until a state change happens during a layout animation. At that point Framer Motion gets 'confused' somehow.

I saw a similar thing in the border radius and opacity animation in https://codesandbox.io/s/framer-motion-border-radius-layout-circle-issue-on-v500-ugyso?file=/src/Modal.tsx:458-590, although the layout animation itself is correct there.

rijk commented 2 years ago

On alpha.13 we're back to square 1 (Sandbox):

https://user-images.githubusercontent.com/159500/126905083-62713aca-e0a3-4da6-b788-9a1f321ee808.mov

rijk commented 2 years ago

This is on 5.0.0-beta.22 (Sandbox):

https://user-images.githubusercontent.com/159500/131312613-accbece7-ca4e-41be-a4cc-80951283bfc2.mov

rijk commented 2 years ago

Fixed in 5.0.0-beta.24.