Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RN 0.61: Animated.color causes "excessive number of pending callbacks" #381

Closed
felippepuhle opened this issue Sep 5, 2019 · 22 comments
Closed

Comments

@felippepuhle
Copy link

felippepuhle commented Sep 5, 2019

Hey guys, how are you?

Just updated react native version to 0.61 (rc) and now my app is crashing on some screens that use color interpolation.

We have a theme based template, so I created a function that parses hex colors using polished:

import Animated from 'react-native-reanimated'
import { parseToRgb } from 'polished'

const interpolateColors = (animation, { inputRange, outputRange }) => {
  const colors = outputRange.map(parseToRgb)

  const r = Animated.interpolate(animation, {
    inputRange,
    outputRange: colors.map(c => c.red),
    extrapolate: Animated.Extrapolate.CLAMP,
  })

  const g = Animated.interpolate(animation, {
    inputRange,
    outputRange: colors.map(c => c.green),
    extrapolate: Animated.Extrapolate.CLAMP,
  })

  const b = Animated.interpolate(animation, {
    inputRange,
    outputRange: colors.map(c => c.blue),
    extrapolate: Animated.Extrapolate.CLAMP,
  })

  return Animated.color(r, g, b)

Any tips on this?

Screen Shot 2019-09-05 at 11 41 22 AM

Thanks!

@marcorm
Copy link

marcorm commented Sep 9, 2019

I get the same error in some conditions using react-navigation 5.0.0 (unstable)

@rt2zz
Copy link

rt2zz commented Sep 11, 2019

I have the same on: [email protected], react-navigation@4 and latest reanimated.

I am not sure how to debug this, but will keep looking.

@Jarred-Sumner
Copy link

Jarred-Sumner commented Sep 11, 2019

i wonder if it's something to do with the TurboModules API changes

@Jarred-Sumner
Copy link

This commit looks relevant: facebook/react-native@1e45a41. This warning was added in 0.61.0.

Do you experience this issue on devices running in release mode? Or only development mode in the simulator? I currently only see it in development mode on the simulator.

My guess is: for most libraries, this warning message is a good way to find certain types of bugs. However, for this library, a lot of callbacks is sometimes expected behavior (e.g. stuff happening after a pan gesture recognizer updates coordinates)...and everything is just slower in development mode.

@felippepuhle
Copy link
Author

You're right!

I couldn't replicate this on release mode. It seems to happen only on debug mode.

That's sad, I had to remove a lot of animations temporarily 'cause of it... any idea how we could solve it? Can we fix on our side or only contacting react-native team?

Thanks!

@BrendonSled
Copy link

The discussion for 0.61.0 is happening here:
react-native-community/releases#140

@elicwhite
Copy link

Cc @sahrens

@elicwhite
Copy link

elicwhite commented Sep 15, 2019

Is anyone able to create a simple repro we can drop into a fresh project, without using any third party libraries? I think that would be necessary for us to be able to effectively debug and fix this.

edit I didn’t realize this was the reanimated repo. Heh, whoops. Needing to make a stand-alone repro without 3rd party deps doesn’t make much sense then. ;-)

@felippepuhle
Copy link
Author

@TheSavior how can I help on this?

@elicwhite
Copy link

@felippepuhle, we need an answer to the why this is happening. I imagine the smallest repro you can (with reanimated) would be a step in the right direction.

@felippepuhle
Copy link
Author

@TheSavior the best example I could get for now was this: https://github.com/felippepuhle/rn-reanimated-bug

Nothing happens immediately. But changing duration here with fast refresh enabled will crash the app.

In my app it happens only on some screens, but it's not related to the fast refresh itself. Maybe something related to mounting/unmounting components (my app crashes just after changing the condition to mount/unmount some components).

I don't know if I was clear, if you need anything else just let me know.

Thanks!

@kmagiera
Copy link
Member

Thanks for working on the repo case for the issue @felippepuhle

Unfortunately I couldn't get the crash you mentioned when trying it. Can you share some more details about your environment? E.g. are you running on iOS/Android simulator or device? Xcode version etc

@kmagiera
Copy link
Member

I also tried changing duration mutliple times and saving file in order for fast refresh to pick it up (at least 50 times) and still didn't get that issue. After all of that I've set a breakpoint in https://github.com/facebook/react-native/blob/master/Libraries/BatchedBridge/MessageQueue.js#L220 and saw that the are 0 callbacks

@kmagiera
Copy link
Member

But now after some time on it I suspect that I know where your issue is coming from.

There is only one place in reanimated where we use RN's callbacks and it is when the animated node detaches. If you have more than 500 nodes that unmount at the same time we'd enqueue an equal number of calls with callbacks (we use callback on detach to remember the last value of animated node in JS so that if it is being reused we initiate it in native with that value). The relevant code in reanimated is here https://github.com/kmagiera/react-native-reanimated/blob/c841c34d1037e6b00a9f425e17782ccfbdb92a12/src/core/InternalAnimatedValue.js#L25 – I'll try to think how we can workaround this limitation (maybe we could use events, or maybe detect which nodes are not going to be reused such that we only use callbacks on the ones that will)

@Jarred-Sumner
Copy link

Not sure if this helps, but here's code that runs into this issue, particularly when panX and panY are updating a lot (those are Animated.Value<number>):

export const DeleteFooter = ({ onDelete, panY, panX }) => {
  const distance = React.useRef(new Animated.Value(0));

  const opacity = React.useRef(
    Animated.interpolate(
      distance.current,
      {
        inputRange: DELETE_RANGE,
        outputRange: [1, 1, 0.95, 0.5]
      }
    )
  );

  const scaleTransform = React.useRef(
    Animated.interpolate(
      distance.current,

      {
        inputRange: DELETE_RANGE,
        outputRange: [1.01, 1.0, 0.97, 0.95]
      }
    )
  );

  const backgroundColor = React.useRef(
    interpolateColor(
      distance.current,
      {
        inputRange: DELETE_RANGE,
        outputRange: [
          {
            r: 120,
            g: 120,
            b: 120
          },
          {
            r: 75,
            g: 75,
            b: 75
          },
          {
            r: 50,
            g: 50,
            b: 50
          },
          {
            r: 0,
            g: 0,
            b: 0
          }
        ]
      },
      "rgb"
    )
  );

  return (
    <View pointerEvents="none" style={[styles.footer, styles.footerCenter]}>
      <Animated.Code
        exec={Animated.block([
          Animated.set(
            distance.current,
            Animated.sqrt(
              Animated.add(
                Animated.multiply(
                  Animated.sub(MID_X_DELETE_BUTTON, panX),
                  Animated.sub(MID_X_DELETE_BUTTON, panX)
                ),
                Animated.multiply(
                  Animated.sub(MID_Y_DELETE_BUTTON, panY),
                  Animated.sub(MID_Y_DELETE_BUTTON, panY)
                )
              )
            )
          )
        ])}
      />
      <View
        pointerEvents="none"
        style={[
          styles.footerSide,
          { alignItems: "center", justifyContent: "center" }
        ]}
      >
        <IconButton
          onPress={onDelete}
          Icon={IconTrash}
          color="#fff"
          size={DELETE_SIZE}
          backgroundColor={backgroundColor.current}
          opacity={opacity.current}
          transform={[{ scale: scaleTransform.current }]}
          borderColor="#fff"
          type="fill"
        />
      </View>
    </View>
  );
};

This callback is triggered when x and y update as well (via Animated.call):

handlePan = ({
    blockId,
    isPanning,
    x,
    y
  }: {
    blockId: string;
    isPanning: boolean;
    x: number;
    y: number;
  }) => {
    if (isPanning) {
      const focusType = FocusType.panning;
      if (blockId !== this.state.focusedBlockId) {
        this.focusedBlockValue.setValue(blockId.hashCode());
      }

      if (this.state.focusType !== focusType) {
        this.focusTypeValue.setValue(focusType);
      }

      this.setState({
        focusedBlockId: blockId,
        focusType
      });
    } else {
      const { focusedBlockId, focusType } = this.state;

      if (
        focusedBlockId === blockId &&
        focusType === FocusType.panning &&
        isDeletePressed(x, y)
      ) {
        this.deleteNode(blockId);
        sendLightFeedback();
      } else {
        this.setState({
          focusedBlockId: null,
          focusType: null
        });

        this.focusedBlockValue.setValue(-1);
        this.focusTypeValue.setValue(-1);
      }
    }
  };

@kmagiera
Copy link
Member

kmagiera commented Sep 20, 2019

Yes, I think I know what the issue is now. Note that the callbacks number check only runs in DEV which explains why you are not seeing this on release builds: facebook/react-native@1e45a41

@felippepuhle
Copy link
Author

Sure, I'm running IOS simulator with Xcode 10.2.1, but it already happened on my device too.
Release builds are safe, but it's almost impossible to work without debug mode.

I didn't try on Android yet (our app is 100% IOS for now).

Hope that helps.
If you need any kind of help just let me know @kmagiera.

Thank you!

@computerjazz
Copy link

@felippepuhle I dug into node_modules and commented out the offending lines as a stopgap. Hopefully it allows you to continue working until a fix is ready: https://github.com/facebook/react-native/blob/1e45a41dc2d4699d4e439994e81ca5ab82d630b3/Libraries/BatchedBridge/MessageQueue.js#L213-L227

@elicwhite
Copy link

We just landed a PR to core that changes this warning to only fire once the first time it occurs. That might also make this issue less high severity.

@fungilation
Copy link

The PR in question: https://github.com/facebook/react-native/pull/26508/files

Thanks for the heads up @TheSavior

@bdrobinson
Copy link

That PR has shipped in RN 0.61.2 and has fixed the issue for me 👍

@felippepuhle
Copy link
Author

felippepuhle commented Oct 24, 2019

Yeah, it's solved! Can we close this @kmagiera @osdnk?

@osdnk osdnk closed this as completed Oct 24, 2019
Ashoat added a commit to CommE2E/comm that referenced this issue Oct 6, 2020
Summary:
This warning happens when `react-native-reanimated` nodes are unmounted. I don't think the amount of Reanimated nodes we're using is excessive, since performance is okay on old Android phones.

Some more context [here](software-mansion/react-native-reanimated#381). Apparently this warning should stop occurring in Reanimated 2.

Reviewers: palys-swm

Reviewed By: palys-swm

Subscribers: KatPo, zrebcu411, Adrian

Differential Revision: https://phabricator.ashoat.com/D185
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests