software-mansion / react-native-reanimated

React Native's Animated library reimplemented
https://docs.swmansion.com/react-native-reanimated/
MIT License
8.66k stars 1.27k forks source link

[iOS] Animations don't always run on component initialization #3296

Open computerjazz opened 2 years ago

computerjazz commented 2 years ago

Description

In reanimated 2.8.0, animations that run on component initialization don't always animate. I can't figure out why they work sometimes and don't work other times. The strangest thing is that adding a console.log within useAnimatedStyle seems to fix the issue. Wrapping the call that sets the shared value to kick off the initial animation in a setTimeout also seems to fix it (most of the time).

This bug does not occur in 2.3.1.

I believe this is the underlying issue behind this react-native-bottom-sheet issue: https://github.com/gorhom/react-native-bottom-sheet/issues/925

Expected behavior

Animations run on component initialization. (Below is with a console.log added in useAnimatedStyle):

https://user-images.githubusercontent.com/6730148/173665563-8df0791a-0a05-41de-a828-d9179f013849.MP4

Actual behavior & steps to reproduce

Create a component that has animations that run on initialization. Show/hide component. Animations run sometimes, but sometimes they just jump to final value (or partially animate).

(Without console.log in useAnimatedStyle):

https://user-images.githubusercontent.com/6730148/173665704-8c6f93eb-56e3-4cfd-88cd-7f64b1b02252.MP4

Snack or minimal code example

Run on iOS device to see issue: https://snack.expo.dev/@computerjazz/reanimated-init-bug

Package versions

2.8.0

name version
react-native 0.68.2
react-native-reanimated 2.8.0
expo 45

Affected platforms

Titozzz commented 2 years ago

I can confirm the bug, the console.log trick. It also affect moti. I'll test the PR and report back

EDIT: also affects android so the PR won't fix all

EDIT2: The PR doesn't fix this issue on iOS for me at least.

Titozzz commented 2 years ago

My issue is in fact the same as : https://github.com/software-mansion/react-native-reanimated/issues/3209. Maybe related?

computerjazz commented 2 years ago

A few clues (and maybe the reason why console.log fixes it): setting optimalization = 0 here also seems to fix it: https://github.com/software-mansion/react-native-reanimated/blob/main/src/reanimated2/hook/useAnimatedStyle.ts#L481 (or returning false here: https://github.com/software-mansion/react-native-reanimated/blob/main/src/reanimated2/hook/utils.ts#L167-L175)

I believe this is also why console.log fixes it -- the babel transform gives different values here depending on whether a console.log exists, which drives optimalization: https://github.com/software-mansion/react-native-reanimated/blob/main/plugin.js#L745 (isFunctionCall becomes true when a console.log exists)

computerjazz commented 2 years ago

Since setting optimalization = 0 seems to fix it, my gut says that the issues lies somewhere around here: https://github.com/software-mansion/react-native-reanimated/blob/150e787d1101249122871128be5805c4fe288286/Common/cpp/Tools/Mapper.cpp#L25-L50

maybe some kind of race condition with jsViewDescriptors being initialized on mount? I have verified that forcing the non-optimized path by hardcoding if(true){ on line 25 also fixes it.

unfortunately i don't know C++ (yet 😛) so this may be about as far as i can go

computerjazz commented 2 years ago

@piaskowyk I think the issue has something to do with the performance optimazations or shared styles here: https://github.com/software-mansion/react-native-reanimated/pull/1470 https://github.com/software-mansion/react-native-reanimated/pull/1879

notjosh commented 2 years ago

Can you try it again now that #3374 is merged? It looks like the same behaviour.

naftalibeder commented 1 year ago

I can confirm. The explanation @computerjazz gave here looks correct, because any function call (not just console.log) fixes it. Even executing a no-op like

const style = useAnimatedStyle(() => {
  (() => {})();

  return {
    ...
  };
});

causes the style to update propertly.

I applied the changes here, but it did not fix the problem.

andreibahachenka commented 1 year ago

has anyone found a fix for that?

lightrow commented 1 year ago

You can use v1 until they fix v2 in v3

andreibahachenka commented 1 year ago

it worked with version 2.2.1

lightrow commented 1 year ago

I mean you can use v1 API with the latest version of reanimated, it doesn't have this bug, at least i haven't been able to reproduce it once i rewrote the affected component with V1 API.

lightrow commented 1 year ago

https://docs.swmansion.com/react-native-reanimated/docs/1.x.x/declarative

the same API is still available in 2.x, except Easing functions for v1 are now exported as EasingNode

andreibahachenka commented 1 year ago

got it! but I use moti and moti uses reanimated under the hood

lightrow commented 1 year ago

well, not sure why you'd need to use another abstraction when reanimated itself is already very straightforward to use

andreibahachenka commented 1 year ago

make the process even easier :) it’s weird that a few people have this bug although reanimated used by a lot

lightrow commented 1 year ago

maybe not that many people try to animate height/width of components

freeboub commented 1 year ago

Short update to notice that this issue can be reproduced on 2.11.0 and also affects android device (I use android tv, but should not impact behavior). workaround here: https://github.com/software-mansion/react-native-reanimated/issues/3296#issuecomment-1211069576 also fix the issue.

sadia-onyxtec commented 1 year ago

I am facing the same issue in v2.8.0.

lightrow commented 1 year ago

simplest reproduction case

const Collapsible = () => {
    const childrenHeight = useSharedValue(0);

    const animStyle = useAnimatedStyle(() => {
        return {
            height: childrenHeight.value,
        };
    }, []);

    useEffect(() => {
        childrenHeight.value = 500;
    }, []);

    return <Animated.View style={[{ width: 500, backgroundColor: 'red' }, animStyle]} />;

};

delaying childrenHeight.value = 500; with setTimeout helps, but the timeout duration has to be substantial (not 0, but 50-100) for it to be a reliable "fix", and at that point the jump becomes too noticeable for users. It also seems to happen more often when JS thread is busy with other stuff (e.g. running effects with API calls)

adding (() => {})(); inside useAnimatedStyle doesn't help in this case

const Collapsible = () => {
    const childrenHeight = useValue(0);

    useEffect(() => {
        childrenHeight.setValue(500);
    }, []);

    return <Animated.View style={[{ width: 500, backgroundColor: 'red', height: childrenHeight }]} />;
};

this works perfectly fine please don't deprecate V1 without fixing this breaking issue, there is no other reliable workaround for this!

vancerbtw commented 1 year ago

Any update on this? I am suffering from the same issue with a fade in animation that is triggered on component mount in useEffect. console.logging the shared value is my only current work around. just triggering a () => {} function call does not solve my issue

davoam commented 1 year ago

This still happens in reanimated version 3.1.0

vancerbtw commented 1 year ago

I had this crash happen when changing the content size of a scroll view when it was being animated and the content size changing affecting the current viewport. I solved this by delaying my content change until the scroll animation was completed. still not ideal :(

Parveshdhull commented 1 year ago

Weird workaround

(hopefully it will work for you)

Observation:

Problems are with sudden changes during initialization. For example, in @lightrow's example, he is changing childrenHeight from 0 to 500 instantly.

Not so good

childrenHeight.value = withSequence(withTiming(0, {duration: 0}), withTiming(500, {duration: 0}))

This solution works for me, as it also has 0 total duration, and somehow using sequence fixes issue.

Problem: This works great when you are creating/rendering view, but if view is already there like on hot reload, screen will flicker. Because we are hiding view and again showing

Better solution

childrenHeight.value = withSequence(withTiming(501, {duration: 0}), withTiming(500, {duration: 0}))

lightrow commented 1 year ago

tried the above, at first it looked like it worked, but it wasn't reliable and sometimes the height still wouldn't be set. Flickering was also still noticable when component fails to set height for the first part of the sequence. All in all it's the same as adding a timeout, only in this case the duration is random and depends on how long the 0 duration animation took to finish

edit: nevermind, i had another component that i forgot to apply this method to inbetween two other components. It seems to be working reliably now!

edit2: well it works but there is visible flickering sometimes, like 1 in 10 cases

edit3: i don't know what happened or changed, but i cannot reproduce this anymore, not even my own example above, which now works fine without any workarounds, on both iOS 16.2 simulator and physical iPhone 13 Pro on iOS 15.7. I'm still on reanimated v3.3.0,

edit4: no it still happens when JS thread is busy, with or without workarounds

ngokevin commented 9 months ago

I ran into this issue on certain Android phones. It's really a race condition that manifests reliably on some phones. In my case, it was with Moti which uses Reanimated. I was on Reanimated v3.3.0.

const anim = useDynamicAnimation(() => ({ opacity: 0 });

useEffect(function () {
  anim.animateTo({ opacity: 1});
}, []);

return anim;

It did help to add delays, or only run the initial onMount animation after a view's onLayout, but it still manifested. Just wanted to comment it wasn't an iOS-only issue for me.

I'll update to see if I can trigger it with Reanimated raw API.

henrymoulton commented 4 months ago

@piaskowyk any ideas on what we can do to give a better repro given it's a race condition?

AlexSapoznikov commented 1 month ago

Has anyone figured out a workaround for it so far? Above workarounds do not work always as @lightrow described.

Edit: Here is a "hackish" workaround I came up with until it gets fixed. I run checks to see if the animation is applied inside an interval with a retry limit. It animates x, y position but can be adjusted to use width and height.

// util: setIntervalWithLimit.ts

export type SetIntervalWithLimitFnStop = () => void;
export type SetIntervalWithLimitFnArgs = {
  stop: SetIntervalWithLimitFnStop;
  sequel: number;
};

export type SetIntervalWithLimitFn = (args: SetIntervalWithLimitFnArgs) => void;

export type SetIntervalWithLimitResponse = {
  ref: NodeJS.Timeout;
  stop: SetIntervalWithLimitFnStop;
};

export const setIntervalWithLimit = (
  fn: SetIntervalWithLimitFn,
  maxLimit: number,
  interval: number,
  cb?: () => void,
): SetIntervalWithLimitResponse => {
  let sequel = 0;

  const stopFn = (intervalRef: NodeJS.Timeout) => () => {
    clearInterval(intervalRef);
    cb?.();
  };

  const ref = setInterval(() => {
    const stop = stopFn(ref);
    sequel++;

    // Run function.
    fn({ stop, sequel });

    // If limit reached, stop.
    if (sequel >= maxLimit) {
      stop();
    }
  }, interval);

  return { ref, stop: stopFn(ref) };
};

// Your component
...
  const ref = useRef<AnimatedView>(null);
  const mountIntervalRef = useRef<SetIntervalWithLimitResponse>();

    // update position when item size changes
  useEffect(() => {
    // Sometimes animations won't run because re-animated has a bug https://github.com/software-mansion/react-native-reanimated/issues/3296#issuecomment-1573900172:
    activeX.value = x; // your actual target x
    activeY.value = y; // your actual target y

    // So we set an interval to try execute animation until it actually completes with a limit of X times in order to avoid infinite loop.
    mountIntervalRef.current?.stop();
    mountIntervalRef.current = setIntervalWithLimit(
      ({ stop, sequel }) => {
        ref.current?.measure((originX, originY, _width, _height, _pageX, _pageY) => {
          // Not correct values.
          if (originX !== activeX.value && originY !== activeY.value) {
            // If still broken, try using this:
            activeX.value = withSequence(withTiming(x - 1, { duration: 0 }), withTiming(x, { duration: 0 }));
            activeY.value = withSequence(withTiming(y - 1, { duration: 0 }), withTiming(y, { duration: 0 }));
            return;
          }

          // Success.
          stop();
        });
      },
      20, // Duration,
      50, // Max times we run interval.
    );
  }, [width, height, activeX, activeY]);

  const animatedStyle = useAnimatedStyle(() => {
    return {
      transform: [{ translateX: activeX.value }, { translateY: activeY.value }],
    };
  });

  return (
    <Animated.View ref={ref} style={[{ position: 'absolute', width, height }, animatedStyle]}>
      {children}
    </Animated.View>
  )
piaskowyk commented 1 month ago

I know that this is an old issue, but could someone confirm whether this problem still exists in the latest version of Reanimated or if it has been resolved?