Wrap Reachability in a protocol to aid testability

[palpatim]

Is your feature request related to a problem? Please describe.

We currently inject Reachability directly into consuming types. We should wrap it in a protocol to aid testability. That should lay the groundwork for better test coverage of mutation queue behavior when the network stops/resumes.

Describe the solution you'd like

Extract the Reachability functions we're currently using into a protocol; and conform Reachability to it. Replace directly-instantiated Reachability client with init params.

Describe alternatives you've considered

None. Reachability is a popular framework that provides useful utilities. It doesn't make sense to roll our own, and a protocol wrapper is a cheap enhancement.

Additional context

N/A

[larryonoff]

One more thing I would like to suggest is moving Reachability inside the framework, not as dependency of AppSync. This will exclude conflicts when someone uses newer or older version of dependency, so that AppSync update won't be possible until updating dependency.

It can be done via git submodules or Carthage.

[palpatim]

Sorry I missed your comment, @larryonoff. I see what you mean about moving the Reachability code inside the framework. Let us think about this a bit; I'm concerned both about the philosophical implications (we don't want to look like we're hiding the dependency or forking it) as well as the practical implications (Would a project attempting to consume AppSync while they have a different Reachability version installed encounter aberrant behavior from having two packages installed? Wouldn't we still face swift ABI compatibility issues as we attempt to import, say, Carthage binaries built with different versions of Swift?)

[larryonoff]

@palpatim I see several options here.

1. CocoaPods dependency or Carthage dependency

I think this's popular option. The only disadvantage I see is dependency versions conflict if someone uses newer or older version of the dependency that AppSync uses.

2. Internal dependency as a source, NOT as framework

The advantage that it will eliminate versions conflict.

The disadvantage that it's harder to manage, e.g. git submodules, Carthage via source code.

3. Internal dependency as framework

This's the worse scenario, because

1. Swift ABI not stable.
2. Dependency is bundled and hidden inside the framework.

I think that options 1 and don't hide what dependencies used via dependencies list or source code.

[palpatim]

Hi @larryonoff,

I agree with your assessment, and choosing what you're outlining as option 1, with a modification that makes it behave like Option 2: I'm currently testing adding all of our Pods to source control (#150), which would remove the need for developers who consume this library via Carthage or source to install CocoaPods and run a pod update before building.

• Carthage developers who consume binaries (that is, who are using the same version of Swift we use to build the SDK) will be unaffected.
• Developers who consume from source (either carthage update --no-use-binaries, using a git submodule, etc) should be able to build without a pod install

Dependencies will continue to be documented within the Cartfile and Podfile (except for Apollo as we noted above), but won't require action on the developer's part to update.

I don't see any mechanism to resolve conflicts between different versions of libraries, short of importing as source (your option 3) and manually renaming the exported symbols or module. But, we'd be bloating a customer's codebase if they are able to actually consume an updated version, and really working hard against the point of a dependency management system.

[palpatim]

I'm closing this issue since it relates specifically to Reachability. Let's continue any further conversation on dependency management strategies on #150.