diff --git a/CHANGELOG.md b/CHANGELOG.md index ae131ec..8ea0855 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,11 +10,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed excessive recompositions and broken Switch animation on `FeatureFlipScreen`: item keys now use the stable `feature.name` instead of `hashCode()`, `FeatureHandler` caches its Flow to prevent `collectAsStateWithLifecycle` from restarting on every recomposition, and redundant explicit `remember` keys have been removed from `derivedStateOf` blocks. (`devview-featureflip`) - Added `distinctUntilChanged()` to `FeatureHandler.isFeatureEnabledFlow` and `getFeatures` to suppress recompositions when DataStore emits structurally identical values. (`devview-featureflip`) -- Fixed `AnalyticsScreen` `LazyColumn` using `log.hashCode()` as item key instead of the stable `log.timestamp`; removed redundant explicit keys from all `derivedStateOf` blocks. (`devview-analytics`) +- Fixed `AnalyticsScreen` `LazyColumn` item key: `log.hashCode()` was replaced by `log.timestamp`, then `log.timestamp` caused a crash because multiple events can share the same millisecond; the key is now the log's original position in the append-only `AnalyticsLogger.logs` list via `withIndex()`. Redundant explicit keys also removed from all `derivedStateOf` blocks. (`devview-analytics`) - Fixed `HomeScreen` `LazyColumn` using `module.hashCode()` as item key instead of the stable `module.moduleName`. (`devview`) - Fixed `NetworkMockScreen` using `collectAsState()` instead of `collectAsStateWithLifecycle()`, causing unnecessary state collection when the screen is off-stack or the app is backgrounded. (`devview-networkmock`) - Added `distinctUntilChanged()` to `MockStateRepository.observeState()` to suppress recompositions triggered by structurally equal `NetworkMockState` emissions from DataStore. (`devview-networkmock-core`) +### Documentation +- Added Compose List Keys rules to the contributing guide (`code-style.md`): LazyColumn/LazyRow keys must be unique, stable under state changes, and semantically meaningful. Added matching item to the PR checklist. + ## [0.1.3] - 2026-07-21 ### Added diff --git a/devview-analytics/src/androidDeviceTest/kotlin/com/worldline/devview/analytics/AnalyticsScreenTest.kt b/devview-analytics/src/androidDeviceTest/kotlin/com/worldline/devview/analytics/AnalyticsScreenTest.kt index 7dbb2c7..546bcd5 100644 --- a/devview-analytics/src/androidDeviceTest/kotlin/com/worldline/devview/analytics/AnalyticsScreenTest.kt +++ b/devview-analytics/src/androidDeviceTest/kotlin/com/worldline/devview/analytics/AnalyticsScreenTest.kt @@ -120,10 +120,32 @@ class AnalyticsScreenTest { waitUntilTagCount(tag = "analytics_log_item_login_click", expectedCount = 0) } + @Test + fun logsWithSameTimestampAllRender() = runComposeUiTest { + val sameTimestamp = 1_700_000_000_000L + val logs = listOf( + AnalyticsLog(tag = "event_a", screenClass = "TestScreen", timestamp = sameTimestamp, type = AnalyticsLogCategory.Action.Click), + AnalyticsLog(tag = "event_b", screenClass = "TestScreen", timestamp = sameTimestamp, type = AnalyticsLogCategory.Action.Click), + AnalyticsLog(tag = "event_c", screenClass = "TestScreen", timestamp = sameTimestamp, type = AnalyticsLogCategory.Action.Click), + ) + + setContent { + CompositionLocalProvider(LocalAnalytics provides logs) { + AnalyticsScreen(highlightedAnalyticsLogTypes = persistentListOf()) + } + } + + waitUntilTagCount(tag = "analytics_log_item_event_a", expectedCount = 1) + waitUntilTagCount(tag = "analytics_log_item_event_b", expectedCount = 1) + waitUntilTagCount(tag = "analytics_log_item_event_c", expectedCount = 1) + } + + private var logIdCounter = 0L + private fun log( tag: String, type: AnalyticsLogType, - timestamp: Long = 1_700_000_000_000 + timestamp: Long = logIdCounter++ ): AnalyticsLog = AnalyticsLog( tag = tag, diff --git a/devview-analytics/src/commonMain/kotlin/com/worldline/devview/analytics/AnalyticsScreen.kt b/devview-analytics/src/commonMain/kotlin/com/worldline/devview/analytics/AnalyticsScreen.kt index e44deb9..182f2e5 100644 --- a/devview-analytics/src/commonMain/kotlin/com/worldline/devview/analytics/AnalyticsScreen.kt +++ b/devview-analytics/src/commonMain/kotlin/com/worldline/devview/analytics/AnalyticsScreen.kt @@ -162,14 +162,14 @@ public fun AnalyticsScreen( val filteredAnalytics by remember { derivedStateOf { val now = Clock.System.now().toEpochMilliseconds() - analytics.filter { + analytics.withIndex().filter { (_, log) -> val matchesQuery = filterQuery.isBlank() || - it.tag.contains(other = filterQuery, ignoreCase = true) || - it.screenClass.contains(other = filterQuery, ignoreCase = true) + log.tag.contains(other = filterQuery, ignoreCase = true) || + log.screenClass.contains(other = filterQuery, ignoreCase = true) val matchesCategory = selectedCategories.isEmpty() || - it.type.category in selectedCategories + log.type.category in selectedCategories val matchesTimeRange = selectedTimeRange.durationMillis?.let { duration -> - now - it.timestamp <= duration + now - log.timestamp <= duration } ?: true matchesQuery && matchesCategory && matchesTimeRange } @@ -388,8 +388,9 @@ public fun AnalyticsScreen( } itemsIndexed( items = filteredAnalytics, - key = { _, log -> log.timestamp } - ) { index, log -> + key = { _, item -> item.index } + ) { index, item -> + val log = item.value Column( modifier = Modifier .animateItem() diff --git a/devview-featureflip/src/androidDeviceTest/kotlin/com/worldline/devview/featureflip/FeatureFlipScreenTest.kt b/devview-featureflip/src/androidDeviceTest/kotlin/com/worldline/devview/featureflip/FeatureFlipScreenTest.kt index d47630b..799399a 100644 --- a/devview-featureflip/src/androidDeviceTest/kotlin/com/worldline/devview/featureflip/FeatureFlipScreenTest.kt +++ b/devview-featureflip/src/androidDeviceTest/kotlin/com/worldline/devview/featureflip/FeatureFlipScreenTest.kt @@ -193,4 +193,26 @@ class FeatureFlipScreenTest { waitUntilTagCount(tag = "feature_item_dark_mode", expectedCount = 1) waitUntilTagCount(tag = "feature_item_new_checkout", expectedCount = 1) } + + @Test + fun multipleFeaturesOfDifferentTypesAllRender() = runComposeUiTest { + val handler = FeatureHandler( + dataStore = FakePreferencesDataStore(), + initialFeatures = listOf( + Feature.LocalFeature(name = "feature_local_a", description = null, isEnabled = false), + Feature.LocalFeature(name = "feature_local_b", description = null, isEnabled = true), + Feature.RemoteFeature(name = "feature_remote_a", description = null, defaultRemoteValue = true, state = FeatureState.REMOTE), + ) + ) + + setContent { + CompositionLocalProvider(LocalFeatureHandler provides handler) { + FeatureFlipScreen() + } + } + + waitUntilTagCount(tag = "feature_item_feature_local_a", expectedCount = 1) + waitUntilTagCount(tag = "feature_item_feature_local_b", expectedCount = 1) + waitUntilTagCount(tag = "feature_item_feature_remote_a", expectedCount = 1) + } } diff --git a/devview-networkmock/src/androidDeviceTest/kotlin/com/worldline/devview/networkmock/NetworkMockScreenTest.kt b/devview-networkmock/src/androidDeviceTest/kotlin/com/worldline/devview/networkmock/NetworkMockScreenTest.kt index 53f4468..a90f162 100644 --- a/devview-networkmock/src/androidDeviceTest/kotlin/com/worldline/devview/networkmock/NetworkMockScreenTest.kt +++ b/devview-networkmock/src/androidDeviceTest/kotlin/com/worldline/devview/networkmock/NetworkMockScreenTest.kt @@ -132,6 +132,14 @@ class NetworkMockScreenTest { ) } + @Test + fun multipleEndpointCardsAllRender() = runComposeUiTest { + setScreen(uiState = MockScreenTestData.contentState()) + + onNodeWithTag(testTag = "endpoint_card_example_staging_getUser").assertIsDisplayed() + onNodeWithTag(testTag = "endpoint_card_example_staging_createUser").assertIsDisplayed() + } + private fun ComposeUiTest.setScreen( uiState: NetworkMockUiState, onGlobalToggle: (Boolean) -> Unit = {}, diff --git a/docs/contributing/code-style.md b/docs/contributing/code-style.md index ba561f8..2079e7b 100644 --- a/docs/contributing/code-style.md +++ b/docs/contributing/code-style.md @@ -45,6 +45,18 @@ The pre-commit hook runs this automatically on every `git commit`, so issues are - Remove unused code and imports - Prefer immutable data structures +## Compose List Keys + +Every `LazyColumn` or `LazyRow` with a `key` argument must follow these rules: + +1. **Unique within the list** — the key must be distinct for every item visible simultaneously. `hashCode()` and `timestamp` are not safe (`hashCode` can collide; millisecond timestamps are not unique under rapid logging). Use a domain identifier: a stable name, a composite key, a database ID. +2. **Stable under state changes** — the key must not change when the item's mutable state changes. Keying a feature on `feature.hashCode()` changes when `isEnabled` changes; `feature.name` does not. +3. **Semantically meaningful** — the key should communicate what uniquely identifies the item, not an implementation detail. + +**Rule of thumb:** if the key field doubles as a DataStore preference key, database primary key, or URL path segment, it is the right choice. + +Every `LazyColumn` with a `key` argument must have at least one device test that renders two or more items and asserts all items are visible (using `waitUntilTagCount` or `assertIsDisplayed`). This test catches duplicate-key crashes before they reach production. + ## Next Steps - See [Development Setup](development.md) for environment configuration - Review [Pull Requests](pull-requests.md) for contribution process diff --git a/docs/contributing/pull-requests.md b/docs/contributing/pull-requests.md index 88f8734..2731766 100644 --- a/docs/contributing/pull-requests.md +++ b/docs/contributing/pull-requests.md @@ -62,6 +62,7 @@ Note: PRs are squash-merged using the PR title (`type: description`) as the fina - [ ] Detekt passes - [ ] Documentation updated - [ ] CHANGELOG updated +- [ ] Any `LazyColumn`/`LazyRow` with a `key` argument has a device test rendering 2+ items (see [Code Style](code-style.md#compose-list-keys)) ## Review Process - PRs are reviewed by maintainers. Be responsive to feedback.