chore: Migrate core Workflow module off of ReactiveSwift#392
Conversation
7a76782 to
f5021fb
Compare
| /// A publisher of the `Rendering` values produced by the root workflow in the hierarchy. Emits a new value | ||
| /// after each render pass that occurs following subscription. Use `rendering` to read the current value. | ||
| public var renderingPublisher: AnyPublisher<WorkflowType.Rendering, Never> { | ||
| renderingSubject.dropFirst().eraseToAnyPublisher() |
There was a problem hiding this comment.
🤖 renderingPublisher applying dropFirst() at the public boundary removes the atomic current-value-plus-future-updates behavior previously available from Property.producer. A render between reading rendering and subscribing becomes the subject’s current value and is then dropped. Could this return renderingSubject.eraseToAnyPublisher() and move .dropFirst() to WorkflowHostingController, which already consumes the initial rendering separately?
There was a problem hiding this comment.
This was done because previously rendering.signal produced only changed renderings so without this all existing rendering.signal callers had to include .dropFirst(), which felt like a footgun for migrations.
Because ReactiveSwift's Property provides both options, we have to pick which behavior we adopt here:
Property.signal: Only emits updatesProperty.producer: Emits current value + updates
There was a problem hiding this comment.
Keeping the producer semantics and letting callers use .dropFirst() feels more idiomatic (for example, matches the behavior of @Published properties), so I'll go with that.
There was a problem hiding this comment.
Thanks Eric! Matching the idiomatic @Published behavior feels like the best approach to me, too.
johnnewman-square
left a comment
There was a problem hiding this comment.
This looks good on my end. My agent just had a few suggestions.
f5021fb to
abb8c90
Compare
abb8c90 to
2cae038
Compare
johnnewman-square
left a comment
There was a problem hiding this comment.
Thanks for making those tweaks.
Bump Workflow to 6.0 and address the ReactiveSwift removal from square/workflow-swift#392
Changes
Remove
ReactiveSwiftusage from the coreWorkflowandWorkflowUImodules.The only usages there were for emitting outputs from WorkflowHosts to external consumers. The internal implementation of Workflow itself did not use
ReactiveSwiftat all.WorkflowHostAPI changes:WorkflowHostingControllerAPI changes:Checklist