Remove dependencies that rely on CombineSchedulers

[p4checo]

Hi @stephencelis @mbrandonw 👋🏼

I've been bringing the ReactiveSwift fork of TCA up to date with the latest upstream TCA changes, and while I think it's awesome that Dependencies are now on its own repo, the fact that MainQueue (and MainRunLoop**) has been included (and thus CombineSchedulers) is causing some issues when integrating with the ReactiveSwift fork.

The problem arises because ReactiveSwift uses its own scheduler types and protocols which are not compatible with CombineScheduler's (and Combine's). While this is isn't a problem in Linux due to the #if canInport(Combine), it does create a conflict in Apple platforms which isn't easy to circumvent.

My suggestion is then to move MainQueue (and MainRunLoop**) back to the TCA repo as "internal" dependencies, effectively removing the dependency on CombineSchedulers from Dependencies. I also assume Clocks are the way forward so I expect the reliance on schedulers to be temporary anyway.

What do you think? Thanks! 🙏🏼

Notes:

** We aren't using MainRunLoop in the ReactiveSwift fork, but for the sake of consistency it would make sense to migrate it as well since it also depends on CombienSchedulers.

[mbrandonw]

Hi @p4checo, unfortunately we cannot do that because it would be a breaking change for anyone using the dependencies library. Also, beyond that, it's a goal of this library to provide a base set of dependencies that nearly every application will need to make use of, such as schedulers.

Can you describe more what is the exact problem you are having? Can you not disambiguate the Scheduler type by doing Combine.Scheduler vs ReactiveSwift.Scheduler?

And if this problem really is insurmountable without removing Combine schedulers, then you may have to fork this library to unblock you.

[p4checo]

unfortunately we cannot do that because it would be a breaking change for anyone using the dependencies library.
Also, beyond that, it's a goal of this library to provide a base set of dependencies that nearly every application will need to make use of, such as schedulers.

I was afraid you would say that, but I understand 🥲

Can you describe more what is the exact problem you are having? Can you not disambiguate the Scheduler type by doing Combine.Scheduler vs ReactiveSwift.Scheduler?

Sure, apologies for not being clear from the start. Yes, we can easily disambiguate as you mention but the problem is that in the ReactiveSwift fork of TCA we've been defining the \.mainQueue dependency as a direct "translation" of the upstream TCA counterpart:

extension DependencyValues {
  public var mainQueue: DateScheduler {
    get { self[MainQueueKey.self] }
    set { self[MainQueueKey.self] = newValue }
  }

  private enum MainQueueKey: DependencyKey {
    static let liveValue = QueueScheduler.main as DateScheduler
    static let testValue = UnimplementedScheduler() as DateScheduler
  }
}

However with the new standalone dependencies by default it will now use the one defined as AnySchedulerOf<DispatchQueue> in MainQueue.swift causing builds to break, e.g.:

  final class ComposableArchitectureTests: XCTestCase {
    func testScheduling() async {
      struct Counter: ReducerProtocol {
        typealias State = Int
        enum Action: Equatable {
          case incrAndSquareLater
          case incrNow
          case squareNow
        }
        @Dependency(\.mainQueue) var mainQueue
        func reduce(into state: inout State, action: Action) -> EffectTask<Action> {
          switch action {
          case .incrAndSquareLater:
            return .merge(
              EffectTask(value: .incrNow)
                .deferred(for: 2, scheduler: self.mainQueue), // compile error here 
              EffectTask(value: .squareNow)
                .deferred(for: 1, scheduler: self.mainQueue), // compile error here
              EffectTask(value: .squareNow)
                .deferred(for: 2, scheduler: self.mainQueue) // compile error here
            )
          case .incrNow:
            state += 1
            return .none
          case .squareNow:
            state *= state
            return .none
          }
        }
      }
   // (...)
}

Failing with the following error message:

Argument type 'AnySchedulerOf' (aka 'AnyScheduler<DispatchQueue.SchedulerTimeType, DispatchQueue.SchedulerOptions>') does not conform to expected type 'DateScheduler'
)

Possible solutions:

1. Creating/keeping a ReactiveSwift "version" of MainQueue.swift shadowing/redefining the \.mainQueue property works but only when used inside the module where it is defined (e.g. defining it in ComposableArchitectureTests builds tests, but defining it inside ComposableArchitecture isn't enough for ComposableArchitectureTests to build). This also imposes an undesirable burden on library users while not scaling in multi module set ups.

2. We could work around it by defining a dependency having a different name with the DateScheduler type (e.g. \.mainScheduler) but we would be sacrificing the symmetry we've been trying to keep in regards to upstream TCA.

3. I also tried making AnySchedulerOf<DispatchQueue> conform to ReactiveSwift's DateScheduler which could work but sadly ReactiveSwift's Scheduler: AnyObject meaning another no-go with AnySchedulerOf<DispatchQueue> being a struct. We could always try to lift that restriction in ReactiveSwift though... 🤔 Linux support has to be ensured via DateScheduler anyway since it doesn't support Combine.

Hope this made it a bit clearer. Do you have any other suggestion? I might be missing something or simply holding it wrong™ 😅

And if this problem really is insurmountable without removing Combine schedulers, then you may have to fork this library to unblock you.

Yeah this is always an option, but I would like to avoid it if possible as it creates a new set of problems and increases the gap between both TCA forks 😐

[mbrandonw]

I tend to agree with @pyrtsa and think we should avoid @_disfavoredOverload just to help out libraries that build upon this library. I think reactiveswift-composable-architecture renaming their dependency is the best long term solution, but also @pyrtsa's idea of having an escape hatch for unavoidable conflicts sounds good to me. @stephencelis and I will chat about that soon.

[tgrapperon]

I also concede that it would be exploiting the feature beyond its intended scope. If Dependency has an init with a DependencyKey type argument, end-users will be able to define their own overloads locally. It will only work at the module level though, but if they don't modularize their apps, it could work relatively seamlessly for them.
I have more liberty with my library, and I still have a few path to explore to solve this problem, especially considering a hypothetical @Dependency(DependencyKey.Type).
I also hope that the day they remove @_disfavoredOverload (if it ever comes), we will have more tools at hand to implement this functionality.

[davdroman]

Proper module qualification should probably exist before Apple removes @_disfavoredOverload. I don't really see any other way they could justify removing it otherwise. Then it'd make sense to have @Dependency(ReactiveSwift::MainQueue.self) (hypothetical syntax).

Though at that point you might as well use namespaced keypaths e.g. @Dependency(\.reactiveSwift.mainQueue).

[p4checo]

@p4checo Just to clarify, the key path squatting was mostly a problem that my own library will create, but the same solution I'm planning to implement to mitigate this problem can be implemented in swift-dependencies too.

Oh I see. Thanks @tgrapperon for clarifying as I thought it was the other way around.

Your idea of disambiguating via key path namespacing is interesting, but expecting everyone to follow a pattern/idiom might not be easy 😄

While @_disfavoredOverload clearly works around the issue, it is not the right tool for the job as it exists currently (emphasis mine):

This is a fair point. While I don't expect this to be a common issue, I agree we should avoid relying on these "unofficial"/"unstable" attributes, especially in scenarios they haven't even been designed for (which seems to be the case).

I agree that your suggestion of allowing explicit type path qualification in the Dependency property wrapper is much more robust and should scale well for most or even all multi module ambiguity scenarios.

I think reactiveswift-composable-architecture renaming their dependency is the best long term solution, but also @pyrtsa's idea of having an escape hatch for unavoidable conflicts sounds good to me. @stephencelis and I will chat about that soon.

I agree. Will follow your previous naming suggestion as I believe it achieves a nice balance. Hopefully we should see schedulers being used less and less in TCA soon 😁

Thanks all for your inputs! 🙏🏼

[mbrandonw]

Hi @p4checo, thanks for being understanding and for adapting. It means a lot!

If this kind of thing continues to be an issue we will consider @pyrtsa's suggestion of an escape hatch to access dependencies by key, but for now we will keep things as-is.