motion icon indicating copy to clipboard operation
motion copied to clipboard

[BUG] Animations do not resume after components resume after suspense

Open Pinpickle opened this issue 2 years ago • 15 comments
trafficstars

1. Read the FAQs 👇

2. Describe the bug

Animations that are interrupted with suspense have weird behaviour. Sometimes they will freeze in place. Sometimes they will freeze at the beginning. Sometimes they will complete.

3. IMPORTANT: Provide a CodeSandbox reproduction of the bug

I made a StackBlitz, hopefully that's OK: https://stackblitz.com/edit/vitejs-vite-uhm4h9?file=src%2FApp.tsx

4. Steps to reproduce

Steps to reproduce the behavior:

  1. Wait for the animation to show "suspended"
  2. Notice how the first animation is frozen in the wrong spot at the end

5. Expected behavior

For the animation to complete eventually once the suspended component re-mounts. Any one of these would be reasonable IMO:

  • Proceed the animation by the amount of time the component has been suspended (so if it suspends at t=0.5 for 1 second, it resumes at t=1.5)
  • Start the animation again
  • Mount with the animation completed

There would be different behaviours for repeating animations and unmounting animations.

6. Video or screenshots

https://github.com/framer/motion/assets/3238878/372e34a3-b089-43bf-9f9a-ee9555819c00

7. Environment details

Chrome on macOS (M1) Version 114.0.5735.198 (Official Build) (arm64)

Pinpickle avatar Aug 01 '23 02:08 Pinpickle

@Pinpickle Hi, it seems the following SuspenseTest component works, it uses a ref to store the promise instead of a state, not forcing any re-render when setting or resetting.

import { useEffect, useRef } from 'react';
import { motion } from 'framer-motion';
export const SuspenseTest = ({ shouldSuspend }: { shouldSuspend: boolean }) => {
  const promiseRef = useRef<null | Promise<void>>(null);
  const resolvePromiseTimeoutRef = useRef<null | NodeJS.Timeout>(null);

  useEffect(() => {
    if (shouldSuspend) {
      const setupPromiseTimeout = setTimeout(() => {
        promiseRef.current = new Promise(resolve => {
          resolvePromiseTimeoutRef.current = setTimeout(() => {
            resolve();
            promiseRef.current = null;
          }, 1000);
        });
      }, 1000);

      return () => {
        clearTimeout(setupPromiseTimeout);
        if (resolvePromiseTimeoutRef.current != null) {
          clearTimeout(resolvePromiseTimeoutRef.current);
        }
      };
    }
  }, []);

  if (shouldSuspend && promiseRef.current) {
    throw promiseRef.current;
  }

  return (
    <motion.div
      style={{ fontWeight: 'bold' }}
      initial={{ opacity: 0.2, scale: 1 }}
      animate={{ opacity: 1, scale: 2 }}
      transition={{ duration: 2 }}
    >
      {shouldSuspend ? 'With suspense' : 'Without suspense'}
    </motion.div>
  );
};

gurkerl83 avatar Aug 02 '23 22:08 gurkerl83

@gurkerl83 you're right that this no longer causes the animation to become stuck. But that's because the component never suspends, so it just means it's no longer running into the bug in Motion.

In this example you can technically create the promise completely outside of React, but this was just a contrived example to get to causing suspense as fast as possible.

A more useful example could be using React.lazy, which triggers suspense:

const LazyDisplayNumber2 = lazy(async () => {
  // artificial network delay
  await new Promise((r) => setTimeout(r, 1000));
  return import("./DisplayNumber2");
});

const SuspenseTest = ({ shouldSuspend }: { shouldSuspend: boolean }) => {
  const [isDisplaying, setIsDisplaying] = useState(false);

  useEffect(() => {
    // We need to make sure the component manages to mount
    // and start the animation before triggering suspense.
    setIsDisplaying(true);
  }, []);

  return (
    <div style={{ display: "flex", justifyContent: "center" }}>
      <motion.div
        style={{ fontWeight: "bold" }}
        initial={{ opacity: 0.2, scale: 1 }}
        animate={{ opacity: 1, scale: 2 }}
        transition={{ duration: 2 }}
      >
        {isDisplaying && shouldSuspend ? <LazyDisplayNumber2 /> : "1"}
      </motion.div>
    </div>
  );
};

This has the same problem of "getting stuck"

Codesandbox (I couldn't get Stackblitz to work this time 🙃): https://codesandbox.io/s/vigorous-lamarr-sjssxd?file=/src/App.tsx

Pinpickle avatar Aug 02 '23 23:08 Pinpickle

You are absolutely right, the promise is never thrown, at least not when no state change gets triggered during the promise gets created and resolves in the timeout.

Working with Suspense often requires the usage of the useTransition hook. Props put into a component wrapped by a suspense boundary require doing a state update starting a transition. But in the second example state update is executed within. Maybe lazy automatically adds the Suspense component over LazyDisplayNumber2, then this makes sense.

From the new example is this what you want to achieve?

You need useTransition from react. Suspense and Transition has improved over the last year dramatically I recommend installing a 18.3 canary version of react and react-dom.

const SuspenseTest = ({ shouldSuspend }: { shouldSuspend: boolean }) => {
  const [isDisplaying, setIsDisplaying] = useState(false);

  const [, startTransition] = useTransition();

  useEffect(() => {
    if (shouldSuspend) {
      startTransition(() => {
        setIsDisplaying(true);
      });
    }
  }, []);

  return (
    <div style={{ display: "flex", justifyContent: "center" }}>
      <motion.div
        style={{ fontWeight: "bold" }}
        initial={{ opacity: 0.2, scale: 1 }}
        animate={{ opacity: 1, scale: 2 }}
        transition={{ duration: 5 }}
      >
        {isDisplaying && shouldSuspend ? <LazyDisplayNumber2 /> : "1"}
      </motion.div>
    </div>
  );
};

gurkerl83 avatar Aug 02 '23 23:08 gurkerl83

@gurkerl83 there are plenty of workarounds to make sure a component doesn't suspend in isolated cases.

Regardless, I'm pretty sure this is a bug in Framer Motion and (in my opinion), animations should still work even if they've been interrupted with suspense.

Pinpickle avatar Aug 03 '23 02:08 Pinpickle

The technical reason behind this looks like when the component throws, the motion.div doesn't re-render but its ref function is passed null (an unmount signal). Then, when the component stops throwing, the motion.div isn't re-rendered or effects don't run, but the ref function does fire. Which is weird IMO. I'll need to get a better handle on React's behaviour here before making any fixes as these lifecycles are in place usually for quite specific reasons.

mattgperry avatar Aug 03 '23 09:08 mattgperry

@mattgperry thanks for investigating! Any suggestions for workarounds in the meantime? Is there a way to force an animation to restart?

Pinpickle avatar Aug 03 '23 23:08 Pinpickle

@mattgperry @Pinpickle Found the issue, with the ref problem, please the the comments made in the component.

https://github.com/framer/motion/blob/c938fd9e3afee6e4e2cfbc8b91a88a2dc38efa07/packages/framer-motion/src/motion/utils/use-motion-ref.ts#L11

export function useMotionRef<Instance, RenderState>(
    visualState: VisualState<Instance, RenderState>,
    visualElement?: VisualElement<Instance> | null,
    externalRef?: React.Ref<Instance>
): React.Ref<Instance> {
    

    useEffect(() => {
        // 2. Requirement
        return () => {
            if (visualElement) {
                visualElement.unmount()
            }
        }
    }, [visualElement])

    return useCallback(
        (instance: Instance) => {
            /** Original Code
            instance && visualState.mount && visualState.mount(instance)

            if (visualElement) {
                instance
                    ? visualElement.mount(instance)
                    : visualElement.unmount()
            */

            /** Pseude Code 
            if (instance && visualState.mount) {
                console.log("visual - State.mount: ", instance)
                visualState.mount(instance)
            }

            if (visualElement) {
                if (instance) {
                    console.log("visual - Element.mount: ", instance)
                    visualElement.mount(instance)
                } else {
                    // the refs corresponding element (instance) was detach from dom
                    // hits when the something suspense, a promise or thenable was catched by a SuspenseBoundary outside of a motion component
                    // immediate after the initial render e.g. using React.lazy()
                    console.log("visual - Element.unmount: ", instance)
                    visualElement.unmount()
                }
            }
            */

            // Workaround
            // 1. Mount visualelement and visualstate in callback
            // 2. Unmount visualelement when the component unmounts in effect cleanup

            // 1. Requirement
            if (instance) {
                visualState.mount && visualState.mount(instance)
                visualElement && visualElement.mount(instance)
            }

            if (externalRef) {
                if (typeof externalRef === "function") {
                    externalRef(instance)
                } else if (isRefObject(externalRef)) {
                    ;(externalRef as any).current = instance
                }
            }
        },
        /**
         * Only pass a new ref callback to React if we've received a visual element
         * factory. Otherwise we'll be mounting/remounting every time externalRef
         * or other dependencies change.
         */
        [visualElement]
    )
}

The final hook looks like this

import * as React from "react"
import { useCallback, useEffect } from "react"
import type { VisualElement } from "../../render/VisualElement"
import { isRefObject } from "../../utils/is-ref-object"
import { VisualState } from "./use-visual-state"

/**
 * Creates a ref function that, when called, hydrates the provided
 * external ref and VisualElement.
 */
export function useMotionRef<Instance, RenderState>(
    visualState: VisualState<Instance, RenderState>,
    visualElement?: VisualElement<Instance> | null,
    externalRef?: React.Ref<Instance>
): React.Ref<Instance> {
    /**
     * This React effect ensures the systematic unmounting of 'visualElement'
     * during the component's unmount phase. This cleanup is especially pivotal
     * in contexts where the component's rendering might have been affected by asynchronous
     * operations, such as with React.lazy and Suspense.
     *
     * Given that 'visualElement' animations are allowed to continue even when certain
     * child components might be rendered invalid by promises from React.lazy, it becomes
     * paramount to ensure that resources or side-effects associated with this DOM node
     * are properly managed and cleaned up to avoid potential pitfalls.
     */
    useEffect(() => {
        return () => {
            if (visualElement) {
                visualElement.unmount()
            }
        }
    }, [visualElement])

    return useCallback(
        (instance: Instance) => {
            /**
             * This section manages the lifecycle of 'visualState' and 'visualElement' based on
             * the presence of the 'instance' variable, which corresponds to a real DOM node.
             *
             * - When a valid DOM node (represented by 'instance') is detected, both 'visualState'
             *   and 'visualElement' are triggered to mount. This signifies the preparation or
             *   setup of visual components or states based on the detected node.
             *
             * - A complex scenario emerges when 'instance' becomes null, particularly within
             *   the environment of an outer Suspense boundary. With React.lazy, components are
             *   loaded lazily and the promises (or thenables) might render certain child components
             *   invalid based on their resolution or rejection. This can lead to situations where
             *   the expected DOM node isn't available. Yet, in these cases, the 'visualElement'
             *   doesn't get immediately unmounted. Animations tied to it persist, maintaining a
             *   consistent visual experience.
             */
            if (instance) {
                visualState.mount && visualState.mount(instance)
                visualElement && visualElement.mount(instance)
            }

            if (externalRef) {
                if (typeof externalRef === "function") {
                    externalRef(instance)
                } else if (isRefObject(externalRef)) {
                    ;(externalRef as any).current = instance
                }
            }
        },
        /**
         * Only pass a new ref callback to React if we've received a visual element
         * factory. Otherwise we'll be mounting/remounting every time externalRef
         * or other dependencies change.
         */
        [visualElement]
    )
}

gurkerl83 avatar Aug 08 '23 10:08 gurkerl83

While framer-motion can execute the animation with the current changes, it doesn't truly resume the animation. Specifying Suspense outside of a motion component doesn't seem advantageous. Without modifying any sources, the same outcome can be achieved by integrating Suspense directly within the component and subsequently using lazy loading. By doing this, the promise or "thenable" throw is confined specifically to this Suspense boundary, ensuring that framer-motion remains uninterrupted.

For illustrative purposes, please avoid using an external Suspense component in this example.

import { motion } from 'framer-motion';
import { useState, useEffect, FC, lazy, Suspense } from 'react';

function delayForDemo(promise) {
  return new Promise(resolve => {
    setTimeout(resolve, 2000);
  }).then(() => promise);
}

const LazyDisplayNumber2 = lazy(() => delayForDemo(import('./DisplayNumber2')));

export const SuspenseTest: FC<{ shouldSuspend: boolean }> = ({
  shouldSuspend
}) => {
  const [isDisplaying, setIsDisplaying] = useState(false);

  useEffect(() => {
    // We need to make sure the component manages to mount
    // and start the animation before triggering suspense.
    setIsDisplaying(true);
  }, []);

  return (
    <div style={{ display: 'flex', justifyContent: 'center' }}>
      <motion.div
        style={{ fontWeight: 'bold' }}
        initial={{ opacity: 0.2, scale: 1 }}
        animate={{ opacity: 1, scale: 2 }}
        transition={{ duration: 4 }}
      >
        {isDisplaying && shouldSuspend ? (
          <Suspense fallback={<div>Suspended</div>}>
            <LazyDisplayNumber2 />
          </Suspense>
        ) : (
          1
        )}
      </motion.div>
    </div>
  );
};

gurkerl83 avatar Aug 08 '23 14:08 gurkerl83

The provided PR #2283 is certainly not the right approach to fix this problem.

gurkerl83 avatar Aug 11 '23 21:08 gurkerl83

@mattgperry I don't suppose you've had any time to further investigate this? The bug is particularly problematic with AnimatePresence where a suspense can prevent an element from unmounting successfully.

Pinpickle avatar Feb 05 '24 22:02 Pinpickle

Thanks to the double invocation of ref functions in React 19 strict mode I believe this will be fixed with that.

mattgperry avatar Jun 06 '24 09:06 mattgperry

@mattgperry are you saying that this should work out-of-the-box in React 19, or the required fixes for double-invocation in strict mode will also fix this?

Going by the React 19 docs, I'm assuming it's the latter?

For example, during development, Strict Mode will double-invoke ref callback functions on initial mount, to simulate what happens when a mounted component is replaced by a Suspense fallback.

Pinpickle avatar Jun 06 '24 23:06 Pinpickle

Yeah the latter. I'm working on a PR that will merge in the logic changes I made for React 19 so it could be fixed before then.

mattgperry avatar Jun 19 '24 09:06 mattgperry

Ah apologises this doesn't seem to have fixed it.

mattgperry avatar Jun 19 '24 09:06 mattgperry

@mattgperry yeah unfortunately it looks like the repro is still failing in 11.3.2

For what it's worth, another instance in the app I'm working on where suspense stopped an animation isn't happening anymore. Unfortunately I'm not sure what the difference between that case and this case is.

Pinpickle avatar Jul 12 '24 04:07 Pinpickle