Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The reversed data flow from the standard SwiftUI views back to the AppState #59

Open
mohammad19991 opened this issue May 28, 2021 · 8 comments

Comments

@mohammad19991
Copy link

Hi @nalexn

I like what you did with this clean-architecture approach, I read some of your articles related to SwiftUI State and I saw the below implementation of reversing the data flow from views to AppState:

extension Binding where Value: Equatable {
    func dispatched<State>(to state: Store<State>,
                           _ keyPath: WritableKeyPath<State, Value>) -> Self {
        return onSet { state[keyPath] = $0 }
    }
}

extension Binding {
    typealias ValueClosure = (Value) -> Void
    
    func onSet(_ perform: @escaping ValueClosure) -> Self {
        return .init(get: {
            self.wrappedValue
        }, set: {
            self.wrappedValue = $0
            perform($0)
        })
    }
}

where you will use it like this:

@State var routingState: Routing = .init()
private var routingBinding: Binding<Routing> {
    $routingState.dispatched(to: injected.appState, \.routing.home)
}

It works in terms of updating the AppState, but if we look at the implementation of dispatched/onSet function we notice that it wraps the Value in a new Binding instance and this will cause the view to rerender again while it's not needed, I wanna check if you are aware about this case or maybe I didn't understand the idea properly.

I can show you and example if needed.

@nalexn
Copy link
Owner

nalexn commented Jun 5, 2021

Hey @mohammad19991 , thanks for the heads up! Not sure if I understood the case fully, here is the recap of the mechanics to make sure we're on the same page:

There is an @State variable that we want to pass along for mutation using Binding AND update the AppState when the change happens. Normally we'd just used $ prefix to access the Binding, but this won't give us ability to listen to mutations. Thus, we create a wrapper Binding that proxies the get and set.

From what I see, the setter does assign the new value to the @State every time a value is set, even to the same value - this can be optimized by discarding the same values to assure the view is not rerendered.

The second side effect of the setter is updating the AppState - which may cause rerender unless we filter out the same values, which we do.

So if I understand the problem correctly, you're referring to the rerender caused by setting the same values to the @State, is that right? Please let me know your view on this issue.

@mohammad19991
Copy link
Author

mohammad19991 commented Jun 6, 2021

yes that's right @nalexn, I'm referring to the rerender caused by setting the same values to the @State. specifically am talking about this func onSet

extension Binding {
    typealias ValueClosure = (Value) -> Void
    
    func onSet(_ perform: @escaping ValueClosure) -> Self {
        return .init(get: { () -> Value in
            self.wrappedValue
        }, set: { value in
            self.wrappedValue = value
            perform(value)
        })
    }
}

as I understand this function, every time we access it then it returns a new instance of Binding which will cause the view to re-render if my understanding is correct.

And regarding filtering are you referring to this func:

    func updates<Value>(for keyPath: KeyPath<Output, Value>) ->
        AnyPublisher<Value, Failure> where Value: Equatable {
        return map(keyPath).removeDuplicates().eraseToAnyPublisher()
    }

@nalexn
Copy link
Owner

nalexn commented Jun 6, 2021

Gotcha. That's an easy fix then:

extension Binding where Value: Equatable {
    typealias ValueClosure = (Value) -> Void
    
    func onSet(_ perform: @escaping ValueClosure) -> Self {
        return .init(get: { () -> Value in
            self.wrappedValue
        }, set: { value in
            if self.wrappedValue != value {
                self.wrappedValue = value
            }
            perform(value)
        })
    }
}

I've pushed the fix. Thanks for reporting!

@nalexn nalexn closed this as completed Jun 6, 2021
@mohammad19991
Copy link
Author

Thanks, I'll test it and let you know.

@mohammad19991
Copy link
Author

@nalexn it's not working, to help you understand more about the issue I made this project to demonstrate the issue, there are 2 branches the first one is the main branch which uses the Binding proxy func dispatched(to:..) and the other branch is just using the local Routing. I added Print statements to show when the views are being rendered:

  1. The Project is just a button and presenting a sheet, here's the main branch so when we use the routingBinding which returns a new instance it will make the ContentView re-render again where it's not needed
    2021-06-06 22 45 42

  2. In this branch we are not using the routingBinding so it just renders the details screen when it's presented without re-rendering the ContentView
    2021-06-06 22 44 45

I hope this makes it more clear for you to understand the problem, please let me know if you need more details.

@mohammad19991
Copy link
Author

Another concern I would mention that having two Routings one local and another on in AppState this means we have two source of truth maybe it would be better just to use the one in AppState without making another one locally somehow.

@nalexn
Copy link
Owner

nalexn commented Jun 21, 2021

Hey, yep, I'll have a look, thanks for composing a sample!

@nalexn nalexn reopened this Jun 21, 2021
@nalexn
Copy link
Owner

nalexn commented Sep 26, 2021

Hey @mohammad19991 ,

I've done some experiments based on the points you've made. To fully understand what's going on I was adding init with print to see when the views are being created (as opposed to "rendered" via body call), and also adding

if #available(iOS 15.0, *) {
    print("\(Self._printChanges())")
}

inside body to see why the view is re-rendered. Additionally we can print from onReceive

So if we take the original version, which gets "re-rendered" when we press the button, the following sequence takes place:

  1. Button callback changes the state directly on the appState
  2. The AppState publisher triggers onReceive for the ContentView, updating the local @State
  3. That @State change notifies SwiftUI that ContentView requires rerender. _printChanges says: ContentView: _routingState changed. - makes sence.
  4. The body call for ContentView constructs a new struct (which we return as the body)
  5. As part of the rendering routine, SwiftUI connects the Content View to the Environment and the external values store, which holds the values for @State variables. The fresh struct from the body receives its first onReceive callback which rewrites the same value for the @State variable. This seems redundant, but this is the result of using CurrentValueSubject, which always pushes the value upon subscription.
  6. Instructed by the actual state value, the sheet modifier calls the builder closure and constructs the Details view
  7. Done. Both views are rendered on screen

Now, if we keep all the prints in place and run the version that changes the local @State directly, with AppState being out of the game, the sequence is much shorter:

  1. Button callback changes the local view's @State
  2. SwiftUI takes a shortcut and instead of calling body reuses the old struct returned as the body
  3. The body is rendered with the updated input, the sheet creates Details view as the result

Although in both cases we have only the @State changing the value, SwiftUI behaves differently. The decision not to call the body in the second case is clearly made for optimization. I can only guess what makes SwiftUI play safe in the first experiment and turn off optimization by calling body again - could it be the onReceived modifier?


I've tried to address the issue with duplicated routing state (AppState plus the local @State).

There were two approaches I've taken - first was to feed the views with Binding that were reading directly from the AppState, the second - using EnvironmentObject as the source of truth for Routing.

  1. For constructing the Binding directly off the AppState I've added the following function:
extension Store {
    func binding<Value>(for keyPath: WritableKeyPath<Output, Value>) -> Binding<Value> where Value: Equatable {
        return .init(get: {
            self.value[keyPath: keyPath]
        }, set: { newValue in
            self.value[keyPath: keyPath] = newValue
        })
    }
}

so that the ContentView would only see the local Binding and nothing else:

struct ContentView: View {
    @Binding var routing: Routing
    
    init(_ routing: Binding<Routing>) {
        _routing = routing
    }
    ....
}

@main
struct CleanArchRoutingApp: App {
    
    @Environment(\.injected) var injected
    
    var body: some Scene {
        WindowGroup {
            NavigationView {
                ContentView(injected.appState.binding(for: \.routing.contentView))
            }
        }
    }
}

In this case, the AppState was changing correctly as the result of the press on the button, but the hierarchy was never reacting to it.

My understanding is that SwiftUI hierarchy requires explicit notification from @State (or other @thing except from @Binding), from the current view, or from one of its predecessors up in the hierarchy. This notification should come at least from the very root view, otherwise, SwiftUI assumes there are no changes and no re-rendering is needed.

In this example, the root view has reference to the mutated object though @Environment, which does not notify about the change because its value (the reference to the object) has never changed.

The updates start to come as you change @Environment to be a local @State variable on the root view. This also makes the whole hierarchy, starting from the root view, to be re-rendered, which is the original problem we've been trying to fight introducing the "Publisher - @State" routing.

  1. I've re-tested the approach with EnvironmentObject ruling the routing for all the views (I've added a few extra layers of modal screens presenting one another with a press on a button).

The result was the same as a couple of years ago when I first noticed the downside of ObservableObjects - the entire hierarchy gets rerendered because every view notices that its EnvironmentObject reported changes, even though those changes had nothing to do with most of the views.


Conclusion: I still cannot see a more optimal way to organize programmatic navigation in SwiftUI than Publisher - @State binding. Yes, SwiftUI seems to optimize the rendering when you use just the local @State - but it's not global programmatic navigation, so we need to make our choice.

As SwiftUI remains a black box, I cannot tell why in seemingly identical cases it either reconstructs the body or reuses the old version. This behavior keeps changing as SwiftUI evolves, but I cannot see this being a problem yet, especially since Publisher - @State binding is a perfectly legit way of delivering updates to the SwiftUI Views.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants