Implement native Animated value listeners on Android

[janicduplessis]

Adds support for Animated.Value#addListener for native driven nodes on Android. This is based on work by @skevy in the exponent RN fork. Also adds a UIExplorer example.

** Test plan **
Run unit tests

Tested that by adding a listener to a native driven animated node and checked that the listener callback is called properly.

Also tested that it doesn't crash on iOS that doesn't support this yet.

[ghost]

By analyzing the blame information on this pull request, we identified @buba447 and @kmagiera to be potential reviewers.

[eslint-bot]

property current Property not found in React component

[kmagiera]

Looks good to me. For sending events to JS you should use a subclass of EventDispatcher (updates for a single animated value can be coalesced which is one of the reasons to use EventDispatcher over just calling JS module directly). Also: do you mind adding a simple test for listeners to NativeAnimatedNodeTraversalTest.java?

[janicduplessis]

@kmagiera EventDispatcher seems pretty coupled with component events. It would require quite a bit of refactoring to be able to dispatch events for a native module. I looked at it a bit and every native module seems to call directly into RCTDeviceEventEmitter.

But I agree that being able to coalesce these event especially when JS thread is busy it would be nice not to queue up an event for every frame. I actually had to implement a similar logic for requestIdleCallback recently so having some generic way to coalesce events from native modules would be great like we have for native components.

[kmagiera]

Ah ok, I see now that EventDispatcher sends events to RCTEventEmitter and here you want to deliver them to RCTDeviceEventEmitter. Although, so far we haven't been using RCTDeviceEventEmitter for sending events that could be coalesced (e.g. keyboard show/hide, passing network data, app state changes). So I think we should at least create an issue to refactor EventDispatcher to support device events and to use it here.

[brentvatne]

@kmagiera - once that issue is created is this good to ship? cc @janicduplessis

[ryangomba]

Aren't we going to have a problem if we happen to re-render during an animation? Namely, out-of-date JS values will override up-to-date native values when the view us updated. Contrived example:

Tick 1 (first render):

• js=0
• native=0

Tick 2 (animation driven natively):

• js=0
• native=0.1

Tick 3: (still driving, listener called on the JS side)

• js=0.1
• native=0.2

Tick 4: (still driving, we call a re-render)

• js=0.2
• native=0.3

Tick 5 (re-rendered):

• js=0.2
• native=0.2 (but we'd expect 0.4)

It seems like the old __getValue, this updated __getValue behavior, and __getAnimatedValue all have slightly different intents:

(1) What prop values should we be setting on the native view when we render?
(2) What are the current values of animated props?
(3) What values are we driving and may need update via setNativeProps? (inverse of 1)

It seems like there is a great tension between an animate prop being treated like any other prop, and being managed independently.

[kmagiera]

I would not see this as an issue. It's similar with all the events we receive from native. E.g. when you register onLayout event it doesn't necessarily mean the values you're getting are the actual dimensions of the element, those are just values that has been calculated during some layout pass in the past and might have already been changed from JS before we managed to process onLayout event

[ryangomba]

It's fine that the values passed to JS aren't representative of the true state of the element. What's not okay is if we take this stale data and use it to update the element, which we will do here whenever we render the element again. Concretely, you might see the animated view unexpectedly snap backward in time when you call setState, which would be jarring.

[janicduplessis]

I'll take another look at this to make sure it doesn't cause issues when re-rendering during an animation when there is no value listener. It did work properly when using one since we update the js value of the node.

[ryangomba]

Another contrived example, but imagine there is no value listener and you do the following:

• render a view with opacity = new Animated.Value(0)
• call anim.setValue(1)
• drive a native animation from 1 to 0.5
• call setState({foo: 'bar'}); the view will be re-rendered with opacity=1

The bug will pop up when both the following conditions are true:

1. the value has changed on the JS side since the last render
2. the value on the JS side and the native side are out of sync

[kmagiera]

@brentvatne Yes, I believe this is good to be merged. Just would prefer someone who is more actively maintaining this piece of code to merge this in :)

[facebook-github-bot]

@janicduplessis updated the pull request.

[facebook-github-bot]

@janicduplessis updated the pull request.

[facebook-github-bot]

@janicduplessis updated the pull request.

[janicduplessis]

@ryangomba You were right about issues with removing the __isNative checks so I went ahead and used your solution from #9120 and just changed the condition to !value.__isNative || value instanceof AnimatedStyle in AnimatedProps and now re-rendering doesn't break the native driven animation.

[janicduplessis]

This should be good to go now.

@kmagiera You're probably still the one that knows the most about this code so feel free to shipit :)

[ide]

I'll ship this tomorrow if kmagiera isn't able to get around to it.

[ide]

@facebook-github-bot shipit

[ghost]

Thanks for importing.If you are an FB employee go to Phabricator to review internal test results.