From 1eeca17b21b931fd6a5cc683a2cd659fb0441c66 Mon Sep 17 00:00:00 2001 From: Brandon Williams <135203+mbrandonw@users.noreply.github.com> Date: Thu, 6 Jun 2024 17:16:34 -0500 Subject: [PATCH] Make `Shared.wrappedValue` setter unavailable from async and introduce `Shared.withLock` (#3136) * Add withValue to Shared, deprecate direct mutation. * updates * wip * wip * wip * wip * Available noasync * withLock * clean up * wip * wip * Update SyncUpsListTests.swift * wip * wip * wip * wip * wip --------- Co-authored-by: Stephen Celis --- Examples/SyncUps/SyncUps/AppFeature.swift | 2 +- .../SyncUpsTests/AppFeatureTests.swift | 4 +- .../SyncUpsTests/SyncUpDetailTests.swift | 2 +- .../MigrationGuides/MigrationTo1.11.md | 99 +++++++++++++++ .../Articles/SharingState.md | 39 +++++- .../Documentation.docc/Extensions/Shared.md | 41 ++++++ .../Extensions/SharedReader.md | 32 +++++ .../PersistingSyncUps-01-code-0005.swift | 18 +++ .../PersistenceKey/AppStorageKey.swift | 32 ++--- .../PersistenceKey/FileStorageKey.swift | 12 +- .../PersistenceKey/InMemoryKey.swift | 6 +- .../References/ValueReference.swift | 72 ++++++++++- .../SharedState/Shared.swift | 118 ++++++++++++++---- .../SharedState/SharedReader.swift | 40 +++++- .../AppStorageTests.swift | 2 +- .../FileStorageTests.swift | 2 +- .../SharedTests.swift | 13 +- 17 files changed, 464 insertions(+), 70 deletions(-) create mode 100644 Sources/ComposableArchitecture/Documentation.docc/Articles/MigrationGuides/MigrationTo1.11.md create mode 100644 Sources/ComposableArchitecture/Documentation.docc/Extensions/Shared.md create mode 100644 Sources/ComposableArchitecture/Documentation.docc/Extensions/SharedReader.md create mode 100644 Sources/ComposableArchitecture/Documentation.docc/Tutorials/BuildingSyncUps/05-PersistingSyncUps/PersistingSyncUps-01-code-0005.swift diff --git a/Examples/SyncUps/SyncUps/AppFeature.swift b/Examples/SyncUps/SyncUps/AppFeature.swift index be884c90bf09..e951f9cbfe22 100644 --- a/Examples/SyncUps/SyncUps/AppFeature.swift +++ b/Examples/SyncUps/SyncUps/AppFeature.swift @@ -30,7 +30,7 @@ struct AppFeature { } Reduce { state, action in switch action { - case let .path(.element(id, .detail(.delegate(delegateAction)))): + case let .path(.element(_, .detail(.delegate(delegateAction)))): switch delegateAction { case let .startMeeting(sharedSyncUp): state.path.append(.record(RecordMeeting.State(syncUp: sharedSyncUp))) diff --git a/Examples/SyncUps/SyncUpsTests/AppFeatureTests.swift b/Examples/SyncUps/SyncUpsTests/AppFeatureTests.swift index 054c5a9a57d3..4e96faeb464c 100644 --- a/Examples/SyncUps/SyncUpsTests/AppFeatureTests.swift +++ b/Examples/SyncUps/SyncUpsTests/AppFeatureTests.swift @@ -12,7 +12,7 @@ final class AppFeatureTests: XCTestCase { AppFeature() } - let sharedSyncUp = try XCTUnwrap($syncUps[id: syncUp.id]) + let sharedSyncUp = try XCTUnwrap(Shared($syncUps[id: syncUp.id])) await store.send(\.path.push, (id: 0, .detail(SyncUpDetail.State(syncUp: sharedSyncUp)))) { $0.path[id: 0] = .detail(SyncUpDetail.State(syncUp: sharedSyncUp)) @@ -44,7 +44,7 @@ final class AppFeatureTests: XCTestCase { AppFeature() } - let sharedSyncUp = try XCTUnwrap($syncUps[id: syncUp.id]) + let sharedSyncUp = try XCTUnwrap(Shared($syncUps[id: syncUp.id])) await store.send(\.path.push, (id: 0, .detail(SyncUpDetail.State(syncUp: sharedSyncUp)))) { $0.path[id: 0] = .detail(SyncUpDetail.State(syncUp: sharedSyncUp)) diff --git a/Examples/SyncUps/SyncUpsTests/SyncUpDetailTests.swift b/Examples/SyncUps/SyncUpsTests/SyncUpDetailTests.swift index 8d011a6b16b2..f6a3cc2ab646 100644 --- a/Examples/SyncUps/SyncUpsTests/SyncUpDetailTests.swift +++ b/Examples/SyncUps/SyncUpsTests/SyncUpDetailTests.swift @@ -118,7 +118,7 @@ final class SyncUpDetailTests: XCTestCase { // TODO: Can this exhaustively be caught? defer { XCTAssertEqual([], syncUps) } - let sharedSyncUp = try XCTUnwrap($syncUps[id: syncUp.id]) + let sharedSyncUp = try XCTUnwrap(Shared($syncUps[id: syncUp.id])) let store = TestStore(initialState: SyncUpDetail.State(syncUp: sharedSyncUp)) { SyncUpDetail() } diff --git a/Sources/ComposableArchitecture/Documentation.docc/Articles/MigrationGuides/MigrationTo1.11.md b/Sources/ComposableArchitecture/Documentation.docc/Articles/MigrationGuides/MigrationTo1.11.md new file mode 100644 index 000000000000..33a18ef893b6 --- /dev/null +++ b/Sources/ComposableArchitecture/Documentation.docc/Articles/MigrationGuides/MigrationTo1.11.md @@ -0,0 +1,99 @@ +# Migrating to 1.11 + +Update your code to use the new ``Shared/withLock(_:)`` method for mutating shared state from +asynchronous contexts, rather than mutating the underlying wrapped value directly. + +## Overview + +The Composable Architecture is under constant development, and we are always looking for ways to +simplify the library, and make it more powerful. This version of the library introduced 2 new +APIs and deprecated 1 API. + +> Important: Before following this migration guide be sure you have fully migrated to the newest +> tools of version 1.10. See for more information. + +## Mutating shared state concurrently + +Version 1.10 of the Composable Architecture introduced a powerful tool for +[sharing state]() amongst your features. And you can mutate a piece of shared +state directly, as if it were just a normal property on a value type: + +```swift +case .incrementButtonTapped: + state.count += 1 + return .none +``` + +And if you only ever mutate shared state from a reducer, then this is completely fine to do. +However, because shared values are secretly references (that is how data is shared), it is possible +to mutate shared values from effects, which means concurrently. And prior to 1.11, it was possible +to do this directly: + +```swift +case .delayedIncrementButtonTapped: + return .run { _ in + @Shared(.count) var count + count += 1 + } + +Now, `Shared` is `Sendable`, and is technically thread-safe in that it will not crash when writing +to it from two different threads. However, allowing direct mutation does make the value susceptible +to race conditions. If you were to perform `count += 1` from 1,000 threads, it is possible for +the final value to not be 1,000. + +We wanted the [`@Shared`]() type to be as ergonomic as possible, and that is why we make +it directly mutable, but we should not be allowing these mutations to happen from asynchronous +contexts. And so now the ``Shared/wrappedValue`` setter has been marked unavailable from +asynchronous contexts, with a helpful message of how to fix: + +```swift +case .delayedIncrementButtonTapped: + return .run { _ in + @Shared(.count) var count + count += 1 // ⚠️ Use '$shared.withLock' instead of mutating directly. + } +``` + +To fix this deprecation you can use the new ``Shared/withLock(_:)`` method on the projected value of +`@Shared`: + +```swift +case .delayedIncrementButtonTapped: + return .run { _ in + @Shared(.count) var count + $count.withLock { $0 += 1 } + } +``` + +This locks the entire unit of work of reading the current count, incrementing it, and storing it +back in the reference. + +Technically it is still possible to write code that has race conditions, such as this silly example: + +```swift +let currentCount = count +$count.withLock { $0 = currentCount + 1 } +``` + +But there is no way to 100% prevent race conditions in code. Even actors are susceptible to problems +due to re-entrancy. To avoid problems like the above we recommend wrapping as many mutations of the +shared state as possible in a single ``Shared/withLock(_:)``. That will make sure that the full unit +of work is guarded by a lock. + +## Supplying mock read-only state to previews + +A new ``SharedReader/constant(_:)`` helper on ``SharedReader`` has been introduced to simplify +supplying mock data to Xcode previews. It works like SwiftUI's `Binding.constant`, but for shared +references: + +```swift +#Preview { + FeatureView( + store: Store( + initialState: Feature.State(count: .constant(42)) + ) { + Feature() + } + ) +) +``` diff --git a/Sources/ComposableArchitecture/Documentation.docc/Articles/SharingState.md b/Sources/ComposableArchitecture/Documentation.docc/Articles/SharingState.md index ca5792fcdaf1..b085170cb011 100644 --- a/Sources/ComposableArchitecture/Documentation.docc/Articles/SharingState.md +++ b/Sources/ComposableArchitecture/Documentation.docc/Articles/SharingState.md @@ -38,6 +38,7 @@ as SQLite. * [Testing tips](#Testing-tips) * [Read-only shared state](#Read-only-shared-state) * [Type-safe keys](#Type-safe-keys) +* [Concurrent mutations to shared state](#Concurrent-mutations-to-shared-state) * [Shared state in pre-observation apps](#Shared-state-in-pre-observation-apps) * [Gotchas of @Shared](#Gotchas-of-Shared) @@ -269,8 +270,6 @@ case .countUpdated(let count): If `count` changes, then `$count.publisher` emits, causing the `countUpdated` action to be sent, causing the shared `count` to be mutated, causing `$count.publisher` to emit, and so on. - - ## Initialization rules Because the state sharing tools use property wrappers there are special rules that must be followed @@ -570,8 +569,8 @@ shared state in an effect, and then increments from the effect: ```swift case .incrementButtonTapped: - return .run { [count = state.$count] _ in - count.wrappedValue += 1 + return .run { [sharedCount = state.$count] _ in + sharedCount.withLock { $0 += 1 } } ``` @@ -810,7 +809,7 @@ func testIncrement() async { } ``` -This will fail if you accidetally remove a `@Shared` from one of your features. +This will fail if you accidentally remove a `@Shared` from one of your features. Further, you can enforce this pattern in your codebase by making all `@Shared` properties `fileprivate` so that they can never be mutated outside their file scope: @@ -991,6 +990,36 @@ struct FeatureView: View { } ``` +## Concurrent mutations to shared state + +While the [`@Shared`]() property wrapper makes it possible to treat shared state +_mostly_ like regular state, you do have to perform some extra steps to mutate shared state from +an asynchronous context. This is because shared state is technically a reference deep down, even +though we take extra steps to make it appear value-like. And this means it's possible to mutate the +same piece of shared state from multiple threads, and hence race conditions are possible. + +To mutate a piece of shared state in an isolated fashion, use the ``Shared/withLock(_:)`` method +defined on the `@Shared` projected value: + +```swift +state.$count.withLock { $0 += 1 } +``` + +That locks the entire unit of work of reading the current count, incrementing it, and storing it +back in the reference. + +Technically it is still possible to write code that has race conditions, such as this silly example: + +```swift +let currentCount = state.count +state.$count.withLock { $0 = currentCount + 1 } +``` + +But there is no way to 100% prevent race conditions in code. Even actors are susceptible to +problems due to re-entrancy. To avoid problems like the above we recommend wrapping as many +mutations of the shared state as possible in a single ``Shared/withLock(_:)``. That will make +sure that the full unit of work is guarded by a lock. + ## Gotchas of @Shared There are a few gotchas to be aware of when using shared state in the Composable Architecture. diff --git a/Sources/ComposableArchitecture/Documentation.docc/Extensions/Shared.md b/Sources/ComposableArchitecture/Documentation.docc/Extensions/Shared.md new file mode 100644 index 000000000000..b33a86f92705 --- /dev/null +++ b/Sources/ComposableArchitecture/Documentation.docc/Extensions/Shared.md @@ -0,0 +1,41 @@ +# ``ComposableArchitecture/Shared`` + +## Topics + +### Creating a shared value + +- ``init(_:fileID:line:)-9d3q`` +- ``init(_:)`` +- ``init(projectedValue:)`` + +### Creating a persisted value + +- ``init(wrappedValue:_:fileID:line:)-512rh`` +- ``init(wrappedValue:_:fileID:line:)-7a80y`` +- ``init(_:fileID:line:)-8zcy1`` +- ``init(_:fileID:line:)-8jqg5`` +- ``init(_:fileID:line:)-gluj`` + +### Accessing the value + +- ``wrappedValue`` +- ``projectedValue`` +- ``reader`` +- ``subscript(dynamicMember:)-6kmzm`` +- ``subscript(dynamicMember:)-22ga9`` + +### Isolating the value + +- ``withLock(_:)`` + +### Unit testing the value + +- ``assert(_:file:line:)`` + +### SwiftUI integration + +- ``elements`` + +### Combine integration + +- ``publisher`` diff --git a/Sources/ComposableArchitecture/Documentation.docc/Extensions/SharedReader.md b/Sources/ComposableArchitecture/Documentation.docc/Extensions/SharedReader.md new file mode 100644 index 000000000000..79ae341e5d57 --- /dev/null +++ b/Sources/ComposableArchitecture/Documentation.docc/Extensions/SharedReader.md @@ -0,0 +1,32 @@ +# ``ComposableArchitecture/SharedReader`` + +## Topics + +### Creating a shared value + +- ``init(_:)-3a38z`` +- ``init(_:)-42f43`` +- ``init(projectedValue:)`` +- ``constant(_:)`` + +### Creating a persisted value + +- ``init(wrappedValue:_:fileID:line:)-7q52`` +- ``init(wrappedValue:_:fileID:line:)-6asu2`` +- ``init(_:fileID:line:)-41rb8`` +- ``init(_:fileID:line:)-3lxyf`` +- ``init(_:fileID:line:)-hzp`` + +### Getting the value + +- ``wrappedValue`` +- ``projectedValue`` +- ``subscript(dynamicMember:)-34wfb`` + +### SwiftUI integration + +- ``elements`` + +### Combine integration + +- ``publisher`` diff --git a/Sources/ComposableArchitecture/Documentation.docc/Tutorials/BuildingSyncUps/05-PersistingSyncUps/PersistingSyncUps-01-code-0005.swift b/Sources/ComposableArchitecture/Documentation.docc/Tutorials/BuildingSyncUps/05-PersistingSyncUps/PersistingSyncUps-01-code-0005.swift new file mode 100644 index 000000000000..9c5b87dacfb1 --- /dev/null +++ b/Sources/ComposableArchitecture/Documentation.docc/Tutorials/BuildingSyncUps/05-PersistingSyncUps/PersistingSyncUps-01-code-0005.swift @@ -0,0 +1,18 @@ +import ComposableArchitecture +import SwiftUI + +@main +struct SyncUpsApp: App { + @MainActor + static let store = Store(initialState: SyncUpsList.State()) { + SyncUpsList() + } + + var body: some Scene { + WindowGroup { + NavigationStack { + SyncUpsListView(store: Self.store) + } + } + } +} diff --git a/Sources/ComposableArchitecture/SharedState/PersistenceKey/AppStorageKey.swift b/Sources/ComposableArchitecture/SharedState/PersistenceKey/AppStorageKey.swift index 1f432972a332..aa743670eab0 100644 --- a/Sources/ComposableArchitecture/SharedState/PersistenceKey/AppStorageKey.swift +++ b/Sources/ComposableArchitecture/SharedState/PersistenceKey/AppStorageKey.swift @@ -163,49 +163,49 @@ public struct AppStorageKey { AppStorageKeyID(key: self.key, store: self.store) } - public init(_ key: String) where Value == Bool { + fileprivate init(_ key: String) where Value == Bool { @Dependency(\.defaultAppStorage) var store self.lookup = CastableLookup() self.key = key self.store = store } - public init(_ key: String) where Value == Int { + fileprivate init(_ key: String) where Value == Int { @Dependency(\.defaultAppStorage) var store self.lookup = CastableLookup() self.key = key self.store = store } - public init(_ key: String) where Value == Double { + fileprivate init(_ key: String) where Value == Double { @Dependency(\.defaultAppStorage) var store self.lookup = CastableLookup() self.key = key self.store = store } - public init(_ key: String) where Value == String { + fileprivate init(_ key: String) where Value == String { @Dependency(\.defaultAppStorage) var store self.lookup = CastableLookup() self.key = key self.store = store } - public init(_ key: String) where Value == URL { + fileprivate init(_ key: String) where Value == URL { @Dependency(\.defaultAppStorage) var store self.lookup = URLLookup() self.key = key self.store = store } - public init(_ key: String) where Value == Data { + fileprivate init(_ key: String) where Value == Data { @Dependency(\.defaultAppStorage) var store self.lookup = CastableLookup() self.key = key self.store = store } - public init(_ key: String) + fileprivate init(_ key: String) where Value: RawRepresentable, Value.RawValue == Int { @Dependency(\.defaultAppStorage) var store self.lookup = RawRepresentableLookup(base: CastableLookup()) @@ -213,7 +213,7 @@ public struct AppStorageKey { self.store = store } - public init(_ key: String) + fileprivate init(_ key: String) where Value: RawRepresentable, Value.RawValue == String { @Dependency(\.defaultAppStorage) var store self.lookup = RawRepresentableLookup(base: CastableLookup()) @@ -221,49 +221,49 @@ public struct AppStorageKey { self.store = store } - public init(_ key: String) where Value == Bool? { + fileprivate init(_ key: String) where Value == Bool? { @Dependency(\.defaultAppStorage) var store self.lookup = OptionalLookup(base: CastableLookup()) self.key = key self.store = store } - public init(_ key: String) where Value == Int? { + fileprivate init(_ key: String) where Value == Int? { @Dependency(\.defaultAppStorage) var store self.lookup = OptionalLookup(base: CastableLookup()) self.key = key self.store = store } - public init(_ key: String) where Value == Double? { + fileprivate init(_ key: String) where Value == Double? { @Dependency(\.defaultAppStorage) var store self.lookup = OptionalLookup(base: CastableLookup()) self.key = key self.store = store } - public init(_ key: String) where Value == String? { + fileprivate init(_ key: String) where Value == String? { @Dependency(\.defaultAppStorage) var store self.lookup = OptionalLookup(base: CastableLookup()) self.key = key self.store = store } - public init(_ key: String) where Value == URL? { + fileprivate init(_ key: String) where Value == URL? { @Dependency(\.defaultAppStorage) var store self.lookup = OptionalLookup(base: URLLookup()) self.key = key self.store = store } - public init(_ key: String) where Value == Data? { + fileprivate init(_ key: String) where Value == Data? { @Dependency(\.defaultAppStorage) var store self.lookup = OptionalLookup(base: CastableLookup()) self.key = key self.store = store } - public init(_ key: String) + fileprivate init(_ key: String) where R.RawValue == Int, Value == R? { @Dependency(\.defaultAppStorage) var store self.lookup = OptionalLookup(base: RawRepresentableLookup(base: CastableLookup())) @@ -271,7 +271,7 @@ public struct AppStorageKey { self.store = store } - public init(_ key: String) + fileprivate init(_ key: String) where R.RawValue == String, Value == R? { @Dependency(\.defaultAppStorage) var store self.lookup = OptionalLookup(base: RawRepresentableLookup(base: CastableLookup())) diff --git a/Sources/ComposableArchitecture/SharedState/PersistenceKey/FileStorageKey.swift b/Sources/ComposableArchitecture/SharedState/PersistenceKey/FileStorageKey.swift index 631f829c5b63..794e4c4b3f5b 100644 --- a/Sources/ComposableArchitecture/SharedState/PersistenceKey/FileStorageKey.swift +++ b/Sources/ComposableArchitecture/SharedState/PersistenceKey/FileStorageKey.swift @@ -17,17 +17,17 @@ extension PersistenceReaderKey { /// /// Use ``PersistenceReaderKey/fileStorage(_:)`` to create values of this type. public final class FileStorageKey: PersistenceKey, Sendable { - fileprivate let storage: FileStorage - let isSetting = LockIsolated(false) - let url: URL - let value = LockIsolated(nil) - let workItem = LockIsolated(nil) + private let storage: FileStorage + private let isSetting = LockIsolated(false) + private let url: URL + private let value = LockIsolated(nil) + private let workItem = LockIsolated(nil) public var id: AnyHashable { FileStorageKeyID(url: self.url, storage: self.storage) } - public init(url: URL) { + fileprivate init(url: URL) { @Dependency(\.defaultFileStorage) var storage self.storage = storage self.url = url diff --git a/Sources/ComposableArchitecture/SharedState/PersistenceKey/InMemoryKey.swift b/Sources/ComposableArchitecture/SharedState/PersistenceKey/InMemoryKey.swift index d75a46e3a11b..58e263faa075 100644 --- a/Sources/ComposableArchitecture/SharedState/PersistenceKey/InMemoryKey.swift +++ b/Sources/ComposableArchitecture/SharedState/PersistenceKey/InMemoryKey.swift @@ -24,9 +24,9 @@ extension PersistenceReaderKey { /// /// See ``PersistenceReaderKey/inMemory(_:)`` to create values of this type. public struct InMemoryKey: PersistenceKey, Sendable { - let key: String - let store: InMemoryStorage - public init(_ key: String) { + private let key: String + private let store: InMemoryStorage + fileprivate init(_ key: String) { @Dependency(\.defaultInMemoryStorage) var defaultInMemoryStorage self.key = key self.store = defaultInMemoryStorage diff --git a/Sources/ComposableArchitecture/SharedState/References/ValueReference.swift b/Sources/ComposableArchitecture/SharedState/References/ValueReference.swift index f51468214d1c..12edf1588183 100644 --- a/Sources/ComposableArchitecture/SharedState/References/ValueReference.swift +++ b/Sources/ComposableArchitecture/SharedState/References/ValueReference.swift @@ -9,6 +9,13 @@ import Foundation #endif extension Shared { + /// Creates a shared reference to a value using a persistence key. + /// + /// - Parameters: + /// - value: A default value that is used when no value can be returned from the persistence + /// key. + /// - persistenceKey: A persistence key associated with the shared reference. It is responsible + /// for loading and saving the shared reference's value from some external source. public init( wrappedValue value: @autoclosure @escaping () -> Value, _ persistenceKey: some PersistenceKey, @@ -45,6 +52,11 @@ extension Shared { ) } + /// Creates a shared reference to an optional value using a persistence key. + /// + /// - Parameters: + /// - persistenceKey: A persistence key associated with the shared reference. It is responsible + /// for loading and saving the shared reference's value from some external source. @_disfavoredOverload public init( _ persistenceKey: some PersistenceKey, @@ -54,6 +66,14 @@ extension Shared { self.init(wrappedValue: nil, persistenceKey, fileID: fileID, line: line) } + /// Creates a shared reference to a value using a persistence key. + /// + /// If the given persistence key cannot load a value, an error is thrown. For a non-throwing + /// version of this initializer, see ``init(wrappedValue:_:fileID:line:)-512rh``. + /// + /// - Parameters: + /// - persistenceKey: A persistence key associated with the shared reference. It is responsible + /// for loading and saving the shared reference's value from some external source. @_disfavoredOverload public init( _ persistenceKey: some PersistenceKey, @@ -108,6 +128,11 @@ extension Shared { ) } + /// Creates a shared reference to a value using a persistence key with a default value. + /// + /// - Parameters: + /// - persistenceKey: A persistence key associated with the shared reference. It is responsible + /// for loading and saving the shared reference's value from some external source. public init( _ persistenceKey: PersistenceKeyDefault, fileID: StaticString = #fileID, @@ -121,14 +146,21 @@ extension Shared { ) } + /// Creates a shared reference to a value using a persistence key by overriding its default value. + /// + /// - Parameters: + /// - value: A default value that is used when no value can be returned from the persistence + /// key. + /// - persistenceKey: A persistence key associated with the shared reference. It is responsible + /// for loading and saving the shared reference's value from some external source. public init( - wrappedValue: @autoclosure @escaping () -> Value, + wrappedValue value: @autoclosure @escaping () -> Value, _ persistenceKey: PersistenceKeyDefault, fileID: StaticString = #fileID, line: UInt = #line ) where Key.Value == Value { self.init( - wrappedValue: wrappedValue(), + wrappedValue: value(), persistenceKey.base, fileID: fileID, line: line @@ -137,6 +169,13 @@ extension Shared { } extension SharedReader { + /// Creates a shared reference to a read-only value using a persistence key. + /// + /// - Parameters: + /// - value: A default value that is used when no value can be returned from the persistence + /// key. + /// - persistenceKey: A persistence key associated with the shared reference. It is responsible + /// for loading the shared reference's value from some external source. public init( wrappedValue value: @autoclosure @escaping () -> Value, _ persistenceKey: some PersistenceReaderKey, @@ -172,6 +211,11 @@ extension SharedReader { ) } + /// Creates a shared reference to an optional, read-only value using a persistence key. + /// + /// - Parameters: + /// - persistenceKey: A persistence key associated with the shared reference. It is responsible + /// for loading the shared reference's value from some external source. @_disfavoredOverload public init( _ persistenceKey: some PersistenceReaderKey, @@ -181,6 +225,14 @@ extension SharedReader { self.init(wrappedValue: nil, persistenceKey, fileID: fileID, line: line) } + /// Creates a shared reference to a read-only value using a persistence key. + /// + /// If the given persistence key cannot load a value, an error is thrown. For a non-throwing + /// version of this initializer, see ``init(wrappedValue:_:fileID:line:)-7q52``. + /// + /// - Parameters: + /// - persistenceKey: A persistence key associated with the shared reference. It is responsible + /// for loading the shared reference's value from some external source. @_disfavoredOverload public init( _ persistenceKey: some PersistenceReaderKey, @@ -234,6 +286,11 @@ extension SharedReader { ) } + /// Creates a shared reference to a read-only value using a persistence key with a default value. + /// + /// - Parameters: + /// - persistenceKey: A persistence key associated with the shared reference. It is responsible + /// for loading the shared reference's value from some external source. public init( _ persistenceKey: PersistenceKeyDefault, fileID: StaticString = #fileID, @@ -247,14 +304,21 @@ extension SharedReader { ) } + /// Creates a shared reference to a value using a persistence key by overriding its default value. + /// + /// - Parameters: + /// - value: A default value that is used when no value can be returned from the persistence + /// key. + /// - persistenceKey: A persistence key associated with the shared reference. It is responsible + /// for loading the shared reference's value from some external source. public init( - wrappedValue: @autoclosure @escaping () -> Value, + wrappedValue value: @autoclosure @escaping () -> Value, _ persistenceKey: PersistenceKeyDefault, fileID: StaticString = #fileID, line: UInt = #line ) where Key.Value == Value { self.init( - wrappedValue: wrappedValue(), + wrappedValue: value(), persistenceKey.base, fileID: fileID, line: line diff --git a/Sources/ComposableArchitecture/SharedState/Shared.swift b/Sources/ComposableArchitecture/SharedState/Shared.swift index 7e6713201137..119305722da1 100644 --- a/Sources/ComposableArchitecture/SharedState/Shared.swift +++ b/Sources/ComposableArchitecture/SharedState/Shared.swift @@ -21,6 +21,10 @@ public struct Shared { self.keyPath = keyPath } + /// Wraps a value in a shared reference. + /// + /// - Parameters: + /// - value: A value to wrap. public init(_ value: Value, fileID: StaticString = #fileID, line: UInt = #line) { self.init( reference: ValueReference>( @@ -32,10 +36,27 @@ public struct Shared { ) } + /// Creates a shared reference from another shared reference. + /// + /// You don't call this initializer directly. Instead, Swift calls it for you when you use a + /// property-wrapper attribute on a binding closure parameter. + /// + /// - Parameter projectedValue: A shared reference. public init(projectedValue: Shared) { self = projectedValue } + /// Unwraps a shared reference to an optional value. + /// + /// ```swift + /// @Shared(.currentUser) var currentUser: User? + /// + /// if let sharedCurrentUser = Shared($currentUser) { + /// sharedCurrentUser // Shared + /// } + /// ``` + /// + /// - Parameter base: A shared reference to an optional value. public init?(_ base: Shared) { guard let initialValue = base.wrappedValue else { return nil } @@ -45,27 +66,31 @@ public struct Shared { ) } + /// Perform an operation on shared state with isolated access to the underlying value. + public func withLock(_ transform: @Sendable (inout Value) -> Void) { + transform(&self._wrappedValue) + } + + /// The underlying value referenced by the shared variable. + /// + /// This property provides primary access to the value's data. However, you don't access + /// `wrappedValue` directly. Instead, you use the property variable created with the ``Shared`` + /// attribute. In the following example, the shared variable `topics` returns the value of + /// `wrappedValue`: + /// + /// ```swift + /// struct State { + /// @Shared var subscriptions: [Subscription] + /// + /// var isSubscribed: Bool { + /// !subscriptions.isEmpty + /// } + /// } + /// ``` public var wrappedValue: Value { - get { - @Dependency(\.sharedChangeTracker) var changeTracker - if changeTracker != nil { - return self.snapshot ?? self.currentValue - } else { - return self.currentValue - } - } - nonmutating set { - @Dependency(\.sharedChangeTracker) var changeTracker - if changeTracker != nil { - self.snapshot = newValue - } else { - @Dependency(\.sharedChangeTrackers) var changeTrackers: Set - for changeTracker in changeTrackers { - changeTracker.track(self.reference) - } - self.currentValue = newValue - } - } + get { _wrappedValue } + @available(*, noasync, message: "Use '$shared.withLock' instead of mutating directly.") + nonmutating set { _wrappedValue = newValue } } /// A projection of the shared value that returns a shared reference. @@ -104,7 +129,19 @@ public struct Shared { } #if canImport(Combine) - // TODO: Should this be wrapped in a type we own instead of `AnyPublisher`? + /// Returns a publisher that emits events when the underlying value changes. + /// + /// Useful when a feature needs to execute logic when a shared reference is updated outside of + /// the feature itself. + /// + /// ```swift + /// case .onAppear: + /// return .run { [currentUser = state.$currentUser] send in + /// for await _ in currentUser { + /// await send(.currentUserUpdated) + /// } + /// } + /// ``` public var publisher: AnyPublisher { func open(_ reference: some Reference) -> AnyPublisher { reference.publisher @@ -115,6 +152,20 @@ public struct Shared { } #endif + /// Returns a shared reference to the resulting value of a given key path. + /// + /// You don't call this subscript directly. Instead, Swift calls it for you when you access a + /// property of the underlying value. In the following example, the property access + /// `$signUpData.topics` returns the value of invoking this subscript with `\SignUpData.topics`: + /// + /// ```swift + /// @Shared var signUpData: SignUpData + /// + /// $signUpData.topics // Shared> + /// ``` + /// + /// - Parameter keyPath: A key path to a specific resulting value. + /// - Returns: A new binding. public subscript( dynamicMember keyPath: WritableKeyPath ) -> Shared { @@ -157,6 +208,29 @@ public struct Shared { } } + fileprivate var _wrappedValue: Value { + get { + @Dependency(\.sharedChangeTracker) var changeTracker + if changeTracker != nil { + return self.snapshot ?? self.currentValue + } else { + return self.currentValue + } + } + nonmutating set { + @Dependency(\.sharedChangeTracker) var changeTracker + if changeTracker != nil { + self.snapshot = newValue + } else { + @Dependency(\.sharedChangeTrackers) var changeTrackers: Set + for changeTracker in changeTrackers { + changeTracker.track(self.reference) + } + self.currentValue = newValue + } + } + } + private var currentValue: Value { get { func open(_ reference: some Reference) -> Value { @@ -315,7 +389,7 @@ where Value: MutableCollection & RandomAccessCollection, Value.Index: Hashable { return self[position, default: DefaultSubscript(self.wrappedValue[position])] } set { - self.wrappedValue[position] = newValue.wrappedValue + self._wrappedValue[position] = newValue.wrappedValue } } } diff --git a/Sources/ComposableArchitecture/SharedState/SharedReader.swift b/Sources/ComposableArchitecture/SharedState/SharedReader.swift index ee47963315d9..c3978153eafe 100644 --- a/Sources/ComposableArchitecture/SharedState/SharedReader.swift +++ b/Sources/ComposableArchitecture/SharedState/SharedReader.swift @@ -21,10 +21,27 @@ public struct SharedReader { self.init(reference: reference, keyPath: \Value.self) } + /// Creates a read-only shared reference from another read-only shared reference. + /// + /// You don't call this initializer directly. Instead, Swift calls it for you when you use a + /// property-wrapper attribute on a binding closure parameter. + /// + /// - Parameter projectedValue: A read-only shared reference. public init(projectedValue: SharedReader) { self = projectedValue } + /// Unwraps a read-only shared reference to an optional value. + /// + /// ```swift + /// @SharedReader(.currentUser) var currentUser: User? + /// + /// if let sharedCurrentUser = SharedReader($currentUser) { + /// sharedCurrentUser // SharedReader + /// } + /// ``` + /// + /// - Parameter base: A read-only shared reference to an optional value. public init?(_ base: SharedReader) { guard let initialValue = base.wrappedValue else { return nil } @@ -34,6 +51,9 @@ public struct SharedReader { ) } + /// Creates a read-only shared reference from a shared reference. + /// + /// - Parameter base: A shared reference. public init(_ base: Shared) { self = base.reader } @@ -57,6 +77,22 @@ public struct SharedReader { Shared(value).reader } + /// The underlying value referenced by the shared variable. + /// + /// This property provides primary access to the value's data. However, you don't access + /// `wrappedValue` directly. Instead, you use the property variable created with the + /// ``SharedReader`` attribute. In the following example, the shared variable `topics` returns the + /// value of `wrappedValue`: + /// + /// ```swift + /// struct State { + /// @SharedReader var subscriptions: [Subscription] + /// + /// var isSubscribed: Bool { + /// !subscriptions.isEmpty + /// } + /// } + /// ``` public var wrappedValue: Value { func open(_ reference: some Reference) -> Value { reference.value[ @@ -66,6 +102,7 @@ public struct SharedReader { return open(self.reference) } + /// A projection of the read-only shared value that returns a shared reference. public var projectedValue: Self { get { reference.access() @@ -78,6 +115,7 @@ public struct SharedReader { } } + /// Returns a shared reference to the resulting value of a given key path. public subscript( dynamicMember keyPath: KeyPath ) -> SharedReader { @@ -94,7 +132,7 @@ public struct SharedReader { } #if canImport(Combine) - // TODO: Should this be wrapped in a type we own instead of `AnyPublisher`? + /// Returns a publisher that emits events when the underlying value changes. public var publisher: AnyPublisher { func open(_ reference: R) -> AnyPublisher { return reference.publisher diff --git a/Tests/ComposableArchitectureTests/AppStorageTests.swift b/Tests/ComposableArchitectureTests/AppStorageTests.swift index 31d39de0eed8..d4b4612c1ce9 100644 --- a/Tests/ComposableArchitectureTests/AppStorageTests.swift +++ b/Tests/ComposableArchitectureTests/AppStorageTests.swift @@ -46,7 +46,7 @@ final class AppStorageTests: XCTestCase { @Shared(.appStorage("url")) var url: URL? = URL(string: "https://pointfree.co") XCTAssertEqual(defaults.url(forKey: "url"), URL(string: "https://pointfree.co")) - url = URL(string: "https://example.com") + url = URL(string: "https://example.com")! XCTAssertEqual(url, URL(string: "https://example.com")) XCTAssertEqual(defaults.url(forKey: "url"), URL(string: "https://example.com")) } diff --git a/Tests/ComposableArchitectureTests/FileStorageTests.swift b/Tests/ComposableArchitectureTests/FileStorageTests.swift index 891f6fe0501a..0a651009d236 100644 --- a/Tests/ComposableArchitectureTests/FileStorageTests.swift +++ b/Tests/ComposableArchitectureTests/FileStorageTests.swift @@ -153,7 +153,7 @@ final class FileStorageTests: XCTestCase { } operation: { @Shared(.fileStorage(.fileURL)) var users = [User]() - users.append(.blob) + $users.withLock { $0.append(.blob) } NotificationCenter.default .post(name: willResignNotificationName, object: nil) await Task.yield() diff --git a/Tests/ComposableArchitectureTests/SharedTests.swift b/Tests/ComposableArchitectureTests/SharedTests.swift index 13c7e8a0d778..83a0875329ee 100644 --- a/Tests/ComposableArchitectureTests/SharedTests.swift +++ b/Tests/ComposableArchitectureTests/SharedTests.swift @@ -365,7 +365,7 @@ final class SharedTests: XCTestCase { case .startTimer: return .run { [count = state.$count] send in for await _ in self.queue.timer(interval: .seconds(1)) { - count.wrappedValue += 1 + count.withLock { $0 += 1 } await send(.timerTick) } } @@ -376,7 +376,7 @@ final class SharedTests: XCTestCase { .run { [count = state.$count] _ in Task { try await self.queue.sleep(for: .seconds(1)) - count.wrappedValue = 42 + count.withLock { $0 = 42 } } } ) @@ -682,7 +682,6 @@ final class SharedTests: XCTestCase { XCTAssertEqual(observations, [0]) await store.send(.incrementInReducer) { - dump($0.$count) $0.count += 1 } XCTAssertEqual(observations, [0, 1]) @@ -698,7 +697,7 @@ final class SharedTests: XCTestCase { XCTAssertEqual(isOn, true) } - func testSharedDefaults_MultipleWithDifferentDefaults() async throws { + func testSharedDefaults_MultipleWithDifferentDefaults() { @Shared(.isOn) var isOn1 @Shared(.isOn) var isOn2 = true @Shared(.appStorage("isOn")) var isOn3 = true @@ -823,7 +822,7 @@ final class SharedTests: XCTestCase { try XCTAssertThrowsError(SharedReader(.noDefaultIsOn)) } - func testSharedReaderDefaults_MultipleWithDifferentDefaults() async throws { + func testSharedReaderDefaults_MultipleWithDifferentDefaults() { @Shared(.appStorage("isOn")) var isOn = false @SharedReader(.isOn) var isOn1 @SharedReader(.isOn) var isOn2 = true @@ -983,7 +982,7 @@ private struct SharedFeature { case .longLivingEffect: return .run { [sharedCount = state.$sharedCount] _ in try await self.mainQueue.sleep(for: .seconds(1)) - sharedCount.wrappedValue += 1 + sharedCount.withLock { $0 += 1 } } case .noop: return .none @@ -1022,7 +1021,7 @@ private struct SimpleFeature { switch action { case .incrementInEffect: return .run { [count = state.$count] _ in - count.wrappedValue += 1 + count.withLock { $0 += 1 } } case .incrementInReducer: state.count += 1