Allow sending actions to deleted IdentifiedArray items?

Related Projects Swift Composable Architecture
1

In my app, it's possible that delayed effects can be sent to items that have been removed from an IdentifiedArray.

This results in hitting the fatalError in IdentifiedArray.subscript(id:).set because Reducer.forEach assumes that the identified item will always be there.

Is there a better way of handling this activity? I've thought about:

Adding an optional to my forEach reducer:

    arrayReducer.forEach(state: \.array, action: /AppAction.arrayAction, environment: { () }).optional,

However, this doesn't work because AppState.array is not an array of optionals.

If I added a guard to Reducer.forEach, the problem is resolved:

  public func forEach<GlobalState, GlobalAction, GlobalEnvironment, ID>(
    state toLocalState: WritableKeyPath<GlobalState, IdentifiedArray<ID, State>>,
    action toLocalAction: CasePath<GlobalAction, (ID, Action)>,
    environment toLocalEnvironment: @escaping (GlobalEnvironment) -> Environment
  ) -> Reducer<GlobalState, GlobalAction, GlobalEnvironment> {
    .init { globalState, globalAction, globalEnvironment in
      guard let (id, localAction) = toLocalAction.extract(from: globalAction) else { return .none }

      // Ensure that the localState is still present in the identified array
      guard globalState[keyPath: toLocalState][id: id] != nil else { return .none }

      return self.optional
        .reducer(
          &globalState[keyPath: toLocalState][id: id],
          localAction,
          toLocalEnvironment(globalEnvironment)
        )
        .map { toLocalAction.embed((id, $0)) }
    }
  }

However, this doesn't seem like a great change to make to forEach — should this be a separate method? Am i just going about this all wrong?

2

But what if the index still exists but is now referencing the wrong item,. Say you have an array of 3 items [a,b,c] and the Effect will modify b (the second item), then b is removed. The index now references c rather than b. I don't think this is what you intended. Also if the effect was going to transform c but c's index changed from 2 to 1 then the reducer wouldn't work in this case either.

3

These indexes are meant to be unique. It's an IdentifiedArray, so the id is the Element.ID, as opposed to a numeric index. If the index still exists then it's considered to always be pointing at the "correct" item. At least, that's my understanding of it.

4

Another possibility to just concatenate a bunch of Effect.cancel when I mutate the state to remove items from the identified array.

Since Effect.cancel doesn't really contain an action, you don't even need to do much of anything, other than generate the correct CancelId:

    public func cancel<GlobalAction>() -> Effect<GlobalAction, Never> {
        if haveADelayedEffect {
            return Effect.cancel(id: cancelId)
        } else {
            return Effect.none
        }
    }

This assumes that all effects are cancelable. That's not always the case though (perhaps there's an outstanding network request that can't be cancelled).

5

Ok, missed that.

7

IdentifiedArray is currently designed to be used via ID the same way Array is designed to be used via Index. Direct subscript access is unsafe and assumes that you know the element is in the array.

The optionality of the subscript may be the cause for confusion, but is necessary due to some bugs with SwiftUI and ForEach, which can continue to call the body of a row view even if its corresponding derived state has gone missing upstream (actually, I may be confusing things with a different, related bug, but the optionality here is definitely a workaround for some bug). If anyone has ideas on how to better address this, we're definitely open to changes in the design!

8

In the end, I believe correct solution is to create your own higher-order forEach reducer like so:

extension Reducer {
    func forEach<GlobalState, GlobalAction, GlobalEnvironment, ID>(
        ifPresent toLocalState: WritableKeyPath<GlobalState, IdentifiedArray<ID, State>>,
        action toLocalAction: CasePath<GlobalAction, (ID, Action)>,
        environment toLocalEnvironment: @escaping (GlobalEnvironment) -> Environment
    ) -> Reducer<GlobalState, GlobalAction, GlobalEnvironment> {
        .init { globalState, globalAction, globalEnvironment in
            guard let (id, localAction) = toLocalAction.extract(from: globalAction) else { return .none }
            guard globalState[keyPath: toLocalState][id: id] != nil else { return .none }

            return self.run(&globalState[keyPath: toLocalState][id: id]!,
                            localAction,
                            toLocalEnvironment(globalEnvironment))
                .map { toLocalAction.embed((id, $0)) }
        }
    }
}