Native Animated - Restore default values when removing props on Android

[janicduplessis]

Same as #11819 but for Android. I didn't notice the bug initially in my app because I was using different animations on Android which did not trigger this issue.

Test plan
Created a simple repro example and tested that this fixes it. https://gist.github.com/janicduplessis/0f3eb362dae63fedf99a0d3ee041796a

[mkonicek]

Hi Janic, do you know who can review changes to native animated on Android?

[janicduplessis]

Replied on slack

[janicduplessis]

Not sure if there is a better way to do this, we have to inject a mock in here but there is no way currently. I tried using PowerMockito.whenNew but could not get it to work.

[janicduplessis]

Ideally we'd also test multithreading here to make sure everything gets called on the proper thread and is safe but I'm not sure how to do that.

[janicduplessis]

Also added a regression test and a few extra tests around the scheduling of NativeAnimatedNodesManager operations.

[hramos]

Sorry for the delayed review. Is there someone on the React or RN team that you think should review this? I can reach out to them.

[hramos]

Is reactCtx defined at this point?

[janicduplessis]

You are right, I messed that up when fixing conflicts.

[hramos]

Was this method used anywhere?

[janicduplessis]

This is a method native modules can override that gets called when batches are completed so it is fine to remove it since it will just call the one from the super class.

[janicduplessis]

@AaaChiuuu or Andrew Chen could review this

[janicduplessis]

@hramos Could you try to have this reviewed?

[hramos]

Just pinged @AaaChiuuu

[hramos]

Approving this without any further review as it has been a while since the review was requested, and Janic has already been using this in production for a while. I don't want to slow down contributions from the core team unnecessarily.

[facebook-github-bot]

@hramos 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.

[pull-bot]

This PR has been submitted by a core contributor.

Attention: @janicduplessis

Generated by 🚫 dangerJS

[janicduplessis]

@hramos Oups, there was a buck build issue and a test I forgot to update, circle is green now it should land smoothly.

[facebook-github-bot]

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

[hramos]

ac43548 is in the process of being reverted. The NativeAnimatedModuleTest started failing when this landed:

javac_jar
stderr: ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedModuleTest.java:67: error: cannot find symbol
        mUIBlocks.add(invocation.getArgumentAt(0, UIBlock.class));
                                ^
  symbol:   method getArgumentAt(int,java.lang.Class<com.facebook.react.uimanager.UIBlock>)
  location: variable invocation of type org.mockito.invocation.InvocationOnMock

ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedModuleTest.java:74: error: cannot find symbol
        mUIBlocks.add(0, invocation.getArgumentAt(0, UIBlock.class));
                                   ^
  symbol:   method getArgumentAt(int,java.lang.Class<com.facebook.react.uimanager.UIBlock>)
  location: variable invocation of type org.mockito.invocation.InvocationOnMock
Errors: 2. Warnings: 0.

At this point I am not yet sure why this is breaking internally, when the PR passed tests. Tests also passed on master after this landed.

[janicduplessis]

@hramos Strange that it doesn't fail on OSS CI, is it possible that you are using a different version of mockito internally? According to OSS gradle.properties (https://github.com/facebook/react-native/blob/master/ReactAndroid/gradle.properties#L10) we are using version 1.10.19, same with buck https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/third-party/java/mockito/BUCK#L19.