Strange behavior with IfCaseLet, SwitchStore, and TabView

[nikita-bor]

Hi! I'm trying out protocol-beta branch with Xcode 14 RC, iOS 16 and I see some behaviors I don't quite understand. I'd appreciate any insights on what's happening here.

There's TabView with two tabs. The first one has some kind of an async loading process and SwitchStore with ifCaseLet reducers. The second one is a dumb empty tab just for demonstration purposes.

struct AppState: Equatable {

    enum Tab: Equatable {
      case first
      case second
    }

    var first = First.State()
    var second = Second.State()
    var selectedTab: Tab = .first
}

struct FirstTabState: Equatable {

    enum LoadingState: Equatable {
      case loading(LoadingFeature.State)
      case loaded(ColorFeature.State)
    }

    var loadingState: LoadingState = .loading(.init())
}

To simulate the loading process in the first tab I return .task effect with Task.sleep inside of it, expecting, that SwitchStore will replace the loading view with the content view. And it does, but changing tabs during this sleep simulated loading breaks just about everything. I see a warning mentioning ifCaseLet reducer and the first tab's view stops updating.

I'm not sure if the problem is with how I configured things or if there's a bug in some of the TCA new fancy tools. Also, I suppose there might be better ways to organize states like load data from the first screen's main reducer instead of doing it in the loading state reducer.

Here's sample project: https://github.com/swasta/tca-switchstore-test

[mbrandonw]

This is definitely strange, and I think this would happen even on main right now, but it seems to be a bug in SwiftUI. Take for example this simple vanilla SwiftUI view:

struct ContentView: View {
  @State var isLoading = true

  var body: some View {
    TabView {
      VStack {
        Text("Tab 1")
        if self.isLoading {
          ProgressView()
          Text("Loading")
            .onAppear { print("onAppear") }
        }
      }
      .tabItem { Label("First", systemImage: "list.dash") }

      Text("Tab 2")
        .tabItem { Label("Second", systemImage: "list.dash") }
    }
    .task {
      try? await Task.sleep(nanoseconds: NSEC_PER_SEC * 2)
      self.isLoading = false
    }
  }
}

If I run this, and quickly navigate to the 2nd tab before the 2 second sleep finishes, I will see "onAppear" printed to the console twice. Once when the loading text actually appears, and then again 2 seconds later when it should be disappearing.

So it seems that SwiftUI is sending an onAppear event at a completely wrong time, and because of that an action is being sent when state is nil. We have filed a ton of bugs in the past about weird onAppear/onDisappear behavior in SwiftUI, and looks like this is another one. 🫤

[nikita-bor]

Thanks for the reply! It's indeed weird to see "onAppear" printed a second time. Do you think that's the reason why the SwitchStore stops switching views in my example? I mean it wouldn't be so bad if it was just the warning about "action sent to the store while state was another case", I would probably blame SwiftUI and move on. But in my sample project changing tab during sleep makes the first tab stops updating, refresh button doesn't really switch states anymore, and so on. And if I go from the first tab to the second one and again to the first one, I see the loading state that never changes. Doesn't seem to be the case in the example you shared.

[nikita-bor]

Good news: I've checked out the latest commit from protocol-branch and now everything magically works. No idea what change exactly fixed my issue, but I'm sure branch at commit abec326 did exhibit weird bugs in my scenario

[oliverfoggin]

I'm a bit late to this thread but the behaviour you are describing seems suspiciously similar to what I've just discovered with TabView in SwiftUI.

It seems that the onAppear is being triggered erroneously for tabs that have been previously opened but aren't the current tab.

Here's a quick gist that shows the behaviour.

https://gist.github.com/oliverfoggin/25f54a66c7eb8dfa8a3ed6f0db239764

When you tap on "Login" and then navigate to another tab then tap "Logout" you will see the "On Appear" in the console for the tabs you visited.

[ibrahimkteish]

@oliverfoggin

It seems that the onAppear is being triggered erroneously for tabs that have been previously opened but aren't the current tab.

This happened to me and lead to firing unnecessary effects.

Here is my TabView implementation to workaround the useless renders to the non-current-selected tab and also have onFirstAppear modifier in the main tab views:

import ComposableArchitecture
import Foundation
import Generated
import HomeFeature
import NotificationsFeature
import Styleguide
import SwiftUI

public struct TabView: View {
  let store: StoreOf<Tab>

  public init(store: StoreOf<Tab>) {
    self.store = store
  }

  public var body: some View {
    WithViewStore(self.store.scope(state: ViewState.init, action: Tab.Action.init)) { viewStore in
      SwiftUI.TabView(selection: viewStore.binding(get: \.selectedTab, send: ViewAction.selectTab)) {
        Group {
          if viewStore.selectedTab == .home {
            NavigationView {
              HomeView(
                store: store.scope(
                  state: \.homeState,
                  action: Tab.Action.home
                )
              )
            }
            .accentColor(.white)
            .navigationViewStyle(StackNavigationViewStyle())

          } else {
            Text("Not Initialised: Home")
          }
        }
        .tabItem {
          Label("Home", systemImage: "calendar")
        }
        .tag(TabItem.home)

        Group {
          if viewStore.selectedTab == .notifications {
            NavigationView {
              NotificationsView(
                store: store.scope(
                  state: \.notificationsState,
                  action: Tab.Action.notifications
                )
              )
            }
            .accentColor(.white)
            .navigationViewStyle(StackNavigationViewStyle())
          } else {
            Text("Not Initialised: Notifications")
          }
        }
        .tabItem {
          Label(L10n.notifications, systemImage: "bell.fill")
        }
        .backport
        .badge(viewStore.unreadNotificationCount)
        .tag(TabItem.notifications)
      }
    }
    .alert(self.store.scope(state: \.alert), dismiss: .alertDismissed)
  }
}

public extension TabView {
  struct ViewState: Hashable {
    var selectedTab: TabItem
    var unreadNotificationCount: Int

    init(_ state: Tab.State) {
      self.selectedTab = state.selectedTab
      self.unreadNotificationCount = state
        .notificationsState
        .unreadCount
    }
  }
}

public extension TabView {
  enum ViewAction: Hashable {
    case selectTab(TabItem)
  }
}

extension Tab.Action {
  init(_ action: TabView.ViewAction) {
    switch action {
    case let .selectTab(tabItem):
      self = .selectTab(tabItem)
    }
  }
}

// MARK: Tab Previews

struct Tab_Previews: PreviewProvider {
  static var previews: some View {
    TabView(
      store: .init(
        initialState: .init(
          homeState: .init(timeZone: .autoupdatingCurrent),
          notificationsState: .init(timeZone: .autoupdatingCurrent),
          timeZone: .autoupdatingCurrent
        ),
        reducer: Tab()
      )
    )
  }
}

[fabstu]

Could you try the wrapper from #1461 (reply in thread) for the TabView(selection:) part? I think the problem there was that the viewStore rerenders every tab when the selection changed due to ObservedObject doing what it does. The workaround there was to use the specialty of SwiftUI.State to notify the TabView to change the tab without rerendering everything. Could be wrong though.

[oliverfoggin]

I can try but this error happens in vanilla SwiftUI. So it's not related to TCA.

[fabstu]

@oliverfoggin This example here uses selection:. WithIntelligentBinding in the other thread was able to avoid redraws when changings tabs, so if we do not hit your bug then avoiding re-renders might be possible with the wrapper as well. Its to see whether that alternative works.

Although the alternative here using Hashable looks interesting.

[oliverfoggin]

Thanks. I'll try that tomorrow.

[fabstu]

@oliverfoggin WithIntelligentBinding will probably not be helpful at all though, since ur not using selection:. Yeah, the wrapper might be useless for .onAppear, but still worth a try to avoid re-renders when using selection:.