Animated - Add missing super calls to fix native animated crash

[janicduplessis]

There was some missing super.__detach calls in some Animated nodes, we rely on the base class being called to drop the node in the native implementation, not doing so will cause some nodes to leak. It also resulted in a crash when removing certain nodes because they would get updated after being detached.

Test plan
Reproduced the crash by unmounting a view that uses a DiffClamp node and made sure this fixes it. Also tested that other native animations still worked properly.

Fixes #11317

[janicduplessis]

cc @ide @ericvicenti

[ide]

@facebook-github-bot shipit

[mkonicek]

Nice! Merging.

[facebook-github-bot]

@mkonicek has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[facebook-github-bot]

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

[nihgwu]

ping @mkonicek

[satya164]

@facebook-github-bot shipit

[facebook-github-bot]

@satya164 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[nihgwu]

import failed, wish this could be in 0.44RC @janicduplessis @javache

[facebook-github-bot]

@javache has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[javache]

Please rebase this diff so our internal CI can process it, thanks!

[janicduplessis]

@javache Done!

[facebook-github-bot]

@javache has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[javache]

This is failing one of the jest tests

 FAIL  Libraries/Animated/src/__tests__/AnimatedNative-test.js
  ● Native Animated › Animated Graph › creates and detaches nodes
    expect(jest.fn()).toHaveBeenCalledTimes(2)
    
    Expected mock function to have been called two times, but it was called three times.
      
      at Object.<anonymous> (Libraries/Animated/src/__tests__/AnimatedNative-test.js:216:53)

[janicduplessis]

@javache The test was asserting the broken behavior, it creates 3 nodes so it should call into dropAnimatedNode 3 times.

[facebook-github-bot]

@javache has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[neiker]

Still having this error on RN 0.62

[AwrminKhodaei]

@janicduplessis still having this error on RN 0.63.3

[janicduplessis]

Can you open an issue with code to reproduce the problem and I can have a look.

[AwrminKhodaei]

@janicduplessis Sure. I will create a new repo containing the example after that will mention repo in the issue.

[littlehome-eugene]

Is it the same error as the following?
We are having it with 0.63.3

Caused by com.facebook.react.bridge.JSApplicationCausedNativeException: Illegal node ID set as an input for Animated.Add node
       at com.facebook.react.animated.AdditionAnimatedNode.update(AdditionAnimatedNode.java:41)
       at com.facebook.react.animated.NativeAnimatedNodesManager.updateNodes(NativeAnimatedNodesManager.java:544)
       at com.facebook.react.animated.NativeAnimatedNodesManager.runUpdates(NativeAnimatedNodesManager.java:457)
       at com.facebook.react.animated.NativeAnimatedModule$1.doFrameGuarded(NativeAnimatedModule.java:104)
       at com.facebook.react.uimanager.GuardedFrameCallback.doFrame(GuardedFrameCallback.java:29)
       at com.facebook.react.modules.core.ReactChoreographer$ReactChoreographerDispatcher.doFrame(ReactChoreographer.java:175)
       at com.facebook.react.modules.core.ChoreographerCompat$FrameCallback$1.doFrame(ChoreographerCompat.java:85)
       at android.view.Choreographer$CallbackRecord.run(Choreographer.java:997)
       at android.view.Choreographer.doCallbacks(Choreographer.java:797)
       at android.view.Choreographer.doFrame(Choreographer.java:728)
       at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:984)
       at android.os.Handler.handleCallback(Handler.java:883)
       at android.os.Handler.dispatchMessage(Handler.java:100)
       at android.os.Looper.loop(Looper.java:237)
       at android.app.ActivityThread.main(ActivityThread.java:8167)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:496)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1100)

[damo-veda]

@littlehome-eugene I have the same errror in RN 0.62.2
Any solutions?

[littlehome-eugene]

@damo-veda unfortunately not yet.. we are replacing an old library that uses animation and will see if it helps..

[damo-veda]

@littlehome-eugene Any fixes for it so far?
Thanks.

[jennifer-elyse]

Any fixes for this? My app just randomly crashes with this error. I tried downgrading to reanimated 1.9.0 but this didn't help

[FallenDestiny]

Anything on this? We are using RN 0.63.4 and we have same issue 🙏

[arkisoul]

Any fixes for this? My app just randomly crashes with this error. I tried downgrading to reanimated 1.9.0 but this didn't help

I am facing a similar issue. Only on Android devices. Did you find any solution for this issue or the root cause?

[MursiDirect]

Any Fix for this problem, yet?

[meliodev]

Still facing this issue randomly.