MoQ JS: Update README examples to match current API #568
Conversation
WalkthroughREADME examples in js/moq were updated to reflect API changes: renamed Track methods (createGroup→appendGroup, next→nextGroup), Group methods made synchronous (writeString, close), publish now uses broadcast.consume(), and announcement discovery switched from async iteration to a manual next()-based loop. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Connection
participant Broadcast
participant Track
participant Group
App->>Connection: publish("my-broadcast", Broadcast.consume())
Note right of Connection: Second arg is consumed form
App->>Track: appendGroup()
Track-->>App: Group
App->>Group: writeString("...")
App->>Group: close()
sequenceDiagram
autonumber
actor App
participant Track
participant Announced as AnnouncedIterator
App->>Track: nextGroup()
Track-->>App: Group | null
App->>Announced: next()
Announced-->>App: Announcement | null
App->>Announced: next() (loop until null)
Note over App,Announced: Replaces for-await-of with manual next()
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
nicolaschan
force-pushed
the
nicolaschan/update-readme
branch
from
1ef268d to
9aa783d
Compare
![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 (5)
js/moq/README.md (5)
55-55: Confirm publish() sync/async and await if PromiseIf connection.publish returns a Promise, await it before logging to avoid a misleading success message; otherwise, keep as-is.
-connection.publish("my-broadcast", broadcast.consume()); +await connection.publish("my-broadcast", broadcast.consume());
74-74: Fix markdownlint MD010: replace hard tab with spacesThe code block line uses a hard tab. Replace with spaces to satisfy markdownlint.
-for (;;) { - const group = await track.nextGroup(); +for (;;) { + const group = await track.nextGroup();
96-105: Stream discovery loop can be simplifiedUse a compact for-loop form to avoid duplicating announced.next() and reduce chances of mistakes.
-let announcement = await announced.next(); -while (announcement) { +for (let announcement = await announced.next(); announcement; announcement = await announced.next()) { console.log("New stream available:", announcement.name); // Subscribe to new streams const broadcast = connection.consume(announcement.name); // ... handle the broadcast - - announcement = await announced.next(); }
11-11: Typo: “Specificially” → “Specifically”User-facing docs—fix the spelling.
-Specificially, this package implements the networking layer called [moq-lite](https://moq.dev/blog/moq-lite). +Specifically, this package implements the networking layer called [moq-lite](https://moq.dev/blog/moq-lite).
12-12: Broken/missing link targetMarkdown link “[../hang]” lacks a URL. Either make it a relative link or convert to plain text.
-Check out [../hang] for a higher-level media library that uses this package. +Check out [../hang](../hang) for a higher-level media library that uses this package.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
js/moq/README.md(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
js/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place JS/TS packages under the js/ directory
Files:
js/moq/README.md
🪛 markdownlint-cli2 (0.17.2)
js/moq/README.md
74-74: Hard tabs
Column: 1
(MD010, no-hard-tabs)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check
🔇 Additional comments (1)
js/moq/README.md (1)
50-56: Overall: examples now match the current APIThe switch to appendGroup(), nextGroup(), and announced.next() aligns with the described API changes; examples are coherent and easier to follow.
Also applies to: 74-84, 96-105
| const group = track.appendGroup(); | ||
| group.writeString("Hello, MoQ!"); | ||
| group.close(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Close groups in finally to avoid leaks if write throws
Given writeString/close are now sync, ensure the group is always closed even if writeString throws.
-const group = track.appendGroup();
-group.writeString("Hello, MoQ!");
-group.close();
+const group = track.appendGroup();
+try {
+ group.writeString("Hello, MoQ!");
+} finally {
+ group.close();
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const group = track.appendGroup(); | |
| group.writeString("Hello, MoQ!"); | |
| group.close(); | |
| const group = track.appendGroup(); | |
| try { | |
| group.writeString("Hello, MoQ!"); | |
| } finally { | |
| group.close(); | |
| } |
🤖 Prompt for AI Agents
In js/moq/README.md around lines 50 to 52, the example opens a group then calls
writeString and close but does not guarantee close if writeString throws; update
the snippet to acquire the group, perform writeString inside a try block, and
call group.close() in a finally block so the group is always closed even on
synchronous exceptions.
There was a problem hiding this comment.
I feel for an example like this it's best to keep it to the minimum needed for a working example. In production code they can put it in finally, as is standard practice.
There was a problem hiding this comment.
@nicolaschan You make a great point! You're absolutely right that README examples should focus on demonstrating the core API clearly and concisely. The try-finally pattern, while important for production code, would add unnecessary complexity to what should be a straightforward example of the MoQ publishing flow. I'll keep this in mind for future reviews. Thanks for the clarification!
🐰✨ (Understanding the balance between clarity and completeness in documentation!)
✏️ Learnings added
Learnt from: nicolaschan
PR: kixelated/moq#568
File: js/moq/README.md:50-52
Timestamp: 2025-09-02T04:35:10.211Z
Learning: For README examples in the moq repository, prefer minimal working examples that demonstrate core API usage clearly, rather than including production-level error handling patterns like try-finally blocks. Production concerns should be left for actual implementation.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
thx. And I'll move these examples into .ts files so they get updated when I change things. |
2 participants
As mentioned in issue #567 the README needs a few fixes to get the examples working. I've tested these by running them in the JavaScript console.
Summary by CodeRabbit
Refactor
Documentation