framer / motion

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

Fix AnimatePresence won't unmount fastly changing content #1569

Closed JaeSeoKim closed 1 year ago

JaeSeoKim commented 1 year ago

Description

exiting object is not enclosed for useRef() as shown below

-   const exiting = new Set<ComponentKey>()
+   const exiting = useRef(new Set<ComponentKey>()).current

Because the object exiting is a new object each time it is re-rendered, the onExit event validates the Defer re-rendering untill all exiting child have indented left incorrectly.

const onExit = () => {
    allChildren.delete(key)
    exiting.delete(key)

    // Remove this child from the present children
    const removeIndex = presentChildren.current.findIndex(
        (presentChild) => presentChild.key === key
    )
    presentChildren.current.splice(removeIndex, 1)

    // Defer re-rendering until all exiting children have indeed left
    if (!exiting.size) {
        presentChildren.current = filteredChildren

        if (isMounted.current === false) return

        forceRender()
        onExitComplete && onExitComplete()
    }
}

2022-06-11 update

A bug occurred because the onExit function is newly defined each time when a fastly outgoing elements unmount instantly.

To solve this problem, solved it by memoizing exitingChild.

// If the component was exiting, reuse the previous component to preserve state
let extingChild = exitingChildren.get(key)
if (extingChild) return extingChild

const onExit = () => {
    ...
}

extingChild = (
    <PresenceChild
        key={key}
        isPresent={false}
        onExitComplete={onExit}
        custom={custom}
        presenceAffectsLayout={presenceAffectsLayout}
    >
        {child}
    </PresenceChild>
)
exitingChildren.set(key, extingChild)
return extingChild

But when I solved this problem, I saw a different problem.

The rendering array will have a strange change position.

Generally, if a change is made from ["A", "D", "E", "F", "B", "C"] to ["1", "A", "2", "B", "3", "C"] during animation, it is expected to be ["1" ,"A" ,"D" ,"E" ,"F" ,"2" ,"B" ,"3" ,"C"]

It does not work as intended and is output in the form ["1", "D", "E", "F", "A", "2", "B", "3", "C"].

// unnatural position
const insertionIndex = presentKeys.indexOf(key)

const onExit = () => {
  ...
}

// If the current array is smaller than "insertionIndex", it will not be inserted in the intended location
childrenToRender.splice(
    insertionIndex,
    0,
    <PresenceChild
      ...
    </PresenceChild>
)

If a change occurs in the rendering array, insert the chunk where the change occurred in the previous location.

 presentChildren  ->  children
     [A]                 [1]
     [D]                 [A]
     [E]                 [2]
     [F]                 [B]
     [B]                 [3]
     [C]                 [C]

  init ->  animate -> Exit Complete

             [1]        [1]     <--- targetChunk - 1
   [A]       [A]        [A]     <--- preservingKey
   [D]       [D]
   [E]       [E]                <--- presentChunk - 1
   [F]       [F]
             [2]        [2]     <--- targetChunk - 2
   [B]       [B]        [B]     <--- preservingKey
             [3]        [3]     <--- targetChunk - 3
   [C]       [C]        [C]     <--- preservingKey

Demo

original
https://user-images.githubusercontent.com/48559454/173114437-7111d473-8626-404e-b2a3-d0bb7ea10125.mov
fixed:

https://user-images.githubusercontent.com/48559454/173112041-f4996ce6-be01-4b11-989f-1fa74952a9e6.mov

original
https://user-images.githubusercontent.com/48559454/173114631-cab937ef-da5f-463a-954f-583d8097691c.mov
fixed:

https://user-images.githubusercontent.com/48559454/173112065-e8eafab0-90d7-4155-918a-c1d8082aed98.mov

fixed: #907 fixed: #1439 fixed: #1534

JaeSeoKim commented 1 year ago

Remove unneccery assignment code

because it has no meaning. childrenToRender = [...childrenToRender]

image

mattgperry commented 1 year ago

Hey thanks for taking a look at this! However I think this approach replaces the bug with another - fastly outgoing elements unmount instantly, rather than waiting for their animation to finish.

JaeSeoKim commented 1 year ago

@mattgperry I fixed it again for that problem. Can I review it again?

mattgperry commented 1 year ago

@JaeSeoKim thanks for taking another look at this. Are these sandboxes updates with the latest fixes?

JaeSeoKim commented 1 year ago

@mattgperry I updated the sandbox samples.

I would like to contribute after hearing feedback on additional problems or improvements, but I think it will be difficult to contribute additionally as I will join the army from the 27th due to the military service obligation of the Republic of Korea.

JaeSeoKim commented 1 year ago

@mattgperry Now, I can work after work. Is there anything that needs to be updated or improved?(Even though the development environment is not good 😂)

JaeSeoKim commented 1 year ago

Origin : https://codesandbox.io/s/bold-shannon-3u5zqd?file=/src/App.tsx Fixed: https://codesandbox.io/s/weathered-star-9keswr?file=/src/App.tsx

avkvak commented 1 year ago

@JaeSeoKim Thank you for this fix! Is there any chances to review/merge it?

utileetdulce commented 1 year ago

@JaeSeoKim Why did you close this pull request? @maintainers, what are your plans concerning this issue ?

utileetdulce commented 1 year ago

@mattgperry I have tested @JaeSeoKim commit 29c4a5bd and even wrote a test but don't understand what you meant with your comment concerning missing animations. Shall I create a pull request including the test i wrote or can you explain under what conditions "fastly outgoing elements unmount instantly, rather than waiting for their animation to finish"?

JaeSeoKim commented 1 year ago

@utileetdulce I don't have enough time to work because of my mandatory military service in South Korea. and two fixes are included in one PR and i couldn't write the test codes, so i closed it with the intention of uploading it again later.

A problem with commit 29c4a5bdc1367910271b1efe2aec68c1a9491ba4 occurs when you wrap PresenceChild for the exiting component.

Because onExit objects are newly declared at every rendering, React detects changes in Props and treats them as different components. This means that the previously rendered onExit is operated by componentdidmount and is immediately deleted from the rendering tree. So I wrote a code that memorizes and solves the component through allChildren.

https://github.com/framer/motion/blob/main/packages/framer-motion/src/components/AnimatePresence/index.tsx#L180-L224

rafaelrcamargo commented 1 year ago

Hey there,

I was actually having the same issue with AnimatePresence not removing elements properly when multiple elements were removed in a short amount of time. However, I was able to fix the issue by using the usePresence hook from the Framer Motion documentation.

The usePresence hook provides a way to animate elements that are entering and exiting a component. It works by keeping track of the presence of the element using a boolean value, and then triggering an animation when the element enters or exits the component.

By using usePresence in conjunction with AnimatePresence, I was able to ensure that elements were properly removed even when multiple elements were removed in a short amount of time.

I highly recommend checking out the Framer Motion documentation on usePresence for more information and examples: https://www.framer.com/motion/animate-presence/#usepresence

Hope this helps!