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!: cancel _checkInternetReachability request on timeout & cancel via AbortController #678

Conversation

jcarrag
Copy link
Contributor

@jcarrag jcarrag commented Jul 25, 2023

Taken from #439 (comment)

@jcarrag jcarrag requested a review from matt-oakes as a code owner July 25, 2023 13:50
@jcarrag jcarrag force-pushed the fix_checkInternetReachability_timeout branch from 7766ff1 to 0f71dd9 Compare July 25, 2023 17:57
@jcarrag jcarrag changed the title fix: cancel _checkInternetReachability request on timeout fix: cancel _checkInternetReachability request on timeout & cancel Jul 25, 2023
@jcarrag
Copy link
Contributor Author

jcarrag commented Aug 7, 2023

Hi @mikehardy does this look ok to you? If you want tests for this change then I can give them a go

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.

Neat trick! Thank you for submitting the PR and thank you for your patience

@mikehardy
Copy link
Contributor

I am closing an re-opening just to see if that will kick off CI

@mikehardy mikehardy closed this Nov 1, 2023
@mikehardy mikehardy reopened this Nov 1, 2023
@mikehardy mikehardy added the pending merge A PR that will be merged shortly, waiting for CI or final comment label Nov 1, 2023
eslint behaves differently right now between windows/macOS and ubuntu
the CI check is on ubuntu, and some problems are only showing up there
BREAKING CHANGES: netinfo now requires AbortController, node v16 / edge required
it will likely not work on internet explorer from this version onwards
@mikehardy
Copy link
Contributor

  • I fixed the lint issues that frustratingly-for-everyone-I-am-sure only show up on Ubuntu
  • I bumped CI to node v16 as AbortController is only available from v16 up, this represents a breaking change since it will likely not be present on Internet Explorer either (though it is on edge...) so I'm marking it as a breaking change also

@mikehardy mikehardy changed the title fix: cancel _checkInternetReachability request on timeout & cancel fix!: cancel _checkInternetReachability request on timeout & cancel via AbortController Nov 1, 2023
@mikehardy mikehardy merged commit dc7a892 into react-native-netinfo:master Nov 1, 2023
2 checks passed
@jcarrag
Copy link
Contributor Author

jcarrag commented Nov 1, 2023

Cheers Mike

mikehardy pushed a commit that referenced this pull request Nov 1, 2023
…cancel via AbortController (#678)

* fix!: cancel _checkInternetReachability request on timeout & cancel

Taken from #439 (comment)

* style(lint): yarn validate:estlint --fix

eslint behaves differently right now between windows/macOS and ubuntu
the CI check is on ubuntu, and some problems are only showing up there

* fix!: node v16 is now the minimum supported version - AbortController

BREAKING CHANGE: netinfo now requires AbortController, node v16 / edge required
it will likely not work on internet explorer from this version onwards
github-actions bot pushed a commit that referenced this pull request Nov 1, 2023
# [10.0.0](v9.5.0...v10.0.0) (2023-11-01)

* fix(cancel)!: cancel _checkInternetReachability request on timeout & cancel via AbortController (#678) ([4bfd3e2](4bfd3e2)), closes [#678](#678) [/github.com//issues/439#issue-787487438](https://github.com//github.com/react-native-netinfo/react-native-netinfo/issues/439/issues/issue-787487438)

### BREAKING CHANGES

* netinfo now requires AbortController, node v16 / edge required
it will likely not work on internet explorer from this version onwards
@mikehardy mikehardy removed the pending merge A PR that will be merged shortly, waiting for CI or final comment label Nov 1, 2023
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.

None yet

2 participants