zendesk / react-native-sunshine-conversations

React Native wrapper for Smooch.io
MIT License
36 stars 26 forks source link

Fix completionhandler and podspec #47

Closed adbl closed 6 years ago

adbl commented 6 years ago

login / logout are not working due to the completionHandler argument. This PR now returns a promise which will resolve or reject with the error information. I took the liberty of renaming logoutWithCompletionHandler to just logout (completion handler is an implementation detail). The methods can be just called and ignore the promise result as well, like before.

Something worrying is that under certain conditions I can see warnings in XCode console, such as:

<SMOOCH::WARNING> User X is already logged in. Ignoring!
<SMOOCH::WARNING> Login called with nil or empty jwt. Ignoring!

In these cases the promise will neither resolve nor reject because completionHandler isn't being called. Not calling the completionHandler seems like a bad idea to me but that problem belongs to the Smooch iOS library. However, not resolving a promise is really bad because it would break code that waits for this promise. Not sure what to do about that? Maybe it's better I just set completionHandler to nil and pretend nothing ever fails? That would be sad though because the whole point was to be able to detect errors...

The podspec was also broken due to paths which has also been fixed here.

Fixes #46