google / accompanist

A collection of extension libraries for Jetpack Compose
https://google.github.io/accompanist
Apache License 2.0
7.31k stars 592 forks source link

[WebView] Question about backwards writes #1737

Closed KevinnZou closed 6 months ago

KevinnZou commented 6 months ago

@bentrengrove Hello! I'm aware that the WebView module has been deprecated. As a result, I decided to fork it and create a multiplatform version. It is built entirely based on this library and includes support for iOS and Desktop platforms. However, I'm currently facing an issue regarding backward writes that is causing an infinite recomposition.

The thing is that: we have a loadingState property in WebViewState and we will update its status by inspecting webview's loading status. However, if we inspect the loadingState inside the WebView composable like below, it will lead to an infinite loop.

@Composable
public fun WebView(
    state: WebViewState,
    layoutParams: FrameLayout.LayoutParams,
    modifier: Modifier = Modifier,
    captureBackPresses: Boolean = true,
    navigator: WebViewNavigator = rememberWebViewNavigator(),
    onCreated: (WebView) -> Unit = {},
    onDispose: (WebView) -> Unit = {},
    client: AccompanistWebViewClient = remember { AccompanistWebViewClient() },
    chromeClient: AccompanistWebChromeClient = remember { AccompanistWebChromeClient() },
    factory: ((Context) -> WebView)? = null,
){
        LaunchedEffect(state.loadingState) {
            if (state.loadingState is LoadingState.Finished) {
                ....
            }
        }
}

I did some searches and found that it may related to backward writes. We read the loadingState before and write it when the WebView's loading state updates.

However, I'm still unsure if it's a case of backward writes since it works well on Android but fails on Desktop. I would appreciate it if you could provide a more detailed explanation of backward writes and the issue I am encountering? You can find the full code here.

Thank you so much!

bentrengrove commented 6 months ago

Hey @KevinnZou, cool project! That does look like a backwards write. Instead of keying your launched effect with state.loadingState try use snapshotFlow instead, this will avoid the backwards write as you won't be observing the state in composition. This will also be more performant if it can avoid recomposing the whole WebView composable.

Something like:

LaunchedEffect(state) {
   snapshotFlow { state.loadingState }.collect {
      // do something
   }
}
KevinnZou commented 6 months ago

@bentrengrove Thank you for your response. It is greatly appreciated and very helpful.

KevinnZou commented 6 months ago

@bentrengrove I have a followup question regarding backwards write issue.

At here, we read the state.webview: val webView = state.webView Then, we update it at here: state.webView = it

Is that also a case for backwards write?

bentrengrove commented 5 months ago

That one is ok because it happens in the factory method which only occurs once, so at most causes one more recomposition. Still not ideal if it can be avoided, you want your composables to settle in one recomposition if possible, not take multiple compositions to arrive at their final state. But one more isn't the biggest deal if it can't be avoided.

The backwards writes you have to watch out for are the infinite loops like the one in the example here. https://developer.android.com/jetpack/compose/performance/bestpractices#avoid-backwards

KevinnZou commented 5 months ago

@bentrengrove Got you! Thanks for your explanation!