Skip to content

Commit 7d0f1c8

Browse files
authored
Merge pull request #839 from synonymdev/refactor/immutable-collections
fix: ui state stability to reduce render cycles
2 parents a84233f + 6b8861e commit 7d0f1c8

79 files changed

Lines changed: 637 additions & 356 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

AGENTS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,10 @@ suspend fun getData(): Result<Data> = withContext(Dispatchers.IO) {
232232
- ALWAYS prefer `when (subject)` with Kotlin guard conditions (`if`) over condition-based `when {}` with `is` type checks, e.g. `when (event) { is Foo if event.x == y -> ... }` instead of `when { event is Foo && event.x == y -> ... }`
233233
- ALWAYS prefer `sealed interface` over `sealed class` when no shared state or constructor is needed
234234
- NEVER duplicate error logging in `.onFailure {}` if the called method already logs the same error internally
235+
- ALWAYS use `ImmutableList`/`ImmutableMap`/`ImmutableSet` instead of `List`/`Map`/`Set` for composable function parameters and UiState data class fields
236+
- ALWAYS annotate UiState data classes with `@Immutable`; use `@Stable` instead when any field holds a non-immutable type (e.g. `Throwable`, external library types from `bitkitcore`/`ldknode`/`vssclient`, or types containing plain `List`/`Map`/`Set`)
237+
- ALWAYS use `.toImmutableList()`, `.toImmutableMap()`, `.toImmutableSet()` when producing collections for UI state
238+
- ALWAYS use `persistentListOf()`, `persistentMapOf()`, `persistentSetOf()` for default values in UiState fields
235239

236240
### Device Debugging (adb)
237241

app/build.gradle.kts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,7 @@ dependencies {
225225
implementation(libs.material)
226226
implementation(libs.datastore.preferences)
227227
implementation(libs.kotlinx.datetime)
228+
implementation(libs.kotlinx.collections.immutable)
228229
implementation(libs.biometric)
229230
implementation(libs.zxing)
230231
implementation(libs.barcode.scanning)

app/src/main/java/to/bitkit/repositories/ActivityRepo.kt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package to.bitkit.repositories
22

33
import androidx.annotation.VisibleForTesting
4+
import androidx.compose.runtime.Immutable
45
import com.synonym.bitkitcore.Activity
56
import com.synonym.bitkitcore.ActivityFilter
67
import com.synonym.bitkitcore.ActivityTags
@@ -11,6 +12,9 @@ import com.synonym.bitkitcore.OnchainActivity
1112
import com.synonym.bitkitcore.PaymentState
1213
import com.synonym.bitkitcore.PaymentType
1314
import com.synonym.bitkitcore.SortDirection
15+
import kotlinx.collections.immutable.ImmutableList
16+
import kotlinx.collections.immutable.persistentListOf
17+
import kotlinx.collections.immutable.toImmutableList
1418
import kotlinx.coroutines.CoroutineDispatcher
1519
import kotlinx.coroutines.TimeoutCancellationException
1620
import kotlinx.coroutines.async
@@ -600,7 +604,7 @@ class ActivityRepo @Inject constructor(
600604
runCatching {
601605
coreService.activity.allPossibleTags()
602606
}.onSuccess { tags ->
603-
_state.update { it.copy(tags = tags) }
607+
_state.update { it.copy(tags = tags.toImmutableList()) }
604608
}.onFailure {
605609
Logger.error("getAllAvailableTags error", it, context = TAG)
606610
}
@@ -677,6 +681,7 @@ class ActivityRepo @Inject constructor(
677681
}
678682
}
679683

684+
@Immutable
680685
data class ActivityState(
681-
val tags: List<String> = emptyList(),
686+
val tags: ImmutableList<String> = persistentListOf(),
682687
)

app/src/main/java/to/bitkit/repositories/BlocktankRepo.kt

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package to.bitkit.repositories
22

3+
import androidx.compose.runtime.Stable
34
import com.synonym.bitkitcore.BtOrderState2
45
import com.synonym.bitkitcore.ChannelLiquidityOptions
56
import com.synonym.bitkitcore.ChannelLiquidityParams
@@ -14,6 +15,9 @@ import com.synonym.bitkitcore.calculateChannelLiquidityOptions
1415
import com.synonym.bitkitcore.getDefaultLspBalance
1516
import com.synonym.bitkitcore.giftOrder
1617
import com.synonym.bitkitcore.giftPay
18+
import kotlinx.collections.immutable.ImmutableList
19+
import kotlinx.collections.immutable.persistentListOf
20+
import kotlinx.collections.immutable.toImmutableList
1721
import kotlinx.coroutines.CoroutineDispatcher
1822
import kotlinx.coroutines.CoroutineScope
1923
import kotlinx.coroutines.SupervisorJob
@@ -104,7 +108,7 @@ class BlocktankRepo @Inject constructor(
104108
.collect { paidOrderIds ->
105109
_blocktankState.update { state ->
106110
state.copy(
107-
paidOrders = state.orders.filter { order -> order.id in paidOrderIds },
111+
paidOrders = state.orders.filter { order -> order.id in paidOrderIds }.toImmutableList(),
108112
)
109113
}
110114
}
@@ -148,9 +152,9 @@ class BlocktankRepo @Inject constructor(
148152
val cachedCjitEntries = coreService.blocktank.cjitEntries(refresh = false)
149153
_blocktankState.update { state ->
150154
state.copy(
151-
orders = cachedOrders,
152-
cjitEntries = cachedCjitEntries,
153-
paidOrders = cachedOrders.filter { order -> order.id in paidOrderIds },
155+
orders = cachedOrders.toImmutableList(),
156+
cjitEntries = cachedCjitEntries.toImmutableList(),
157+
paidOrders = cachedOrders.filter { order -> order.id in paidOrderIds }.toImmutableList(),
154158
)
155159
}
156160

@@ -159,9 +163,9 @@ class BlocktankRepo @Inject constructor(
159163
val cjitEntries = coreService.blocktank.cjitEntries(refresh = true)
160164
_blocktankState.update { state ->
161165
state.copy(
162-
orders = orders,
163-
cjitEntries = cjitEntries,
164-
paidOrders = orders.filter { order -> order.id in paidOrderIds },
166+
orders = orders.toImmutableList(),
167+
cjitEntries = cjitEntries.toImmutableList(),
168+
paidOrders = orders.filter { order -> order.id in paidOrderIds }.toImmutableList(),
165169
)
166170
}
167171

@@ -287,7 +291,7 @@ class BlocktankRepo @Inject constructor(
287291
updatedOrders[index] = order
288292
}
289293

290-
_blocktankState.update { state -> state.copy(orders = updatedOrders) }
294+
_blocktankState.update { state -> state.copy(orders = updatedOrders.toImmutableList()) }
291295

292296
return@runCatching order
293297
}.onFailure {
@@ -406,8 +410,8 @@ class BlocktankRepo @Inject constructor(
406410

407411
_blocktankState.update {
408412
it.copy(
409-
orders = payload.orders,
410-
cjitEntries = payload.cjitEntries,
413+
orders = payload.orders.toImmutableList(),
414+
cjitEntries = payload.cjitEntries.toImmutableList(),
411415
info = payload.info,
412416
)
413417
}
@@ -516,10 +520,11 @@ class BlocktankRepo @Inject constructor(
516520
}
517521
}
518522

523+
@Stable
519524
data class BlocktankState(
520-
val orders: List<IBtOrder> = emptyList(),
521-
val paidOrders: List<IBtOrder> = emptyList(),
522-
val cjitEntries: List<IcJitEntry> = emptyList(),
525+
val orders: ImmutableList<IBtOrder> = persistentListOf(),
526+
val paidOrders: ImmutableList<IBtOrder> = persistentListOf(),
527+
val cjitEntries: ImmutableList<IcJitEntry> = persistentListOf(),
523528
val info: IBtInfo? = null,
524529
val minCjitSats: Int? = null,
525530
)

app/src/main/java/to/bitkit/repositories/CurrencyRepo.kt

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
package to.bitkit.repositories
22

3+
import androidx.compose.runtime.Stable
4+
import kotlinx.collections.immutable.ImmutableList
5+
import kotlinx.collections.immutable.persistentListOf
6+
import kotlinx.collections.immutable.toImmutableList
37
import kotlinx.coroutines.CoroutineDispatcher
48
import kotlinx.coroutines.CoroutineScope
59
import kotlinx.coroutines.SupervisorJob
@@ -112,7 +116,7 @@ class CurrencyRepo @Inject constructor(
112116
rate.quote == settings.selectedCurrency
113117
}
114118
_currencyState.value.copy(
115-
rates = cachedData.cachedRates,
119+
rates = cachedData.cachedRates.toImmutableList(),
116120
selectedCurrency = settings.selectedCurrency,
117121
displayUnit = settings.displayUnit,
118122
primaryDisplay = settings.primaryDisplay,
@@ -248,8 +252,9 @@ class CurrencyRepo @Inject constructor(
248252
override fun convertSatsToFiatString(sats: Long): String = convertSatsToFiatPair(sats).getOrNull()?.second ?: ""
249253
}
250254

255+
@Stable
251256
data class CurrencyState(
252-
val rates: List<FxRate> = emptyList(),
257+
val rates: ImmutableList<FxRate> = persistentListOf(),
253258
val error: Throwable? = null,
254259
val hasStaleData: Boolean = false,
255260
val selectedCurrency: String = "USD",

app/src/main/java/to/bitkit/repositories/LightningRepo.kt

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package to.bitkit.repositories
22

3+
import androidx.compose.runtime.Stable
34
import com.google.firebase.messaging.FirebaseMessaging
45
import com.synonym.bitkitcore.AddressType
56
import com.synonym.bitkitcore.ClosedChannelDetails
@@ -10,6 +11,9 @@ import com.synonym.bitkitcore.Scanner
1011
import com.synonym.bitkitcore.createChannelRequestUrl
1112
import com.synonym.bitkitcore.createWithdrawCallbackUrl
1213
import com.synonym.bitkitcore.lnurlAuth
14+
import kotlinx.collections.immutable.ImmutableList
15+
import kotlinx.collections.immutable.persistentListOf
16+
import kotlinx.collections.immutable.toImmutableList
1317
import kotlinx.coroutines.CoroutineDispatcher
1418
import kotlinx.coroutines.CoroutineScope
1519
import kotlinx.coroutines.Job
@@ -1170,8 +1174,8 @@ class LightningRepo @Inject constructor(
11701174
it.copy(
11711175
nodeId = getNodeId().orEmpty(),
11721176
nodeStatus = getStatus(),
1173-
peers = getPeers().orEmpty(),
1174-
channels = getChannels().orEmpty(),
1177+
peers = getPeers().orEmpty().toImmutableList(),
1178+
channels = getChannels().orEmpty().toImmutableList(),
11751179
balances = getBalances(),
11761180
)
11771181
}
@@ -1388,12 +1392,13 @@ class NodeRunTimeoutError(opName: String) : AppError("Timeout waiting for node t
13881392
class GetPaymentsError : AppError("It wasn't possible get the payments")
13891393
class SyncUnhealthyError : AppError("Wallet sync failed before send")
13901394

1395+
@Stable
13911396
data class LightningState(
13921397
val nodeId: String = "",
13931398
val nodeStatus: NodeStatus? = null,
13941399
val nodeLifecycleState: NodeLifecycleState = NodeLifecycleState.Stopped,
1395-
val peers: List<PeerDetails> = emptyList(),
1396-
val channels: List<ChannelDetails> = emptyList(),
1400+
val peers: ImmutableList<PeerDetails> = persistentListOf(),
1401+
val channels: ImmutableList<ChannelDetails> = persistentListOf(),
13971402
val balances: BalanceDetails? = null,
13981403
val isSyncingWallet: Boolean = false,
13991404
val isGeoBlocked: Boolean = false,

app/src/main/java/to/bitkit/repositories/WalletRepo.kt

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
package to.bitkit.repositories
22

3+
import androidx.compose.runtime.Immutable
34
import com.synonym.bitkitcore.AddressType
45
import com.synonym.bitkitcore.PreActivityMetadata
56
import com.synonym.bitkitcore.Scanner
7+
import kotlinx.collections.immutable.ImmutableList
8+
import kotlinx.collections.immutable.persistentListOf
9+
import kotlinx.collections.immutable.toImmutableList
610
import kotlinx.coroutines.CoroutineDispatcher
711
import kotlinx.coroutines.CoroutineScope
812
import kotlinx.coroutines.Job
@@ -434,7 +438,7 @@ class WalletRepo @Inject constructor(
434438
_walletState.update {
435439
it.copy(
436440
bip21 = "",
437-
selectedTags = if (clearTags) emptyList() else it.selectedTags,
441+
selectedTags = if (clearTags) persistentListOf() else it.selectedTags,
438442
bip21AmountSats = null,
439443
bip21Description = "",
440444
)
@@ -474,7 +478,7 @@ class WalletRepo @Inject constructor(
474478
preActivityMetadataRepo.addPreActivityMetadataTags(paymentId, listOf(newTag)).onSuccess {
475479
_walletState.update {
476480
it.copy(
477-
selectedTags = (it.selectedTags + newTag).distinct()
481+
selectedTags = (it.selectedTags + newTag).distinct().toImmutableList()
478482
)
479483
}
480484
settingsStore.addLastUsedTag(newTag)
@@ -495,7 +499,7 @@ class WalletRepo @Inject constructor(
495499
.onSuccess {
496500
_walletState.update {
497501
it.copy(
498-
selectedTags = it.selectedTags.filterNot { tagItem -> tagItem == tag }
502+
selectedTags = it.selectedTags.filterNot { tagItem -> tagItem == tag }.toImmutableList()
499503
)
500504
}
501505
}.onFailure {
@@ -508,7 +512,7 @@ class WalletRepo @Inject constructor(
508512
if (paymentId == null || paymentId.isEmpty()) return@withContext
509513

510514
preActivityMetadataRepo.resetPreActivityMetadataTags(paymentId).onSuccess {
511-
_walletState.update { it.copy(selectedTags = emptyList()) }
515+
_walletState.update { it.copy(selectedTags = persistentListOf()) }
512516
}.onFailure {
513517
Logger.error("Failed to reset tags for pre-activity metadata", it, context = TAG)
514518
}
@@ -588,13 +592,14 @@ class WalletRepo @Inject constructor(
588592
}
589593
}
590594

595+
@Immutable
591596
data class WalletState(
592597
val onchainAddress: String = "",
593598
val bolt11: String = "",
594599
val bip21: String = "",
595600
val bip21AmountSats: ULong? = null,
596601
val bip21Description: String = "",
597-
val selectedTags: List<String> = listOf(),
602+
val selectedTags: ImmutableList<String> = persistentListOf(),
598603
val walletExists: Boolean = false,
599604
)
600605

app/src/main/java/to/bitkit/ui/ContentView.kt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import androidx.compose.runtime.CompositionLocalProvider
1313
import androidx.compose.runtime.DisposableEffect
1414
import androidx.compose.runtime.LaunchedEffect
1515
import androidx.compose.runtime.Stable
16-
import androidx.compose.runtime.collectAsState
1716
import androidx.compose.runtime.getValue
1817
import androidx.compose.runtime.mutableStateOf
1918
import androidx.compose.runtime.remember
@@ -41,6 +40,7 @@ import androidx.navigation.toRoute
4140
import dev.chrisbanes.haze.HazeState
4241
import dev.chrisbanes.haze.hazeSource
4342
import dev.chrisbanes.haze.rememberHazeState
43+
import kotlinx.collections.immutable.persistentListOf
4444
import kotlinx.coroutines.delay
4545
import kotlinx.coroutines.launch
4646
import kotlinx.serialization.Serializable
@@ -343,7 +343,7 @@ fun ContentView(
343343
}
344344

345345
val balance by walletViewModel.balanceState.collectAsStateWithLifecycle()
346-
val currencies by currencyViewModel.uiState.collectAsState()
346+
val currencies by currencyViewModel.uiState.collectAsStateWithLifecycle()
347347

348348
// Keep backups in sync
349349
LaunchedEffect(backupsViewModel) { backupsViewModel.observeAndSyncBackups() }
@@ -385,7 +385,7 @@ fun ContentView(
385385
}
386386

387387
is Sheet.Receive -> {
388-
val walletState by walletViewModel.walletState.collectAsState()
388+
val walletState by walletViewModel.walletState.collectAsStateWithLifecycle()
389389
ReceiveSheet(
390390
walletState = walletState,
391391
navigateToExternalConnection = {
@@ -661,7 +661,7 @@ private fun RootNavHost(
661661
)
662662
}
663663
composableWithDefaultTransitions<Routes.Funding> {
664-
val hasSeenSpendingIntro by settingsViewModel.hasSeenSpendingIntro.collectAsState()
664+
val hasSeenSpendingIntro by settingsViewModel.hasSeenSpendingIntro.collectAsStateWithLifecycle()
665665
val isGeoBlocked by appViewModel.isGeoBlocked.collectAsStateWithLifecycle()
666666

667667
FundingScreen(
@@ -799,7 +799,7 @@ private fun NavGraphBuilder.home(
799799

800800
SavingsWalletScreen(
801801
isGeoBlocked = isGeoBlocked,
802-
onchainActivities = onchainActivities.orEmpty(),
802+
onchainActivities = onchainActivities ?: persistentListOf(),
803803
onAllActivityButtonClick = { navController.navigateToAllActivity(activityListViewModel::clearFilters) },
804804
onActivityItemClick = { navController.navigateToActivityItem(it) },
805805
onEmptyActivityRowClick = { appViewModel.showSheet(Sheet.Receive) },
@@ -821,7 +821,7 @@ private fun NavGraphBuilder.home(
821821

822822
SpendingWalletScreen(
823823
channels = lightningState.channels,
824-
lightningActivities = lightningActivities.orEmpty(),
824+
lightningActivities = lightningActivities ?: persistentListOf(),
825825
onAllActivityButtonClick = { navController.navigateToAllActivity(activityListViewModel::clearFilters) },
826826
onActivityItemClick = { navController.navigateToActivityItem(it) },
827827
onEmptyActivityRowClick = { appViewModel.showSheet(Sheet.Receive) },

0 commit comments

Comments
 (0)