rjrjr / compose-backstack

Simple composable for rendering transitions between backstacks.
Other
504 stars 23 forks source link

Fix TransitionController to no longer perform side effects directly in composition. #58

Closed zach-klippenstein closed 3 years ago

zach-klippenstein commented 3 years ago

Performing side effects in composition, without using one of the effect functions, is dangerous because if compositions may be ran on multiple threads in parallel, and if the composition does not complete successfully, the side effects may be running in an invalid state. TransitionController was updating non-MutableState properties directly from updateTransition(which is called directly from a composition) and even launching coroutines to perform transition animations. This is particularly bad because if the composition later failed, we'd be animating a transition which, as far as compose is concerned, never actually occured.

Instead, now TransitionController.updateBackstack simply sets targetKeys (which is now a MutableState) and ensures that displayedKeys (previously called activeKeys) is initialized on the first call. Animations are performed by a long-running LaunchedEffect which uses snapshotFlow to observe targetKeys changes and run the animations inline. Using a conflated Flow means we also don't need to explicitly synchronize to avoid overlapping transitions – the flow collector exerts backpressure while a transition is animating, and any backstack updates that occur during that time will be conflated.

This change also makes all the properties of TransitionController that are updated by rememberTransitionController MutableStates, because while we don't need to observe them for changes, we do need to make sure they are thread-safe and rolled back if the composition fails.