Skip to content

Commit 4016008

Browse files
stephencelisstephencelis
authored
Assorted fixes for collections of state (#25)
* Assorted fixes for collections of state Fixes #21. * Update IdentifiedArrayTests.swift * Update case studies * Run swift-format on _push_ to master * Format
1 parent d638733 commit 4016008

File tree

7 files changed

+138
-10
lines changed

7 files changed

+138
-10
lines changed

‎.github/workflows/format.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
name: Format
22

33
on:
4-
pull_request:
4+
push:
55
branch: 'master'
66

77
jobs:

‎Examples/CaseStudies/SwiftUICaseStudies/03-Navigation-Lists-LoadThenNavigate.swift

+3-3
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,15 @@ let lazyListNavigationReducer = Reducer<
5252

5353
case .setNavigation(selection: .none):
5454
if let selection = state.selection {
55-
state.rows[selection.id]?.count = selection.count
55+
state.rows[id: selection.id]?.count = selection.count
5656
state.selection = nil
5757
}
5858
return .none
5959

6060
case let .setNavigationSelectionDelayCompleted(id):
61-
state.rows[id]?.isActivityIndicatorVisible = false
61+
state.rows[id: id]?.isActivityIndicatorVisible = false
6262
state.selection = Identified(
63-
CounterState(count: state.rows[id]?.count ?? 0),
63+
CounterState(count: state.rows[id: id]?.count ?? 0),
6464
id: id
6565
)
6666
return .none

‎Examples/CaseStudies/SwiftUICaseStudies/03-Navigation-Lists-NavigateAndLoad.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,14 @@ let eagerListNavigationReducer = Reducer<
4949

5050
case .setNavigation(selection: .none):
5151
if let selection = state.selection, let count = selection.value?.count {
52-
state.rows[selection.id]?.count = count
52+
state.rows[id: selection.id]?.count = count
5353
state.selection = nil
5454
}
5555
return .cancel(id: CancelId())
5656

5757
case .setNavigationSelectionDelayCompleted:
5858
guard let id = state.selection?.id else { return .none }
59-
state.selection?.value = CounterState(count: state.rows[id]?.count ?? 0)
59+
state.selection?.value = CounterState(count: state.rows[id: id]?.count ?? 0)
6060
return .none
6161
}
6262
},

‎Sources/ComposableArchitecture/Reducer.swift

+12-1
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,17 @@ public struct Reducer<State, Action, Environment> {
194194
guard let (index, localAction) = toLocalAction.extract(from: globalAction) else {
195195
return .none
196196
}
197+
// NB: This does not need to be a fatal error because of the index subscript that follows it.
198+
assert(
199+
index < globalState[keyPath: toLocalState].endIndex,
200+
"""
201+
Index out of range. This can happen when a reducer that can remove the last element from \
202+
an array is then combined with a "forEach" from that array. To avoid this and other \
203+
index-related gotchas, consider using an "IdentifiedArray" of state instead. Or, combine \
204+
your reducers so that the "forEach" comes before any reducer that can remove elements from \
205+
its array.
206+
"""
207+
)
197208
return self.reducer(
198209
&globalState[keyPath: toLocalState][index],
199210
localAction,
@@ -238,7 +249,7 @@ public struct Reducer<State, Action, Environment> {
238249
guard let (id, localAction) = toLocalAction.extract(from: globalAction) else { return .none }
239250
return self.optional
240251
.reducer(
241-
&globalState[keyPath: toLocalState][id],
252+
&globalState[keyPath: toLocalState][id: id],
242253
localAction,
243254
toLocalEnvironment(globalEnvironment)
244255
)

‎Sources/ComposableArchitecture/SwiftUI/ForEachStore.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ where Data: Collection, ID: Hashable, Content: View {
105105
ForEach(viewStore.state, id: viewStore.id) { element in
106106
content(
107107
store.scope(
108-
state: { $0[element[keyPath: viewStore.id]] ?? element },
108+
state: { $0[id: element[keyPath: viewStore.id]] ?? element },
109109
action: { (element[keyPath: viewStore.id], $0) }
110110
)
111111
)

‎Sources/ComposableArchitecture/SwiftUI/IdentifiedArray.swift

+6-2
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,15 @@ where ID: Hashable {
9292
}
9393
}
9494

95-
public subscript(id: ID) -> Element? {
95+
public subscript(id id: ID) -> Element? {
9696
get {
9797
self.dictionary[id]
9898
}
9999
_modify {
100100
yield &self.dictionary[id]
101+
if self.dictionary[id] == nil {
102+
self.ids.removeAll(where: { $0 == id })
103+
}
101104
}
102105
}
103106

@@ -115,6 +118,7 @@ where ID: Hashable {
115118
}
116119
}
117120

121+
@discardableResult
118122
public mutating func remove(at position: Int) -> Element {
119123
let id = self.ids.remove(at: position)
120124
let element = self.dictionary[id]!
@@ -138,7 +142,7 @@ where ID: Hashable {
138142
}
139143

140144
public mutating func remove(atOffsets offsets: IndexSet) {
141-
for offset in offsets {
145+
for offset in offsets.reversed() {
142146
_ = self.remove(at: offset)
143147
}
144148
}

‎Tests/ComposableArchitectureTests/IdentifiedArrayTests.swift

+113
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import XCTest
2+
3+
@testable import ComposableArchitecture
4+
5+
final class IdentifiedArrayTests: XCTestCase {
6+
func testIdSubscript() {
7+
struct User: Equatable, Identifiable {
8+
let id: Int
9+
var name: String
10+
}
11+
12+
var array: IdentifiedArray = [User(id: 1, name: "Blob")]
13+
14+
XCTAssertEqual(array[id: 1], .some(User(id: 1, name: "Blob")))
15+
16+
array[id: 1] = nil
17+
XCTAssertEqual(array, [])
18+
}
19+
20+
func testInsert() {
21+
struct User: Equatable, Identifiable {
22+
let id: Int
23+
var name: String
24+
}
25+
26+
var array: IdentifiedArray = [User(id: 1, name: "Blob")]
27+
28+
array.insert(User(id: 2, name: "Blob Jr."), at: 0)
29+
XCTAssertEqual(array, [User(id: 2, name: "Blob Jr."), User(id: 1, name: "Blob")])
30+
}
31+
32+
func testInsertContentsOf() {
33+
struct User: Equatable, Identifiable {
34+
let id: Int
35+
var name: String
36+
}
37+
38+
var array: IdentifiedArray = [User(id: 1, name: "Blob")]
39+
40+
array.insert(contentsOf: [User(id: 3, name: "Blob Sr."), User(id: 2, name: "Blob Jr.")], at: 0)
41+
XCTAssertEqual(
42+
array,
43+
[User(id: 3, name: "Blob Sr."), User(id: 2, name: "Blob Jr."), User(id: 1, name: "Blob")]
44+
)
45+
}
46+
47+
func testRemoveAt() {
48+
struct User: Equatable, Identifiable {
49+
let id: Int
50+
var name: String
51+
}
52+
53+
var array: IdentifiedArray = [
54+
User(id: 3, name: "Blob Sr."),
55+
User(id: 2, name: "Blob Jr."),
56+
User(id: 1, name: "Blob"),
57+
]
58+
59+
array.remove(at: 1)
60+
XCTAssertEqual(array, [User(id: 3, name: "Blob Sr."), User(id: 1, name: "Blob")])
61+
}
62+
63+
func testRemoveAllWhere() {
64+
struct User: Equatable, Identifiable {
65+
let id: Int
66+
var name: String
67+
}
68+
69+
var array: IdentifiedArray = [
70+
User(id: 3, name: "Blob Sr."),
71+
User(id: 2, name: "Blob Jr."),
72+
User(id: 1, name: "Blob"),
73+
]
74+
75+
array.removeAll(where: { $0.name.starts(with: "Blob ") })
76+
XCTAssertEqual(array, [User(id: 1, name: "Blob")])
77+
}
78+
79+
func testRemoveAtOffsets() {
80+
struct User: Equatable, Identifiable {
81+
let id: Int
82+
var name: String
83+
}
84+
85+
var array: IdentifiedArray = [
86+
User(id: 3, name: "Blob Sr."),
87+
User(id: 2, name: "Blob Jr."),
88+
User(id: 1, name: "Blob"),
89+
]
90+
91+
array.remove(atOffsets: [0, 2])
92+
XCTAssertEqual(array, [User(id: 2, name: "Blob Jr.")])
93+
}
94+
95+
func testMoveFromOffsets() {
96+
struct User: Equatable, Identifiable {
97+
let id: Int
98+
var name: String
99+
}
100+
101+
var array: IdentifiedArray = [
102+
User(id: 3, name: "Blob Sr."),
103+
User(id: 2, name: "Blob Jr."),
104+
User(id: 1, name: "Blob"),
105+
]
106+
107+
array.move(fromOffsets: [0], toOffset: 2)
108+
XCTAssertEqual(
109+
array,
110+
[User(id: 2, name: "Blob Jr."), User(id: 3, name: "Blob Sr."), User(id: 1, name: "Blob")]
111+
)
112+
}
113+
}

0 commit comments

Comments
 (0)