Updated deprecated NetInfo usage

[kbrandwijk]

Fixes #39.

• fetch() -> getConnectionInfo()
• change -> connectionChange
• updated reach values

[sorodrigo]

So I’ve been busy this past few weeks but I’m trying to make space this weekend to test this and work on other stuff that’s pending. Just wanted to let you know. Thanks

[harshmaur]

Hey Guys, Thanks for the PR, When can this be merged? @sorodrigo

[sorodrigo]

@wacii this introduces a breaking change because the react-native API changed. This is related with all other breaking changes that many libs had to make to stay up to date with react 16 and react-native >= 0.47.

Before the API redux-offline exposed the values in uppercase (this decision was taken because in android values were uppercased and in iOS they weren't) perhaps a better solution back then would've been to respect what react-native exposed (as it's done now).
I really want to include this PR now.

So shall we make a breaking change bump on next release?

EDIT: As an alternative the methods can be updated to use latest react-native version, but continue to expose that data in uppercase.

[wacii]

Sounds good. Eventually I would like to move the defaults into their own package/s to isolate these changes, as this obviously doesn't affect the library when used on the web. But that is for another time.

The NetInfo data isn't technically exposed. There are no selectors for it and it isn't mentioned in the documentation. In fact, it isn't actually used anywhere in the library. So change the data to whatever is convenient.

[kbrandwijk]

@wacii I was just looking to the source code for that exact same reason. To see where it was actually used or exposed. And yes, it is part of the NetInfo in the store, but it's still an internal implementation detail as far as I can see, so I think it's not a breaking change to the public interface.

[wacii]

@kbrandwijk It doesn't change the interface but won't this code no longer work with older versions of React Native?

[kbrandwijk]

@wacii I agree. If there is a policy for version support, I can make it support n-2 or something. Changes would include case-insensitivity of connectionType, and fallback to use the old methods if the new ones are not available.

[sorodrigo]

Perhaps the best solution is to include another default (something like detectNetwork.native.legacy.js) compatible with the previous version. Along with a deprecation notice.

[kbrandwijk]

It would be unclear for me how those would automagically be linked together. How would that work?

[sorodrigo]

It wouldn't, if we want to make it automagically best I can think of is:

// detectNetwork.native.legacy.js
export default DetectNetwork;

// detectNetwork.native.js
import LegacyDetectNetwork from './detectNetwork.native.legacy.js'

/* new code */

const isLegacy = typeof NetInfo.getConnectionInfo === 'undefined';
export default callback => (isLegacy ? LegacyDetectNetwork(callback) : DetectNetwork(callback))

I'm open to other suggestions.

[kbrandwijk]

Although that would lead to some code duplication, I guess it's better indeed to keep the two implementations isolated from each other, so it can easily be deprecated later on.

[kbrandwijk]

Do you want to do something to normalize connectionTypes across the two implementations, or should the RN connectionTypes be leading? I would prefer the latter.

[sorodrigo]

I think the way it is now is just fine. 🤙