I'm seeing a timing issue with staticCompositionLo...
# compose
a
I'm seeing a timing issue with staticCompositionLocal, if I update the value of my local 2 times quickly, not all places that refer to the local always get updated. I have been trying to produce a demo project but I haven't been able to hit the problem in isolation yet, the place I've seen that doesn't update has a lazy list with a bunch of images loading in it so its the slowest thing to recompose. What I found interesting is if I use a non-static compositionLocal then it works fine. The local i'm defining is used to define window size class/foldable information which gets updated 2 times after rotation / activity recreate due to how the androidx.window async API works on Samsung devices. (I mentioned this here a couple weeks ago thinking that just referring to state wasn't refreshing, but it seems to be actually the static composition local not the state). There was other strange behavior I saw falling out of this, if I have a state value that is updated after a flow collect inside LaunchedEffect, when the bug happens one of the places on screen does not have the launchedeffect value. All fallout of not everything being updated..
z
Hm this sounds tricky if you can't reproduce reliably. But cc @Chuck Jazdzewski [G] in case it rings a bell for him.
c
It doesn't sound familiar. Please file a bug and a repro steps would be appreciated. I highly recommend you use dynamic composition locals for cases like this but a static composition local should work too .
a
I will switch it to a non-static for now, it seemed like static is what we would want if its supposed to have better performance in situations where updating the value should recompose everything and you don't need the state tracking. We use this local in a few places at the top of the tree (topappbar, main content, etc, to decide if we're in a 2 pane or single pane layout), but yeah in reality its only read from a handful of additional places. I'll try a little more to see if I can get a standalone test app to more closely match the case I discovered the bug in (which is also demo/testbed app)
z
it seemed like static is what we would want if its supposed to have better performance in situations where updating the value should recompose everything
Not really. Dynamic locals will also cause everything that reads them to recompose. Static locals are meant for times when it's expected that the value will never change (eg values that are by nature tied to the lifetime of the composition itself, such as the hosting View), because when it does change it recomposes literally everything underneath it. In your case, where it's conceivable that the layout mode could change to react to config changes, and especially since you're only reading the value from a few places, it really doesn't make sense to use static because the small overhead of doing a little state tracking is probably way cheaper than recomposing basically every function in your app when there's a change that the vast majority of them don't care about.
a
Putting the potential bug aside, I would recommend avoiding composition locals in general for a case like this in favor of explicitly passing down the value. > We use this local in a few places at the top of the tree (topappbar, main content, etc) If you’re only using it in a few places near the root of the tree, it might be much clearer to just pass down the state normally. It’s easier to test/preview just by passing different values
a
Right, in our demo app we only query foldable properties, window size class, dimension type things in a few places. Our real app looks at these for a few more things like types of FAB options and visibility of some buttons. I had been treating it the same as LocalContext, LocalConfiguration, as its a global thing which usually doesn't change, if it does at least in our app we recreate on configuration changes (but I suppose the fold open/partially open state is the one thing that isn't static). Another comparison was thinking that its kind of like Accompanist's LocalWindowInsets, its a similar number of places we're using that too