From f9f66faf89eda86cfd9c65df0879986dcdbf95a9 Mon Sep 17 00:00:00 2001 From: Ben Follington <5009316+bfollington@users.noreply.github.com> Date: Fri, 23 Feb 2024 10:47:40 +1000 Subject: [PATCH 1/4] Make send async --- Sources/ObservableStore/ObservableStore.swift | 68 ++++++++++--------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/Sources/ObservableStore/ObservableStore.swift b/Sources/ObservableStore/ObservableStore.swift index be6b197..e2ed2b3 100644 --- a/Sources/ObservableStore/ObservableStore.swift +++ b/Sources/ObservableStore/ObservableStore.swift @@ -356,44 +356,46 @@ where Model: ModelProtocol let actionString = String(describing: action) logger.debug("Action: \(actionString, privacy: .public)") } - - // Dispatch action before state change - _actions.send(action) - - // Create next state update - let next = Model.update( - state: self.state, - action: action, - environment: self.environment - ) - // Set `state` if changed. - // - // Mutating state (a `@Published` property) will fire `objectWillChange` - // and cause any views that subscribe to store to re-evaluate - // their body property. - // - // If no change has occurred, we avoid setting the property - // so that body does not need to be reevaluated. - if self.state != next.state { - // If transaction is specified by update, set state with - // that transaction. + + Task(priority: .userInitiated) { + // Dispatch action before state change + _actions.send(action) + + // Create next state update + let next = Model.update( + state: self.state, + action: action, + environment: self.environment + ) + // Set `state` if changed. + // + // Mutating state (a `@Published` property) will fire `objectWillChange` + // and cause any views that subscribe to store to re-evaluate + // their body property. // - // Otherwise, if transaction is nil, just set state, and - // defer to global transaction. - if let transaction = next.transaction { - withTransaction(transaction) { + // If no change has occurred, we avoid setting the property + // so that body does not need to be reevaluated. + if self.state != next.state { + // If transaction is specified by update, set state with + // that transaction. + // + // Otherwise, if transaction is nil, just set state, and + // defer to global transaction. + if let transaction = next.transaction { + withTransaction(transaction) { + self.state = next.state + } + } else { self.state = next.state } - } else { - self.state = next.state } + + // Run effects + self.subscribe(to: next.fx) + + // Dispatch update after state change + self._updates.send(next) } - - // Run effects - self.subscribe(to: next.fx) - - // Dispatch update after state change - self._updates.send(next) } } From 8030c54ce612bbfbd0d32ce18aad1cf97331eabc Mon Sep 17 00:00:00 2001 From: Ben Follington <5009316+bfollington@users.noreply.github.com> Date: Fri, 23 Feb 2024 11:15:25 +1000 Subject: [PATCH 2/4] Lock task to MainActor --- Sources/ObservableStore/ObservableStore.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/ObservableStore/ObservableStore.swift b/Sources/ObservableStore/ObservableStore.swift index e2ed2b3..edf6fe3 100644 --- a/Sources/ObservableStore/ObservableStore.swift +++ b/Sources/ObservableStore/ObservableStore.swift @@ -357,9 +357,9 @@ where Model: ModelProtocol logger.debug("Action: \(actionString, privacy: .public)") } - Task(priority: .userInitiated) { + Task.detached { @MainActor in // Dispatch action before state change - _actions.send(action) + self._actions.send(action) // Create next state update let next = Model.update( From cff26f50c343a3c6bf16610222db464da822ff3f Mon Sep 17 00:00:00 2001 From: Ben Follington <5009316+bfollington@users.noreply.github.com> Date: Fri, 1 Mar 2024 08:51:36 +1000 Subject: [PATCH 3/4] Change which part is deferred --- Sources/ObservableStore/ObservableStore.swift | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/Sources/ObservableStore/ObservableStore.swift b/Sources/ObservableStore/ObservableStore.swift index edf6fe3..34c24ba 100644 --- a/Sources/ObservableStore/ObservableStore.swift +++ b/Sources/ObservableStore/ObservableStore.swift @@ -357,39 +357,39 @@ where Model: ModelProtocol logger.debug("Action: \(actionString, privacy: .public)") } - Task.detached { @MainActor in - // Dispatch action before state change - self._actions.send(action) - - // Create next state update - let next = Model.update( - state: self.state, - action: action, - environment: self.environment - ) - // Set `state` if changed. - // - // Mutating state (a `@Published` property) will fire `objectWillChange` - // and cause any views that subscribe to store to re-evaluate - // their body property. + // Dispatch action before state change + self._actions.send(action) + + // Create next state update + let next = Model.update( + state: self.state, + action: action, + environment: self.environment + ) + // Set `state` if changed. + // + // Mutating state (a `@Published` property) will fire `objectWillChange` + // and cause any views that subscribe to store to re-evaluate + // their body property. + // + // If no change has occurred, we avoid setting the property + // so that body does not need to be reevaluated. + if self.state != next.state { + // If transaction is specified by update, set state with + // that transaction. // - // If no change has occurred, we avoid setting the property - // so that body does not need to be reevaluated. - if self.state != next.state { - // If transaction is specified by update, set state with - // that transaction. - // - // Otherwise, if transaction is nil, just set state, and - // defer to global transaction. - if let transaction = next.transaction { - withTransaction(transaction) { - self.state = next.state - } - } else { + // Otherwise, if transaction is nil, just set state, and + // defer to global transaction. + if let transaction = next.transaction { + withTransaction(transaction) { self.state = next.state } + } else { + self.state = next.state } + } + Task.detached { @MainActor in // Run effects self.subscribe(to: next.fx) From 4020901e5ef3386a02b45860850ff7154465673c Mon Sep 17 00:00:00 2001 From: Ben Follington <5009316+bfollington@users.noreply.github.com> Date: Fri, 1 Mar 2024 13:01:38 +1000 Subject: [PATCH 4/4] Switch to DispatchQueue.main.async --- Sources/ObservableStore/ObservableStore.swift | 20 +++-- Tests/ObservableStoreTests/BindingTests.swift | 36 ++++---- .../ComponentMappingTests.swift | 82 +++++++++++-------- .../ObservableStoreTests.swift | 14 ++-- .../ObservableStoreTests/ViewStoreTests.swift | 36 ++++---- 5 files changed, 107 insertions(+), 81 deletions(-) diff --git a/Sources/ObservableStore/ObservableStore.swift b/Sources/ObservableStore/ObservableStore.swift index 34c24ba..485fe18 100644 --- a/Sources/ObservableStore/ObservableStore.swift +++ b/Sources/ObservableStore/ObservableStore.swift @@ -352,6 +352,12 @@ where Model: ModelProtocol /// `send(_:)` is run *synchronously*. It is up to you to guarantee it is /// run on main thread when SwiftUI is being used. public func send(_ action: Model.Action) { + DispatchQueue.main.async { @MainActor in + self.sendSync(action) + } + } + + public func sendSync(_ action: Model.Action) { if loggingEnabled { let actionString = String(describing: action) logger.debug("Action: \(actionString, privacy: .public)") @@ -388,14 +394,12 @@ where Model: ModelProtocol self.state = next.state } } - - Task.detached { @MainActor in - // Run effects - self.subscribe(to: next.fx) - - // Dispatch update after state change - self._updates.send(next) - } + + // Run effects + self.subscribe(to: next.fx) + + // Dispatch update after state change + self._updates.send(next) } } diff --git a/Tests/ObservableStoreTests/BindingTests.swift b/Tests/ObservableStoreTests/BindingTests.swift index 50a5427..65e74cc 100644 --- a/Tests/ObservableStoreTests/BindingTests.swift +++ b/Tests/ObservableStoreTests/BindingTests.swift @@ -59,14 +59,16 @@ final class BindingTests: XCTestCase { view.text = "Foo" view.text = "Bar" - XCTAssertEqual( - store.state.text, - "Bar" - ) - XCTAssertEqual( - store.state.edits, - 2 - ) + DispatchQueue.main.async { + XCTAssertEqual( + store.state.text, + "Bar" + ) + XCTAssertEqual( + store.state.edits, + 2 + ) + } } /// Test creating binding for an address @@ -86,13 +88,15 @@ final class BindingTests: XCTestCase { view.text = "Foo" view.text = "Bar" - XCTAssertEqual( - store.state.text, - "Bar" - ) - XCTAssertEqual( - store.state.edits, - 2 - ) + DispatchQueue.main.async { + XCTAssertEqual( + store.state.text, + "Bar" + ) + XCTAssertEqual( + store.state.edits, + 2 + ) + } } } diff --git a/Tests/ObservableStoreTests/ComponentMappingTests.swift b/Tests/ObservableStoreTests/ComponentMappingTests.swift index 2386d97..c7eaee1 100644 --- a/Tests/ObservableStoreTests/ComponentMappingTests.swift +++ b/Tests/ObservableStoreTests/ComponentMappingTests.swift @@ -140,14 +140,16 @@ class ComponentMappingTests: XCTestCase { send(.setText("Foo")) send(.setText("Bar")) - XCTAssertEqual( - store.state.child.text, - "Bar" - ) - XCTAssertEqual( - store.state.edits, - 2 - ) + DispatchQueue.main.async { + XCTAssertEqual( + store.state.child.text, + "Bar" + ) + XCTAssertEqual( + store.state.edits, + 2 + ) + } } func testKeyedCursorUpdate() throws { @@ -164,11 +166,13 @@ class ComponentMappingTests: XCTestCase { store.send(.keyedChild(action: .setText("BBB"), key: "a")) store.send(.keyedChild(action: .setText("AAA"), key: "a")) - XCTAssertEqual( - store.state.keyedChildren["a"]?.text, - "AAA", - "KeyedCursor updates model at key" - ) + DispatchQueue.main.async { + XCTAssertEqual( + store.state.keyedChildren["a"]?.text, + "AAA", + "KeyedCursor updates model at key" + ) + } } func testCursorUpdateTransaction() throws { @@ -194,15 +198,17 @@ class ComponentMappingTests: XCTestCase { store.send(.setText("Woo")) store.send(.setText("Woo")) - XCTAssertEqual( - store.state.child.text, - "Woo", - "Cursor updates child model" - ) - XCTAssertEqual( - store.state.edits, - 2 - ) + DispatchQueue.main.async { + XCTAssertEqual( + store.state.child.text, + "Woo", + "Cursor updates child model" + ) + XCTAssertEqual( + store.state.edits, + 2 + ) + } } func testKeyedCursorUpdateMissing() throws { @@ -217,15 +223,18 @@ class ComponentMappingTests: XCTestCase { environment: () ) store.send(.keyedChild(action: .setText("ZZZ"), key: "z")) - XCTAssertEqual( - store.state.keyedChildren.count, - 3, - "KeyedCursor update does nothing if key is missing" - ) - XCTAssertNil( - store.state.keyedChildren["z"], - "KeyedCursor update does nothing if key is missing" - ) + + DispatchQueue.main.async { + XCTAssertEqual( + store.state.keyedChildren.count, + 3, + "KeyedCursor update does nothing if key is missing" + ) + XCTAssertNil( + store.state.keyedChildren["z"], + "KeyedCursor update does nothing if key is missing" + ) + } } func testKeyedCursorUpdateTransaction() throws { @@ -244,9 +253,12 @@ class ComponentMappingTests: XCTestCase { action: .setText("Foo"), environment: () ) - XCTAssertNotNil( - update.transaction, - "Transaction is preserved by cursor" - ) + + DispatchQueue.main.async { + XCTAssertNotNil( + update.transaction, + "Transaction is preserved by cursor" + ) + } } } diff --git a/Tests/ObservableStoreTests/ObservableStoreTests.swift b/Tests/ObservableStoreTests/ObservableStoreTests.swift index fd164d6..1dd72d2 100644 --- a/Tests/ObservableStoreTests/ObservableStoreTests.swift +++ b/Tests/ObservableStoreTests/ObservableStoreTests.swift @@ -94,7 +94,9 @@ final class ObservableStoreTests: XCTestCase { store.send(.increment) - XCTAssertEqual(store.state.count, 1, "state is advanced") + DispatchQueue.main.async { + XCTAssertEqual(store.state.count, 1, "state is advanced") + } } /// Tests that the immediately-completing empty Fx used as the default for @@ -112,7 +114,7 @@ final class ObservableStoreTests: XCTestCase { let expectation = XCTestExpectation( description: "cancellable removed when publisher completes" ) - DispatchQueue.main.async { + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { XCTAssertEqual( store.cancellables.count, 0, @@ -120,7 +122,7 @@ final class ObservableStoreTests: XCTestCase { ) expectation.fulfill() } - wait(for: [expectation], timeout: 0.1) + wait(for: [expectation], timeout: 0.2) } /// Tests that immediately-completing Fx get removed from the cancellables. @@ -145,7 +147,7 @@ final class ObservableStoreTests: XCTestCase { let expectation = XCTestExpectation( description: "cancellable removed when publisher completes" ) - DispatchQueue.main.async { + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { XCTAssertEqual( store.cancellables.count, 0, @@ -153,7 +155,7 @@ final class ObservableStoreTests: XCTestCase { ) expectation.fulfill() } - wait(for: [expectation], timeout: 0.1) + wait(for: [expectation], timeout: 0.2) } func testAsyncFxRemovedOnComplete() { @@ -229,7 +231,7 @@ final class ObservableStoreTests: XCTestCase { let expectation = XCTestExpectation( description: "publisher does not fire when state does not change" ) - DispatchQueue.main.async { + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { // Publisher should fire twice: once for initial state, // once for state change. XCTAssertEqual( diff --git a/Tests/ObservableStoreTests/ViewStoreTests.swift b/Tests/ObservableStoreTests/ViewStoreTests.swift index b481e05..9ca3c46 100644 --- a/Tests/ObservableStoreTests/ViewStoreTests.swift +++ b/Tests/ObservableStoreTests/ViewStoreTests.swift @@ -107,14 +107,16 @@ final class ViewStoreTests: XCTestCase { viewStore.send(.setText("Foo")) - XCTAssertEqual( - store.state.child.text, - "Foo" - ) - XCTAssertEqual( - store.state.edits, - 1 - ) + DispatchQueue.main.async { + XCTAssertEqual( + store.state.child.text, + "Foo" + ) + XCTAssertEqual( + store.state.edits, + 1 + ) + } } /// Test creating binding for an address @@ -131,13 +133,15 @@ final class ViewStoreTests: XCTestCase { viewStore.send(.setText("Foo")) - XCTAssertEqual( - store.state.child.text, - "Foo" - ) - XCTAssertEqual( - store.state.edits, - 1 - ) + DispatchQueue.main.async { + XCTAssertEqual( + store.state.child.text, + "Foo" + ) + XCTAssertEqual( + store.state.edits, + 1 + ) + } } }