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

Fix recompose whole app after change languageTag #9

Conversation

phucynwa
Copy link
Contributor

I used Lyricist in my project and got an issue.

By using the code generated by KSP, after change languageTag by below way, whole app recomposed and lost current state. That is not what I want.

    val lyricist = rememberStrings()
    ProvideStrings(lyricist) {
        AppContent()
    }
    LaunchedEffect(state.languageCode) {
        if (lyricist.languageTag != state.languageCode) {
            lyricist.languageTag = state.languageCode
        }
    }

As the description, I expect that it only changes wherever text, but not recompose whole compose tree.

Use the Lyricist instance provided by rememberStrings() to change the current locale. This will trigger a recomposition that will update the strings wherever they are being used.

That happen because you use staticCompositionLocalOf which described

Unlike compositionLocalOf, reads of a staticCompositionLocalOf are not tracked by the composer and changing the value provided in the CompositionLocalProvider call will cause the entirety of the content to be recomposed instead of just the places where in the composition the local value is used. This lack of tracking, however, makes a staticCompositionLocalOf more efficient when the value provided is highly unlikely to or will never change.

So I think we should move to compositionLocalOf because it provides flexible ability. I.E: change language in app settings feature.

@DevSrSouza DevSrSouza merged commit 9ee4c03 into adrielcafe:main May 10, 2023
@hichamboushaba
Copy link

@DevSrSouza a remark regarding this change, I wonder if this change was the right call, compositionLocalOf has an additional cost to track changes, and the official documentation recommends this:

If the value provided to the CompositionLocal is highly unlikely to change or will never change, use staticCompositionLocalOf to get performance benefits.
https://developer.android.com/jetpack/compose/compositionlocal#creating-apis

I think that the language of the app is something that falls under the category of "unlikely to change", and the use of staticCompositionLocalOf was more appropriate, just like the MatherialTheme properties (for example the colors property is backed by a static composition local).
What do you think?

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

Successfully merging this pull request may close these issues.

3 participants