A modern programming language that makes developers happier.

kotlinlang

Hey folks!
I’m seeing some weird behaviors on `Modifier.composed` + `Modifier.then` and I’m not totally sure why. I’ll explain more in :thread:

I currently have these modifiers:
```fun Modifier.impression(
    id: String,
): Modifier = composed {
    Logger.v(tag = "ImpressionTracking") { "Creating modifier($id)" }
    LaunchedEffect(key1 = id) {
        Logger.v(tag = "ImpressionTracking") { "Launching effect($id)" }
    }
    this
}

fun Modifier.placeholderBackground(): Modifier =
    this then background(Color(0xFF272727))```
Example usage (my case is actually more complex but I think this is the minimum case):
```Box (
    modifier = Modifier
        .impression("1")
        .placeholderBackground()
)```
That `LaunchedEffect` gets launched multiple times on recomposition. If I remove `then` from `placeholderBackground` :
```fun Modifier.placeholderBackground(): Modifier =
    this.background(Color(0xFF272727))```
Then it only gets launched once.

It seems `composed` is not keeping state. I found this issue <https://issuetracker.google.com/issues/270963071>, but my case is with `then` on other modifiers. In the end it can possibly be the same issue. But this seems like a footgun that is not obvious at all.

Now I don’t know much about the implementation details, but it seems with `then` it duplicates the modifiers that were before into two different paths and ends up materialising `composed` twice, If I have more modifiers with `then` , it creates more paths and more `composed` materialisations, one “state” for each possible path on the tree.

And why does removing `then` solve it, when predefined modifiers also use it?

No its correct. You seem to have misunderstood the APIs.

To fix: Change to
```fun Modifier.placeholderBackground(): Modifier =
  this then Modifier.background(...)```
OR just
```fun Modifier.placeholderBackground(): Modifier =
  this.background(...)```
It should be clear what the issue now is but if not:

The modifier extensions are *shortcuts* for chaining the `this`  modifier with some _other modifier_ so your old code actually did
```this then (this.background(...))```
which is basically
```this then (this.then(BackgroundModifier(...)))```

_So why does_ 
```this then Modifier.background(...)```
_work? …_

Look at the source of `Modifier` ’s companion object is which is the empty `Modifier` , you can see that its `then` method, short circuits to the _other modifier_

Ah, that makes much sense. It seems I was lacking some fundamental knowledge about this :smile:
But it seems like an easy mistake to do though

Yeah I admit its easy to fall for because its an other wise simple language feature. Even accompanist had a <https://github.com/google/accompanist/issues/1525|bug> related to this at some point.

I managed to find that, thats how I found the issue that I linked before. But wording made it seem it only applied to `composed` , which now I see is not the case.

I guess a lint check could really be useful here

Yeah. Pls star the issue to add a lint check for this <https://issuetracker.google.com/issues/270963071>