From d9a3cfb5fa0586599e8125635fbc2950691d753c Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Thu, 8 Feb 2024 12:33:54 -0800 Subject: [PATCH] Better document explicit `id` with scoped `ForEach` (#2784) `ForEachStore` implicitly used `id: \.state.id` under the hood, so a naive migration to `ForEach` might miss this detail. Let's be more explicit in our documentation and migration guide to avoid this issue. In the future it might be possible to address better in the library itself, but for now the requirement for stores to be identifiable for other forms of navigation means allowing for this. --- .../MigrationGuides/MigratingTo1.7.md | 20 ++++++++++++------- .../Articles/ObservationBackport.md | 4 ++-- .../Articles/Performance.md | 2 +- .../IdentifiedArray+Observation.swift | 20 ++++++++++++++++--- 4 files changed, 33 insertions(+), 13 deletions(-) diff --git a/Sources/ComposableArchitecture/Documentation.docc/Articles/MigrationGuides/MigratingTo1.7.md b/Sources/ComposableArchitecture/Documentation.docc/Articles/MigrationGuides/MigratingTo1.7.md index 89f6a5b4a25a..da83e5711f0a 100644 --- a/Sources/ComposableArchitecture/Documentation.docc/Articles/MigrationGuides/MigratingTo1.7.md +++ b/Sources/ComposableArchitecture/Documentation.docc/Articles/MigrationGuides/MigratingTo1.7.md @@ -204,27 +204,33 @@ struct Feature { Then you would have made use of ``ForEachStore`` in the view like this: ```swift -ForEachStore(store.scope(state: \.rows, action: \.rows)) { childStore in +ForEachStore( + store.scope(state: \.rows, action: \.rows) +) { childStore in ChildView(store: childStore) } ``` This can now be updated to use the vanilla `ForEach` view in SwiftUI, along with -``Store/scope(state:action:)-1nelp``: +``Store/scope(state:action:)-1nelp``, identified by the state of each row: ```swift -ForEach(store.scope(state: \.rows, action: \.rows)) { childStore in +ForEach( + store.scope(state: \.rows, action: \.rows), id: \.state.id +) { childStore in ChildView(store: childStore) } ``` -If your usage of `ForEachStore` relied on the identity of the state of each row (_e.g._, the state's -`id` is also associated with a selection binding), you must explicitly use the `id` parameter: +If your usage of `ForEachStore` did not depend on the identity of the state of each row (_e.g._, the +state's `id` is not associated with a selection binding), you can omit the `id` parameter, as the +`Store` type is identifiable by its object identity: ```diff ForEach( - store.scope(state: \.rows, action: \.rows), -+ id: \.state.id +- store.scope(state: \.rows, action: \.rows), +- id: \.state.id, ++ store.scope(state: \.rows, action: \.rows) ) { childStore in ChildView(store: childStore) } diff --git a/Sources/ComposableArchitecture/Documentation.docc/Articles/ObservationBackport.md b/Sources/ComposableArchitecture/Documentation.docc/Articles/ObservationBackport.md index f1c17a25cff9..0e31cdf3c52b 100644 --- a/Sources/ComposableArchitecture/Documentation.docc/Articles/ObservationBackport.md +++ b/Sources/ComposableArchitecture/Documentation.docc/Articles/ObservationBackport.md @@ -71,7 +71,7 @@ This means that even if you wrap the body of the view in `WithPerceptionTracking ```swift WithPerceptionTracking { - ForEach(store.scope(state: \.rows, action: \.rows) { store in + ForEach(store.scope(state: \.rows, action: \.rows), id: \.state.id) { store in Text(store.title) } } @@ -84,7 +84,7 @@ The fix for this is to wrap the content of the trailing closure in another `With ```swift WithPerceptionTracking { - ForEach(store.scope(state: \.rows, action: \.rows) { store in + ForEach(store.scope(state: \.rows, action: \.rows), id: \.state.id) { store in WithPerceptionTracking { Text(store.title) } diff --git a/Sources/ComposableArchitecture/Documentation.docc/Articles/Performance.md b/Sources/ComposableArchitecture/Documentation.docc/Articles/Performance.md index 5ecce134cce2..36ca9aefa28b 100644 --- a/Sources/ComposableArchitecture/Documentation.docc/Articles/Performance.md +++ b/Sources/ComposableArchitecture/Documentation.docc/Articles/Performance.md @@ -351,7 +351,7 @@ Another example is scoping to some collection of a child domain in order to use ``ForEachStore``: ```swift -ForEachStore(store.scope(state: \.rows, action: \.rows) { store in +ForEachStore(store.scope(state: \.rows, action: \.rows)) { store in RowView(store: store) } ``` diff --git a/Sources/ComposableArchitecture/Observation/IdentifiedArray+Observation.swift b/Sources/ComposableArchitecture/Observation/IdentifiedArray+Observation.swift index 4e3d140045f5..a800b7411751 100644 --- a/Sources/ComposableArchitecture/Observation/IdentifiedArray+Observation.swift +++ b/Sources/ComposableArchitecture/Observation/IdentifiedArray+Observation.swift @@ -5,8 +5,8 @@ extension Store where State: ObservableState { /// Scopes the store of an identified collection to a collection of stores. /// - /// This operator is most often used with SwiftUI's `ForEach` view. For example, suppose you have - /// a feature that contains an `IdentifiedArray` of child features like so: + /// This operator is most often used with SwiftUI's `ForEach` view. For example, suppose you + /// have a feature that contains an `IdentifiedArray` of child features like so: /// /// ```swift /// @Reducer @@ -38,7 +38,7 @@ /// /// var body: some View { /// List { - /// ForEach(store.scope(state: \.rows, action: \.rows) { store in + /// ForEach(store.scope(state: \.rows, action: \.rows), id: \.state.id) { store in /// ChildView(store: store) /// } /// } @@ -46,6 +46,20 @@ /// } /// ``` /// + /// > Tip: If you do not depend on the identity of the state of each row (_e.g._, the state's + /// > `id` is not associated with a selection binding), you can omit the `id` parameter, as the + /// > `Store` type is identifiable by its object identity: + /// > + /// > ```diff + /// > ForEach( + /// > - store.scope(state: \.rows, action: \.rows), + /// > - id: \.state.id, + /// > + store.scope(state: \.rows, action: \.rows) + /// > ) { childStore in + /// > ChildView(store: childStore) + /// > } + /// > ``` + /// /// - Parameters: /// - state: A key path to an identified array of child state. /// - action: A case key path to an identified child action.