From ab9d66fc444a1054e9beb13a24f1d907879a4f7c Mon Sep 17 00:00:00 2001 From: Daniel Peter Date: Wed, 23 Mar 2022 12:13:21 +0100 Subject: [PATCH 1/5] Avoid force-unwrapping cached value in ForEachStore --- .../SwiftUI/ForEachStore.swift | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift b/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift index b9651684d899..da6f13d78f00 100644 --- a/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift +++ b/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift @@ -89,27 +89,30 @@ where Data: Collection, ID: Hashable, Content: View { EachContent: View, Data == IdentifiedArray, Content == WithViewStore< - OrderedSet, (ID, EachAction), ForEach, ID, EachContent> + OrderedSet, (ID, EachAction), ForEach, ID, EachContent?> > { self.data = store.state.value self.content = { WithViewStore(store.scope(state: { $0.ids })) { viewStore in - ForEach(viewStore.state, id: \.self) { id -> EachContent in + ForEach(viewStore.state, id: \.self) { id -> EachContent? in // NB: We cache elements here to avoid a potential crash where SwiftUI may re-evaluate // views for elements no longer in the collection. // // Feedback filed: https://gist.github.com/stephencelis/cdf85ae8dab437adc998fb0204ed9a6b - var element = store.state.value[id: id]! - return content( - store.scope( - state: { - element = $0[id: id] ?? element - return element - }, - action: { (id, $0) } + if var element = store.state.value[id: id] { + return content( + store.scope( + state: { + element = $0[id: id] ?? element + return element + }, + action: { (id, $0) } + ) ) - ) + } else { + return nil + } } } } From a8921c5dd8863a6b01a3049a984e15ca6194923e Mon Sep 17 00:00:00 2001 From: Daniel Peter Date: Fri, 25 Mar 2022 10:44:36 +0100 Subject: [PATCH 2/5] ForEach closure based caching approach --- .../SwiftUI/ForEachStore.swift | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift b/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift index da6f13d78f00..7a672270fd28 100644 --- a/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift +++ b/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift @@ -88,31 +88,26 @@ where Data: Collection, ID: Hashable, Content: View { where EachContent: View, Data == IdentifiedArray, + EachState: Equatable, Content == WithViewStore< - OrderedSet, (ID, EachAction), ForEach, ID, EachContent?> + IdentifiedArray, (ID, EachAction), ForEach, ID, EachContent> > { self.data = store.state.value self.content = { - WithViewStore(store.scope(state: { $0.ids })) { viewStore in - ForEach(viewStore.state, id: \.self) { id -> EachContent? in + WithViewStore(store) { viewStore in + ForEach(viewStore.state, id: viewStore.state.id) { element in // NB: We cache elements here to avoid a potential crash where SwiftUI may re-evaluate // views for elements no longer in the collection. // // Feedback filed: https://gist.github.com/stephencelis/cdf85ae8dab437adc998fb0204ed9a6b - if var element = store.state.value[id: id] { - return content( - store.scope( - state: { - element = $0[id: id] ?? element - return element - }, - action: { (id, $0) } - ) + let id = element[keyPath: viewStore.state.id] + content( + store.scope( + state: { collection in collection[id: id] ?? element }, + action: { action in (id, action) } ) - } else { - return nil - } + ) } } } From 98b39c70a79168bf8619b9e36ed63d15447adc0d Mon Sep 17 00:00:00 2001 From: Daniel Peter Date: Fri, 25 Mar 2022 10:56:30 +0100 Subject: [PATCH 3/5] Uses unnamed closure parameters --- Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift b/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift index 7a672270fd28..6690edc1f789 100644 --- a/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift +++ b/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift @@ -104,8 +104,8 @@ where Data: Collection, ID: Hashable, Content: View { let id = element[keyPath: viewStore.state.id] content( store.scope( - state: { collection in collection[id: id] ?? element }, - action: { action in (id, action) } + state: { $0[id: id] ?? element }, + action: { (id, $0) } ) ) } From 5f36457d31b91e456cd3949f55b31c339f542af5 Mon Sep 17 00:00:00 2001 From: Daniel Peter Date: Wed, 6 Apr 2022 14:56:08 +0200 Subject: [PATCH 4/5] Define custom remove duplicates to only reevaluate when IDs change --- Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift b/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift index 6690edc1f789..ebe560591026 100644 --- a/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift +++ b/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift @@ -88,14 +88,16 @@ where Data: Collection, ID: Hashable, Content: View { where EachContent: View, Data == IdentifiedArray, - EachState: Equatable, Content == WithViewStore< IdentifiedArray, (ID, EachAction), ForEach, ID, EachContent> > { self.data = store.state.value self.content = { - WithViewStore(store) { viewStore in + WithViewStore( + store, + removeDuplicates: { old, new in old.ids == new.ids } + ) { viewStore in ForEach(viewStore.state, id: viewStore.state.id) { element in // NB: We cache elements here to avoid a potential crash where SwiftUI may re-evaluate // views for elements no longer in the collection. From 7b28dbff8854674cd221fa4c98a32c8ee938b91f Mon Sep 17 00:00:00 2001 From: Stephen Celis Date: Wed, 21 Jun 2023 17:02:31 -0700 Subject: [PATCH 5/5] fix --- .../SwiftUI/ForEachStore.swift | 35 +++++++++---------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift b/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift index 1d94c26b8110..fb8101fd1af4 100644 --- a/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift +++ b/Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift @@ -100,27 +100,26 @@ public struct ForEachStore< where Data == IdentifiedArray, Content == WithViewStore< - IdentifiedArray, (ID, EachAction), ForEach, ID, EachContent> + IdentifiedArray, (ID, EachAction), + ForEach, ID, EachContent> > { self.data = store.state.value - self.content = { - WithViewStore( - store, - observe: { $0 }, - removeDuplicates: { areOrderedSetsDuplicates($0.ids, $1.ids) } - ) { viewStore in - ForEach(viewStore.state, id: viewStore.state.id) { element in - var element = element - let id = element[keyPath: viewStore.state.id] - content( - store.scope( - state: { - element = $0[id: id] ?? element - return element - }, - action: { (id, $0) } - ) + self.content = WithViewStore( + store, + observe: { $0 }, + removeDuplicates: { areOrderedSetsDuplicates($0.ids, $1.ids) } + ) { viewStore in + ForEach(viewStore.state, id: viewStore.state.id) { element in + var element = element + let id = element[keyPath: viewStore.state.id] + content( + store.scope( + state: { + element = $0[id: id] ?? element + return element + }, + action: { (id, $0) } ) ) }