SwiftUI drag to dismiss crash

I have the following code

struct AppState {
  var detailViewState: DetailViewState?
}

enum AppAction {
  case showDetailView
  case detailView(DetailViewAction)
}

let appReducer = Reducer<AppState, AppAction, Void>.combine( 
detailReducer.optional.pullback(
  state: \AppState.detailViewState,
  action: /AppAction.detailView,
  environment: { _ in () }),
  
Reducer { state, action, _ in 
  switch action {
    .case showDetailView:
      state.detailViewState = DetailViewState()
      
    .case detailView(let detailViewAction):
      switch detailViewAction {
        case buttonPressed:
          //logic for buttonPressed
          return .init { .detailView(.dismiss)
          
        case .dismiss:
          state.detailViewState = nil    
          
      }
  }
})

struct DetailViewState {}

enum DetailAction {
  case buttonPressed
  case dimiss
}

let detailReducer = Reducer<DetailViewState, DetailAction> {
	//logic here
}

struct DetailView: View {
  let store: Store<DetailViewState, DetailViewAction>
  
  var body: some View {
    WithViewStore(store) { viewStore in 
      Button.init(
        action: viewStore.send(.dismiss),
        label: { Text("Button".localized()) })
      }
      .onDisappear { viewStore.send(.dismiss) }
  }
}

So basically I have a view that presents DetailView modally through an IfLetStore. I want the user to be able to 'drag to dismiss' this DetailView, so that is why I have .onDisapper { viewStore.send(.dismiss) }.

But, the app crashes when the user presses the Button in the DetailView. When the button is pressed, I perform some logic, then dismiss the view in the AppReducer. This 'dismiss' action sets the DetailViewState to nil. But when the DetailView disappears, the viewStore sends .dismiss again. TCA forces a crash here because detailViewState has already been set to nil. The crash is forced in Reducer.swift, line 169.

Is there a simple way to prevent this? The only way that I can think of right now is to add a new action clearDetailView in the AppAction, then have something like this in the parent view

IfLetStore(
  self.store.scope(
    state: { $0.detailViewState },
    action: RevenueSplits.Action.detailViewAction),
  then: DetailView(store:))
  .onDisappear { viewStore.send(.clearDetailView) }

and remove .onDisappear from DetailView.

Hey @Nathan_Mann! Any chance you can share a fully compiling repro? It should be possible to coordinate state and actions in a way that does not cause the optional assertion to get called, but knowing how your parent/child views are structured should help us figure out a path forward.

@stephencelis here you go!

Combining optional reducers is tricky and can lead to subtle bugs when done out of order, which is why we have these preconditions, but we could probably improve the documentation better with some fleshed out examples. One problem with hiding an action in a .onDisappear is that action is sent when state is nil it will be ignored, and you might miss out on performing some logic that you depend on there.

So instead of using .onDisappear you can either leverage the sheet's onDismiss action closure, or better yet, avoid the constant binding and instead derive a binding from the view store:

.sheet(
  isPresented: viewStore.binding(
    get: { $0.detailState != nil },
    send: { $0 ? .presentDetail : .detailAction(.dismiss) }
  )
) { ...

For another example, @darrarski just ran into a similar issue here: IfLetStore and Effect cancellation on view disappear

We're replying to it now.

@stephencelis thanks

I'm actually having similar issues with dismissing views. I didn't realise until I updated the library to a version that had these assertions. My issue is that I have a "close" action in a LocalAction that is triggered by a LocalView but that is handled by the "parent"Reducer. If I chain parentReduer -> localReducer.optional it will always complain. So I have to do a little bit of gymnastics to change the order of the combine to avoid this.
Is that really what you guys recommend?

We might need to figure out how to better document this, but reducer ordering is very important, and the rule of thumb is you almost always want to run local reducers before more global ones.

In your example it is tempting to have a local close action that a parent hooks into but does nothing on its own in the child reducer. While this seems perfectly reasonable, and makes it sound like the order of the combine doesn't matter, if you come back to this code later and try to instrument it so that the local reducer fires off an effect when it receives close you will have a bug on your hands if the global reducer runs first:

  1. Global reducer intercepts .close and nils out state
  2. Local reducer cannot run with optional state so .close logic is never run

This is why we prefer to crash here and not silently ignore actions sent to optional reducers, because it almost always means that there's a bug just waiting to happen. For instance, in the thread mentioned above, the precondition caught the fact that timer cancellation was not actually happening.

Also in that thread @mbrandonw has shared a higher-order reducer that may make it easier to reason about these kinds of onAppear and onDisappear lifecycle-y reducers: IfLetStore and Effect cancellation on view disappear - #2 by mbrandonw

That's the bit I was missing.

I really like the assertion, as you say it helps cause tricky issues. I was just missing some guidance to know if it was expected that order was so important or if was just that I was doing something wrong.

Thanks for clarifying.