Android: Support RN50-51 changes #652
Conversation
…lingResource sync by any means, but should help in dire straits.
rotemmiz
changed the title
rotemmiz
changed the title
|
|
||
| static { | ||
| try { | ||
| Class reactNativeVersion = Class.forName("com.facebook.react.modules.systeminfo.ReactNativeVersion"); |
There was a problem hiding this comment.
This class is present only in RN 50 and above.
Later, the getMinor call could fail on these older versions. As we don't actually use this API on older versions, I think we could just set it to 49.5.0 in the catch as a default version.
| if (callback != null) { | ||
| callback.onTransitionToIdle(); | ||
| } | ||
| Log.i(LOG_TAG, "AnimatedModule is idle, no operations"); |
There was a problem hiding this comment.
I think we shouldn't log out idle states. Only busy states.
Originally, I also logged both states, but after the initial debugging it become overwhelming.
| if (callback != null) { | ||
| callback.onTransitionToIdle(); | ||
| } | ||
| Log.i(LOG_TAG, "AnimatedModule is idle, no operations"); |
There was a problem hiding this comment.
I looked at this diff.
I am not even sure that we need an animatedIdlingResource at all on later RN versions. It seems, they delegate everything to the UIModule. I don't fully understand the new flow in that code though.
The check here certainly doesn't do any harm.
There was a problem hiding this comment.
Well, it was a trial and error kinda fix :)
But I think that there's a small gap between the time when the UIBlock is posted and when it starts executing.
|
Done deal |
|
Awesome! 👍 |
This PR aims to:
waitForReactNativeLoadby adding Thread.sleep() on RN>=50, not ideal, but will enable usage of Detox on RN51 projects even before we find the true culprit of this sync defect.