cashapp / molecule

Build a StateFlow stream using Jetpack Compose
https://cashapp.github.io/molecule/docs/1.x/
Apache License 2.0
1.78k stars 76 forks source link

Compose stops recomposing in tests #326

Open vRallev opened 8 months ago

vRallev commented 8 months ago

In a weird combination of StateFlow, backgroundScope, UnconfinedTestDispatcher, collectAsState() and the Immediate recomposition mode the Compose runtime stops recomposing and Flows produced by Molecule stop sending events.

This test passes:

    @Test
    fun `something breaks`() = runTest {
        val _strings = MutableStateFlow("one")
        val strings: Flow<String> = _strings

        val molecule = (backgroundScope + UnconfinedTestDispatcher(testScheduler)).launchMolecule(RecompositionMode.Immediate) {
            val string by strings.collectAsState("one")
            string
        }

        molecule.test {
            assertThat(awaitItem()).isEqualTo("one")

            _strings.value = "two"
            assertThat(awaitItem()).isEqualTo("two")

            _strings.value = "three"
            assertThat(awaitItem()).isEqualTo("three")
        }
    }

If I change the initial value to something else such as collectAsState("abc"), then only "one" from the StateFlow is emitted but no other value anymore even though the StateFlow value changes. The composable function only recomposes when the initial value of the StateFlow and the collectAsState function are equal. This seems to be the bug.

Changing the flow to a MutableSharedFlow fixes the issue, because there’s no default value. Removing the UnconfinedTestDispatcher also fixes the issue. In this case it emits the initial value from collectAsState first, then the default value from the StateFlow and then all other changes. Using the same initial values also resolves the bug (the problem here is that StateFlow is an implementation detail of the underlying API, this example is simplified). It only breaks when the initial values are different.

jingibus commented 8 months ago

This checks out.

We're discussing this with the compose folks. No clue what's going on yet!

jingibus commented 6 months ago

Another note to drop here:

Our current suspicion is that this is caused by UnconfinedTestDispatcher. Here's what we suspect is happening:

  1. GatedFrameClock's withFrameNanos implementation is called from Compose
  2. This sends a Unit to an internal conflated Channel, which acts as a trampoline for invoking sendFrame()
  3. Due to UnconfinedTestDispatcher's immediate dispatch behavior, instead of a waiting for dispatch to execute the trampolined sendFrame(), sendFrame() executes immediately
  4. This forms a reentrant call within Compose, which borks Compose's state and makes it fail to pick up the new frame

So the workaround for this is just to not use UnconfinedTestDispatcher. (This is also my personal recommendation)

jingibus commented 3 months ago

h/u @vRallev, we just got a fix from upstream on #396 which might have the same root cause. I have not retested, but this issue might be fixed now.

vRallev commented 3 months ago

Am I under the right assumption that the fix needs to trickle into the Jetbrains Compose runtime before I can verify the fix?

|    |    |         +--- app.cash.molecule:molecule-runtime:1.4.1
|    |    |         |    \--- app.cash.molecule:molecule-runtime-jvm:1.4.1
|    |    |         |         +--- org.jetbrains.compose.runtime:runtime:1.6.0
JakeWharton commented 3 months ago

Only if you are experiencing it on targets other than Android. If you are only using it on Android, an explicit dependency on the newer version is all you need.

vRallev commented 3 months ago

Yes, JVM unit tests mainly. We haven't seen an issue on Android.

JakeWharton commented 3 months ago

Yes, alas, we must wait. It's not even merged into their builds yet.