Hi <@UHAJKUSTU>, for the `labelsChannel()` , I hav...

# mvikotlin

[CXwudi]

Hi @Arkadii Ivanov, for the labelsChannel() , I have some improvement idea in the comment section

[CXwudi]

I was experiencing a problem from using labelsChannel() from runTest unit test. If I pass the TestScope instance from runTest into label channel like:

runTest(testDispatcher) {
    val labelsChannel = landingPageStore.labelsChannel(this)
    ...
}

Then the test will be stuck and never finish.

If I do:

runTest(testDispatcher) {
    val separateScope = TestScope(testDispatcher) // separate scope for label channel
    val labelsChannel = landingPageStore.labelsChannel(separateScope)
    ....
    separateScope.cancel()
}

Then the test works.

[CXwudi]

I soon realize that the labelsChannel() is creating a coroutine that simply just waiting for cancellation and then call other shutdown hooks.

However, the this TestScope from runTest are not going to be cancelled until all children coroutines are finished. Manually call this.cancel() in runTest will also be traded as a test failure. Hence, this creates a deadlock unless a separate coroutine scope is used for the labelsChannel()

[CXwudi]

So with all being said, my suggestion is that:
Can we have another extension function that doesn't rely on coroutine scope cancellation for calling the shutdown hooks? Instead, it relies on the shutdown hooks from the store itself. Just like how toFlow method is doing:

@ExperimentalMviKotlinApi
fun <Label : Any> Store<*, *, Label>.labelsChannel(
  capacity: Int = Channel.BUFFERED,
): ReceiveChannel<Label> {
  val channel = Channel<Label>(capacity = capacity)
  val disposable = labels(
    observer(
      onComplete = { channel.close() },
      onNext = channel::trySend
    )
  )
  channel.invokeOnClose { disposable.dispose() }
  return channel
}

[CXwudi]

Another way of improvement could be:

@ExperimentalMviKotlinApi
fun <Label : Any> Store<*, *, Label>.labelsChannel(
  sendScope: CoroutineScope,
  capacity: Int = Channel.BUFFERED,
): ReceiveChannel<Label> {
  val channel = Channel<Label>(capacity = capacity)
  val disposable = labels(observer(
    onComplete = {
      channel.close()
    },
    onNext = {
      if (sendScope.isActive) {
        sendScope.launch {
          channel.send(it)
        }
      }
    }
  ))
  channel.invokeOnClose { disposable.dispose() }
  return channel
}

This one simply use channel.send instead of channel.trySend, and then a coroutine is required for this send method.

[Arkadii Ivanov]

I think none of the provided solutions would work. The only reason why the scope is required is because the Store can live longer than the channel. This happens when the Store is retained (e.g. placed in InstanceKeeper), but the channel is created in a Component/Fragment/etc. This essentially equivalent to Flow.shareIn.

On the other hand, you probably shouldn't pass the scope from runTest. Usually a TestDispatcher or Dispatchers.Unconfined is used for testing.

[CXwudi]

I see, that's right, the Store can live longer than the Component (via InstanceKeeper), then it makes sense to rely on the coroutine scope lifecycle to call the dispose method.

I will stick with using a separate dispatcher when testing, like what you suggested

[CXwudi]

As always, thanks for your clarification and maintaining such a great framework. Really enjoyed every aspect of it