Make update a protocol#41
Conversation
...and receive actions on main, only updating state on main.
Update becomes a concrete implementation of UpdateProtocol. This enables ObservableStore models to describe custom update structs that contain additioanl information.
This prevents name ambiguity with Update<Model>, which was tripping up Swift's type inference
| { | ||
| /// Stores cancellables by ID | ||
| private(set) var cancellables: [UUID: AnyCancellable] = [:] | ||
| private var cancelTransactions: AnyCancellable? |
There was a problem hiding this comment.
This doesn't seem to be used anywhere anymore?
There was a problem hiding this comment.
Combine subscribers automatically cancel when their cancellable is released. So you have to hold on to the cancellable to keep the subscriber alive. We hold on to the cancellable within the instance so that its lifetime matches the store lifetime. The subscriber will continue receiving as long as the store instance exists.
There was a problem hiding this comment.
Update: after talking through, there was a confusion. The OP is in response to the fact that cancelTransactions is an additional cancellable property that doesn't need to be there. Followed up with fix in #43.
| private var fx: AnyPublisher<Model.Action, Never> { | ||
| _fxBatches | ||
| .flatMap({ publisher in publisher }) | ||
| .receive(on: DispatchQueue.main) | ||
| .eraseToAnyPublisher() | ||
| } |
| let package = Package( | ||
| name: "ObservableStore", | ||
| platforms: [.macOS(.v10_15), .iOS(.v15)], | ||
| platforms: [.macOS(.v10_15), .iOS(.v16)], |
There was a problem hiding this comment.
Is this a required bump? I haven't got a problem with raising the target to iOS 16 just curious if a particular API motivated this change.
There was a problem hiding this comment.
Good catch. I thought it would be required for some of the changes I would make, but it turned out it was not needed. Backed it out.
bfollington
left a comment
bfollington
left a comment
There was a problem hiding this comment.
LGTM, but I would like to see that unused cancellable removed before merge (if it is really unused)
This reverts commit c85e83d.
2 participants
Model.updateto return some type that conforms toUpdateProtocol.Update<Model>becomes a concrete implementation ofUpdateProtocolStore.updatespublisher. Updates are guaranteed to publish after state change.This is a non-breaking change for existing consumers of ObservableStore. The motivating reason for the change is that we wished to use ObservableStore to power complex UIViewControllers in Subconscious, where a diffing-based approach was not practicable. Allowing for the customization of
Updatelets individual implementations of Store provide additional metadata to controllers.While working on this PR, I also discovered a simpler way to implement effects that does not require us to dynamically manage cancellables, by using
flatMapto consume batches of fx.