More docs around Reducer.combine and ordering (#215)

* More docs around Reducer.combine and ordering

* typo

* Update Sources/ComposableArchitecture/Reducer.swift

Co-authored-by: Brandon Williams <[email protected]>

* more

* Update Reducer.swift

* more

Co-authored-by: Brandon Williams <[email protected]>

Author: 2 people

Sources/ComposableArchitecture/Reducer.swift

@@ -55,17 +55,99 @@ public struct Reducer<State, Action, Environment> {
     Self { _, _, _ in .none }
   }
 
-  /// Combines many reducers into a single one by running each one on the state, and merging
+  /// Combines many reducers into a single one by running each one on state in order, and merging
   /// all of the effects.
   ///
+  /// It is important to note that the order of combining reducers matter. Combining `reducerA` with 
+  /// `reducerB` is not necessarily the same as combining `reducerB` with `reducerA`.
+  ///
+  /// This can become an issue when working with reducers that have overlapping domains. For
+  /// example, if `reducerA` embeds the domain of `reducerB` and reacts to its actions or modifies
+  /// its state, it can make a difference if `reducerA` chooses to modify `reducerB`'s state
+  /// _before_ or _after_ `reducerB` runs.
+  ///
+  /// This is perhaps most easily seen when working with `optional` reducers, where the parent
+  /// domain may listen to the child domain and `nil` out its state. If the parent reducer runs
+  /// before the child reducer, then the child reducer will not be able to react to its own action.
+  ///
+  /// Similar can be said for a `forEach` reducer. If the parent domain modifies the child
+  /// collection by moving, removing, or modifying an element before the `forEach` reducer runs, the
+  /// `forEach` reducer may perform its action against the wrong element, an element that no longer
+  /// exists, or an element in an unexpected state.
+  ///
+  /// Running a parent reducer before a child reducer can be considered an application logic
+  /// error, and can produce assertion failures. So you should almost always combine reducers in
+  /// order from child to parent domain.
+  ///
+  /// Here is an example of how you should combine an `optional` reducer with a parent domain:
+  ///
+  ///     let parentReducer = Reducer<ParentState, ParentAction, ParentEnvironment>.combine(
+  ///       // Combined before parent so that it can react to `.dismiss` while state is non-`nil`.
+  ///       childReducer.optional.pullback(
+  ///         state: \.child,
+  ///         action: /ParentAction.child,
+  ///         environment: { $0.child }
+  ///       ),
+  ///       // Combined after child so that it can `nil` out child state upon `.child(.dismiss)`.
+  ///       Reducer { state, action, environment in
+  ///         switch action
+  ///         case .child(.dismiss):
+  ///           state.child = nil
+  ///           return .none
+  ///         ...
+  ///         }
+  ///       },
+  ///     )
+  ///
   /// - Parameter reducers: A list of reducers.
   /// - Returns: A single reducer.
   public static func combine(_ reducers: Reducer...) -> Reducer {
     .combine(reducers)
   }
 
-  /// Combines an array of reducers into a single one by running each one on the state, and
-  /// merging all of the effects.
+  /// Combines many reducers into a single one by running each one on state in order, and merging
+  /// all of the effects.
+  ///
+  /// It is important to note that the order of combining reducers matter. Combining `reducerA` with
+  /// `reducerB` is not necessarily the same as combining `reducerB` with `reducerA`.
+  ///
+  /// This can become an issue when working with reducers that have overlapping domains. For
+  /// example, if `reducerA` embeds the domain of `reducerB` and reacts to its actions or modifies
+  /// its state, it can make a difference if `reducerA` chooses to modify `reducerB`'s state
+  /// _before_ or _after_ `reducerB` runs.
+  ///
+  /// This is perhaps most easily seen when working with `optional` reducers, where the parent
+  /// domain may listen to the child domain and `nil` out its state. If the parent reducer runs
+  /// before the child reducer, then the child reducer will not be able to react to its own action.
+  ///
+  /// Similar can be said for a `forEach` reducer. If the parent domain modifies the child
+  /// collection by moving, removing, or modifying an element before the `forEach` reducer runs, the
+  /// `forEach` reducer may perform its action against the wrong element, an element that no longer
+  /// exists, or an element in an unexpected state.
+  ///
+  /// Running a parent reducer before a child reducer can be considered an application logic
+  /// error, and can produce assertion failures. So you should almost always combine reducers in
+  /// order from child to parent domain.
+  ///
+  /// Here is an example of how you should combine an `optional` reducer with a parent domain:
+  ///
+  ///     let parentReducer = Reducer<ParentState, ParentAction, ParentEnvironment>.combine(
+  ///       // Combined before parent so that it can react to `.dismiss` while state is non-`nil`.
+  ///       childReducer.optional.pullback(
+  ///         state: \.child,
+  ///         action: /ParentAction.child,
+  ///         environment: { $0.child }
+  ///       ),
+  ///       // Combined after child so that it can `nil` out child state upon `.child(.dismiss)`.
+  ///       Reducer { state, action, environment in
+  ///         switch action
+  ///         case .child(.dismiss):
+  ///           state.child = nil
+  ///           return .none
+  ///         ...
+  ///         }
+  ///       },
+  ///     )
   ///
   /// - Parameter reducers: An array of reducers.
   /// - Returns: A single reducer.
@@ -75,8 +157,52 @@ public struct Reducer<State, Action, Environment> {
     }
   }
 
-  /// Combines the current reducer with another given reducer by running each one on the state,
-  /// and merging their effects.
+  /// Combines many reducers into a single one by running each one on state in order, and merging
+  /// all of the effects.
+  ///
+  /// It is important to note that the order of combining reducers matter. Combining `reducerA` with
+  /// `reducerB` is not necessarily the same as combining `reducerB` with `reducerA`.
+  ///
+  /// This can become an issue when working with reducers that have overlapping domains. For
+  /// example, if `reducerA` embeds the domain of `reducerB` and reacts to its actions or modifies
+  /// its state, it can make a difference if `reducerA` chooses to modify `reducerB`'s state
+  /// _before_ or _after_ `reducerB` runs.
+  ///
+  /// This is perhaps most easily seen when working with `optional` reducers, where the parent
+  /// domain may listen to the child domain and `nil` out its state. If the parent reducer runs
+  /// before the child reducer, then the child reducer will not be able to react to its own action.
+  ///
+  /// Similar can be said for a `forEach` reducer. If the parent domain modifies the child
+  /// collection by moving, removing, or modifying an element before the `forEach` reducer runs, the
+  /// `forEach` reducer may perform its action against the wrong element, an element that no longer
+  /// exists, or an element in an unexpected state.
+  ///
+  /// Running a parent reducer before a child reducer can be considered an application logic
+  /// error, and can produce assertion failures. So you should almost always combine reducers in
+  /// order from child to parent domain.
+  ///
+  /// Here is an example of how you should combine an `optional` reducer with a parent domain:
+  ///
+  ///     let parentReducer: Reducer<ParentState, ParentAction, ParentEnvironment> =
+  ///       // Run before parent so that it can react to `.dismiss` while state is non-`nil`.
+  ///       childReducer
+  ///         .optional
+  ///         .pullback(
+  ///           state: \.child,
+  ///           action: /ParentAction.child,
+  ///           environment: { $0.child }
+  ///         )
+  ///         // Combined after child so that it can `nil` out child state upon `.child(.dismiss)`.
+  ///         .combined(
+  ///           with: Reducer { state, action, environment in
+  ///             switch action
+  ///             case .child(.dismiss):
+  ///               state.child = nil
+  ///               return .none
+  ///             ...
+  ///             }
+  ///           }
+  ///         )
   ///
   /// - Parameter other: Another reducer.
   /// - Returns: A single reducer.
@@ -140,7 +266,8 @@ public struct Reducer<State, Action, Environment> {
   /// only running the non-optional reducer when state is non-nil.
   ///
   /// Often used in tandem with `pullback` to transform a reducer on a non-optional local domain
-  /// into a reducer on a global domain that contains an optional local domain:
+  /// into a reducer that can be combined with a reducer on a global domain that contains some
+  /// optional local domain:
   ///
   ///     // Global domain that holds an optional local domain:
   ///     struct AppState { var modal: ModalState? }
@@ -151,12 +278,20 @@ public struct Reducer<State, Action, Environment> {
   ///     let modalReducer = Reducer<ModalState, ModalAction, ModalEnvironment { ... }
   ///
   ///     // Pullback the local modal reducer so that it works on all of the app domain:
-  ///     let appReducer: Reducer<AppState, AppAction, AppEnvironment> =
+  ///     let appReducer = Reducer<AppState, AppAction, AppEnvironment>.combine(
   ///       modalReducer.optional().pullback(
   ///         state: \.modal,
   ///         action: /AppAction.modal,
   ///         environment: { ModalEnvironment(mainQueue: $0.mainQueue) }
-  ///       )
+  ///       ),
+  ///       Reducer { state, action, environment in
+  ///         ...
+  ///       }
+  ///     )
+  ///
+  /// Take care when combining optional reducers into parent domains, as order matters. Always
+  /// combine optional reducers _before_ parent reducers that can `nil` out the associated optional
+  /// state.
   ///
   /// - See also: `IfLetStore`, a SwiftUI helper for transforming a store on optional state into a
   ///   store on non-optional state.
@@ -205,12 +340,19 @@ public struct Reducer<State, Action, Environment> {
   ///     let todoReducer = Reducer<Todo, TodoAction, TodoEnvironment> { ... }
   ///
   ///     // Pullback the local todo reducer so that it works on all of the app domain:
-  ///     let appReducer: Reducer<AppState, AppAction, AppEnvironment> =
+  ///     let appReducer = Reducer<AppState, AppAction, AppEnvironment>.combine(
   ///       todoReducer.forEach(
   ///         state: \.todos,
   ///         action: /AppAction.todo(index:action:),
   ///         environment: { _ in TodoEnvironment() }
-  ///       )
+  ///       ),
+  ///       Reducer { state, action, environment in
+  ///         ...
+  ///       }
+  ///     )
+  ///
+  /// Take care when combining `forEach` reducers into parent domains, as order matters. Always
+  /// combine `forEach` reducers _before_ parent reducers that can modify the collection.
   ///
   /// - Parameters:
   ///   - toLocalState: A key path that can get/set an array of `State` elements inside.
@@ -276,12 +418,19 @@ public struct Reducer<State, Action, Environment> {
   ///     let todoReducer = Reducer<Todo, TodoAction, TodoEnvironment> { ... }
   ///
   ///     // Pullback the local todo reducer so that it works on all of the app domain:
-  ///     let appReducer: Reducer<AppState, AppAction, AppEnvironment> =
+  ///     let appReducer = Reducer<AppState, AppAction, AppEnvironment>.combine(
   ///       todoReducer.forEach(
   ///         state: \.todos,
   ///         action: /AppAction.todo(id:action:),
   ///         environment: { _ in TodoEnvironment() }
-  ///       )
+  ///       ),
+  ///       Reducer { state, action, environment in
+  ///         ...
+  ///       }
+  ///     )
+  ///
+  /// Take care when combining `forEach` reducers into parent domains, as order matters. Always
+  /// combine `forEach` reducers _before_ parent reducers that can modify the collection.
   ///
   /// - Parameters:
   ///   - toLocalState: A key path that can get/set a collection of `State` elements inside
@@ -342,6 +491,9 @@ public struct Reducer<State, Action, Environment> {
   /// A version of `pullback` that transforms a reducer that works on an element into one that works
   /// on a dictionary of element values.
   ///
+  /// Take care when combining `forEach` reducers into parent domains, as order matters. Always
+  /// combine `forEach` reducers _before_ parent reducers that can modify the dictionary.
+  ///
   /// - Parameters:
   ///   - toLocalState: A key path that can get/set a dictionary of `State` values inside
   ///     `GlobalState`.