beekai-oss / react-simple-animate

🎯 React UI animation made easy
https://react-simple-animate.now.sh/
MIT License
1.82k stars 61 forks source link

play value from useAnimate cannot be use in useEffect #52

Closed marcin-piela closed 5 years ago

marcin-piela commented 5 years ago

Describe the bug Value of play is changing during the animation and it cannot be used inside useEffect

To Reproduce I want to play animation depends on isOpened prop from parent and I need value of isPlaying to conditionally show children (I think that is not doable using Animation component)


const { play, style, isPlaying } = useAnimate({
  start: { opacity: 0 },
  end: { opacity: 1 },
  play: isOpened,
});

useEffect(() => {
  play(isOpened)
}, [isOpened, play]);

return (isOpened || isPlaying) ? children : null;

Above code produces infinite loop inside useEffect.

bluebill1049 commented 5 years ago

@marcin-piela I think it's because [play] can you remove play with the dependency see if it's working.

marcin-piela commented 5 years ago

@bluebill1049 Yes it is but regarding recommended practice this variable should be there :) And to do that I need add comment // eslint-disable-line react-hooks/exhaustive-deps, and why this function is changing in time?

bluebill1049 commented 5 years ago

it's not changing, I think it's because Object.is which is what react use to diff the dependency (return true) all the time. I have this problem at work as well, I haven't found a good solution yet. to be honest I find the eslint rule bit annoying.

marcin-piela commented 5 years ago

It's changing cause play function in hook is not memoized, probably wrapping it in useCallback will resolve the issue

bluebill1049 commented 5 years ago

oh i think you are right! @marcin-piela want to submit a PR for this since you discover the issue?

marcin-piela commented 5 years ago

I will do, but give me time until tomorrow :)

bluebill1049 commented 5 years ago

hey, @marcin-piela I did try to add useCallback in the weekend, but I don't think it works as expected. when I start to pass useCallback with dependency inside the useAnimate, it runs into the same issue of infinite running, I think it's the problem when the object compares in the dependency, maybe we can leave play out of useEffect dep. happy to see your approach too and suggestion.

marcin-piela commented 5 years ago

@bluebill1049 I've created a PR, example of use:

import React, { useEffect } from 'react';
import { useAnimate } from 'react-simple-animate';

import { DropdownPopupProps } from './DropdownPopup.types';

const start = { opacity: 0 };
const end = { opacity: 1 };

export const DropdownPopup: React.FC<DropdownPopupProps> = ({ style, children, isOpened }) => {
  const { play, style: animationStyle, isPlaying } = useAnimate({
    start,
    end,
  });

  useEffect(() => {
    play(isOpened);
  }, [isOpened, play]);

  return isOpened || isPlaying ? <div style={{ ...style, ...animationStyle }}>{children}</div> : null;
};

And I think that value of isPlaying was wrong, for me it should change to false after animation end, please take a look on PR

bluebill1049 commented 5 years ago

@marcin-piela I have published the change. thanks very much 🙏

marcin-piela commented 5 years ago

Thanks, now I can replace react-transition-group with you library (2kB less in build 😄 )