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

RenderContext passed into StatefulWorkflow#render is new instance every time #988

Open
weverb2 opened this issue Apr 11, 2023 · 3 comments

Comments

@weverb2
Copy link

weverb2 commented Apr 11, 2023

Found issue when digging into to recompositions with @steve-the-edwards:

When render is called in a StatefulWorkflow, a new instance of RenderContext is passed each time, causing compose to be unhappy about any lambdas that would capture the RenderContext (like for actionSink calls).

We believe this issue to be the cast on L172 of StatefulWorkflow.kt never succeeding and always requiring a wrapped baseContext

Screenshot 2023-04-11 at 11 34 30 AM

@steve-the-edwards
Copy link
Contributor

Problem first noted in #849 then we tried to fix in #850 but this had a bug because we were recreating anyway in order to get the StateT info in.

I believe the reason we have these StatefulWorkflow.RenderContext and StatelessWorkflow.RenderContext concrete types that just delegate to the BaseRenderContext instance passed in (which is a RealRenderContext) is so that StatelessWorkflows don't have to worry about typing the StateT.

That being said, we always cast to a StatefulWorkflow when creating the nodes (see asStatefulWorkflow) so by the time we create the node I don't see why we can't do the cast write away for the RenderContext and only deal with that.

Will spike on this.

@steve-the-edwards
Copy link
Contributor

Note that to currently handle this for lambdas passed in teh rendering to Composables, we box the lambdas on the Compose side like so:

@Composable
fun rememberLambda(lambda: () -> Unit): () -> Unit {
  val latestLambda = rememberUpdatedState(lambda)
  return remember { { latestLambda.value.invoke() } }
}

So that we save the latest lambda but hide it from the other Composables until its invoked.

@steve-the-edwards
Copy link
Contributor

Consider "Strong Skipping" - as discussed here - may make instance equality all we need for skipping - https://www.reddit.com/r/androiddev/comments/1an5emm/comment/kqf8dz0/

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

No branches or pull requests

2 participants