pointfreeco / swift-composable-architecture

A library for building applications in a consistent and understandable way, with composition, testing, and ergonomics in mind.
https://www.pointfree.co/collections/composable-architecture
MIT License
12.61k stars 1.46k forks source link

They are making decisions affecting the future of TCA on Swift Evolution! #1087

Closed johnno1962 closed 2 years ago

johnno1962 commented 2 years ago

The Composable Architecture uses GitHub issues for bugs. For more general discussion and help, please use GitHub Discussions or the Swift forum first.

Hi, I feel like you have some skin in this game, https://forums.swift.org/t/se-0354-regex-literals/57037/195. Do you care to comment?

stephencelis commented 2 years ago

@johnno1962 We appreciate the communication! We did our best to address our stance here: https://forums.swift.org/t/se-0354-regex-literals/57037/260

If you have any comments, let us know!

johnno1962 commented 2 years ago

Thanks for braving S/E for us. The tone of your post is just right. 10,000 clones a week! Congratulations!

johnno1962 commented 2 years ago

While I have you on the line, how receptive would you be to upstreaming the the changes we made to support "injection" of reducers at runtime? They were relatively minor changes which have proven quite robust in the field and could be gated by a -DINJECTION so people would have to opt-in .

stephencelis commented 2 years ago

@johnno1962 We saw @krzysztofzablocki's post about this recently! Really cool stuff!

We have some major changes to TCA's types planned in the coming months, and we're not sure we'll be able to maintain the functionality of this patch at the time, so I think we'd be worried to introduce functionality that people depend on and then not be able to maintain that functionality. We wonder, though, if the changes we have planned will make it easier to instrument TCA with injection, maybe even from outside the library.

If you'd like a preview of the changes, a version is being experimented with on the proto-2 branch. The major changes are:

johnno1962 commented 2 years ago

Thanks for getting back. It was just a heads up and if there are major changes in the pipeline, particularly changes that might make TCA more injection friendly it'll definitely keep until then.

DavidKmn commented 2 years ago

@stephencelis Hey Stephen, interesting stuff happening on the proto-2 branch especially around DI. I like the approach however some potential issues I see: 1) The inability to inject other dependencies into each other. For example we may have a some sort of a repository (e.g. LoggedInUserRepository) that itself requires an http client and a persistent storage client (CoreData, Realm, ...). We would need to construct this LoggedInUserRepository after user logs in and then inject it in, however this repository itself would need other dependencies we initialized sometime before, higher up the composition tree. Not sure how this could be done.
2) Another case that I see is where we need to update a dependency at runtime, for example we may start the app off with an unauthorized http client and then at some point once the user logs in we would want to swap in an authorized http client to be used throughout the app (and then back to unauthorized after he logs out).

stephencelis commented 2 years ago

@DavidKmn:

1. The inability to inject other dependencies into each other. For example we may have a some sort of a repository (e.g. LoggedInUserRepository) that itself requires an http client and a persistent storage client (CoreData, Realm, ...). We would need to construct this LoggedInUserRepository after user logs in and then inject it in, however this repository itself would need other dependencies we initialized sometime before, higher up the composition tree. Not sure how this could be done.

I haven't explored this too much, but it seems like a dependency could use @Dependency itself, but even if it can't, I think you could use Reducer.dependency to prepare your dependency tree.

2. Another case that I see is where we need to update a dependency at runtime, for example we may start the app off with an unauthorized http client and then at some point once the user logs in we would want to swap in an authorized http client to be used throughout the app (and then back to unauthorized after he logs out).

In general, a dependency itself should be static, though it may have some private mutable state. Check out the live isowords API client for an example, where the API client can be authenticated without requiring the dependency itself from being swapped out.

DavidKmn commented 2 years ago

@stephencelis Thanks for the response. I see what you are doing in the API client example, I think this is actually a prime example of what I was saying on point 1. Here the API client dependency depends on the user defaults client which ideally would be injected as a dependency into the API client. I am actually not using TCA atm for the current project so the Reducer.dependency would not work...

stephencelis commented 2 years ago

Thanks for the response. I see what you are doing in the API client example, I think this is actually a prime example of what I was saying on point 1. Here the API client dependency depends on the user defaults client which ideally would be injected as a dependency into the API client.

One thing to consider is that this is a live implementation of a dependency, and its use of user defaults is incidental (and could be swapped out for some other storage mechanism at any time) and isolated. Because of this, there isn't really a need to inject defaults from the outside, as no other dependency should depend on the incidental fact that the live API client uses user defaults on the inside.

DavidKmn commented 2 years ago

For 1, so if we go with an approach of using structs for modelling dependencies I do not see how we could inject using @Dependency. To pass in we would need something like func live(_ dependencyOfDepedency: ). Or am I missing something

DavidKmn commented 2 years ago

Also any reason you decided to use @TaskLocal, I know what it does however don't see the point in the context of DependencyValues

stephencelis commented 2 years ago

@DavidKmn @TaskLocal allows you to temporarily mutate some potentially global state within the context of a task. Reducer.dependency uses it to override a dependency that it accesses from @Dependency.

DavidKmn commented 2 years ago

@stephencelis Yeah I saw that, mostly used for testing and previews from what I gathered, is that the intended use, would you use it elsewhere? Also any thought on performance of Existential types (saw your usage of them in dependencies), have you been benchmarking their use?

johnno1962 commented 2 years ago

@DavidKmn, I've been been benchmarking the use of existential types, looking into one of the justifications for the massively source breaking SE-0335 using the script below. The tl;dr is: function call overhead is not a practical consideration in real world usage. Sure, existentials are have about a 20% greater overhead so you will only be able to make 400,000,000 invocations instead of 500,000,000 per second which isn't really worth fretting about.

#!/usr/bin/swift -O

import Foundation

//@objc // uncomment to make 100x slower (dynamic dispatch)
protocol P {
    func inc()
}

var c = 0

//final
class
//struct
    C: P {
//    var c = 0 // uncomment to make 25x slower (strangely)
    var x = "", y = "", z = ""
    func inc() {
        c += 1
    }
    func str() -> String {
        return x + y + z
    }
}

struct S {
    func existential(p: P) {
        p.inc()
    }
    func generic<G: P>(p: G) {
        p.inc()
    }
//    func opaque(p: some P) {
//        p.inc()
//    }
}

var a = C(), b = S()
var p: P = a
var repetitions = 100_000_000, i = repetitions
var t0 = Date.timeIntervalSinceReferenceDate
while i != 0 {
    b.existential(p: a)
    b.existential(p: a)
    b.existential(p: a)
    b.existential(p: a)
    b.existential(p: a)
    b.existential(p: a)
    b.existential(p: a)
    b.existential(p: a)
    b.existential(p: a)
    b.existential(p: a)
    i -= 1
}
var t1 = Date.timeIntervalSinceReferenceDate
print(t1-t0, c, a.str())

i = repetitions
t0 = t1
while i != 0 {
    b.generic(p: a)
    b.generic(p: a)
    b.generic(p: a)
    b.generic(p: a)
    b.generic(p: a)
    b.generic(p: a)
    b.generic(p: a)
    b.generic(p: a)
    b.generic(p: a)
    b.generic(p: a)
    i -= 1
}
t1 = Date.timeIntervalSinceReferenceDate
print(t1-t0, c, a.str())

//i = repetitions
//t0 = t1
//while i != 0 {
//    b.opaque(p: a)
//    i -= 1
//}
//t1 = Date.timeIntervalSinceReferenceDate
//print(t1-t0, c, a.str())

//let p: P = a
//b.generic(p: p)
stephencelis commented 2 years ago

@DavidKmn As @johnno1962 points out, it's nothing worth worrying about. We were just curious, so we benchmarked and found it introduced < 100 ns of overhead. An in the scope of TCA this is likely just accessing a boxed dependency or two per action, not accessing an array of millions of [any]s.

DavidKmn commented 2 years ago

@DavidKmn:

  1. The inability to inject other dependencies into each other. For example we may have a some sort of a repository (e.g. LoggedInUserRepository) that itself requires an http client and a persistent storage client (CoreData, Realm, ...). We would need to construct this LoggedInUserRepository after user logs in and then inject it in, however this repository itself would need other dependencies we initialized sometime before, higher up the composition tree. Not sure how this could be done.

I haven't explored this too much, but it seems like a dependency could use @Dependency itself, but even if it can't, I think you could use Reducer.dependency to prepare your dependency tree.

  1. Another case that I see is where we need to update a dependency at runtime, for example we may start the app off with an unauthorized http client and then at some point once the user logs in we would want to swap in an authorized http client to be used throughout the app (and then back to unauthorized after he logs out).

In general, a dependency itself should be static, though it may have some private mutable state. Check out the live isowords API client for an example, where the API client can be authenticated without requiring the dependency itself from being swapped out.

Was wondering on question 1 @stephencelis, would something like this make sense, where HomeProductsService is a struct for a specific product feature.

private enum SpotProductsServiceKey: LiveDependencyKey {
        @Dependency(\.httpGateway) static private var httpGateway
        static let liveValue: HomeProductsService = .live(httpGateway)
        static let testValue: HomeProductsService = .mock(failing: false)
}
johnno1962 commented 2 years ago

@stephencelis, I've had a quick look the new protocol-beta branch and moving to ReducerProtocol is sufficient to have the InjectionIII app work with TCA.

stephencelis commented 2 years ago

@johnno1962 Good to hear! I'm curious if the change makes it easier to use injection without a fork? Or is there more work that needs to be done in the core library to make that possible?

johnno1962 commented 2 years ago

Changes to TCA no longer required once you move to types conforming to ReducerProtocol, nor changes to the client application though it will need "Other Links Flags" -Xlinker -interposiable during development. Good news indeed.

johnno1962 commented 1 year ago

Hi again 👋, I was watching the pointfree videos on observation and I have a question for you. It is possible to use the InjectionIII app with SwiftUI but at the moment it's a bit of a song and dance for example the HotSwiftUI project. The current state is that you need to add .eraseToAnyView() to the body of all SwiftUI views which seems unavoidable and, in addition, you have to add a @ObserveInjection property wrapper to force redrawing which just contains a ObservableObject with a published injection counter. So, all views at all levels need to look like this:

    var body: some View {
        // Your SwiftUI code...
        .eraseToAnyView()
    }

    @ObserveInjection var redraw

I've tried to find a solution where only the .enableInjection() call is necessary, trying wrapping views etc, and it feels like it should be possible but without something in the view or any views it contains to observe it never feels the need to redraw. Do you have any ideas how this could be avoided?? Any tips would be greatly appreciated.

There is a small "getting started" project https://github.com/johnno1962/SwiftUIPlaygrounds if you've never tangled with Injection before.