Hey guys, I stumbled on this piece of <documentat...

# compose

[Radoslaw Juszczyk]

Hey guys,

I stumbled on this piece of documentation:

An example of a bad practice is creating a CompositionLocal that holds the ViewModel of a particular screen so that all composables in that screen can get a reference to the ViewModel to perform some logic. This is a bad practice because not all composables below a particular UI tree need to know about a ViewModel.

and am wondering how to interpret it.

If you take lets say Coils LocalImageLoader it will turn out to be a bad practice too as not all composables below a particular UI tree need to know about the ImageLoader. Then if you think about it there would be no good useage of a CompositionLocal

Then for the part :

Avoid CompositionLocal for concepts that aren't thought as tree-scoped or sub-hierarchy scoped.

Lets have a comopsable XyzScreen which utilizes XyzViewModel and defines local composition of XyzViewModel which is used inside XyzScreen composables - isnt it a tree-scoped or sub-hierarchy scoped concept and therefore a good candidate to use CompositionLocal?

[Stylianos Gakis]

The ViewModel would outlive the composition tree though, maybe this is part of the reason why it’s a bad idea. And the even bigger reason why it’s a bad idea to rely on this is that then you suddenly make these composables impossible to use in any other place, and completely kill their reusability. Even worse that you do so in a hidden way, meaning that other composables outside of that “tree” will be perfectly able to call it, but you’ll get a runtime crash. This is a lose-lose situation. At least if you’re gonna do that, put it as part of the normal function arguments so you make this link explicit and impossible to miss.

Also as far as Coil goes, yeah I don’t use the CompositonLocal either, just pass the ImageLoader manually to the children that need it, so in a way yeah there’s no need for the composition local there either, it’s just a convenience since you’re gonna have one global ImageLoader most likely anyway and you’d want to provide it top level so that you can access the exact same one anywhere. Not killing off any reusability in your production code at least.
It does however make Previews harder, since you don’t have the composition local hooked up there. This was what pushed me to simply avoid that entire dependency, and get only the core Coil dependency which doesn’t include this global ImageLoader at all. Then in previews, if your composable needs an ImageLoader you just pass a fake one and you’re good.

[Radoslaw Juszczyk]

Yeah, I understand why it is a bad idea to use it for the ViewModel as this composables would not be reusable and the dependency is not explicit.

But in the docs they use the ViewModel as an example of a concept which is not tree scoped while it seems to me it is.

Then when you read: it is a bad practice because not all composables below a particular tree need to know about the ViewModel

What would be a good example of using composition local? It seems it will always happen that not all composables will be concerned about the provided item.

[Stylianos Gakis]

Oh in fact, Coil has even deprecated it it seems https://github.com/coil-kt/coil/blob/e311caf5118d14c881b6bdf09f03d500c5811ab4/coil[…]ompose-singleton/src/main/java/coil/compose/LocalImageLoader.kt

[Radoslaw Juszczyk]

Yes but due to different reasons

[Stylianos Gakis]

In fact, some people here would definitely agree with your point that “What would be a good example of using composition local? It seems it will always happen that not all composables will be concerned about the provided item.“. There are hardly any good reasons to go for it. In a way you could think of using compositon locals as something to avoid at all costs, unless it’s super inconvenient to do so. I think the existing ones, like the global android related things like context, density etc. and other things that’d be incredibly frustrating to be passing around all the time, like Material3 things are the ones where I feel are justified. Other than that, you can simply never use CompositionLocals. That’s what I’d advocate for at least.

[Radoslaw Juszczyk]

True, tbh I would prefer to pass the ImageLoader explicitly too, but the problem is that if you do not pass it the AsyncImage wont complain and will simply resort to use LocalImageLoader.current . So just to avoid wrong ImageLoader being used we provide it on the most top level. For the other use cases I agree about discouraging it unless it is really frustrating...

[Stylianos Gakis]

Oh if that’s your issue then it’s a solved problem. There’s coil-compose-singleton, but there’s also coil-compose-base which does not have that function at all. So it’s impossible to call the wrong one.
That’s what I meant above when I said “This was what pushed me to simply avoid that entire dependency”, you can depend on the base artifact only
These two here is what I mean I use now.

[Radoslaw Juszczyk]

That is sweet! Ups I did not understand that entire dependency because I was not aware there were two. Thank you for bringing it up 🙏

[Stylianos Gakis]

Yes, I realize I wasn't clear at all there. Well you have your solution now, go on and enjoy a life free of CompositionLocals and other such evils 😅

[dewildte]

In real life, Air is a CompositionLocal.
Arbitrary things, at arbitrary positions in the existence tree, need access to it to operate.

[Stylianos Gakis]

Dammit... there goes my plan