Scoping inside body when using TCAFeatureAction protocol

[ibrahimkteish]

lets say we are developing a login feature with TCAFeatureAction with the following code.

public struct Login: ReducerProtocol {
  // MARK: Login State
  public struct State: Equatable {
    public var alert: AlertState<Action>?
    public var isFormValid = false
    public var isRequestInFlight = false
    @BindableState
    public var isSafariSheetPresented = false
    @BindableState
    public var isSelectRolePresented = false
    @BindableState
    public var isSelectRoleShown = false
    @BindableState
    public var password = ""

    @BindableState
    public var selectedRole: AuthResponse.Role = .init(code: 100, info: "Courier")
    public var roleSelection: [AuthResponse.Role] = []

    public var showPassword = false
    public var token: AuthResponse
    @BindableState
    public var username = ""
    public var welcomeMessage: String = L10n.welcomeToTheApp

    public init(
      alert: AlertState<Action>? = nil,
      isFormValid: Bool = false,
      isRequestInFlight: Bool = false,
      isSafariSheetPresented: Bool = false,
      isSelectRolePresented: Bool = false,
      roleSelection: [AuthResponse.Role] = [],
      password: String = "",
      selectedRole: AuthResponse.Role = .init(code: 100, info: ""),
      showPassword: Bool = false,
      token: AuthResponse = .init(data: .init(idToken: "", roles: [])),
      username: String = "",
      welcomeMessage: String = L10n.welcomeToTheApp
    ) {
      self.alert = alert
      self.isFormValid = isFormValid
      self.isRequestInFlight = isRequestInFlight
      self.isSafariSheetPresented = isSafariSheetPresented
      self.isSelectRolePresented = isSelectRolePresented
      self.roleSelection = roleSelection
      self.password = password
      self.selectedRole = selectedRole
      self.showPassword = showPassword
      self.token = token
      self.username = username
      self.welcomeMessage = welcomeMessage
    }
  }

  // MARK: Login Actions

  public enum Action: Equatable, TCAFeatureAction {
    public enum ViewAction: Equatable, BindableAction {
      case alertDismissed
      case binding(BindingAction<State>)
      case loginButtonTapped
      case showPassword
    }

    public enum InternalAction: Equatable {
      case accessTokenReceived(TaskResult<LoginResponse>)
      case initializeResponse(TaskResult<InitializeResponse>)
      case leaveTypesResponse(TaskResult<LeavesTypeResponse>)
      case loginResponse(TaskResult<AuthResponse>)
      case profilePicDownloaded(TaskResult<Data?>, TimeZone)
    }

    public enum DelegateAction: Equatable {
      case accessTokenReceived(LoginResponse)
      case profilePicDownloaded(Data?, TimeZone)
    }

    case view(ViewAction)
    case delegate(DelegateAction)
    case _internal(InternalAction) // swiftlint:disable:this identifier_name
  }

  // MARK: Login Dependencies

  @Dependency(\.apiClient) var apiClient
  @Dependency(\.networkClient) var networkClient
  @Dependency(\.fileClient) var fileClient

  // MARK: init

  public init() {}

  // MARK: Reducer Body

  public var body: some ReducerProtocol<State, Action> {
    Scope(state: \.self, action: /Action.view) {
      BindingReducer()
    }

    Scope(state: \.self, action: /Action._internal) {
      Reduce { state, internalAction in
        switch internalAction {
          case .accessTokenReceived(.success):
            return .run { send in
              await withThrowingTaskGroup(of: Void.self) { group in
                group.addTask {
                  await send(
                      .leaveTypesResponse(
                        await TaskResult {
                          try await self.apiClient
                            .apiRequest(
                              route: ServerRoute.Api.Route.leave(.types),
                              as: LeavesTypeResponse.self
                            )
                        }
                      )
                  )
                }

                group.addTask {
                  await send(
                      .initializeResponse(
                        await TaskResult {
                          try await self.apiClient
                            .apiRequest(
                              route: ServerRoute.Api.Route.initialize,
                              as: InitializeResponse.self
                            )
                        }
                    )
                  )
                }
              }
            }
          case let .accessTokenReceived(.failure(error)):
            state.alert = AlertState(title: TextState(error.localizedDescription))
            state.isRequestInFlight = false
            return .none
          case let .initializeResponse(.failure(error)):
            state.alert = AlertState(title: TextState(error.localizedDescription))
            state.isRequestInFlight = false
            return .none
          case .initializeResponse(.success):
            return .none // self.initialResponse(&state, initial)
          case .leaveTypesResponse(.failure):
            return .none
          case let .leaveTypesResponse(.success(response)):
            return .fireAndForget {
              try await self.fileClient.saveLeaveTypes(response: response)
            }
          case let .loginResponse(.failure(error)):
            state.alert = AlertState(title: TextState(error.localizedDescription))
            state.isRequestInFlight = false
            return .none

          case .loginResponse(.success):
            return .none // self.loginResponse(&state, response: response)

          case let .profilePicDownloaded(.failure(error), _):
            state.alert = .init(title: .init(error.localizedDescription))
            return .none
          case .profilePicDownloaded:
            return .none // .task { .delegate(.profilePicDownloaded(response, timeZone)) }
        }

      }
    }

    Reduce { state, action in
      switch action {
        case let .view(viewAction):
          switch viewAction {
            case .alertDismissed:
              state.alert = nil
              return .none
            case .binding(\.view.$isSelectRolePresented):
              if state.isSelectRolePresented == false { // means we selected and dismissed or auto-selected
                let token = state.token
                let role = state.selectedRole
                return .task {
                  ._internal(
                    .accessTokenReceived(
                      await TaskResult {
                        try await self.apiClient
                          .request(
                            route: ServerRoute.auth(.token(.init(idToken: token.idToken, role: role.code))),
                            as: LoginResponse.self
                          )
                      }
                    )
                  )
                }
              }
              return .none
            case .binding(\.view.$username), .binding(\.view.$password):
              state.isFormValid = !state.username.isEmpty && !state.password.isEmpty
              return .none
            case .binding:
              return .none
            case .loginButtonTapped:
              return loginButtonTapped(&state)
            case .showPassword:
              state.showPassword.toggle()
              return .none
          }
        case .delegate:
          return .none
        case let ._internal(.accessTokenReceived(.success(response))):
          return .task { .delegate(.accessTokenReceived(response)) }
        case let ._internal(.initializeResponse(.success(initial))):
          return self.initialResponse(&state, initial)
        case let ._internal(.loginResponse(.success(response))):
          return self.loginResponse(&state, response: response)
        case let ._internal(.profilePicDownloaded(.success(response), timeZone)):
          return .task { .delegate(.profilePicDownloaded(response, timeZone)) }
        case ._internal:
          return .none
      }
    }
  }

  private func loginButtonTapped(_ state: inout State) -> Effect<Action, Never> {
   // code
  }

  private func loginResponse(_ state: inout State, response: AuthResponse) -> Effect<Action, Never> {
   // code
      return .task { .view(.binding(.set(\.view.$isSelectRolePresented, false))) }
  }

  private func initialResponse(_ state: inout State, _ initial: InitializeResponse) -> Effect<Action, Never> {
    state.isRequestInFlight = false
    return .task {
      try await self.fileClient.saveInitialize(response: initial)
      // code
      }
}

my question is it ok to have scopes in public var body: some ReducerProtocol<State, Action> { like the

 Scope(state: \.self, action: /Action.view) {
      BindingReducer()
    }

    Scope(state: \.self, action: /Action._internal) {
      Reduce { state, internalAction in
      //.....//
       }
    }

or it is more appropriate to have them inside one Reduce { state, action in BindingReducer() ... }

[tgrapperon]

Hey @ibrahimkteish! While it is functionally OK to do so, I wouldn't do it that way personally.

Your reducer's domain is quite large, and this is probably a sign that you can extract some functionalities into smaller reducers. This should also probably help the compiler. Each smaller domain can still have nested _internal, view, and delegate actions if you want, and I'm not saying that you should try to split your domain along these technical boundaries, but rather according to some coherent functionalities (which is something you can only do on a case by case basis).

This should probably help make your code easier to read, compile and test.

Furthermore, if I would adopt this kind of nested actions/reducers, I would probably define a custom higher order reducer in the same spirit of Scope, but where the \.self State transform would be baked in. For example, a Case<ActionDomain>(_ action: CasePath<Action, ActionDomain>) that could be used like:

Case(/Action.view) {
  BindingReducer()
}
Case(/Action._internal) {
  Reduce { … }
}
Case(/Action.delegate) {
  …
}

You can define this type of reducer relatively straightforwardly. For example, something like:

public struct Case<ParentState, ParentAction, Child: ReducerProtocol>: ReducerProtocol
where Child.State == ParentState {
  public let toChildAction: CasePath<ParentAction, Child.Action>
  public let child: Child

  @inlinable
  public init(
    _ toChildAction: CasePath<ParentAction, Child.Action>,
    @ReducerBuilderOf<Child> _ child: () -> Child
  ) {
    self.toChildAction = toChildAction
    self.child = child()
  }

  @inlinable
  public func reduce(
    into state: inout ParentState, action: ParentAction
  ) -> EffectTask<ParentAction> {
    guard let childAction = self.toChildAction.extract(from: action)
    else { return .none }
    return self.child
      .reduce(into: &state, action: childAction)
      .map(self.toChildAction.embed)
  }
}

Again, this is my personal opinion and not a definitive view on the matter. In general, having to Scope asymmetrically or multiple times is a sign that you can refactor in a way that doesn't involve this kind of procedure.

[tgrapperon]

BTW, there is likely a typo in Reduce { state, action in BindingReducer() ... }. This one is definitely not the recommended way to use child reducers.

[ibrahimkteish]

@tgrapperon indeed it was a Typo, Thank you so much for your answer, I was looking forward to see how others might think about it, I will take your suggestion into consideration.