You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I was looking at the sources and realized there could be improvements made to the recompositions of DI functions.
LocalDi should be made public to allow clients to provide the DI via a uniform syntax. When the localDi function is used, it is not marked as non-skippable, which leads to an additional nesting level in the tree. Instead, the LocalDi composition local should be made public so that clients can provide the DI more efficiently:
@Composable
internalfunProvideDestinationLocals(
component:DestinationComponent,
content: @Composable () ->Unit
) =CompositionLocalProvider(
LocalSubscriberLifecycle provides component.subLifecycle,
LocalDestinationContext provides component,
// LocalDi provides component.diContext // desired, avoids composition and object creation
) {
// current - requires a nesting level in the treeOnDIContext(content = content, context = remember { diContext(component.context) })
}
Some of the functions, such as OnDIContext(context: DIContext<*>, content: @Composable () -> Unit) create the DiContext in-place in the composition. This can lead to unnecessary allocations of contexts on every recomposition and heap pollution. We should avoid creating objects directly in composition and use remember with a lambda-driven DSL where possible.
The functions that provide the DI context do not utilize rememberUpdatedState, which means once a lambda is captured inside the remember block, it will not change. If the user of the code has their lambda be recreated on recomposition, this will lead to issues as the With* functions from kodein that take lambdas will not pick up the updated lambda. This can only manifest if the lambdas capture parameters out of composition that can change. See docs for details
DI value is not properly recreated when incoming params change. An optimized solution will make use of remember keys to properly re-create the di when the incoming parameters change. This is avoided by #3 and #2 but has to be kept in mind when those are addressed.
Would you like for me to take a stab at this feature in a PR?
The text was updated successfully, but these errors were encountered:
Improving / Enhancing / Fixing Compose is my next task on Kodein, but it is a side project, so not sure when. If you have some time and the whiling to contribute, be my guest :)
I was looking at the sources and realized there could be improvements made to the recompositions of DI functions.
LocalDi
should be made public to allow clients to provide the DI via a uniform syntax. When thelocalDi
function is used, it is not marked asnon-skippable
, which leads to an additional nesting level in the tree. Instead, theLocalDi
composition local should be made public so that clients can provide the DI more efficiently:Some of the functions, such as
OnDIContext(context: DIContext<*>, content: @Composable () -> Unit)
create the DiContext in-place in the composition. This can lead to unnecessary allocations of contexts on every recomposition and heap pollution. We should avoid creating objects directly in composition and useremember
with a lambda-driven DSL where possible.The functions that provide the DI context do not utilize
rememberUpdatedState
, which means once a lambda is captured inside theremember
block, it will not change. If the user of the code has their lambda be recreated on recomposition, this will lead to issues as theWith*
functions from kodein that take lambdas will not pick up the updated lambda. This can only manifest if the lambdas capture parameters out of composition that can change. See docs for detailsDI value is not properly recreated when incoming params change. An optimized solution will make use of
remember
keys to properly re-create the di when the incoming parameters change. This is avoided by#3
and#2
but has to be kept in mind when those are addressed.Would you like for me to take a stab at this feature in a PR?
The text was updated successfully, but these errors were encountered: