From d0867d7dac54263b9fd780f991999797ce7b0a11 Mon Sep 17 00:00:00 2001 From: Sean Matheson Date: Tue, 6 Oct 2020 22:05:10 +0800 Subject: [PATCH 1/2] Adds an unmount check to AnimatePresence prior to animation exit callback --- .../__tests__/AnimatePresence.test.tsx | 544 +++++++++--------- src/components/AnimatePresence/index.tsx | 12 + 2 files changed, 268 insertions(+), 288 deletions(-) diff --git a/src/components/AnimatePresence/__tests__/AnimatePresence.test.tsx b/src/components/AnimatePresence/__tests__/AnimatePresence.test.tsx index 3694c1bc89..f533d80c60 100644 --- a/src/components/AnimatePresence/__tests__/AnimatePresence.test.tsx +++ b/src/components/AnimatePresence/__tests__/AnimatePresence.test.tsx @@ -1,11 +1,12 @@ import { render } from "../../../../jest.setup" import * as React from "react" +import { act } from "react-dom/test-utils" import { AnimatePresence, motion } from "../../.." import { motionValue } from "../../../value" describe("AnimatePresence", () => { test("Allows initial animation if no `initial` prop defined", async () => { - const promise = new Promise(resolve => { + const promise = new Promise((resolve) => { const x = motionValue(0) const Component = () => { setTimeout(() => resolve(x.get()), 75) @@ -30,7 +31,7 @@ describe("AnimatePresence", () => { }) test("Suppresses initial animation if `initial={false}`", async () => { - const promise = new Promise(resolve => { + const promise = new Promise((resolve) => { const Component = () => { return ( @@ -58,45 +59,50 @@ describe("AnimatePresence", () => { }) test("Animates out a component when its removed", async () => { - const promise = new Promise(resolve => { - const opacity = motionValue(1) - const Component = ({ isVisible }: { isVisible: boolean }) => { - return ( - - {isVisible && ( - - )} - - ) - } + const opacity = motionValue(1) - const { container, rerender } = render() - rerender() - rerender() + const Component = ({ isVisible }: { isVisible: boolean }) => { + return ( + + {isVisible && ( + + )} + + ) + } + + const { container, rerender } = render() + + rerender() + + await act(async () => { rerender() + }) - // Check it's animating out - setTimeout(() => { - expect(opacity.get()).not.toBe(1) - expect(opacity.get()).not.toBe(0) - }, 50) + await act(async () => { + await new Promise((resolve) => { + // Check it's animating out + setTimeout(() => { + expect(opacity.get()).not.toBe(1) + expect(opacity.get()).not.toBe(0) + }, 50) - // Check it's gone - setTimeout(() => { - resolve(container.firstChild as Element | null) - }, 150) + // Resolve after the animation is expected to have completed + setTimeout(() => { + resolve() + }, 150) + }) }) - const child = await promise - expect(child).toBeFalsy() + expect(container.firstChild).toBeFalsy() }) test("Animates a component back in if it's re-added before animating out", async () => { - const promise = new Promise(resolve => { + const promise = new Promise((resolve) => { const Component = ({ isVisible }: { isVisible: boolean }) => { return ( @@ -134,49 +140,50 @@ describe("AnimatePresence", () => { }) test("Animates a component out after having an animation cancelled", async () => { - const promise = new Promise(resolve => { - const opacity = motionValue(1) - const Component = ({ isVisible }: { isVisible: boolean }) => { - return ( - - {isVisible && ( - - )} - - ) - } - - const { container, rerender } = render() - rerender() - rerender() - rerender() - rerender() - rerender() - rerender() - rerender() + const opacity = motionValue(1) + const Component = ({ isVisible }: { isVisible: boolean }) => { + return ( + + {isVisible && ( + + )} + + ) + } - // Check it's animating out - setTimeout(() => { - expect(opacity.get()).not.toBe(1) - expect(opacity.get()).not.toBe(0) - }, 50) + const { container, rerender } = render() + rerender() + rerender() + rerender() + rerender() + rerender() + rerender() + rerender() + + await act(async () => { + await new Promise((resolve) => { + // Check it's animating out + setTimeout(() => { + expect(opacity.get()).not.toBe(1) + expect(opacity.get()).not.toBe(0) + }, 50) - // Check it's gone - setTimeout(() => { - resolve(container.firstChild as Element | null) - }, 300) + // Resolve after the animation is expected to have completed + setTimeout(() => { + resolve() + }, 300) + }) }) - const child = await promise - expect(child).toBeFalsy() + expect(container.firstChild).toBeFalsy() }) test("Can cycle through multiple components", async () => { - const promise = new Promise(resolve => { + const promise = new Promise((resolve) => { const Component = ({ i }: { i: number }) => { return ( @@ -203,11 +210,11 @@ describe("AnimatePresence", () => { }, 400) }) - return await expect(promise).resolves.toBe(3) + return expect(promise).resolves.toBe(3) }) test("Only renders one child at a time if exitBeforeEnter={true}", async () => { - const promise = new Promise(resolve => { + const promise = new Promise((resolve) => { const Component = ({ i }: { i: number }) => { return ( @@ -234,7 +241,7 @@ describe("AnimatePresence", () => { }, 150) }) - return await expect(promise).resolves.toBe(1) + return expect(promise).resolves.toBe(1) }) test("Exit variants are triggered with `AnimatePresence.custom`, not that of the element.", async () => { @@ -242,54 +249,44 @@ describe("AnimatePresence", () => { enter: { x: 0, transition: { type: false } }, exit: (i: number) => ({ x: i * 100, transition: { type: false } }), } - const promise = new Promise(resolve => { - const x = motionValue(0) - const Component = ({ - isVisible, - onAnimationComplete, - }: { - isVisible: boolean - onAnimationComplete?: () => void - }) => { - return ( - - {isVisible && ( - - )} - - ) - } - const { rerender } = render() - rerender() + const x = motionValue(0) - rerender( - resolve(x.get())} - /> + const Component = ({ + isVisible, + onAnimationComplete, + }: { + isVisible: boolean + onAnimationComplete?: () => void + }) => { + return ( + + {isVisible && ( + + )} + ) + } - rerender( - resolve(x.get())} - /> - ) + const { rerender } = render() + + rerender() + + await act(async () => { + rerender() }) - const resolvedX = await promise - expect(resolvedX).toBe(200) + expect(x.get()).toBe(200) }) test("Exit propagates through variants", async () => { @@ -298,42 +295,41 @@ describe("AnimatePresence", () => { exit: { opacity: 0, transition: { type: false } }, } - const promise = new Promise(resolve => { - const opacity = motionValue(1) - const Component = ({ isVisible }: { isVisible: boolean }) => { - return ( - - {isVisible && ( - - - - + const opacity = motionValue(1) + + const Component = ({ isVisible }: { isVisible: boolean }) => { + return ( + + {isVisible && ( + + + - )} - - ) - } + + )} + + ) + } - const { rerender } = render() + const { rerender } = render() + await act(async () => { rerender() - - resolve(opacity.get()) }) - return await expect(promise).resolves.toBe(0) + expect(opacity.get()).toBe(0) }) test("Handles external refs on a single child", async () => { - const promise = new Promise(resolve => { + const promise = new Promise((resolve) => { const ref = React.createRef() const Component = ({ id }: { id: number }) => { return ( @@ -370,7 +366,7 @@ describe("AnimatePresence", () => { describe("AnimatePresence with custom components", () => { test("Does nothing on initial render by default", async () => { - const promise = new Promise(resolve => { + const promise = new Promise((resolve) => { const x = motionValue(0) const CustomComponent = () => ( @@ -400,7 +396,7 @@ describe("AnimatePresence with custom components", () => { }) test("Suppresses initial animation if `initial={false}`", async () => { - const promise = new Promise(resolve => { + const promise = new Promise((resolve) => { const CustomComponent = () => ( { }) test("Animates out a component when its removed", async () => { - const promise = new Promise(resolve => { - const opacity = motionValue(1) - - const CustomComponent = () => ( - + const opacity = motionValue(1) + + const CustomComponent = () => ( + + ) + const Component = ({ isVisible }: { isVisible: boolean }) => { + return ( + + {isVisible && } + ) - const Component = ({ isVisible }: { isVisible: boolean }) => { - return ( - - {isVisible && } - - ) - } + } - const { container, rerender } = render() - rerender() - rerender() - rerender() + const { container, rerender } = render() + rerender() + rerender() + rerender() - // Check it's animating out - setTimeout(() => { - expect(opacity.get()).not.toBe(1) - expect(opacity.get()).not.toBe(0) - }, 50) + await act(async () => { + await new Promise((resolve) => { + // Check it's animating out + setTimeout(() => { + expect(opacity.get()).not.toBe(1) + expect(opacity.get()).not.toBe(0) + }, 50) - // Check it's gone - setTimeout(() => { - resolve(container.firstChild as Element | null) - }, 150) + // Resolve after the animation is expected to have completed + setTimeout(() => { + resolve() + }, 150) + }) }) - const child = await promise - expect(child).toBeFalsy() + expect(container.firstChild).toBeFalsy() }) test("Can cycle through multiple components", async () => { - const promise = new Promise(resolve => { + const promise = new Promise((resolve) => { const CustomComponent = ({ i }: any) => ( { }, 500) }) - return await expect(promise).resolves.toBe(3) + return expect(promise).resolves.toBe(3) }) test("Exit variants are triggered with `AnimatePresence.custom`, not that of the element.", async () => { @@ -514,54 +511,41 @@ describe("AnimatePresence with custom components", () => { exit: (i: number) => ({ x: i * 100, transition: { type: false } }), } const x = motionValue(0) - const promise = new Promise(resolve => { - const CustomComponent = () => ( - + const CustomComponent = () => ( + + ) + const Component = ({ + isVisible, + onAnimationComplete, + }: { + isVisible: boolean + onAnimationComplete?: () => void + }) => { + return ( + + {isVisible && } + ) - const Component = ({ - isVisible, - onAnimationComplete, - }: { - isVisible: boolean - onAnimationComplete?: () => void - }) => { - return ( - - {isVisible && } - - ) - } - - const { rerender } = render() - rerender() + } - rerender( - resolve(x.get())} - /> - ) + const { rerender } = render() + rerender() - rerender( - resolve(x.get())} - /> - ) + await act(async () => { + rerender() }) - const element = await promise - expect(element).toBe(200) + expect(x.get()).toBe(200) }) test("Exit variants are triggered with `AnimatePresence.custom` throughout the tree", async () => { @@ -573,81 +557,65 @@ describe("AnimatePresence with custom components", () => { } const xParent = motionValue(0) const xChild = motionValue(0) - const promise = new Promise(resolve => { - const CustomComponent = ({ - children, - x, - initial, - animate, - exit, - }: { - children?: any - x: any - initial?: any - animate?: any - exit?: any - }) => ( - ( + + {children} + + ) + const Component = ({ + isVisible, + onAnimationComplete, + }: { + isVisible: boolean + onAnimationComplete?: () => void + }) => { + return ( + - {children} - + {isVisible && ( + + + + )} + ) - const Component = ({ - isVisible, - onAnimationComplete, - }: { - isVisible: boolean - onAnimationComplete?: () => void - }) => { - return ( - - {isVisible && ( - - - - )} - - ) - } + } - const { rerender } = render() - rerender() + const { rerender } = render() + rerender() - rerender( - - resolve([xParent.get(), xChild.get()]) - } - /> - ) - - rerender( - - resolve([xParent.get(), xChild.get()]) - } - /> - ) + await act(async () => { + rerender() }) - const latest = await promise - expect(latest).toEqual([200, 200]) + expect([xParent.get(), xChild.get()]).toEqual([200, 200]) }) test("Exit propagates through variants", async () => { @@ -656,7 +624,7 @@ describe("AnimatePresence with custom components", () => { exit: { opacity: 0 }, } - const promise = new Promise(resolve => { + const promise = new Promise((resolve) => { const opacity = motionValue(1) const Component = ({ isVisible }: { isVisible: boolean }) => { return ( diff --git a/src/components/AnimatePresence/index.tsx b/src/components/AnimatePresence/index.tsx index ae50625617..4d57e295e1 100644 --- a/src/components/AnimatePresence/index.tsx +++ b/src/components/AnimatePresence/index.tsx @@ -1,5 +1,6 @@ import { useContext, + useEffect, useRef, isValidElement, cloneElement, @@ -138,6 +139,14 @@ export const AnimatePresence: React.FunctionComponent = ({ const isInitialRender = useRef(true) + const isMounted = useRef(true) + + useEffect(() => { + return () => { + isMounted.current = false + } + }, []) + // Filter out any children that aren't ReactElements. We can only track ReactElements with a props.key const filteredChildren = onlyElements(children) @@ -225,6 +234,9 @@ export const AnimatePresence: React.FunctionComponent = ({ // Defer re-rendering until all exiting children have indeed left if (!exiting.size) { presentChildren.current = filteredChildren + if (isMounted.current === false) { + return + } forceRender() onExitComplete && onExitComplete() } From 089e43b39d80ae67b486f382e5f966e05e73bea2 Mon Sep 17 00:00:00 2001 From: Matt Perry Date: Fri, 29 Oct 2021 15:35:55 +0200 Subject: [PATCH 2/2] Update index.tsx --- src/components/AnimatePresence/index.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/components/AnimatePresence/index.tsx b/src/components/AnimatePresence/index.tsx index 4a800c7fbd..c1146743f4 100644 --- a/src/components/AnimatePresence/index.tsx +++ b/src/components/AnimatePresence/index.tsx @@ -105,7 +105,9 @@ export const AnimatePresence: React.FunctionComponent = ({ const isInitialRender = useRef(true) const isMounted = useRef(true) - useEffect(() => () => isMounted.current = false, []) + useEffect(() => () => { + isMounted.current = false + }, []) // Filter out any children that aren't ReactElements. We can only track ReactElements with a props.key const filteredChildren = onlyElements(children)