Skip to content
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

Compose stops recomposing in tests #326

Open
vRallev opened this issue Oct 27, 2023 · 7 comments
Open

Compose stops recomposing in tests #326

vRallev opened this issue Oct 27, 2023 · 7 comments

Comments

@vRallev
Copy link

vRallev commented Oct 27, 2023

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
Copy link
Contributor

jingibus commented Oct 30, 2023

This checks out.

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

@jingibus
Copy link
Contributor

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
Copy link
Contributor

jingibus commented Apr 4, 2024

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
Copy link
Author

vRallev commented Apr 8, 2024

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
Copy link
Collaborator

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
Copy link
Author

vRallev commented Apr 8, 2024

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

@JakeWharton
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants