chat docs: Add jetpack compose support ( latest ) #3097
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces Jetpack Compose as a supported language variant in the chat documentation and configuration system. Kotlin version is bumped from 1.0 to 1.1, with a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/docs/chat/getting-started/android.mdx (1)
720-733: Conditional composable function call violates Compose stability rules.The pattern
room?.collectAsPresenceMembers()conditionally invokes a@Composablefunction. Compose requires that composable functions are called unconditionally and in the same order on every recomposition to maintain call-site identity and stability.Restructure to guard the null case at the start and ensure the composable is called unconditionally:
Suggested restructure
`@Composable` fun PresenceStatusUi(room: Room?) { + if (room == null) { + Text( + text = "Online: 0", + style = MaterialTheme.typography.bodyMedium, + modifier = Modifier.padding(start = 8.dp) + ) + return + } + - val membersState = room?.collectAsPresenceMembers() - val members by membersState ?: remember { mutableStateOf(emptyList()) } + val members by room.presence.collectAsPresenceMembers() LaunchedEffect(room) { room?.presence?.enter() } Text( text = "Online: ${members.size}", style = MaterialTheme.typography.bodyMedium, modifier = Modifier.padding(start = 8.dp) ) }
🤖 Fix all issues with AI agents
In `@src/pages/docs/chat/rooms/presence.mdx`:
- Around line 217-228: Remove the unused coroutineScope variable declared in
EnterPresenceComponent; since LaunchedEffect provides a coroutine scope and you
call the suspend function room.presence.enter(...) inside it, delete the val
coroutineScope = rememberCoroutineScope() line so only LaunchedEffect remains
invoking room.presence.enter(...) in EnterPresenceComponent.
In `@src/pages/docs/chat/setup.mdx`:
- Around line 137-145: The code blocks show Groovy Gradle syntax but are labeled
as kotlin/jetpack; update the block language tags to ```groovy for both blocks
(or alternatively convert the dependency lines to the Kotlin DSL form
implementation("com.ably.chat:chat:1.1.0") and
implementation("com.ably.chat:chat-extensions-compose:1.1.0") to match a
```kotlin tag). Ensure the labels and the lines for the strings 'implementation
'com.ably.chat:chat:1.1.0'' and 'implementation
'com.ably.chat:chat-extensions-compose:1.1.0'' are consistent.
🧹 Nitpick comments (8)
src/pages/docs/chat/rooms/message-reactions.mdx (2)
481-509: Consider removing duplicate code block.The
jetpackcode block (lines 481-509) is identical to thekotlincode block (lines 451-479). Since theMessageandMessageReactionSummaryinterface definitions are the same for both Kotlin and Jetpack Compose, consider whether both blocks are needed or if thekotlinblock alone would suffice (with Jetpack users seeing the Kotlin syntax-highlighted version via the alias).
644-661: Potential race condition between LaunchedEffects.The two separate
LaunchedEffectblocks for loading initial messages (lines 644-647) and subscribing to reactions (lines 649-661) run concurrently. This could result in the subscription starting before messages are loaded, potentially missing early reaction events for those messages.Consider combining into a single
LaunchedEffector usingsnapshotFlowto ensure messages are loaded before subscribing:💡 Alternative approach
LaunchedEffect(room) { // init messages messages = room.messages.history(limit = 50).items - } - - LaunchedEffect(room) { // subscribe to message reactions summary events room.messages.reactions.asFlow().collect { event ->src/pages/docs/chat/rooms/occupancy.mdx (1)
202-215: Consider aligning example with section intent.This "Current room occupancy" section describes retrieving occupancy in a "one-off call" (line 183), but
CurrentOccupancyComponentusescollectAsOccupancy()which sets up a continuous subscription. For a true one-off read, you might want to accessroom.occupancy.currentdirectly similar to the Kotlin example on line 199:💡 Suggested alternative for one-off access
`@Composable` fun CurrentOccupancyComponent(room: Room) { - val occupancy by room.occupancy.collectAsOccupancy() + var occupancy by remember { mutableStateOf(room.occupancy.current) } Text("Connections: ${occupancy.connections}") Text("Presence members: ${occupancy.presenceMembers}") }Alternatively, if
collectAsOccupancy()is the idiomatic Compose approach and provides the same functionality, consider updating the section text to clarify that for Jetpack Compose, subscribing is the recommended pattern.src/pages/docs/chat/rooms/messages.mdx (1)
65-79: Add missing imports for completeness.The code example is missing several imports that are used in the snippet. For consistency with other Jetpack examples in this PR (e.g., lines 211-228 which include full imports), consider adding the missing imports.
Suggested imports
```jetpack import com.ably.chat.extensions.compose.collectAsPagingMessagesState +import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.items +import androidx.compose.material.Text +import androidx.compose.runtime.* +import com.ably.chat.Room `@Composable` fun MyComponent(room: Room) {src/pages/docs/chat/rooms/reactions.mdx (1)
121-130: Clarify when automatic vs manual cleanup applies.The comment "Jetpack Compose handles cleanup automatically" is accurate for the
asFlow().collectpattern shown earlier (lines 54-66), but this code block demonstrates the manualsubscribe()pattern which requires explicitunsubscribe()calls. Consider clarifying this distinction in the comment.Suggested clarification
```jetpack -// Jetpack Compose handles cleanup automatically -// When using subscribe directly: +// When using asFlow().collect in LaunchedEffect, cleanup is automatic. +// When using subscribe() directly, manual unsubscribe is needed: val (unsubscribe) = room.reactions.subscribe { event -> println("Received a reaction of type ${event.reaction.name}, and metadata ${event.reaction.metadata}") }src/pages/docs/chat/rooms/presence.mdx (2)
140-149: Consider clarifying the distinction between automatic and manual cleanup.The note at line 107 states Jetpack Compose handles cleanup automatically when using
collectAsPresenceMembers(), but this code block shows manualsubscribe()usage with explicitunsubscribe(). Consider adding a brief comment explaining when developers might choose the manual approach over the automatic one.
483-499: Jetpack example differs semantically from Kotlinpresence.get()example.The Kotlin example above demonstrates a one-shot
presence.get()call, while this Jetpack example usescollectAsPresenceMembers()which is reactive/subscription-based. Consider either:
- Adding a comment clarifying that Jetpack Compose idiomatically uses reactive state
- Or showing a one-shot pattern using
LaunchedEffectwithroom.presence.get()for parityThis helps developers understand the semantic difference between the imperative and reactive approaches.
src/pages/docs/chat/rooms/typing.mdx (1)
204-220: Consider noting SDK throttling behavior in the example.The example calls
keystroke()on everyonValueChange, which aligns with the documentation at line 292 that states "callkeystroke()with every keypress, and the SDK will handle when and if to send a typing indicator."A brief inline comment in the code example could reinforce this SDK-managed behavior for developers unfamiliar with the pattern.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
src/data/languages/languageData.tssrc/data/languages/languageInfo.tssrc/data/languages/types.tssrc/pages/docs/chat/connect.mdxsrc/pages/docs/chat/getting-started/android.mdxsrc/pages/docs/chat/rooms/history.mdxsrc/pages/docs/chat/rooms/index.mdxsrc/pages/docs/chat/rooms/message-reactions.mdxsrc/pages/docs/chat/rooms/messages.mdxsrc/pages/docs/chat/rooms/occupancy.mdxsrc/pages/docs/chat/rooms/presence.mdxsrc/pages/docs/chat/rooms/reactions.mdxsrc/pages/docs/chat/rooms/typing.mdxsrc/pages/docs/chat/setup.mdx
🔇 Additional comments (47)
src/data/languages/types.ts (1)
29-29: LGTM!The
jetpacklanguage key is correctly added to thelanguageKeysarray, enabling type-safe usage across the codebase.src/pages/docs/chat/rooms/message-reactions.mdx (1)
165-200: LGTM!The
SendMessageReactionComponentfollows Compose best practices with proper use ofrememberCoroutineScope()for launching coroutines from click handlers.src/data/languages/languageInfo.ts (1)
89-93: LGTM!The
jetpacklanguage entry is correctly configured with appropriate label, Kotlin syntax highlighting, and alias.src/data/languages/languageData.ts (1)
42-43: LGTM!The version updates are appropriate - both
kotlinandjetpackare set to version1.1for the chat product, reflecting that Jetpack Compose support is built on the chat-kotlin SDK.src/pages/docs/chat/rooms/occupancy.mdx (3)
62-75: LGTM!The
OccupancyComponentcorrectly usescollectAsOccupancy()for reactive occupancy updates in a Compose-idiomatic way.
262-281: LGTM!The
GetOccupancyComponentcorrectly demonstrates one-off occupancy retrieval usingLaunchedEffectwithroom.occupancy.get().
145-147: LGTM!Helpful lifecycle note clarifying that Compose handles cleanup automatically when using
collectAsOccupancy().src/pages/docs/chat/rooms/messages.mdx (8)
20-22: LGTM!Clear introduction of the Jetpack Compose approach with
collectAsPagingMessagesState()as the primary method and mentioning the alternativemessages.subscribe()option.
130-132: LGTM!Correctly notes that
collectAsPagingMessagesState()handles lifecycle and cleanup automatically, which is accurate for Compose's StateFlow collection patterns.
211-229: LGTM!Correct use of
rememberCoroutineScope()andcoroutineScope.launchpattern for calling the suspend functionroom.messages.send()from a button click handler.
273-275: LGTM!The Jetpack example matches the Kotlin example, which is appropriate for a simple API call demonstration.
339-363: LGTM!The Jetpack example correctly demonstrates the update pattern using
rememberCoroutineScope(). The comment clarifying thatoriginalMessageis assumed to be available is helpful for documentation purposes.
459-486: LGTM!Good demonstration of the
asFlow().collectpattern withinLaunchedEffectfor handling message events reactively in Compose. The version comparison logic for updates is correctly implemented.
586-610: LGTM!Consistent pattern with the update example, correctly using
rememberCoroutineScope()for the delete operation.
706-729: LGTM!Correct implementation of delete event handling using
filterNotwith proper version comparison to ensure only older messages are removed.src/pages/docs/chat/rooms/index.mdx (9)
73-76: LGTM!Basic room retrieval API call is appropriately identical to the Kotlin example.
127-140: LGTM!Room options configuration using the Kotlin DSL builder pattern is correctly demonstrated.
174-177: LGTM!Room release API call is appropriately identical to the Kotlin example.
228-231: LGTM!Room attach API call is appropriately identical to the Kotlin example.
259-262: LGTM!Room detach API call is appropriately identical to the Kotlin example.
295-297: LGTM!Clear introduction of the
collectAsStatus()composable function for observing room status in Jetpack Compose.
337-350: LGTM!Correct use of
collectAsStatus()with state delegation (by) and appropriate imports.
359-361: LGTM!Appropriately references
collectAsStatus()for reactive status observation.
381-398: LGTM!Good demonstration of using
LaunchedEffectkeyed onroomStatusto react to status changes. This is the idiomatic Compose approach for side effects triggered by state changes.src/pages/docs/chat/rooms/history.mdx (4)
14-16: LGTM!Clear description of using
collectAsPagingMessagesState()for paginated message history retrieval in Jetpack Compose.
78-100: LGTM!Well-structured example demonstrating
collectAsPagingMessagesState()with theorderByparameter and proper UI rendering in aLazyColumn.
120-122: LGTM!Clear description of using
historyBeforeSubscribe()for retrieving messages before subscription.
201-247: Consider simplifying the subscription and history fetch pattern.The current implementation uses separate
DisposableEffectandLaunchedEffectwith a state variable to coordinate subscription and history fetching. While this works, it's a more complex pattern.A few observations:
- The
DisposableEffect(room)correctly manages the subscription lifecycle- The
LaunchedEffect(subscription)correctly waits for subscription to be available before fetching history- The state accumulation pattern (
messages = messages + ...) creates new lists on each page, which is fine for documentation purposesThe example is functional and demonstrates the concept well.
src/pages/docs/chat/rooms/reactions.mdx (3)
54-66: LGTM!Correct use of
asFlow().collectwithinLaunchedEffectfor observing room reactions in Jetpack Compose.
87-88: LGTM!Correctly notes that Jetpack Compose handles lifecycle and cleanup automatically with
LaunchedEffect.
183-216: LGTM!Good demonstration of sending reactions with and without metadata using the
rememberCoroutineScope()pattern and thejsonObjectDSL builder.src/pages/docs/chat/connect.mdx (6)
31-33: LGTM!Clear description of using
collectAsStatus()for observing connection status in Jetpack Compose.
63-73: LGTM!Correct use of
collectAsStatus()onchatClient.connectionwith state delegation pattern.
102-104: LGTM!Appropriately references
collectAsStatus()for reactive connection status observation.
136-153: LGTM!Well-structured example demonstrating reactive connection status observation with
LaunchedEffectfor side effects on status changes.
188-190: LGTM!Clear introduction of
discontinuityAsFlow()for observing discontinuity events in Jetpack Compose.
228-243: LGTM!Correct use of
discontinuityAsFlow().collectwithinLaunchedEffectfor handling discontinuity events in Jetpack Compose.src/pages/docs/chat/rooms/presence.mdx (3)
67-83: LGTM! Clean Jetpack Compose presence subscription example.The code correctly uses the
collectAsPresenceMembers()extension function with proper state delegation viabykeyword. The composable follows Compose conventions.
289-304: LGTM! Correct coroutine usage for button click handlers.The
rememberCoroutineScope()is appropriately used here sinceonClickis not a suspend function, unlikeLaunchedEffectwhich provides its own scope.
522-532: LGTM! Correct one-shot query pattern.The
LaunchedEffectkeyed on bothroomandclientIdcorrectly triggers a re-query when either dependency changes. The state management withmutableStateOfis appropriate for this use case.src/pages/docs/chat/getting-started/android.mdx (2)
255-269: LGTM! Idiomatic Flow collection in Compose.The change from
DisposableEffectwith manual subscription toLaunchedEffectwith Flow collection (room?.messages?.asFlow()?.collect) is the recommended pattern for Compose. The coroutine automatically cancels when the effect leaves composition.
544-569: LGTM! Correct pattern for subscription with history.The two-phase approach is appropriate:
DisposableEffectestablishes the subscription and stores the referenceLaunchedEffect(subscription)fetches historical messages once the subscription is readyThis ensures messages before the subscription point are retrieved without race conditions.
src/pages/docs/chat/rooms/typing.mdx (2)
58-71: LGTM! Clean typing indicator subscription.The
collectAsCurrentlyTyping()extension follows the same pattern as other Compose extensions. ThejoinToStringusage is appropriate for displaying the set of typing users.
344-350: LGTM! Consistent with earlier example.This component provides a focused example for the "retrieve currently typing" section, even though it's functionally similar to
TypingComponentshown earlier. The repetition is appropriate given the documentation structure.src/pages/docs/chat/setup.mdx (3)
124-131: LGTM! Dependency setup is clear.The separation of standard Kotlin and Jetpack dependencies is appropriate. The Jetpack block correctly includes both the core
chatdependency and thechat-extensions-composeextension library.
202-215: LGTM! Consistent initialization pattern.The Jetpack initialization is identical to the standard Kotlin initialization, which is correct since
ChatClientsetup doesn't differ between regular Kotlin and Jetpack Compose usage. The separate block enables proper documentation filtering.
300-311: LGTM! Logging configuration example.The Jetpack logging example correctly demonstrates the
ChatClientconfiguration lambda with customlogHandlerandlogLevelsettings.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
AndyTWF
left a comment
AndyTWF
left a comment
There was a problem hiding this comment.
If I run this in the review app, selecting jetpack as a language crashes the page with error
"console.js:36 Could not find the language 'jetpack', did you forget to load/include a language module?"
ttypic
left a comment
ttypic
left a comment
There was a problem hiding this comment.
Looks good! added couple of suggestions
| ```jetpack | ||
| @Composable | ||
| fun MyComponent(room: Room) { | ||
| var myMessageList by remember { mutableStateOf<List<Message>>(emptyList()) } |
There was a problem hiding this comment.
I believe Jetpack recommends
| var myMessageList by remember { mutableStateOf<List<Message>>(emptyList()) } | |
| val myMessageList by remember { mutableStateListOf<Message>() } |
There was a problem hiding this comment.
@coderabbitai, according to claude this won't work because
- mutableStateListOf returns SnapshotStateList, not State, so by delegation won't work
- The code later tries to reassign myMessageList = ... but it's declared as val
Can you check if this is true
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Jetpack Compose mutableStateListOf vs mutableStateOf delegation by keyword
💡 Result:
Short answer
- Use mutableStateListOf for observable, in-place mutable lists (add/remove/set notify Compose). mutableStateOf(list) only notifies when the list reference is replaced (so you must assign a new list to trigger recomposition). [1][2]
Delegation (the "by" keyword)
- "by" works with MutableState (mutableStateOf) because it delegates get/set to the state's value (var x by mutableStateOf(...)). It does NOT apply to SnapshotStateList returned by mutableStateListOf — that function returns a SnapshotStateList (a MutableList), not a MutableState, so you cannot delegate a var to it with "by". Use the list directly. [1][2]
When to use which
- If you need to mutate the list in-place (add/remove/update items and have Compose react): remember { mutableStateListOf(...) }. [2]
- If you prefer immutable updates (replace whole list references): remember { mutableStateOf(listOf(...)) } or var list by remember { mutableStateOf(listOf(...)) } and assign new lists on change. This is fine but requires creating new list instances to trigger recomposition. [1][5]
Example snippets
-
mutableStateListOf:
val names = remember { mutableStateListOf() }
names.add("Alice") // Compose observes this change. [2] -
mutableStateOf with delegation:
var names by remember { mutableStateOf(listOf("A", "B")) }
names = names + "C" // must replace reference to recompose. [1]
References
- Android Dev docs — State in Compose (mutableStateOf guidance). [1]
- API / runtime docs — mutableStateListOf returns SnapshotStateList (observable MutableList). [2]
- StackOverflow / community guidance comparing the two and recommending mutableStateListOf for mutable collections. [4]
@sacOO7, Claude is absolutely correct on both points:
-
mutableStateListOfdoesn't supportbydelegation:mutableStateListOf()returns aSnapshotStateList(which implementsMutableList), not aMutableState<T>. Thebykeyword only works with types that support property delegation (likeMutableState<T>). Soval myMessageList by remember { mutableStateListOf<Message>() }won't compile. -
Reassignment requires
var: The code later doesmyMessageList = myMessageList.filterNot { ... }, which creates and assigns a new list. Withval, this wouldn't compile (even if delegation worked).
The current code is correct for this use case:
var myMessageList by remember { mutableStateOf<List<Message>>(emptyList()) }✅mutableStateOfreturnsMutableState<List<Message>>which supports delegationvarwithbyallows the reassignment syntax to work- The pattern of creating new list instances (
myMessageList + item,myMessageList.filterNot { }) triggers recomposition
When to use mutableStateListOf:
- When you want in-place mutations (
list.add(),list.removeAt()) to trigger recomposition - Not suitable here since the code follows an immutable update pattern
The current implementation is the right approach for this scenario.
| fun PresenceStatusUi(room: Room?) { | ||
| val members = room?.collectAsPresenceMembers() | ||
| val membersState = room?.collectAsPresenceMembers() | ||
| val members by membersState ?: remember { mutableStateOf(emptyList()) } |
There was a problem hiding this comment.
I would avoid creating unnecessary "remembered" variable
| val members by membersState ?: remember { mutableStateOf(emptyList()) } | |
| val members = membersState?.value ?: emptyList() |
| @Composable | ||
| fun HistoryBeforeSubscribeComponent(room: Room) { | ||
| var messages by remember { mutableStateOf<List<String>>(emptyList()) } |
There was a problem hiding this comment.
I believe Jetpack recommends
| var messages by remember { mutableStateOf<List<String>>(emptyList()) } | |
| val myMessageList by remember { mutableStateListOf<Message>() } |
There was a problem hiding this comment.
@coderabbitai, according to claude this won't work because
- mutableStateListOf returns SnapshotStateList, not State, so by delegation won't work
- The code later tries to reassign myMessageList = ... but it's declared as val
Can you check if this is true
There was a problem hiding this comment.
@sacOO7, Claude is absolutely correct on both points:
-
mutableStateListOfreturnsSnapshotStateList<T>, notState<T>: The mutableStateListOf function returns an object of type SnapshotStateList<T>. Thebydelegation syntax requires a type that implements thegetValueandsetValueoperators (whichState<T>provides), butSnapshotStateList<T>does not support delegation. Usingbywith mutableStateListOf produces an error: "Type 'TypeVariable(T)' has no method 'getValue(Nothing?, KProperty<*>)' and thus it cannot serve as a delegate". -
Cannot reassign a
val: The current code reassignsmessagesmultiple times (messages = ...), which requiresvar, notval.
The correct way to use mutableStateListOf would be:
val messages = remember { mutableStateListOf<String>() } // No 'by', just '='
// Then use list mutation operations instead of reassignment:
messages.addAll(historicalMessages.items.map { it.text })However, your current approach with var messages by remember { mutableStateOf<List<String>>(emptyList()) } is actually perfectly valid and appropriate for this use case. It uses immutable list replacement rather than mutable list operations. Both patterns work fine in Compose—choose based on whether you prefer immutable list replacement or mutable list operations.
| Button(onClick = { | ||
| coroutineScope.launch { | ||
| // Send a 👍 reaction using the default type | ||
| room.messages.reactions.send(message, name = "👍") |
There was a problem hiding this comment.
I am thinking maybe we should wrap in try-catch block or somehow else show it can throw an unhandled exception that can lead to app crash if it won't be handled somewhere
There was a problem hiding this comment.
At the moment, we only handle the happy path for each public API method. You can find similar examples in the JavaScript and Swift implementations, so this should be sufficient.
You can check -> https://github.com/ably/docs/blob/main/src/pages/docs/chat/rooms/message-reactions.mdx#sending-a-message-reaction-
sacOO7
force-pushed
the
feature/chat-jetpack-compose-docs
branch
from
f319011 to
f7ee1df
Compare
sacOO7
force-pushed
the
feature/chat-jetpack-compose-docs
branch
from
f7ee1df to
3d685c4
Compare
sacOO7
force-pushed
the
feature/chat-jetpack-compose-docs
branch
from
3d685c4 to
dd377ed
Compare
sacOO7
force-pushed
the
feature/chat-jetpack-compose-docs
branch
from
dd377ed to
57c3e71
Compare
sacOO7
force-pushed
the
feature/chat-jetpack-compose-docs
branch
from
57c3e71 to
a72f25d
Compare
- Fixed method signatures of jetpack compose extensions - Removed jetpack alias reference to kotlin, instead will use native jetpack icon - Updated ably/ui to jetpack compatible latest version
sacOO7
force-pushed
the
feature/chat-jetpack-compose-docs
branch
from
a72f25d to
520c5c6
Compare
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.