search

react-native-webrtc / react-native-voip-push-notification

React Native VoIP Push Notification - Currently iOS only
ISC License
213 stars 83 forks source link

#86 Fix singletone class initialisation #87

Closed Romick2005 closed 3 years ago

Romick2005 commented 3 years ago

Also take care of start stop Observing issue on metro bundler restart/reload/hot-reload.

zxcpoiu commented 3 years ago

@Romick2005

So you do not have init multiple times after this PR?

Romick2005 commented 3 years ago

Actually init called multiple times, but initPrivate only ones. Also on multiple call of init the returning result is same initialised only one time object. But generally this PR implements real singletone pattern.

zxcpoiu commented 3 years ago

About startObserving and stopObserving, would you please remove them and treat it as your own workaround in your private branch?

I don't think to ignore stopObserving when debug is a valid solution here.

Reload process has causes similar issues around different libs ( like mis-created multiple instances for react-native-webrtc, IIRC ), given that the reload only happens on DEBUG mode only, and we can not assure that the behavior is a bug or whether it will change in the future or not, not to say there are different algorithms / fallbacks / Engines there ( ex: full reload / HMS Fast Refresh...etc ), I prefer not to add this for general usages. Personally, I only find Fast Refresh useful when I'm tweaking UI stuff, if the test is related to native library, I always did a clean restart ( swipe out ) to prevent any reload-caused issues.

I think the more appropriate way would be needing an event api like onRefresh or onReload for the native side provided by React Native, and user can config them to reset some states.

zxcpoiu commented 3 years ago

@jonastelzio @danjenkins

Hello, would you mind to take a look if this solves your problem described in #85?

zxcpoiu commented 3 years ago

Thanks @Romick2005 I just fix the style to match with the current code base, sorry to push to your branch directly, but I think it's more efficient. If you want to adjust / fix the style, feel free to send another PR, thanks!

zxcpoiu commented 3 years ago

Since this an open source, so it would be better to follow the current code base's style, and makes minimal changes in a single PR.

But fire a PR to adjust them is acceptable for sure

Romick2005 commented 3 years ago

Thank you @zxcpoiu, it's just styling, so never mind. No worries I am ok with this, until it works as expected :).