-
-
Notifications
You must be signed in to change notification settings - Fork 389
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
Add observe extensions for Flow to reduce nesting and boilerplates #4338
base: develop
Are you sure you want to change the base?
Conversation
This depends on #4337. |
93c013c
to
b83783e
Compare
@@ -51,6 +51,7 @@ android { | |||
|
|||
kotlinOptions { | |||
freeCompilerArgs = [ | |||
"-Xcontext-receivers", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For using context receivers feature, see https://github.com/Kotlin/KEEP/blob/master/proposals/context-receivers.md
context(LifecycleOwner) | ||
fun <T> Flow<T>.observe( | ||
collector: FlowCollector<T>? = null | ||
): Job = observe(this@LifecycleOwner, collector) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can use Fragment
's viewLifecycleOwner
instead of itself for all invocations in Fragments? This means we can unify
viewModel.uiEvents.observe(viewLifecycleOwner) {
// Some logic.
}
and
viewModel.uiEvents.observe {
// Some logic.
}
to the same in Fragments, and sync with root Views' lifecycles.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finished in 38bdb76, feel free to review.
That is nice, but also quite a big change. What do you think @charlag? |
Be careful of subtle behavior changes: most of the current code doesn't use |
I think a balance needs to be found between clarity, flexibility and reducing nesting. I think having two levels of nesting is already fine:
A utility function
Then to collect repeatedly, also two levels of nesting:
Note that you can also collect multiple things inside a lifecycle repetition by launching child coroutines. This is also more efficient than launching multiple instances of
In that case it would indeed create 3 levels of nesting. But the collecting code could also be moved to separate suspending functions to improve readability. |
# Conflicts: # app/src/main/java/com/keylesspalace/tusky/components/compose/ComposeViewModel.kt
That is what I'm considering too, remove it for now. |
No description provided.