Colton Idle
01/13/2023, 3:52 PMviewModelScope
seem bad? Like isn't that something that should be injected? But then again, like i'm not smart enough to know what the alternative would be lolCasey Brooks
01/13/2023, 4:02 PMviewModelScope
. The whole idea is that you only want stuff running as long as the ViewModel is active, but the Android framework is what determines how long a ViewModel remains active. If you’re injecting the scope yourself, then you’d responsible for determining when to cancel those coroutineScopes, which you probably don’t want to do. Remember that a ViewModel typically lives longer than the Activity/Fragment it’s associated with, and it’s difficult to properly scope the lifetime of a ViewModel to the Android system lifecycle, which is why it’s best to just let the Android team figure out those details
For things that are intended to live longer than a ViewModel (for example, Singletons, Repositories, etc.) those probably should be managed on your own CoroutineScope where you control their lifetime, rather than using an Android ViewModel and its viewModelScope
kevin.cianfarini
01/13/2023, 4:05 PMviewmodelScope
has a hard dependency on Dispatchers.Main.immediate
. If you use viewModelScope
it will require you to use the horrible Dispatchers.setMain
and Dispatchers.resetMain
APIs.
Recently the arch libs allow for a vararg Closeable
to be injected into their constructor so you should be injecting a CoroutineScope
that can be closed to your viewmodels and using that. Then in your tests you can control what scope is passed in and avoid needing to use viewModelScope
as well as the Dispatchers.setPain
apis.
In an upcoming release the viewModelScope
extension is actually going to be annotated with the @Discouraged
annotation for this very reason.Casey Brooks
01/13/2023, 4:12 PMCoroutineScope
with Closable
in this case, as you still do not want to be trying to manage that lifecycle yourself.Colton Idle
01/13/2023, 4:15 PMkevin.cianfarini
01/13/2023, 4:16 PMkevin.cianfarini
01/13/2023, 4:19 PMclass CloseableCoroutineScope(override val coroutineContext) : CoroutineScope, Closeable {
override fun close() = cancel()
}
class MyFancyViewModel @Inject constructor(scope: CloseableCoroutineScope) : ViewModel(scope)
// In Dagger
@Provides fun providesCloseableCoroutineScope() = CloseableCoroutineScope(SupervisorJob())
kevin.cianfarini
01/13/2023, 4:19 PMColton Idle
01/13/2023, 4:21 PMIf you useyou meanit will require you to use the horribleviewModelScope
andDispatchers.setMain
APIs.Dispatchers.resetMain
If you useRight?it will require you to use the horribleviewModelScope
andDispatchers.setMain
APIs while writing tests.Dispatchers.resetMain
kevin.cianfarini
01/13/2023, 4:21 PMColton Idle
01/13/2023, 4:21 PMColton Idle
01/13/2023, 4:21 PMkevin.cianfarini
01/13/2023, 4:21 PMkevin.cianfarini
01/13/2023, 4:22 PMColton Idle
01/13/2023, 4:23 PMAuthored by Kevin Cianfarini
kevin.cianfarini
01/13/2023, 4:24 PMColton Idle
01/13/2023, 4:26 PMkevin.cianfarini
01/13/2023, 4:27 PMviewModelScope
kevin.cianfarini
01/13/2023, 4:27 PMColton Idle
01/13/2023, 4:28 PMRobert Williams
01/13/2023, 4:42 PMRobert Williams
01/13/2023, 4:43 PMRobert Williams
01/13/2023, 4:43 PMkevin.cianfarini
01/13/2023, 6:00 PMclass ViewModelThing @Inject constructor(
parentScope: CloseableCoroutineScope,
) : ViewModel(CloseableCoroutineScope(SupervisorJob() + scope)) { ... }
Francesc
01/13/2023, 6:23 PMviewmodelScope
with a closeable scope, you could have a look for reference
https://github.com/fvilarino/Weather-Sample/blob/feature/compose/presentation/shar[…]a/com/francescsoftware/weathersample/shared/mvi/MviViewModel.ktFrancesc
01/13/2023, 6:23 PMK Merle
01/14/2023, 2:39 AMviewModelScope
, just with some different configuration, so we could have just a bit more ownership of it. If you want to have ownership of your CoroutineScope
, you can have it already.Francesc
01/14/2023, 3:53 AMviewmodelScope
is the implicit dependency on the main dispatcherK Merle
01/14/2023, 4:13 AMK Merle
01/14/2023, 4:17 AMFrancesc
01/14/2023, 4:43 AMkevin.cianfarini
01/14/2023, 2:08 PMkevin.cianfarini
01/14/2023, 2:11 PMK Merle
01/14/2023, 4:35 PMviewModelScope.launch(myDispatcher)
?kevin.cianfarini
01/14/2023, 4:47 PMkevin.cianfarini
01/14/2023, 4:49 PMK Merle
01/14/2023, 4:54 PMthe in case of runTest you wouldn't be able to pass in a scope that can control virtual time (thus making delay and debounce calls untestable). This could be solved by passing in the test scheduler provided by a test scope, but it seems excessive.I've never tried it, but I'd expect for one of
TestDispatchers
to give you more control over calls in tests.
Also good luck enforcing that every launch, async, flow, etc etc all run in a provided context/dispatcher so you can testWell, I'm not saying it's perfect, I'm just saying that you can still have benefits of
viewModelScope
and inject Dispatcher
over constructor for tests. I thought this served me well in the past.kevin.cianfarini
01/14/2023, 4:59 PMEmptyCoroutineContext
for dispatchers under testkevin.cianfarini
01/14/2023, 5:00 PMassertFailsWith
a lot and if viewmodelScope is dropping those errors on the ground because it has no parent scope to report to then you've got a problem.Patrick Steiger
01/15/2023, 2:09 PMclass ViewModelThing @Inject constructor(
scope: CloseableCoroutineScope,
) : ViewModel(scope) { ... }
you can also just
class ViewModelThing @Inject constructor(
scope: CoroutineScope,
) : ViewModel(scope.asCloseable()) { ... }
fun CoroutineScope.asCloseable() = Closeable { cancel() }
if you don’t want to create a custom CloseableCoroutineScope
interface/class and provide it in your DI graphkevin.cianfarini
01/15/2023, 11:08 PMMarko Novakovic
02/17/2023, 11:34 AMYou should be doing both of those things and injectingyou mean bothfor dispatchers under test (edited)EmptyCoroutineContext
CloseableCoroutineScope
should be passed to ViewModel
and custom dispatchers should be injected to do proper testing?