Event system fixes for compatibility with react-native 0.50

[janicduplessis]

The DummyViewManager technique to register event types doesn't work anymore because events now registered when the component class is used. We actually still need DummyViewManager on Android so native animated knows about the custom event name mapping. iOS has some magic function to convert from topX to onX so it's not needed anymore.

We can now use ReactNativeBridgeEventPlugin to register custom event types instead. Event names should also be topX instead of onX on Android so we can have the same event types as iOS.

Tested a bunch of examples in the example app with RN 0.50.0

This is in no way backward compatible with previous RN versions since ReactNativeBridgeEventPlugin does not exist so I haven't made any effort to make other changes backwards compatible.

[janicduplessis]

If we really want to make this backwards compatible I think we could use requireNativeComponent to register the events from the DummyView but I feed like this is a cleaner solution since we can get rid of that.

[kmagiera]

Thanks @janicduplessis for being on top of this! TBH I believe this library is at the stage when users wouldn't be surprise when they need to deal with breaking changes while upgrading. Yet, since I expect there are still some bugs that may be blocking ppl using this lib for their product, I'd like to wait with updating the core dependency to an "unstable" branch (even though RN rc's are perhaps more "stable" than this library).

I'll be more than happy to merge this in as soon as 0.50 is released. I also like this approach more considering we can get rid of the "DummyView" hack. But if you could offer a workaround that would make it work with 0.50-rc and 0.49 than I'd very much appreciated it. In which case we can also leave this PR open and revisit when 0.50 is out.

[janicduplessis]

@kmagiera 0.50 is out, I don't really have the time to investigate a backwards compatible solution at the moment sadly.

[janicduplessis]

Just tried this PR again and noticed an issue with Swipeable + native animations, not sure if it's related to those changes but I'll have a look.

[kmagiera]

Thanks for checking @janicduplessis. Let me know what your findings are. I'm fine with merging this in now that 0.50 is out

[kmagiera]

Seems I figured it out. The problem was that native driver would register event under "onGestureHandler" name but was actually getting "topGestureHandler" as the event name and couldn't map that to any value (here is the relevant code in RN)

So since I don't understand the recent changes in RN core regarding events I have two possible solutions for the problem:

1. keep DummyViewManager and update its exported direct event constants so that the registration name for "topGestureHandlerEvent" is "onGestureHandlerEvent" (similarly to how was it done for scroll events here: facebook/react-native@ca834f9)
2. In ReactNativeBridgeEventPlugin.processEventTypes use names starting with "on" instead of "top" and update EVENT_NAME in native code to also use names starting with "on"

@janicduplessis If you understand what the events changes are all about I would trust your judgment to select the correct approach. If you are not certain I can ask around or research it on my own.

Otherwise this is great stuff. Thanks!

[janicduplessis]

@kmagiera I have figured out something that works, from what I understand the main issue is that events are now registered lazily the first time the component is rendered which means it will never happen for DummyViewManager. The best solution seem to be to call ReactNativeBridgeEventPlugin.processEventTypes manually but on Android we also need to keep the DummyViewManager since native animated uses the constants it provides to map event names. This is not needed on iOS because of some magic code to convert from topX to onX.

I did try your second solution but it does not work on iOS because of the magic topX to onX conversion. I could platform check but I feel this is better.

Also need to update getShadowNodeClass to return ReactShadowNodeImpl instead.

[kmagiera]

Instead of doing this I'd prefer to change the baseclass of ViewManager and use ReactViewManager instead. It doesn't make much of a difference because views won't be instantiated anyways but may prevent similar incompatibility issues in the future

[kmagiera]

Can you put both "topGestureHandlerEvent" and "onGestureHandlerEvent" as strings here instead of using constant? Or maybe define "onXXX" as a constant in the event class? I think it will make it easier to understand how events are translated in the future as well as may help with spotting typos etc

(same below)

[kmagiera]

I tested this and seems to be working fine. I'm going to merge it and fix the small comments on my own. Thanks for working on this @janicduplessis !