-
Notifications
You must be signed in to change notification settings - Fork 138
Update @moq/lite docs with server-side usage #851
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe README for 🚥 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@js/lite/README.md`:
- Around line 29-33: Replace the unhyphenated heading and occurrences of the
phrase "Server side usage" with the compound adjective "Server-side usage" for
grammar and consistency; update the section heading "Server side usage" and any
example/list item instances (e.g., the occurrence around line showing the
examples and the one at line 64) so they read "Server-side usage".
- Around line 31-33: Update the README wording around WebSocket support so it
distinguishes client vs server support in Node: mention that WebTransport is
unavailable in Node/Bun/Deno server environments so `@moq/lite` falls back to
WebSockets; state that Deno and Bun have built-in client and server WebSocket
support, while Node v21+ includes a built-in WebSocket client API but does not
include a built-in WebSocket server—so Node users need the ws package for
server-side WebSocket functionality (but not for client usage in v21+). Ensure
you reference `@moq/lite`, WebTransport, WebSockets, and the ws package in the
updated sentence.
- Line 64: The example description is incorrect: update the text for the "Server
side usage" link (the line showing **[Server side
usage](https://github.com/sb2702/webcodecs-examples/tree/main/src/moq-server)**)
to accurately describe the repo purpose, e.g., change "Publish from browser to a
server" to "Stream encoded video from server to browser" (or similar wording) so
it reflects server-to-browser streaming.
🧹 Nitpick comments (1)
js/lite/README.md (1)
50-53: UseglobalThisfor cross‑runtime consistency.Prefer
globalThis.WebTransport = WebTransport;to align with the earlier snippet and keep the example valid across Node/Bun/Deno.♻️ Suggested tweak
-import { WebTransport, quicheLoaded } from '@fails-components/webtransport'; -global.WebTransport = WebTransport; +import { WebTransport, quicheLoaded } from '@fails-components/webtransport'; +globalThis.WebTransport = WebTransport;
@moq/liteworks in server JS/TS environments like Node, Bun, Deno but needs some additional setup for Node. Adding some extra docs and a link to an example for server-side usage of@moq/lite