-
Notifications
You must be signed in to change notification settings - Fork 4.2k
fix(#2553) : AsyncSequence.asObservable() runs on background thread #2662
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
b0e5e21
17e8102
1415a54
ef95a2a
83ab029
bf5a92c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -60,7 +60,7 @@ public extension AsyncSequence { | |||
/// - returns: An `Observable` of the async sequence's type | ||||
func asObservable() -> Observable<Element> { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about adding a new param to decide whether to use Task or Task.detached? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think adding a parameter is a good idea to provide flexibility. I'm considering implementing it like this: func asObservable(detached: Bool = true) -> Observable<Element> {
// Use Task.detached or Task based on the parameter
} With true as the default value, it would still solve the original issue by running on a background thread, while giving users the option to use regular Task when they want to maintain the current context's priority. What do you think about this approach and the default value being true? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It sounds good. I’ve also seen a similar implementation in another library, which takes the same approach
About the default value of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've implemented the parameter as suggested, with false as the default value to maintain backward compatibility. func asObservable(detached: Bool = false) -> Observable<Element> {
// Implementation uses Task.detached when detached=true, otherwise uses Task
} All tests are passing. Thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice 🙏 |
||||
Observable.create { observer in | ||||
let task = Task { | ||||
let task = Task.detached { | ||||
do { | ||||
for try await value in self { | ||||
observer.onNext(value) | ||||
|
Uh oh!
There was an error while loading. Please reload this page.