GoldenOwlAsia / react-native-twitter-signin

MIT License
167 stars 302 forks source link

init method return promise, but it is used as sync in example #72

Open punksta opened 6 years ago

punksta commented 6 years ago

https://github.com/GoldenOwlAsia/react-native-twitter-signin/blob/5d57b21260f6b3c3be264c15eca8d5a04ce328be/Example/TwitterButton.js#L24-L40

https://github.com/GoldenOwlAsia/react-native-twitter-signin/blob/5d57b21260f6b3c3be264c15eca8d5a04ce328be/android/src/main/java/com/goldenowl/twittersignin/TwitterSigninModule.java#L47-L56

I am not sure, but looks like possible issue is here. Also: better to wrap init into try {} catch() {} to avoid uncathed native crashes

jslz commented 6 years ago

+1 I think this is confusing at least, and maybe buggy at worst.

https://github.com/facebook/react-native/blob/master/React/Base/RCTBridgeModule.h#L133

https://github.com/GoldenOwlAsia/react-native-twitter-signin/blob/master/ios/RNTwitterSignIn.m#L23

The init() should invoke the resolve or reject, or should be written to not look like it uses promises at all?

frncs-eu commented 5 years ago

I've stumbled into this problem today, and i've spent 4 hours straight debugging my own code, trying to understand why the init was resolving in Android but not on iOS. I then took a look inside the plugin code... The java implementation does indeed work correctly as a promise, while in the obj-C code the resolve parameter is never executed at all:

RCT_EXPORT_METHOD(init: (NSString *)consumerKey consumerSecret:(NSString *)consumerSecret resolver:(RCTPromiseResolveBlock)resolve  
rejecter:(RCTPromiseRejectBlock)reject)
{
    [[Twitter sharedInstance] startWithConsumerKey:consumerKey consumerSecret:consumerSecret];
}

I assume this is wrong, as the js interface should be identical between the two platforms. Let me know if I can be of any help.