Nilling out child state in response to child's action replays old state in child store

[dannyhertz]

Describe the bug In a UIKit based app (have not tried SwiftUI but have a feeling its related to the UIKit IfLet functionality) if you you nil out a child state (in parent reducer) in response to a child's actions (which also changes some state) then child ViewStore publishers seem to emit older state values (after emitting the correct values first).

To Reproduce

TcaCombineApp.zip

import UIKit
import ComposableArchitecture
import Combine

struct Environment {}

struct ChildState: Equatable {
    var alertText: String?
}

enum ChildAction: Equatable {
    case alertDismissed
    case dismissalRequested
    case didAppear
}

let childReducer = Reducer<ChildState, ChildAction, Environment> { state, action, environment in
    switch action {
    case .didAppear:
        state.alertText = "Hey!"
        return .none
    case .alertDismissed:
        state.alertText = nil
        return Effect(value: .dismissalRequested)
    case .dismissalRequested:
        return .none
    }
}

struct AppState: Equatable {
    var child: ChildState?
}

enum AppAction {
    case child(ChildAction)
    case buttonSelected
}

let appReducer: Reducer<AppState, AppAction, Environment> = .combine(
    childReducer.optional()
        .pullback(state: \.child, action: /AppAction.child, environment: { $0 }),
    Reducer<AppState, AppAction, Environment> { state, action, _ in
        switch action {
        case .child(.dismissalRequested):
            state.child = nil
            return .none
        case .buttonSelected:
            state.child = .init()
            return .none
        default:
            return .none
        }
    }
)

class ChildViewController: UIViewController {

    private var cancellables = Set<AnyCancellable>()
    private let store: Store<ChildState, ChildAction>
    private let viewStore: ViewStore<ChildState, ChildAction>

    init(store: Store<ChildState, ChildAction>) {
        self.store = store
        self.viewStore = ViewStore(store)
        super.init(nibName: nil, bundle: nil)
    }

    required init?(coder: NSCoder) {
        fatalError("init(coder:) has not been implemented")
    }

    override func viewDidLoad() {
        super.viewDidLoad()
        view.backgroundColor = .blue

        viewStore.publisher.alertText
            .sink(receiveValue: { [weak self] title in
                guard let self = self, let title = title else { return }
                let alert = UIAlertController(title: title, message: nil, preferredStyle: .alert)
                alert.addAction(.init(title: "Done", style: .default, handler: { _ in
                    self.viewStore.send(.alertDismissed)
                }))

                self.present(alert, animated: true, completion: nil)
            })
            .store(in: &cancellables)
    }

    override func viewDidAppear(_ animated: Bool) {
        super.viewDidAppear(animated)
        viewStore.send(.didAppear)
    }

}

class AppViewController: UIViewController {

    private var cancellables = Set<AnyCancellable>()
    private let store: Store<AppState, AppAction>
    private let viewStore: ViewStore<AppState, AppAction>

    init(store: Store<AppState, AppAction>) {
        self.store = store
        self.viewStore = ViewStore(store)
        super.init(nibName: nil, bundle: nil)
    }

    required init?(coder: NSCoder) {
        fatalError("init(coder:) has not been implemented")
    }

    override func viewDidLoad() {
        super.viewDidLoad()
        view.backgroundColor = .white

        let button = UIButton(type: .system)
        button.setTitle("Show me the child", for: .normal)
        button.setTitleColor(UIColor.black, for: .normal)
        button.sizeToFit()
        button.center = view.center
        button.addTarget(self, action: #selector(buttonSelected), for: .touchUpInside)
        view.addSubview(button)

        store.scope(state: \.child, action: AppAction.child)
            .ifLet(then: { store in
                let childVc = ChildViewController(store: store)
                self.present(childVc, animated: true, completion: nil)
            }, else: {
                self.dismiss(animated: true, completion: nil)
            })
            .store(in: &cancellables)
    }

    @objc func buttonSelected(id: AnyObject) {
        viewStore.send(.buttonSelected)
    }

}

Expected behavior The alert should be dismissed in the child and the child should dismiss (without any additional presentations of the children's alert). If you add a print to the alert publisher or have your console open you should see that after initially turning nil (on alert close) the publisher emits the previous non-nil value again.

***: receive subscription: (RemoveDuplicates)
***: request unlimited
***: receive value: (nil)
***: receive value: (Optional("Hey!"))
***: receive value: (nil)
***: receive value: (Optional("Hey!"))
2021-04-17 18:52:15.531197-0400 TcaCombineApp[67717:15126535] [Presentation] Attempt to present <UIAlertController: 0x7fdeeb80e400> on <TcaCombineApp.ChildViewController: 0x7fdecac09730> (from <TcaCombineApp.ChildViewController: 0x7fdecac09730>) whose view is not in the window hierarchy.
***: receive cancel

Screenshots

https://user-images.githubusercontent.com/407719/115128871-e3ce3500-9fae-11eb-8954-abf0c402e4f4.mov

Environment

• Xcode 12.4
• Swift 5.3
• OS (if applicable): iOS 14

[lukeredpath]

This sounds suspiciously similar to a bug I’ve encountered but in SwiftUI using an IfLetStore. In my case, an onAppear for a view at the root of a navigation hierarchy, which itself is shown by an IfLetStore, is firing when the state for that IfLetStore becomes nil, and even stranger, if I breakpoint in the onAppear and inspect the ViewStore state, it appears to be the original state from when the store was first initialised. https://github.com/pointfreeco/swift-composable-architecture/issues/395#issuecomment-822717766

[stephencelis]

@lukeredpath That behavior makes sense because of what we do here:

https://github.com/pointfreeco/swift-composable-architecture/blob/ba2b9e08e2776bbb02f27ab7123dae767f106fad/Sources/ComposableArchitecture/SwiftUI/IfLetStore.swift#L50-L54

We cache the value when the body first goes non-nil to work around a bug where force unwrapping the value in the store is not safe (that child view code could evaluate even if a parent view should not evaluate that branch).

We're not exactly sure how to write this a different/better way, but if you have any ideas we're all ears.

[lukeredpath]

@stephencelis no ideas for a workaround right now but thank you for pointing out why it was happening because I was very, very confused!

[AppalaNaidu01]

Hey guys, do you have any fix for the issue?

Thanks

[nmccann]

Ran into this same issue when trying to convert to the recently introduced SwitchStore. I am also making use of onAppear in a child view, and it's getting triggered when it's associated state should be nil. (Specifically, this occurs when I send a log out action which should return the user to the login screen, however an existing child view's onAppear gets called first).

I was able to workaround the issue by changing the pullback that is using a CasePath to explicitly pass breakpointOnNil: false. It's not ideal, but aside from reworking my child view to not require onAppear (I haven't given it much thought - it may or may not be doable), I'm not sure of a better solution.

Maybe calling out the caching behaviour in the documentation would be useful (similar to how WithViewStore has callouts for some of the weird ways in which it interacts with some SwiftUI views). Not sure how helpful it would be, I was very confused setting breakpoints wondering how my state was reverting without actually hitting the breakpoints. I'm glad I'm not the only one.

[lukeredpath]

I'm running into this problem again, with a very similar scenario to what @nmccann describes, specifically:

• Our app has a logged out and logged in state, managed by an enum state.
• Our logged in view is displayed using an IfLetStore on the logged in state, so it should only appear when the logged in state is non-nil.
• Within this logged in view, we display a modal sheet - when the user performs some action on this sheet it is possible for the user to get logged out.
• If the user gets logged out, the logged in state becomes nil however the ViewStore binding for the sheet is still sending a dismiss action to the logged in view store, which causes the breakpoint on nil state to fire.

To work around this, I had to set some state that indicates the user should be logged out, and then trigger the logout only after the dismiss action has been fired.

[iampatbrown]

@dannyhertz are you still experiencing this issue? Your sample project seems to work as expected with version 0.20.0 but not with 0.19.0 @lukeredpath or @nmccann any chance on providing a sample project or confirming the issue is still present with the latest release?

[nmccann]

@iampatbrown , unfortunately I'm seeing the same issue on both 0.19.0 and 0.20.0 in my particular case (when used with a SwitchStore). I also tried it against the latest from main, same results. I'll try to prep a sample project next week.

[iampatbrown]

OK no worries. A sample project would be awesome :) looking forward to checking it out.

[nmccann]

I've attached an example that reproduces this issue with SwitchStore: nil-issue.zip

Interestingly, I first tried this without a TabView and didn't encounter the bug. That led me to look for issues related to TabView, and I found this issue which has a very similar example (which still crashes with 0.20.0).

That said, the larger project that I have this problem in doesn't use Apple's TabView, instead it uses a custom implementation that basically amounts to an if statement (if current tab == "first tab", display first tab contents else, second tab contents). But in my case this custom tab view is embedded in a NavigationView, so perhaps that is contributing to the problem.

I also ran this on Xcode 13 Beta 2, and got the same issue. So if it is a SwiftUI bug, it doesn't look like Apple has fixed it yet.

Update: I was able to reproduce the crash without Apple's TabView - rather than being due to a NavigationView like I thought, it was due to the usage of a GeometryReader. See this example for a custom tab view with/without geometry reader: nil-issue-no-tab-view.zip

I wonder if the underlying problem with Apple's TabView is the same (ex. if they're using a geometry reader? Or perhaps they just work similarly internally).

Update 2: with nil-issue-no-tab-view, I was able to work around the issue by replacing switch viewStore.currentTab in the withGeometryReader function with the following:

ZStack {
          firstTab(viewStore).opacity(viewStore.currentTab == .first ? 1 : 0)
          secondTab(viewStore).opacity(viewStore.currentTab == .second ? 1 : 0)
        }

With the above implementation, onAppear isn't called when "switching" between tabs, because the previous view never actually disappears. The underlying problem still occurs (where the state gets reverted to it's initial state due to this line - in this case, currentTab reverts to first upon logout) - but it doesn't cause a crash since onAppear isn't called, and thus we don't send anything to the (now nil) store. This isn't a great "solution" though, and wouldn't necessarily work for all situations.

[ollitapa]

I was having the same issue in my app (for example a list on a sheet being animated empty, just as the sheet is closing) and came up with this solution: #665. The implementation of compactScope is quite straight forward, but maybe a bit too specific to include alongside regular scope, at least not as a public function.

[nmccann]

@ollitapa I've tried your branch out with both of my samples (with and without tab view), and it solved the problem perfectly. I'd definitely be in favour of that being introduced as a private function (I think having it as a public function would be confusing for newcomers, and largely unnecessary). So I hope it gets merged in, thanks for the fix!

[danielhorv]

@ollitapa I also checked your branch and it would also fix my issue https://github.com/pointfreeco/swift-composable-architecture/issues/631. Tried it out with my current project, where I had the problem for the first time, and seems to work everything well! I hope so, it will be merged, nice PR! 🎉

[stephencelis]

As noted in #502, this appears to be fixed!