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(web): verify window exists before attempting to access navigator #666

Merged
merged 1 commit into from May 5, 2023

Conversation

jspizziri
Copy link
Contributor

@jspizziri jspizziri commented Apr 29, 2023

Fix an issue causing an SSR exception if getCurrentState is called.

Overview

As things currently stand, if for any reason you end up calling getCurrentState during SSR you end up getting an exception due to navigator being undefined.

Test Plan

  1. Call getCurrentState during SSR without this change. See you receive an exception.
  2. Call getCurrentState during SSR with this change. See you do not receive an exception.

Fix an issue causing an SSR exception if `getCurrentState` is called.
@jspizziri
Copy link
Contributor Author

@mikehardy given your comment here #655 (comment) I wanted to ping you as @matt-oakes was auto-assigned as the Reviewer when I submitted the PR. Wanted to put it on someone's radar who was still involved in the project. I know you're quite busy so sorry for adding something onto your list!

Anyway, it's a pretty straightforward PR IMHO. Happy to update it to something more stylistically appealing to ya'll (e.g. navigator?.onLine || false, etc.)

✌️

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.

Looks imminently reasonable, thanks! And thanks for your patience + understanding

@mikehardy mikehardy added the pending merge A PR that will be merged shortly, waiting for CI or final comment label May 5, 2023
@mikehardy mikehardy merged commit 60b99f2 into react-native-netinfo:master May 5, 2023
2 checks passed
@mikehardy mikehardy removed the pending merge A PR that will be merged shortly, waiting for CI or final comment label May 5, 2023
github-actions bot pushed a commit that referenced this pull request May 5, 2023
## [9.3.10](v9.3.9...v9.3.10) (2023-05-05)

### Bug Fixes

* **web:** verify `window` exists before attempting to access navigator ([#666](#666)) ([60b99f2](60b99f2))
@matt-oakes
Copy link
Collaborator

🎉 This PR is included in version 9.3.10 🎉

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

3 participants