Support supplying collection context elements #320
Conversation
Do not use internal functions to create channels.
| // Use test-specific unconfined if test scheduler is in use to inherit its virtual time. | ||
| @OptIn(ExperimentalCoroutinesApi::class) // UnconfinedTestDispatcher is still experimental. | ||
| val unconfined = scope.coroutineContext[TestCoroutineScheduler] | ||
| ?.let(::UnconfinedTestDispatcher) | ||
| ?: Unconfined | ||
|
|
||
| val output = Channel<T>(UNLIMITED) | ||
| val job = scope.launch(unconfined, start = UNDISPATCHED) { | ||
| val job = scope.launch(collectContext + unconfined, start = UNDISPATCHED) { |
There was a problem hiding this comment.
I had thought that overriding the dispatcher here would be a big reason folks would want to use this. I do think it would be surprising if I tried to use that parameter for that purpose and found it didn't work.
| val job = scope.launch(collectContext + unconfined, start = UNDISPATCHED) { | |
| val collectDispatcher = collectContext[CoroutineDispatcher] ?: unconfined | |
| val job = scope.launch(collectContext + collectDispatcher, start = UNDISPATCHED) { |
There was a problem hiding this comment.
We should throw if you try to do that. There's no reason to change the dispatcher on which we write into the channel, and anything other than unconfined would open up the potential for correctness problems since you make things like expectMostRecentItem() (or whatever it's called) racy.
There was a problem hiding this comment.
There's no reason to change the dispatcher on which we write into the channel
If this is so (and I think I agree, even though I hate unconfined - we don't do anything inside collect, apart from call trySend, so any dispatcher hop on emit can only be bad), then maybe the right change is this:
flowOn(collectContext).collect { output.trySend(it) }There was a problem hiding this comment.
Yes, that seems nice. Should the parameter be called flowContext? flowOnContext? Something else?
There was a problem hiding this comment.
I'm pretty sure this whole feature isn't needed. Just call flowOn!
JakeWharton
force-pushed
the
jw.more-stuff.2024-04-10
branch
from
b842689
to
e969bd3
Compare
Closes #314
CHANGELOG.md's "Unreleased" section has been updated, if applicable.