pointfreeco / swift-composable-architecture

A library for building applications in a consistent and understandable way, with composition, testing, and ergonomics in mind.
https://www.pointfree.co/collections/composable-architecture
MIT License
12.23k stars 1.42k forks source link

Add Id to environment transform for forEach #44

Closed toddwbates closed 4 years ago

toddwbates commented 4 years ago

It might be useful to have the key, index, id provided with the global environment when transforming to a local environment. For example

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

These identifiers are often only locally unique (such as file names or view id's) and currently the environment has no way of knowing the scope of id's. It might be possible to construct a global path for a state by composing one with the stateTransform but this would mean that the local reducer would have knowledge about global context. For example if a reducer was implemented as per your VoiceMemo sample where the same static Id is used and then the container is extended to support having multiple players simultaneously then the local reducer would have to be modified as well. It might also be difficult if the type of the enclosing Id was different than the local Id, for example a user my have a email for an id while their todo items would be identified with a UUID. Constructing single global id from this might be a bit strange. Would it be a string or an array of any or a nesting structs?

stephencelis commented 4 years ago

This is interesting to think about! In the reusable favoriting component example the favorite higher-order reducer takes an environment function that can describe a favoriting request given an environment: https://github.com/pointfreeco/swift-composable-architecture/blob/4df9a2e17e8931ef1443fd0ce5f6be23f141c5c1/Examples/CaseStudies/SwiftUICaseStudies/04-HigherOrderReducers-ReusableFavoriting.swift#L55-L59

This functionality is then combined with an episodeReducer: https://github.com/pointfreeco/swift-composable-architecture/blob/4df9a2e17e8931ef1443fd0ce5f6be23f141c5c1/Examples/CaseStudies/SwiftUICaseStudies/04-HigherOrderReducers-ReusableFavoriting.swift#L148

With a forEach such as yours above, one could potentially define a favorites as a specialized forEach for favoriting without much work.

I'm trying to think through other use cases and how it affects other versions of forEach (like the one that works with arrays by index and the one that works with dictionaries by key). What could they do if index or key were exposed to the environment part of forEach?

toddwbates commented 4 years ago

After studying the favoriting I'm wonder that this could be implemented as an alternative to the current forEach. It might make things like grouping downloads easier. If the app was web page like, (I used to work at eBay and old habits die hard) then there are times when you want to kill all the loads from a single page. Having a chained environment would make this easier. It also can minimize use of global queues, such as currently being used in the cancelable functionality. It feels a little odd to have the environment produce effects but then cancel is triggered from the Effect class itself.

I'm not sure if its better to do this or have a concept of path.

stephencelis commented 4 years ago

@toddwbates We just got a Swift forum that should be nice for discussions like this and allow us to keep GitHub issues focused on bugs. Would you be up for moving the discussion there and closing this out? Would also love to see some motivating examples of this kind of forEach transformation. Maybe with a PR 🤩

toddwbates commented 4 years ago

Sure, I'll try and get one set up asap.