search

ak1394 / react-native-tts

React Native Text-To-Speech library for Android and iOS
601 stars 154 forks source link

Properly resolving promise for calls to setDefaultEngine #134

Closed tt-driver-apps closed 4 years ago

tt-driver-apps commented 4 years ago

I think that the recent modifications for changing the default engine have a small issue, as they are not resolving the promise. For example, the following scenario doesn't work and waits forever:

await Tts.getInitStatus();
await Tts.setDefaultEngine(myPreferredEngine);

I am not sure about the need of the call to resolveReadyPromise, as I suspect it is only needed in the initialisation and not here, however I decided to leave it there for now (see comment in the code).

ak1394 commented 4 years ago

Yeah, I think you're right! Thanks for spotting the issue. I'm going to merge your change and get rid of resolveReadyPromise() stuff as well. Would you be able to test the changes (at the moment I don't have the environment to test it myself), and I'll publish new release once it's tested.

ak1394 commented 4 years ago

I decided to leave resolveReadyPromise() stuff in place. The issue is that if someone calls getInitStatus() after ready = null; is executed and before ready = (status == TextToSpeech.SUCCESS) ? Boolean.TRUE : Boolean.FALSE; is executed, promise from getInitStatus() will never resolve.

There is a different issue of why someone should call getInitStatus() after calling setDefaultEngine(), but it's still safer to leave it there.

Also, since there are only your changes now, I suppose it's safe to release without additional testing, which I'll do now.