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

Rule feature request: Using derivedStateOf without any State object in calculation #88

Open
svenjacobs opened this issue Sep 30, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@svenjacobs
Copy link

Currently there is no (lint) warning when using derivedStateOf without accessing any State object in its calculation, which can lead to unexpected behaviour. Since no State object is read, the derived value will never update which however might not be obvious just by looking at the code.

Let's take the following Composable for example:

@Composable
fun Amount(
    amount: Int,
    modifier: Modifier = Modifier,
) {
    val isZeroAmount by remember { derivedStateOf { amount == 0 } }

    Box(modifier = modifier) {
        if (isZeroAmount) {
            Text("Amount: Is zero")
        } else {
            Text("Amount: $amount")
        }
    }
}

If amount is 1 initially and becomes 0 on the next recomposition, the Composable will incorrectly display Amount: 0 instead of Amount: Is zero.

This might for instance happen when a State variable with by delegation is changed to a plain type.
The correct code for this case would be:

@Composable
fun Amount(
    amount: State<Int>,
    modifier: Modifier = Modifier,
) {
    val isZeroAmount by remember { derivedStateOf { amount.value == 0 } }

    Box(modifier = modifier) {
        if (isZeroAmount) {
            Text("Amount: Is zero")
        } else {
            Text("Amount: ${amount.value}")
        }
    }
}
@mrmans0n mrmans0n added the enhancement New feature or request label Sep 30, 2022
@mrmans0n
Copy link
Contributor

mrmans0n commented Sep 30, 2022

Interesting idea!

Doing this w/o type resolution might prove hard but I think it might be doable with some heuristics (in a best-effort way at least)

@svenjacobs
Copy link
Author

I assumed that this will be a tricky one without type resolution in the context of the rule's code. Also just ensuring that any read inside the calculation must be done via state.value is not sufficient when by delegation is used.

@chrisbanes
Copy link
Contributor

I like it! It's going to be hard to determine whether something is backed by state though. There's the obvious case of having a State object in the same function, but the state could be anywhere in the call stack, and possible hidden through lambdas or other objects.

I think a better way is for derivedStateOf to throw an exception, when it detects that nothing state backed has been read.

@svenjacobs
Copy link
Author

svenjacobs commented Sep 30, 2022

I like the idea of derivedStateOf throwing an exception. It even becomes trickier when a State is read inside the calculation which however is not part of the resulting value. Then the derived state only updates when the unrelated state is updated, which might be even more confusing.

val isZeroAmount by remember { 
    derivedStateOf {
        log("Product type is: ${productType.value}") // productType is State<String> for example
        amount == 0
    }
}

I think only the Compose compiler or runtime can detect those cases for certain.

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

No branches or pull requests

3 participants