Skip to content

fix(core): use rememberUpdatedState to avoid stale callback capture in WheelPicker#135

Merged
darkokoa merged 1 commit into
mainfrom
fix/dkk/stale-callback-capture
Jul 7, 2026
Merged

fix(core): use rememberUpdatedState to avoid stale callback capture in WheelPicker#135
darkokoa merged 1 commit into
mainfrom
fix/dkk/stale-callback-capture

Conversation

@darkokoa

@darkokoa darkokoa commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Summary

WheelPicker's two long-lived LaunchedEffect + snapshotFlow collectors captured onScrollChanged / onScrollFinished as the lambda instance that was current when the effect started. The effects are keyed on lazyListState and count, so when a parent recomposed and passed new callback instances the effects did not restart (correct) but kept invoking the old lambdas (stale-capture bug).

Root cause

WheelPicker.kt:

LaunchedEffect(lazyListState, count) {
  snapshotFlow { calculateSnappedItemIndex(lazyListState) }
    .distinctUntilChanged()
    .collect { onScrollChanged(it) }   // <- stale capture
}

onScrollChanged / onScrollFinished are function types ((Int) -> Unit, (Int) -> Int?), which Compose treats as unstable. WheelTextPicker (public API) forwards them straight through, so any caller passing an un-remembered inline lambda produces a new instance per recomposition — and the running collector keeps calling the one captured at first composition.

Fix

Wrap both callbacks with rememberUpdatedState and call the wrappers inside the effects:

val latestOnScrollChanged by rememberUpdatedState(onScrollChanged)
val latestOnScrollFinished by rememberUpdatedState(onScrollFinished)

This is the canonical Compose pattern for "long-running effect that should not restart but must invoke the latest handler."

Why rememberUpdatedState (not keys)?

Adding the callbacks as LaunchedEffect keys would cancel and re-subscribe the snapshotFlow on every lambda identity change — wrong, because:

  • the collection lifecycle follows lazyListState / count, not callback identity;
  • re-subscribing resets distinctUntilChanged() state, so the current snapped index is re-emitted to the parent on every lambda identity change.

The callbacks are event handlers, not lifecycle identifiers, so rememberUpdatedState is the correct tool.

Impact

Callers that pass fresh lambdas now have their latest callback invoked on every scroll snap, instead of the one captured at first composition. Concretely:

  • StandardWheelDatePicker and CJKWheelDatePicker — inline lambdas capturing snappedDate plus the user-supplied onSnappedDate / onSnappedDateChanged.
  • StandardWheelTimePicker and AdaptiveWheelDateTimePicker — inline lambdas capturing snappedTime / snappedDateTime plus the user-supplied onSnappedTime / onSnappedDateTime callbacks.

Real-world trigger

When a sibling wheel rewrites snappedDate / snappedTime / snappedDateTime, the parent recomposes and passes fresh lambdas down to WheelPicker. The effect does not restart — lazyListState (from rememberLazyListState(startIndex)) is stable across snappedDate changes, and count only changes when the item count actually changes (e.g. Jan→Feb: 31→28 restarts it, but Jan→Mar: 31→31 does not). So for same-count transitions the collector keeps invoking the lambda captured at first composition, which closes over the user-supplied onSnappedDate / onSnappedDateChanged (or time equivalents) from that point — not the latest one.

Note: the library's own snappedDate / snappedTime / snappedDateTime are backed by mutableStateOf, so they are read freshly even from a stale lambda. The derived dayOfMonths list is plain remember-ed and does go stale, but its content is identical across same-day-count months (values 1..N with the same index mapping), so the staleness doesn't produce observable drift inside this library. The stale-capture path that actually matters is the user-supplied callback — a plain function type with no snapshot-state guarantee.

Test coverage

No regression test added — the repo has no Compose UI test harness today (only pure-Kotlin commonTest for locale/numeral logic). A behavioral test would require bootstrapping ui-test on a jvm target and carefully driving snapshotFlow + recomposition. Flagged as follow-up rather than blocking this fix.

…n WheelPicker

Co-authored-by: Junie <junie@jetbrains.com>
@darkokoa darkokoa merged commit 07293c8 into main Jul 7, 2026
2 checks passed
@darkokoa darkokoa deleted the fix/dkk/stale-callback-capture branch July 7, 2026 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant