Make AnimatePresence CM-safe

[Andarist]

This is just an experiment right now - but Jest's tests pass, so I'm optimistic 😅

The logic within AnimatePresence here is very delicate and my changes affect the timing of refs updates so I need to rethink all scenarios to check if this code is functionally identical to the previous version. If you spot something that might not work properly - let me know.

[Andarist]

We can only update isInitialRender in the effect because render might be fired multiple times. Until this flag is updated (first effect ever) we know we can trust "incoming children" as nothing has been actually committed to the screen yet - that's why we can just early return based on that.

The key to making things CM-safe is to defer any render-related updates to effects and compute render output based on the same values that were previously committed (and props) - we can't manipulate refs in render, at all costs. We need to prepare new values for refs though (so they can be committed in an effect).

What mutable refs we had previously here?

• exiting
• presentChildren
• allChildren

Let's take a closer look to each one:

exiting

This was used to re-insert exiting nodes into the returned elements and to keep track of when we need to force rerender to clear "ghost" DOM nodes in a single batch.

To improve the situation around this I've gone with a route of recomputing this for each render - to compute what's currently exiting we need 2 things: incoming children and currently rendered ones, exiting children can be fully derived from those 2 things. By having this computed in the closure itself we can still track if all exiting children have been exited. We only need to remove them from a mutable presentChildren (those that are rendered) and check if the exiting.size is now 0 to schedule an update in React to clear those "ghosts" all at once.

presentChildren

This is a tricky one - this represents what is being rendered on the screen but yet we use a mutable value for this 😱 It operates on the assumption that it's safe to return different result from render based on the same props. This sounds terrible.

But maybe in this very case it's not? What we care about is a synchronization to the final state of things - in the end we only want to have the same children in the DOM tree that we got in the props. Even if a child exits in the middle of rerendering it seems OK to remove it from a mutable presentChildren (it was only put there because React doesn't support exit animations etc). This only happens if React rerenders in between of commits in such a flow: render-commit-exiting-render-commit. It seems fine to just tell React that something has disappeared from here even if without render that got in between of commits it would be notified about this during render caused by "onExitAll".

This is quite convoluted and I had to rethink this using some more visuals - you might find it helpful as well.

In here we have 3 components: A B C and first we exit, later B. Exiting components are marked with '.

children A  B  C
render   A  B  C
commit   A  B  C

children A  B
render   A  B  C'
commit   A  B  C'

children A
render   A  B' C'
exit           C' this sees B' C' exiting and removes C' from that set
                  but it also removes it from *mutable* presentChildren

render   A  B'    rerender for whatever Reason (e.g. unrelated state update)
                  presentChildren had no chance to be committed yet in the effect
                  but as it was mutated we have computed A B' and this is what React sees
                  it's not much different from React's perspective than a change caused by props being different
                  (although it can know that props have not changed at times)
                  C' still stays on the screen though (up until React removes it)

commit   A  B'    presentChildren are committed here, C' node has been removed by React ✅

Q: Can Framer Motion handle gracefully a "ghost" DOM node that reenters? Even though a "ghost" element gets removed from presentChildren and thus it's not returned by React from a given render it still exists in the DOM and might reappear in the incoming props.children before the DOM node gets actually removed from both the DOM and React's internal structures (latest committed children).

allChildren

This basically stays untouched - only we update this in an effect now. filteredChildren are source of this (because we need to keep original children here and not the wrapped ones) and this should sync correctly without memory leaks as far as I can tell.

---

I expect this might not work properly with React's capabilities to commit higher priority updates first even if a lower priority render has been already computed, rebasing of those updates etc. My mind kinda melts when I think about those - but there is also a lot of unknown regarding those capabilities because they are marketed even less than time-slicing. There is also a chance that this would actually work with those capabilities - I'm not sure. After all, we actually don't want to render those "ghost" children at all - we only try to optimize stuff here by batching final removal or piggybacking on React's rerenders in the middle of the flow.

[vincentriemer]

Hey there, Matt wanted me to chime in because I've been looking into this sort of thing recently.

So I've been working on a version of AnimatePresence for use at FB and since we are all-in on concurrent mode I needed to ensure its compatibility with it. The biggest rule when ensuring concurrent mode compatibility is to never modify mutable data (i.e. refs) in render, since we can't guarantee that any one render will eventually be committed and we can't guarantee the renders will happen in a certain order. A component's render — with a given props and state — should (for the most part) always return the same thing. If there are side-effects that alter mutable data, they should be only done in events or effects.

The approach I ended up taking has a lot of the same logic that already exists in FM's AnimatePresence implementation but the biggest difference is that instead of refs to store presentChildren, allChildren, and exiting — I store them in state. We still need to do the diffing and such as the props change to update the state, and based on what I mentioned above my first instinct would be do perform those changes in an effect. That isn't ideal because if we update state in a useLayoutEffect this causes React to perform a synchronous re-render (i.e. we lose all of the performance benefits of CM).

The actual way to accomplish this is to implement the getDerivedStateFromProps pattern — which in a functional/hooks based component means to actually update state directly in render (https://reactjs.org/docs/hooks-faq.html#how-do-i-implement-getderivedstatefromprops). In my implementation I actually built it as a class component and used getDerivedStateFromProps because, personally, I found it easier to reason about but there is nothing inherently better or worse in either approach.

I hope that this explanation helps and if anything is still unclear I'd be happy to answer any questions you have.

[Andarist]

The biggest rule when ensuring concurrent mode compatibility is to never modify mutable data (i.e. refs) in render

This has been done here - refs are only read in the render, which is also frowned upon but it has been done with extra care (happy to be proven that I've missed some scenarios in which this still ain't safe, this is very tricky to get right so I fully expect that I could have missed something).

The actual way to accomplish this is to implement the getDerivedStateFromProps pattern

This was an approach used by Motion's predecessor - React Pose. So I'm in a sense familiar with it - I just haven't wanted to refactor too much here, especially given what you mention: this pattern with hooks is awkward. We could consider it here - probably even refactoring to a class component like you have done, but things get complicated with this approach because of the fact that Motion tries to only remove exiting components from the "state" when all exiting components actually have exited. So it needs to maintain some mutable copy of that "state" to do that. It also has to maintain its state across rerenders (or just be able to recompute what has been mutated in subsequent renders). This happens here:
https://github.com/framer/motion/blob/0f867960ada7f36c85fb97cd1b11276b788f6da1/src/components/AnimatePresence/index.tsx#L213-L229

exiting is just a plain scope variable and its used for keeping track if all exiting components have been removed, whereas presentChildren is also mutated because:

• if no render happens in between then finally the forced update will happen and React will use that updated presentChildren to actually remove exited components in the next commit
• if render interrupts the process then we just piggyback on React's new render, it won't see the exited component in presentChildren, and thus it will also remove it in the next commit

I hope that this explanation helps and if anything is still unclear I'd be happy to answer any questions you have.

Do u know of any good way to test this stuff? We need to be able to test this with great control over rerenders, handlers, commits - using act doesn't solve this because it waits for things to be flushed and we'd like to actually test different scenarios when things are called in edge case orders.