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

fix: android: make sure initial network value is not stale on app startup #553

Merged

Conversation

cristianoccazinsp
Copy link
Contributor

Overview

When the app is started with no connectivity (e.g., wifi off or airplane mode), the library currently reports unknown and connected = true values because network listeners are never called when there's no network info, so all default values are read.

After this, we now try to properly set the initial network values on register (similar to what the old broadcast received does and was not carried over to the new implementation.

I've also taken the chance to replace the Runnable with a lambda as suggested by the linter.

P.S.: no idea why old commits were carried over, I'm sure I branched from master.

Test Plan

Tested on Google Pixel 5 (Android 12, RN 0.66.3)

Steps to test:

  • Turn airplane mode on
  • Open the app (cold start, no previously stored states)
  • Check results of NetInfo.fetch() on mount or similar, type should be none
  • Turn off airplane mode, and make sure connectivity is reported correctly.

Test again, but now without airplane mode on startup, to make sure the app still reports connectivity when first started if available.

Make sure we use a final reference to the instance variables in case they are modified in another callback while being used.
…ing the app with no connectivity reports it as none instead of unknown.

- Replace Runnable with lambda as suggested by linter
Copy link
Contributor

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

Hmm not sure about the unclean commit history but I can certainly squash it away.
git reset --hard && git fetch upstream && rebase upstream/master would blow it out (assuming upstream is what you call the remote for main repo), then cherry-pick just your commit

I wouldn't do that now - I'll merge and squash as the diff itself is clean but that's what I'd do if my tree was in this state prior to pushing

Anyway, LGTM :-), even the intersectionality with #548 I think - thanks!

@mikehardy mikehardy merged commit c05080f into react-native-netinfo:master Dec 20, 2021
github-actions bot pushed a commit that referenced this pull request Dec 20, 2021
## [7.1.7](v7.1.6...v7.1.7) (2021-12-20)

### Bug Fixes

* **android:** populate network value during initial module startup  ([#553](#553)) ([c05080f](c05080f))
@matt-oakes
Copy link
Collaborator

🎉 This PR is included in version 7.1.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

4 participants