[DX-475] Updated docs section for handling self published message#3215
[DX-475] Updated docs section for handling self published message#3215
Conversation
WalkthroughDocumentation addition to chat messages page that introduces two approaches for handling self-published messages in chat rooms: subscription-only rendering and serial field-based deduplication. Includes multi-language code examples (JavaScript, React, Swift, Kotlin, Android) demonstrating both strategies with optimistic UI patterns. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
sacOO7
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Pull request overview
This PR adds documentation for handling self-published messages in chat applications to prevent message duplication. When a message is sent using send(), the server echoes it back to all subscribers including the sender. This can cause messages to appear twice in the UI if not handled properly.
Changes:
- Added a new "Handle self-published messages" section with guidance on two approaches: only rendering from subscription (simple) or using serial-based deduplication for optimistic UI updates
- Provided code examples in JavaScript, React, Swift, Kotlin, and Android demonstrating the serial-based deduplication approach
- Included explanatory documentation about when to use each approach
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/pages/docs/chat/rooms/messages.mdx (1)
238-240: Optional: add a code example for Approach 1.Approach 1 is described only in text. A minimal code snippet (subscribe-only rendering, no optimistic add) alongside Approach 2 would make the docs consistent and let developers copy-paste either pattern without having to infer what Approach 1 looks like in code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/docs/chat/rooms/messages.mdx` around lines 238 - 240, Add a minimal code example to the "Approach 1" section showing the subscribe-only rendering pattern: show a snippet that subscribes to message events (e.g., onMessage or messagesSubscription handler), appends incoming messages to local state (e.g., setMessages or messagesRef), and calls send() without mutating the UI optimistically; reference the same send() function and the subscription listener used elsewhere in the doc so readers can copy-paste a simple pattern where send() only sends to the server and the subscription callback is solely responsible for adding messages to the local list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/docs/chat/rooms/messages.mdx`:
- Around line 233-241: Add a single clarifying sentence inside the existing
<Aside data-type='important'> block (near the explanation of send() and
subscribe()) stating that setting echoMessages=false does not disable the Chat
SDK's echo behavior—echoes are emitted by the Chat service itself and must be
deduplicated client-side (e.g., via serial or rendering only from
subscriptions). Reference the existing terms send(), subscribe(), serial, and
echoMessages=false so readers can immediately connect the caveat to the
surrounding guidance.
- Around line 297-314: The code mutates the shared var messages from two
concurrent contexts (the unstructured Task processing messagesSubscription and
the outer async scope) causing a data race; confine all mutations to the
MainActor by ensuring any append/contains on messages runs on the MainActor—wrap
the loop body inside the Task handling messagesSubscription so each iteration
does await MainActor.run { if !messages.contains(where: { $0.serial ==
message.serial }) { messages.append(message) } } and do the same for the
optimistic append (await MainActor.run { messages.append(sentMessage) }), or
alternatively annotate the surrounding UI/async scope (the function or enclosing
type that owns messages and invokes room.messages.send/write) with `@MainActor` so
both contexts perform mutations under actor isolation (reference: messages,
messagesSubscription, Task { ... }, and room.messages.send).
- Around line 262-263: The optimistic push after await room.messages.send(...)
is unconditional and can create duplicates; mirror the subscription's serial
check before adding the sent message: after calling room.messages.send(...) and
receiving message, verify the message.serial (or the same unique identifier used
by the subscription callback) is not already present in the messages collection
(e.g., check messages array for an existing entry with that serial) and only
call messages.push(message) when it's missing; apply this symmetric serial check
in the send-side code path that currently does messages.push(message).
---
Nitpick comments:
In `@src/pages/docs/chat/rooms/messages.mdx`:
- Around line 238-240: Add a minimal code example to the "Approach 1" section
showing the subscribe-only rendering pattern: show a snippet that subscribes to
message events (e.g., onMessage or messagesSubscription handler), appends
incoming messages to local state (e.g., setMessages or messagesRef), and calls
send() without mutating the UI optimistically; reference the same send()
function and the subscription listener used elsewhere in the doc so readers can
copy-paste a simple pattern where send() only sends to the server and the
subscription callback is solely responsible for adding messages to the local
list.
sacOO7
force-pushed
the
fix/chat-self-publish-message
branch
from
4e30b7c to
e1362a1
Compare
|
@coderabbitai review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <Aside data-type='important'> | ||
| When you send a message using `send()`, the server echoes it back to all subscribers in the room, including the sender. | ||
| If your application adds the message to the UI immediately after calling `send()` and also appends it when received via `subscribe()`, the message will appear twice. | ||
| To avoid this, we need to add safety check in the subscribe method that validates incoming message serial and version against existing messages. |
There was a problem hiding this comment.
Missing article "a" before "safety check". The sentence should read "we need to add a safety check in the subscribe method..."
| To avoid this, we need to add safety check in the subscribe method that validates incoming message serial and version against existing messages. | |
| To avoid this, we need to add a safety check in the subscribe method that validates incoming message serial and version against existing messages. |
| ``` | ||
|
|
||
| ```kotlin | ||
| val myMessageList: List<Messages> |
There was a problem hiding this comment.
Type should be List<Message> (singular) instead of List<Messages> (plural). The Message type is singular throughout the codebase as shown in other examples like var myMessageList by remember { mutableStateOf<List<Message>>(emptyList()) } on line 301.
| val myMessageList: List<Messages> | |
| val myMessageList: List<Message> |
| @@ -448,11 +549,17 @@ for await message in messagesSubscription { | |||
| ```kotlin | |||
| val myMessageList: List<Messages> | |||
There was a problem hiding this comment.
Type should be List<Message> (singular) instead of List<Messages> (plural). The Message type is singular throughout the codebase as shown in other examples.
| val myMessageList: List<Messages> | |
| val myMessageList: List<Message> |
| @@ -702,11 +825,17 @@ for await message in messagesSubscription { | |||
| ```kotlin | |||
| val myMessageList: List<Messages> | |||
There was a problem hiding this comment.
Type should be List<Message> (singular) instead of List<Messages> (plural). The Message type is singular throughout the codebase as shown in other examples.
| val myMessageList: List<Messages> | |
| val myMessageList: List<Message> |
| @@ -431,12 +525,19 @@ const MyComponent = () => { | |||
| let messagesList: [Message] | |||
There was a problem hiding this comment.
The messagesList is declared as let (immutable) but the code attempts to modify it with append() and index assignment operations on lines 536 and 541. This will cause a compilation error. Change let to var to make the array mutable.
| let messagesList: [Message] | |
| var messagesList: [Message] |
| @@ -685,12 +801,19 @@ const MyComponent = () => { | |||
| let messagesList: [Message] | |||
There was a problem hiding this comment.
The messagesList is declared as let (immutable) but the code attempts to modify it with append() and remove() operations on lines 812 and 817. This will cause a compilation error. Change let to var to make the array mutable.
1 participant
Summary by CodeRabbit
Documentation