prscX / react-native-tooltips

React Native: Native Tooltip View
Apache License 2.0
271 stars 40 forks source link

Bugfix: /android callback #42

Closed danielwinkler closed 4 years ago

danielwinkler commented 4 years ago

Visibility of the tooltip is hooked up to pressing a button too short. With that the tooltip may show up and disappear quickly. If that happens, the following exception is raised, which doesn't really make sense.

com.facebook.react.bridge.CallbackImpl.invoke
CallbackImpl.java, line 25
java.lang.RuntimeException: Illegal callback invocation from native module. This callback type only permits a single invocation from native code

I could not reproduce it with debug and local release builds. However, once it was published and coming from the store, it is easily reproducable

com.facebook.react.bridge.CallbackImpl.invoke CallbackImpl.java:25
px.tooltips.RNTooltipsModule$1$1$1.onHide RNTooltipsModule.java:108
com.github.florent37.viewtooltip.ViewTooltip$TooltipView$2.onAnimationEnd ViewTooltip.java:565
android.view.ViewPropertyAnimator$AnimatorEventListener.onAnimationEnd ViewPropertyAnimator.java:1111
android.animation.Animator$AnimatorListener.onAnimationEnd Animator.java:554
android.animation.ValueAnimator.endAnimation ValueAnimator.java:1242
android.animation.ValueAnimator.doAnimationFrame ValueAnimator.java:1484
android.animation.AnimationHandler.doAnimationFrame AnimationHandler.java:146
android.animation.AnimationHandler.access$100 AnimationHandler.java:37
android.animation.AnimationHandler$1.doFrame AnimationHandler.java:54
android.view.Choreographer$CallbackRecord.run Choreographer.java:965
android.view.Choreographer.doCallbacks Choreographer.java:791
android.view.Choreographer.doFrame Choreographer.java:722
android.view.Choreographer$FrameDisplayEventReceiver.run Choreographer.java:952
android.os.Handler.handleCallback Handler.java:883
android.os.Handler.dispatchMessage Handler.java:100
android.os.Looper.loop Looper.java:214
android.app.ActivityThread.main ActivityThread.java:7356
java.lang.reflect.Method.invoke Method.java
com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run RuntimeInit.java:492
com.android.internal.os.ZygoteInit.main ZygoteInit.java:930

I fixed by adding a workaround that would generate a new callback with each call of Tooltip.Show

    // try to work around duplicate callback invocation error on android
    const cbGenerator = () => () => {
      props.onHide && props.onHide();
    };
prscX commented 4 years ago

Thanks @danielwinkler for raising the PR.

Does the workaround shared works for you? because you are calling cbGenerator() function which returns arrow function itself, which is similar to old implementation. I am not sure how this has helped you.

Thanks </ Pranav >

danielwinkler commented 4 years ago

To be honest, I cannot explain why this fix helped. But I also cannot explain why I get the single invocation RuntimeException in the first place?

Do you have an idea why there seems to be a duplicate invocation?

The rationale behind the current fix was, that there might have been some optimization going on in RN because of the static function and the generation of the Android callback. If it was pure JS/TS then your approach

RNTooltips.Show(target, parent, props, () => {
    props.onHide && props.onHide();
});

will generate a new function with every iteration, so no need to add my workaround.

danielwinkler commented 4 years ago

I just realized I got another such exception with the PR, so you can ignore it. Still, it would be great if we could fix the problem!

prscX commented 4 years ago

Thanks @danielwinkler for sharing detailed explanation. I will look into it, why callback is getting called multiple times. I will update you once I have a fix.

Thanks

danielwinkler commented 4 years ago

@prscX did you have any luck with the bug?