[fix]: update RxSwift bindings to observe on main thread #294
Conversation
| @@ -40,7 +40,7 @@ struct ObservableWorkflow<Value>: Workflow { | |||
| context.runSideEffect(key: "") { [observable] lifetime in | |||
| let disposable = observable | |||
| .map { AnyWorkflowAction(sendingOutput: $0) } | |||
| .subscribe(on: MainScheduler.asyncInstance) | |||
| .observe(on: MainScheduler.asyncInstance) | |||
There was a problem hiding this comment.
we could additionally leave the subscribe logic here... seems like it may not be necessary though if the 'subscription side effects' occur synchronously, since we'll be on the main thread when subscribe(onNext:) is executed.
There was a problem hiding this comment.
edit: decided to 'play it safe' and just leave the existing code too. not sure what may or may not depend on it.
There was a problem hiding this comment.
I'd vote for removing .subscribe(on: since it's just wrong but either way observe(on: is what we want here.
There was a problem hiding this comment.
yeah... maybe it is. my only argument against that is if removing it changes something that was async to sync, causing other issues. although, i suppose you can make the case that this also changes behavior in a similar way... we're taking things that used to synchronously emit in whatever context they were in to ones that are (always?) queued async on the main thread.
jamieQ
force-pushed
the
jquadri/fix-rx-observation
branch
from
5f3552d
to
73c9e4a
Compare
|
Just to link to documentation: RxSwift/Documentation/Schedulers.md (permalink). |
Resolves: #293
Checklist