From cff076b508de4a37f8f9a561a82aaf8f32f67407 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 19 Jan 2024 20:44:43 +0100 Subject: [PATCH] Fix detekt issue: Lambda parameters in a @Composable that are referenced directly inside of restarting effects can cause issues or unpredictable behavior. If restarting the effect is ok, you can add the reference to this parameter as a key in that effect, so when the parameter changes, a new effect is created. However, if the effect is not to be restarted, you will need to use `rememberUpdatedState` on the parameter and use its result in the effect. See https://mrmans0n.github.io/compose-rules/rules/#be-mindful-of-the-arguments-you-use-inside-of-a-restarting-effect for more information. [LambdaParameterInRestartableEffect] --- .../features/ftue/impl/migration/MigrationScreenView.kt | 2 +- .../android/features/invitelist/impl/InviteListView.kt | 2 +- .../features/lockscreen/impl/unlock/PinUnlockHelper.kt | 7 +++++-- .../features/login/impl/changeserver/ChangeServerView.kt | 2 +- .../android/features/logout/impl/ui/LogoutActionDialog.kt | 2 +- .../element/android/features/messages/impl/MessagesView.kt | 2 +- .../impl/attachments/preview/AttachmentsPreviewView.kt | 2 +- .../features/messages/impl/timeline/TimelineView.kt | 4 +++- .../rageshake/api/detection/RageshakeDetectionView.kt | 2 +- .../designsystem/components/async/AsyncActionView.kt | 2 +- .../android/libraries/textcomposer/SoftKeyboardEffect.kt | 7 +++++-- .../element/android/libraries/textcomposer/TextComposer.kt | 6 ++++-- 12 files changed, 25 insertions(+), 15 deletions(-) diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/migration/MigrationScreenView.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/migration/MigrationScreenView.kt index d2674e3f98..a365b1e4dc 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/migration/MigrationScreenView.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/migration/MigrationScreenView.kt @@ -32,7 +32,7 @@ fun MigrationScreenView( modifier: Modifier = Modifier, ) { if (migrationState.isMigrating.not()) { - LaunchedEffect(Unit) { + LaunchedEffect(onMigrationFinished) { onMigrationFinished() } } diff --git a/features/invitelist/impl/src/main/kotlin/io/element/android/features/invitelist/impl/InviteListView.kt b/features/invitelist/impl/src/main/kotlin/io/element/android/features/invitelist/impl/InviteListView.kt index 28ecf4938d..93d552237f 100644 --- a/features/invitelist/impl/src/main/kotlin/io/element/android/features/invitelist/impl/InviteListView.kt +++ b/features/invitelist/impl/src/main/kotlin/io/element/android/features/invitelist/impl/InviteListView.kt @@ -57,7 +57,7 @@ fun InviteListView( modifier: Modifier = Modifier, ) { if (state.acceptedAction is AsyncData.Success) { - LaunchedEffect(state.acceptedAction) { + LaunchedEffect(state.acceptedAction, onInviteAccepted) { onInviteAccepted(state.acceptedAction.data) } } diff --git a/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockHelper.kt b/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockHelper.kt index e1dd7f5be3..6ecf13f591 100644 --- a/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockHelper.kt +++ b/features/lockscreen/impl/src/main/kotlin/io/element/android/features/lockscreen/impl/unlock/PinUnlockHelper.kt @@ -18,6 +18,8 @@ package io.element.android.features.lockscreen.impl.unlock import androidx.compose.runtime.Composable import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.rememberUpdatedState import io.element.android.features.lockscreen.impl.biometric.BiometricUnlockManager import io.element.android.features.lockscreen.impl.biometric.DefaultBiometricUnlockCallback import io.element.android.features.lockscreen.impl.pin.DefaultPinCodeManagerCallback @@ -30,15 +32,16 @@ class PinUnlockHelper @Inject constructor( ) { @Composable fun OnUnlockEffect(onUnlock: () -> Unit) { + val latestOnUnlock by rememberUpdatedState(onUnlock) DisposableEffect(Unit) { val biometricUnlockCallback = object : DefaultBiometricUnlockCallback() { override fun onBiometricUnlockSuccess() { - onUnlock() + latestOnUnlock() } } val pinCodeVerifiedCallback = object : DefaultPinCodeManagerCallback() { override fun onPinCodeVerified() { - onUnlock() + latestOnUnlock() } } biometricUnlockManager.addCallback(biometricUnlockCallback) diff --git a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerView.kt b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerView.kt index 132003eafc..ea18ec407e 100644 --- a/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerView.kt +++ b/features/login/impl/src/main/kotlin/io/element/android/features/login/impl/changeserver/ChangeServerView.kt @@ -63,7 +63,7 @@ fun ChangeServerView( } } is AsyncData.Loading -> ProgressDialog() - is AsyncData.Success -> LaunchedEffect(state.changeServerAction) { + is AsyncData.Success -> LaunchedEffect(state.changeServerAction, onDone) { onDone() } AsyncData.Uninitialized -> Unit diff --git a/features/logout/impl/src/main/kotlin/io/element/android/features/logout/impl/ui/LogoutActionDialog.kt b/features/logout/impl/src/main/kotlin/io/element/android/features/logout/impl/ui/LogoutActionDialog.kt index 636dd68058..b626836c8b 100644 --- a/features/logout/impl/src/main/kotlin/io/element/android/features/logout/impl/ui/LogoutActionDialog.kt +++ b/features/logout/impl/src/main/kotlin/io/element/android/features/logout/impl/ui/LogoutActionDialog.kt @@ -53,7 +53,7 @@ fun LogoutActionDialog( onDismiss = onDismissError, ) is AsyncAction.Success -> - LaunchedEffect(state) { + LaunchedEffect(state, onSuccessLogout) { onSuccessLogout(state.data) } } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt index 394f1f3556..8098f2516c 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt @@ -294,7 +294,7 @@ private fun AttachmentStateView( ) { when (state) { AttachmentsState.None -> Unit - is AttachmentsState.Previewing -> LaunchedEffect(state) { + is AttachmentsState.Previewing -> LaunchedEffect(state, onPreviewAttachments) { onPreviewAttachments(state.attachments) } is AttachmentsState.Sending -> { diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewView.kt index 5b624bd254..5dc7875afc 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/attachments/preview/AttachmentsPreviewView.kt @@ -57,7 +57,7 @@ fun AttachmentsPreviewView( } if (state.sendActionState is SendActionState.Done) { - LaunchedEffect(state.sendActionState) { + LaunchedEffect(state.sendActionState, onDismiss) { onDismiss() } } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt index d2ab635e6a..563eb78d78 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineView.kt @@ -42,6 +42,7 @@ import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.alpha @@ -193,10 +194,11 @@ private fun BoxScope.TimelineScrollHelper( } } + val latestOnScrollFinishedAt by rememberUpdatedState(onScrollFinishedAt) LaunchedEffect(isScrollFinished, isTimelineEmpty) { if (isScrollFinished && !isTimelineEmpty) { // Notify the parent composable about the first visible item index when scrolling finishes - onScrollFinishedAt(lazyListState.firstVisibleItemIndex) + latestOnScrollFinishedAt(lazyListState.firstVisibleItemIndex) } } diff --git a/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/detection/RageshakeDetectionView.kt b/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/detection/RageshakeDetectionView.kt index 996392d879..5982edfdc8 100644 --- a/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/detection/RageshakeDetectionView.kt +++ b/features/rageshake/api/src/main/kotlin/io/element/android/features/rageshake/api/detection/RageshakeDetectionView.kt @@ -73,7 +73,7 @@ private fun TakeScreenshot( onScreenshotTaken: (ImageResult) -> Unit ) { val view = LocalView.current - LaunchedEffect(Unit) { + LaunchedEffect(onScreenshotTaken) { view.screenshot { onScreenshotTaken(it) } diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/async/AsyncActionView.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/async/AsyncActionView.kt index 732a3473a3..102769b7be 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/async/AsyncActionView.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/components/async/AsyncActionView.kt @@ -67,7 +67,7 @@ fun AsyncActionView( } } is AsyncAction.Success -> { - LaunchedEffect(async) { + LaunchedEffect(async, onSuccess) { onSuccess(async.data) } } diff --git a/libraries/textcomposer/impl/src/main/kotlin/io/element/android/libraries/textcomposer/SoftKeyboardEffect.kt b/libraries/textcomposer/impl/src/main/kotlin/io/element/android/libraries/textcomposer/SoftKeyboardEffect.kt index 96b48dca6e..90e9591953 100644 --- a/libraries/textcomposer/impl/src/main/kotlin/io/element/android/libraries/textcomposer/SoftKeyboardEffect.kt +++ b/libraries/textcomposer/impl/src/main/kotlin/io/element/android/libraries/textcomposer/SoftKeyboardEffect.kt @@ -18,6 +18,8 @@ package io.element.android.libraries.textcomposer import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.ui.platform.LocalView import androidx.compose.ui.viewinterop.AndroidView import io.element.android.libraries.androidutils.ui.awaitWindowFocus @@ -40,7 +42,8 @@ internal fun SoftKeyboardEffect( predicate: (T) -> Boolean, ) { val view = LocalView.current - LaunchedEffect(key) { + val latestOnRequestFocus by rememberUpdatedState(onRequestFocus) + LaunchedEffect(key, predicate) { if (predicate(key)) { // Await window focus in case returning from a dialog view.awaitWindowFocus() @@ -49,7 +52,7 @@ internal fun SoftKeyboardEffect( view.showKeyboard(andRequestFocus = true) // Refocus to the correct view - onRequestFocus() + latestOnRequestFocus() } } } diff --git a/libraries/textcomposer/impl/src/main/kotlin/io/element/android/libraries/textcomposer/TextComposer.kt b/libraries/textcomposer/impl/src/main/kotlin/io/element/android/libraries/textcomposer/TextComposer.kt index 06585d3b31..252e35d652 100644 --- a/libraries/textcomposer/impl/src/main/kotlin/io/element/android/libraries/textcomposer/TextComposer.kt +++ b/libraries/textcomposer/impl/src/main/kotlin/io/element/android/libraries/textcomposer/TextComposer.kt @@ -42,6 +42,7 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberUpdatedState import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip @@ -274,12 +275,13 @@ fun TextComposer( } val menuAction = state.menuAction + val latestOnSuggestionReceived by rememberUpdatedState(onSuggestionReceived) LaunchedEffect(menuAction) { if (menuAction is MenuAction.Suggestion) { val suggestion = Suggestion(menuAction.suggestionPattern) - onSuggestionReceived(suggestion) + latestOnSuggestionReceived(suggestion) } else { - onSuggestionReceived(null) + latestOnSuggestionReceived(null) } } }