Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow startNotifier to optionally skip the initial check. #99

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

alwaysthecritic
Copy link

Because sometimes you only want to know when network comes back.
Furthermore, the initial check is not on main thread which can cause some
issues for clients.

Certainly we needed this change to get rid of nasty crashes and difficult thread timing behaviour in our app. In our use case we only cared to know when connectivity was re-established for auto-reload functionality. So we didn't want to initial check, and occasionally the initial callback would arrive before the result of startNotifier had actually been assigned, so we didn't have access to the Reachability object (which we wanted to check if we had WiFi or mobile access). Which caused crashes until we figured out what was going on.

Copy link
Owner

@ashleymills ashleymills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@t-ob Happy to take this update, but please use set the default to false so existing behaviour is unchanged.

Also, please update README.md to match
.

@ashleymills ashleymills changed the base branch from master to develop September 18, 2017 09:16
@ozgur
Copy link

ozgur commented Oct 6, 2018

@ashleymills any update on this? seems like a very cool feature to have. I think checkImmediately is set to true by default to leave the current behavior unchanged. Am I wrong?

I can open a new PR if you don't have time to look.

@ashleymills
Copy link
Owner

I'll take another look at this one

@SlavcoPetkovski
Copy link

Any update on this? I also don't want to be notified initially, only to be notified when there is a change in reachability. If you are not merging this one, can you advice me for a workaround?

@bdkjones
Copy link

This PR has been outstanding for several years and I just had to fork the repo to accomplish the very same thing. It would be good to merge this and get it done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants