Add retained chip text field state - #201
Conversation
📝 WalkthroughWalkthroughAdds Compose runtime-retain support for ChangesRetained chip text field state
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant retainChipTextFieldState
participant ComposeRetain
participant BasicChipTextField
Caller->>retainChipTextFieldState: Request retained state
retainChipTextFieldState->>ComposeRetain: Retain state with optional key
ComposeRetain->>retainChipTextFieldState: Create ChipTextFieldState
BasicChipTextField->>BasicChipTextField: Check reset flag
BasicChipTextField->>BasicChipTextField: Preserve or reset chips
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
chiptextfield-core/src/commonMain/kotlin/com/dokar/chiptextfield/ChipTextFieldState.kt (1)
48-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for both retained overloads. Cover retained state preserving chips after exit/re-entry, remembered state restoring
defaultChips, and key changes creating a fresh state from the new chips.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@chiptextfield-core/src/commonMain/kotlin/com/dokar/chiptextfield/ChipTextFieldState.kt` around lines 48 - 70, Add regression tests covering both retainChipTextFieldState overloads: verify chips survive composition exit and re-entry, remembered state restores defaultChips, and changing the keyed overload’s key creates fresh state initialized from the new chips. Reuse the existing state-test utilities and assert both preservation and replacement behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@chiptextfield-core/src/commonMain/kotlin/com/dokar/chiptextfield/ChipTextFieldState.kt`:
- Around line 54-63: Update the KDoc for retainChipTextFieldState, specifically
the key parameter documentation, to state that the key remains alive for the
lifetime of the retained value. Warn callers not to pass short-lived references
such as Context or View-backed objects.
---
Nitpick comments:
In
`@chiptextfield-core/src/commonMain/kotlin/com/dokar/chiptextfield/ChipTextFieldState.kt`:
- Around line 48-70: Add regression tests covering both retainChipTextFieldState
overloads: verify chips survive composition exit and re-entry, remembered state
restores defaultChips, and changing the keyed overload’s key creates fresh state
initialized from the new chips. Reuse the existing state-test utilities and
assert both preservation and replacement behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f697368-4e24-47ab-99e9-e933281b5df2
📒 Files selected for processing (4)
chiptextfield-core/build.gradlechiptextfield-core/src/commonMain/kotlin/com/dokar/chiptextfield/BasicChipTextField.ktchiptextfield-core/src/commonMain/kotlin/com/dokar/chiptextfield/ChipTextFieldState.ktgradle/libs.versions.toml
| /** | ||
| * Return a [ChipTextFieldState] that is retained after leaving the composition, recreating it when | ||
| * [key] changes. | ||
| * | ||
| * Retained state can survive configuration changes, but it does not survive process death. Use | ||
| * [rememberChipTextFieldState] when the state should only live as long as the current composition. | ||
| * | ||
| * @param chips Default chips | ||
| * @param key An input that identifies the retained state. When this value changes, the previous | ||
| * state is discarded and a new state is created with [chips]. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the target file and inspect the relevant section with line numbers.
sed -n '1,220p' chiptextfield-core/src/commonMain/kotlin/com/dokar/chiptextfield/ChipTextFieldState.ktRepository: dokar3/ChipTextField
Length of output: 6070
🌐 Web query:
androidx.compose.runtime.retain retain(key) key lifetime documentation short-lived reference Context View leak
💡 Result:
The retain API, introduced in androidx.compose.runtime:runtime-retain [1][2], is a mechanism for state memoization that provides a lifespan longer than remember but shorter than persistence solutions like rememberSaveable [1]. While remember is scoped to the immediate composition hierarchy, retained values are stored in a RetainedValuesStore and can persist through transient destruction and recreation (e.g., configuration changes) [3][1]. Crucially, retained values persist longer than the lifespan of the specific composable that created them [3]. This extended lifetime introduces significant risks of memory leaks if not managed correctly [3]. Memory Leak Prevention Guidelines: 1. Avoid Retaining Context and Views: Never retain an Android Context or any object that references a Context, either directly or indirectly [3]. This includes Views, as they hold references to the Context that created them [3]. A rule of thumb is that if you would not store an object in a ViewModel, you should not retain it using the retain API [1]. 2. Key Lifetime: The keys provided to retain are referenced as long as the value is retained [1]. Ensure that these keys themselves do not inadvertently hold onto long-lived references that could prevent garbage collection. 3. Use Annotations: To prevent accidental retention of unsafe types, you can annotate your class definition with androidx.compose.runtime.annotation.DoNotRetain [3]. 4. Resource Management: If your retained object manages resources, implement RetainObserver to receive callbacks like onRetained and onRetired [4][5]. Always release resources or cancel jobs in onRetired to prevent leaks [4][6]. 5. Explicit Disposal: When using utilities like RetainedValuesStoreRegistry, ensure that you call dispose when the registry is no longer needed [6]. Failure to do so may result in leaked memory from undispatched RetainObserver callbacks [6]. In summary, retain is a powerful tool for maintaining state across configuration changes without serialization, but it effectively acts as a cache that lives as long as the app process (or the associated RetainedValuesStore). Treat it with the same memory-safety caution as you would a ViewModel [1].
Citations:
- 1: https://developer.android.com/develop/ui/compose/state-lifespans
- 2: https://android.googlesource.com/platform/frameworks/support/+/c92f111c2d6ae8e4cea1537c9c3ee37af5304041
- 3: https://developer.android.com/reference/kotlin/androidx/compose/runtime/retain/retain.composable
- 4: https://developer.android.com/develop/ui/compose/state-callbacks
- 5: https://developer.android.google.cn/reference/kotlin/androidx/compose/runtime/retain/RetainObserver
- 6: https://developer.android.com/reference/kotlin/androidx/compose/runtime/retain/RetainedValuesStoreRegistry
Document the retained key lifetime constraint.
retainChipTextFieldState(..., key) keeps the key alive as long as the retained value, so avoid passing short-lived references like Context or View-backed objects.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@chiptextfield-core/src/commonMain/kotlin/com/dokar/chiptextfield/ChipTextFieldState.kt`
around lines 54 - 63, Update the KDoc for retainChipTextFieldState, specifically
the key parameter documentation, to state that the key remains alive for the
lifetime of the retained value. Warn callers not to pass short-lived references
such as Context or View-backed objects.
|
Top, thank you! |
Happy to help! |
2 participants
Add new APIs:
Close #184
Summary by CodeRabbit
New Features
Bug Fixes
Chores