https://kotlinlang.org logo
Join Slack
Powered by
A question: shouldn't the `State` interface have `...
# compose
a
A question: shouldn't the 
State
interface have 
out T
and not just 
T
? Like 
State<out T> { ... }
👍 4
👍🏼 1
Turned out there is already a ticket for it: https://issuetracker.google.com/issues/184760940
l
@Adam Powell Could this very easy ticket, reported almost 2 months ago, be prioritized or handled straightaway, please? 🙏🏼 I'm "pinging" you because I don't think we can send PRs for Compose runtime yet (only compiler AFAIK).
a
Perhaps we can send PRs to AOSP? But it will require more efforts with env setup. I have plans to check it in the near future.
z
We can definitely send PRs to compose runtime, i submitted one almost a year ago
env setup is definitely a bit involved the first time though, but it’s well-documented
👍 1
a
don't worry, I saw the OP here. 🙂
🤔 1
while I'm supportive of this for general correctness, wanting it for practical use cases makes me a bit nervous. Generally one should avoid passing 
State<T>
objects as a parameter to things, which is where this starts to become handy
a
Thank you!
👍 1
I needed it for testing purposes, just in case)
a
ah, now CI is reminding me why we didn't do this before; it messes with the 
@BuilderInference
of 
produceState
and friends
l
Why isn't it messing with 
buildList
from stlib but messes with 
produceState
?
a
produceState
accepts an initial value and if you use a sealed class like with the common 
Result<T>
type patterns it ends up inferring the more specific type from that.
Copy code
e: $SUPPORT/compose/integration-tests/docs-snippets/src/main/java/androidx/compose/integration/docs/sideeffects/SideEffects.kt: (220, 40): Type mismatch: inferred type is Result.Error but Result.Loading was expected
e: $SUPPORT/compose/integration-tests/docs-snippets/src/main/java/androidx/compose/integration/docs/sideeffects/SideEffects.kt: (222, 20): Type mismatch: inferred type is Result.Success<Image> but Result.Loading was expected
@Andrey Kulikov I think you and I both messed around with this a while back
l
I'm wondering is there's a resolution possible for that blob thinking fast If there's none, I think it'd be good to have a code or KDoc comment near the declaration that states (no pun intended) why there's no 
out
variance. Otherwise, I think it'll keep coming up on the long term.
a
I'm inclined to think we should do it anyway, it's just kind of unfortunate for this usage. Mitigation is just to specify the generic type of the call explicitly either with 
produceState<Result<Image>>
or upcast the initial value 
produceState(initialValue = Result.Loading as Result<Image>,
a
There type inference contracts available, maybe something could help here?
a
got a link?
a
Let me check
Nope, I was wrong, not for this particular case.
The 
@BuilderInference
should ideally work here, but looks like it works only for methods, not for properties. If 
ProduceStateScope
had 
fun setValue
, then using this method would help with the inference. But since we are using 
value = x
, then it does not work, and so the annotation looks even useless.
Maybe it could be solved on the Kotlin level.
z
I thought you could also fix that problem by using two type params, one as the upper bound of the other, but that kind of makes the case where you want to pass explicit type arguments super gross (assuming it even solves the problem at all)
a
tried that, it doesn't
👍🏻 1
and yes it makes the explicit case extra gross
at the moment I think I'm in camp, "type inference can be improved but the API is going to be around much longer"
➕ 2
a
Personally I don't see any issues with supplying the type manually. The 
State<out T>
API looks more correct and more flexible, the change is binary compatible, but is not source compatible. So some compilations will be broken, but easy to fix.
➕ 1
a
aaaand merged. Should be in next week's build.
🙏 2
👌 1
🎉 4
🙏🏻 1
l
Awesome, thank you! 😊
4 Views
Open in Slack
PreviousNext
Powered by