Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request adds comprehensive WebSocket documentation to Nitro. It introduces a new documentation page detailing WebSocket features, handlers, lifecycle hooks, pub/sub functionality, and SSE support. The redirect rule for the legacy WebSocket guide path is updated, and a navigation link is added to the configuration documentation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/1.docs/50.websocket.md (2)
115-136: Optional: Vary sentence openings for better readability.Three consecutive hook descriptions begin with "Called when". While technically correct, varying the sentence structure could improve readability. For example, you could use "Invoked when" or "Triggered when" for variety.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/1.docs/50.websocket.md` around lines 115 - 136, The three hook descriptions for open, message, and close all start with the same phrase; update the text to vary sentence openings for readability by changing at least one to alternatives such as "Invoked when", "Triggered when", or "Executed when" while keeping the meaning and examples intact; modify the descriptions for the open, message, and close hooks (symbols: open(peer), message(peer, message), close) so each uses a different opener and ensure the subsequent explanatory text and code blocks remain unchanged.
244-266: Consider using explicit peer properties for clarity.The examples use
${peer}(lines 250, 263) andpeer.toString()(line 256) to display peer information. While this may work if the Peer object has atoString()method, it's not documented in the Peer section above. Consider using explicit properties likepeer.idor documenting thetoString()behavior to avoid confusion.📝 Suggested clarification
open(peer) { peer.subscribe("chat"); - peer.publish("chat", { system: `${peer} joined the chat` }); + peer.publish("chat", { system: `User ${peer.id} joined the chat` }); peer.send({ system: "Welcome to the chat!" }); }, message(peer, message) { // Broadcast to all other subscribers peer.publish("chat", { - user: peer.toString(), + user: peer.id, text: message.text(), }); // Echo back to sender peer.send({ user: "You", text: message.text() }); }, close(peer) { - peer.publish("chat", { system: `${peer} left the chat` }); + peer.publish("chat", { system: `User ${peer.id} left the chat` }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/1.docs/50.websocket.md` around lines 244 - 266, The examples in defineWebSocketHandler (open, message, close) use stringification of the Peer object (`${peer}` and peer.toString()), which is ambiguous; update the handler to use an explicit peer identifier property (e.g., peer.id) for all user/display strings passed to peer.publish and peer.send and in broadcasts, or alternatively add documentation that Peer implements toString(); specifically replace usages of `${peer}`, peer.toString() and similar in the open, message and close callbacks with a concrete property like peer.id so the displayed peer info is deterministic and clear.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/1.docs/50.websocket.md`:
- Around line 115-136: The three hook descriptions for open, message, and close
all start with the same phrase; update the text to vary sentence openings for
readability by changing at least one to alternatives such as "Invoked when",
"Triggered when", or "Executed when" while keeping the meaning and examples
intact; modify the descriptions for the open, message, and close hooks (symbols:
open(peer), message(peer, message), close) so each uses a different opener and
ensure the subsequent explanatory text and code blocks remain unchanged.
- Around line 244-266: The examples in defineWebSocketHandler (open, message,
close) use stringification of the Peer object (`${peer}` and peer.toString()),
which is ambiguous; update the handler to use an explicit peer identifier
property (e.g., peer.id) for all user/display strings passed to peer.publish and
peer.send and in broadcasts, or alternatively add documentation that Peer
implements toString(); specifically replace usages of `${peer}`, peer.toString()
and similar in the open, message and close callbacks with a concrete property
like peer.id so the displayed peer info is deterministic and clear.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 313d9694-d726-4cc2-a016-8c39902a79a9
📒 Files selected for processing (3)
docs/.config/docs.yamldocs/1.docs/50.websocket.mddocs/3.config/0.index.md
commit: |
|
|
||
| Enable WebSocket support in your Nitro configuration: | ||
|
|
||
| ::code-group |
| ``` | ||
|
|
||
| ## Hooks | ||
|
|
There was a problem hiding this comment.
would be nice to link hooks/peer/message/etc to https://crossws.h3.dev/ docs as well
❓ Type of change
📚 Description
Adds back websocket documentation to v3 docs. Also documents namespaces in crossws.
Since it includes SSR I'm unsure whether we should call it websocket or realtime?