meteorrn / meteor-react-native

Meteor client for React Native matching Meteor Spec

https://guide.meteor.com/react-native.html

Other

59 stars 31 forks source link

feature: make NetInfo injectable/truly optional #145

Closed jankapunkt closed 8 months ago

jankapunkt commented 9 months ago

Summary

This makes NetInfo a true optional dependency.

If Meteor.connect options contain NetInfo: null it will not be imported and not used. Users need to manually connect/reconnect.

If the options is a custom NetInfo (injected) then this will be used.

Otherwise the default NetInfo is loaded.

This allows also to configure netinfo in a much more custom basis than previously.

Linked issue(s)

143

Involved parts of the project

Meteor.connect / DDP.connect

Added tests?

yes

Targeted Meteor release version

Reproduction

Clone, ceckout branch, run tests A test release is provided soon

bratelefant commented 9 months ago

Should we generally discuss connection managment (maybe at some other place)? I tend to use App State to disconnect when the app is sent to background, and reconnect when the app is reopened. This saves a lot of server resources, since otherwise the sockets remain open at least for a while. However, I have some reconnection issues, don't know if this interferes with NetInfo.

How does the connection state get managed if no NetInfo is present?

jankapunkt commented 9 months ago

Yes let's open a discussion in regards to a 3.0. this PR is only a fix for the linked issue so @derwaldgeist can continue development.

When netinfo is disabled there is all manually to manage right now, including disconnect and reconnect.

I also agree with Appstate as without it we have many glitches when opening the app after a certain time

jankapunkt commented 9 months ago

Released 2.8.0-rc.0 for testing