feat(standard-server): server adapter for connected peers#456
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis change introduces comprehensive support for WebSocket adapters and standardizes HTTP adapter documentation and implementation across the oRPC codebase. It adds new WebSocket client and server adapters for multiple runtimes (native WebSocket, crossws, ws, Bun), with corresponding handler classes and test suites. The documentation is reorganized: new "Adapters" sections for HTTP and WebSocket replace previous "Integrations" docs for specific runtimes, and references across the docs are updated accordingly. The shared package receives new utilities for sequential function execution, async ID queues, and ID generation. A new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as oRPC Client
participant WebSocket as WebSocket Transport
participant Server as oRPC Server
Client->>WebSocket: Send encoded RPC request (via experimental_RPCLink)
WebSocket->>Server: Deliver message
Server->>Server: experimental_RPCHandler parses message
Server->>Server: Handle request (with context, plugins, etc.)
Server->>WebSocket: Send encoded response or event iterator messages
WebSocket->>Client: Deliver response/event messages
Client->>Client: experimental_LinkWebsocketClient decodes and resolves response
Possibly related PRs
Poem
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
c084523 to
6c3f753
Compare
d4a9e0e to
6c3f753
Compare
7191032 to
3c3d111
Compare
There was a problem hiding this comment.
Actionable comments posted: 14
🔭 Outside diff range comments (1)
packages/client/src/adapters/websocket/rpc-link.ts (1)
1-23: 💡 Verification agent🧩 Analysis chain
Confirm WebSocket connection lifecycle management
The implementation doesn't show how connection errors, reconnections, or connection closing is managed. This might be handled in the LinkWebsocketClient, but it's worth verifying that there's robust connection handling.
Can you confirm that the LinkWebsocketClient properly handles:
- Connection establishment errors
- Connection drops and reconnection logic
- Proper cleanup on connection close
🏁 Script executed:
#!/bin/bash # Look for connection handling in the LinkWebsocketClient class rg -A 10 -B 10 "LinkWebsocketClient" --type tsLength of output: 4603
Ensure WebSocket error handling and reconnection in LinkWebsocketClient
The
experimental_LinkWebsocketClientin
packages/client/src/adapters/websocket/link-websocket-client.tscurrently only registers handlers for the WebSocket’smessageandcloseevents. It does not:
- Listen for the
errorevent to catch connection failures- Implement any reconnection or backoff strategy on unexpected disconnects
- Explicitly cleanup or propagate errors upstream on failure
Please update this class to address these gaps:
- Add an
errorlistener (e.g.options.websocket.addEventListener('error', …)) to handle establishment or runtime errors.- Introduce a reconnection/backoff mechanism or document which layer is responsible for retries.
- Ensure resources are cleaned up and callers are notified on both
closeanderror.
♻️ Duplicate comments (2)
packages/standard-server-peer/src/event-iterator.test.ts (1)
299-304: Repeat of unsafethrowpatternSame remark as above applies here; add the
biome-ignorepragma or refactor.🧰 Tools
🪛 Biome (1.9.4)
[error] 303-303: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
packages/standard-server-peer/src/client.test.ts (1)
294-298:⚠️ Potential issueSame unsafe
throwinfinallyas server testThe generator used for the iterator again throws in a
finallyblock (lines 294-298).
Please remove/replace it as suggested earlier to avoid masking real errors and silence static-analysis warnings.🧰 Tools
🪛 Biome (1.9.4)
[error] 297-297: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
🧹 Nitpick comments (42)
apps/content/docs/integrations/remix.md (1)
8-8: Use hyphen in compound adjective
Please hyphenate “full stack JavaScript framework” as “full-stack JavaScript framework” to adhere to compound adjective style.🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ation Remix is a full stack JavaScript framework for building web a...(EN_COMPOUND_ADJECTIVE_INTERNAL)
apps/content/docs/integrations/nuxt.md (1)
8-8: Minor punctuation fix
Add a comma after “server” for clarity.
Apply this diff:-[Nuxt.js](https://nuxtjs.org/) is a popular Vue.js framework for building server-side applications. It built on top of [Nitro](https://nitro.build/) server a lightweight, high-performance Node.js runtime. +[Nuxt.js](https://nuxtjs.org/) is a popular Vue.js framework for building server-side applications. It built on top of [Nitro](https://nitro.build/) server, a lightweight, high-performance Node.js runtime.🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: A comma might be missing here.
Context: ...on top of Nitro server a lightweight, high-performance Node.js...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
apps/content/docs/client/rpc-link.md (1)
10-13: Clarify adapter examples in warningConsider specifying examples of other adapters to give readers more context (e.g., WebSocket, IPC). For instance:
:::warning - This documentation is focused on the [HTTP Adapter](/docs/adapters/http). - Other adapters may remove or change options to keep things simple. + This documentation is focused on the [HTTP Adapter](/docs/adapters/http). + Other adapters (e.g., WebSocket, IPC) may remove or modify options to simplify usage. :::This helps readers immediately connect this warning to the newly introduced WebSocket and IPC adapters.
packages/server/src/adapters/ws/index.ts (1)
1-2: Add test coverage for index fileStatic analysis indicates this index file isn’t covered by tests. Consider adding a simple test that imports from
@orpc/server/adapters/wsand asserts the presence ofWsHandlerandRPCHandlerexports to satisfy coverage requirements and prevent codecov regressions.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 2-2: packages/server/src/adapters/ws/index.ts#L2
Added line #L2 was not covered by testspackages/server/src/adapters/bun-ws/index.ts (1)
1-2: Add test coverage for Bun WebSocket adapter entrySimilarly, the
bun-wsindex needs test coverage. Add a test importing from@orpc/server/adapters/bun-wsto verify the re-exportedhandlerandrpc-handlermodules, ensuring full coverage for this entry point.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 2-2: packages/server/src/adapters/bun-ws/index.ts#L2
Added line #L2 was not covered by testspackages/shared/src/id.ts (1)
1-12: Clean implementation of ID generator with proper edge case handlingThe
SequentialIdGeneratorcorrectly implements sequential ID generation and handles the edge case atNumber.MAX_SAFE_INTEGER.Consider adding a comment about the wrapping behavior for clarity:
export class SequentialIdGenerator { private nextId = 0 generate(): number { + // When reaching MAX_SAFE_INTEGER, return it once then wrap back to 0 if (this.nextId === Number.MAX_SAFE_INTEGER) { this.nextId = 0 return Number.MAX_SAFE_INTEGER } return this.nextId++ } }packages/server/src/adapters/node/rpc-handler.test.ts (1)
21-27: Good test for verifying default security behaviorThe new test correctly verifies that StrictGetMethodPlugin is enabled by default by checking that GET requests to routes without explicit method allowance return 405 Method Not Allowed.
The non-null assertion on the response object isn't necessary:
- expect(res!.status).toEqual(405) + expect(res.status).toEqual(405)packages/shared/src/id.test.ts (1)
1-15: Tests cover all edge cases of SequentialIdGeneratorThe test adequately verifies sequential ID generation, including boundary behavior at MAX_SAFE_INTEGER and proper reset to 0.
Consider using a more explicit approach for testing the edge case instead of directly modifying the private state:
- ;(idGenerator as any).nextId = Number.MAX_SAFE_INTEGER + // Create a new generator with a controlled starting point for testing edge case + const edgeCaseGenerator = new SequentialIdGenerator(); + // Use reflection to set the internal state for testing + Object.defineProperty(edgeCaseGenerator, 'nextId', { value: Number.MAX_SAFE_INTEGER }); - expect(idGenerator.generate()).toBe(Number.MAX_SAFE_INTEGER) - expect(idGenerator.generate()).toBe(0) - expect(idGenerator.generate()).toBe(1) + expect(edgeCaseGenerator.generate()).toBe(Number.MAX_SAFE_INTEGER) + expect(edgeCaseGenerator.generate()).toBe(0) + expect(edgeCaseGenerator.generate()).toBe(1)This approach makes the test intention clearer and reduces coupling with the implementation details.
packages/standard-server-peer/package.json (1)
1-14: Add package descriptionThe package metadata is well-structured, but it's missing a description field that would help clarify the purpose of this package for users.
{ "name": "@orpc/standard-server-peer", "type": "module", "version": "0.0.0", "license": "MIT", + "description": "Standard server peer implementation for oRPC peer communication over WebSocket and IPC", "homepage": "https://unnoq.com", "repository": { "type": "git", "url": "git+https://github.com/unnoq/orpc.git", "directory": "packages/standard-server-peer" }, "keywords": [ "orpc" ],packages/shared/src/function.ts (1)
18-28: Well-implemented sequential execution utilityThe
sequentialfunction effectively ensures asynchronous function calls execute in order by chaining them on a promise. The error handling approach prevents previous errors from blocking subsequent calls.Consider adding a brief comment documenting that errors from previous calls are silently ignored to make this behavior explicit to users.
export function sequential<A extends any[], R>( fn: (...args: A) => Promise<R>, ): (...args: A) => Promise<R> { let lastOperationPromise: Promise<any> = Promise.resolve() return (...args: A): Promise<R> => { + // Chain to previous promise but ignore errors to prevent blocking subsequent calls return lastOperationPromise = lastOperationPromise.catch(() => { }).then(() => { return fn(...args) }) } }packages/client/src/adapters/websocket/rpc-link.test.ts (2)
25-41: Consider strengthening the success path test.The test correctly verifies the request encoding and response handling, but relies on setTimeout(0) for async operations which can be fragile.
Consider using more explicit async/await patterns or a more deterministic approach to handle asynchronous testing.
- await new Promise(resolve => setTimeout(resolve, 0)) + // Wait for the next event loop tick more explicitly + await Promise.resolve()Additionally, consider adding assertions to verify that the WebSocket event listeners are properly set up and that the message is correctly encoded before sending.
1-50: Consider adding tests for additional edge cases.The current tests cover the basic happy path and connection closure, but there are additional edge cases that would be valuable to test:
- Handling malformed responses
- Network errors during communication
- Timeout handling
- Multiple concurrent requests
Would you like me to suggest implementation examples for any of these test cases?
packages/standard-server-peer/README.md (1)
69-72: Add alt text to the sponsors image for better accessibility.The sponsors image is missing alternative text, which is important for accessibility. Consider adding descriptive alt text.
- <img src='https://cdn.jsdelivr.net/gh/unnoq/unnoq/sponsors.svg'/> + <img src='https://cdn.jsdelivr.net/gh/unnoq/unnoq/sponsors.svg' alt='oRPC sponsors'/>🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
71-71: Images should have alternate text (alt text)
null(MD045, no-alt-text)
packages/shared/src/iterator.ts (2)
3-9:isAsyncIteratorObjectonly checks for the symbol – consider a stronger guard
Right now any plain object that merely exposes aSymbol.asyncIteratorfunction is accepted, even if the returned iterator lacksnext,return, orthrow. This may cause subtle runtime failures when such a pseudo-iterator is consumed.
A lightweight enhancement is to additionally verify that the result ofobj[Symbol.asyncIterator]()is an object with anextmethod.-return Symbol.asyncIterator in maybe && typeof maybe[Symbol.asyncIterator] === 'function' +return Symbol.asyncIterator in maybe + && typeof maybe[Symbol.asyncIterator] === 'function' + && typeof (maybe as any)[Symbol.asyncIterator]()?.next === 'function'
67-75: Property-key expression forasyncDisposeis hard to read and may mis-compile
The current key …async [(Symbol as any).asyncDispose as typeof Symbol extends { asyncDispose: infer T } ? T : any ?? Symbol.for('asyncDispose')] ()… mixes value‐level and type-level assertions with a
??fallback, making precedence unclear and hampering maintainability. Wrapping it in a helper constant keeps the intent obvious and avoids potential edge-case errors whenSymbol.asyncDisposeisundefined.-async [(Symbol as any).asyncDispose as typeof Symbol extends { asyncDispose: infer T } ? T : any ?? Symbol.for('asyncDispose')]() { +const asyncDisposeKey: symbol = + (Symbol as any).asyncDispose ?? Symbol.for('asyncDispose') + +async [asyncDisposeKey]() {packages/server/src/adapters/ws/handler.ts (2)
20-22: Avoid the extraBloballocation in Node runtimes
wsalready deliversBuffer,ArrayBufferorstring. Re-packing into aBlobadds an unnecessary copy and requires Node ≥ 18 (or a polyfill). Passing the raw data topeer.message(and letting it acceptArrayBuffer | Uint8Array | string) will be faster and more portable.
34-36: Detach listeners on close to prevent potential leaks
You close the peer but leave themessagelistener attached. While no more frames should arrive after'close', explicitly removing the handler (ws.removeEventListener('message', handler)) makes the lifecycle explicit and future-proof (e.g., if someone re-uses the same ws instance).packages/shared/src/iterator.test.ts (1)
168-181: Overriding the globalSymbolobject is risky – isolate viavi.spyOninstead
ReplacingglobalThis.Symbolwith a stub can break other tests that rely on native symbol behaviour if an assertion fails before thefinallyblock restores it. Prefer spying onSymbol.foronly:const spy = vi.spyOn(Symbol, 'for').mockImplementation((key) => { expect(key).toBe('asyncDispose') return fallbackSymbol }) // … test … spy.mockRestore()This keeps global state intact even on early errors.
packages/server/src/adapters/ws/rpc-handler.test.ts (2)
36-52:ping_request_messageandping_buffer_request_messageappear identicalThese two test message variables have identical content but different names. If they're meant to test different message formats, consider making them distinctly different or adding a comment explaining why they're duplicated.
8-11: Test setup could use a more descriptive variable initializationThe
signalvariable is declared but initialized withundefined as any, which is later referenced in tests. Consider initializing it with a more type-safe approach.- signal = undefined as any + signal = new AbortController().signalpackages/server/src/adapters/websocket/handler.ts (1)
15-15: Consider adding prefix support in WebSocket handlerThe handler omits the
prefixoption fromFriendlyStandardHandleOptions, but doesn't provide an alternative way to set a prefix. If prefixing is needed for routing, consider accepting it as a constructor parameter or through theupgrademethod.apps/content/docs/adapters/websocket.md (1)
117-132: Add example of handling connection errors and reconnectionThe client adapter section would benefit from an example of how to handle connection errors and implement reconnection logic. While the tip about partysocket is helpful, a basic example would make the documentation more complete.
// Example with basic error handling and reconnection import { experimental_RPCLink as RPCLink } from '@orpc/client/websocket' let websocket: WebSocket let link: typeof RPCLink.prototype function connectWebSocket() { websocket = new WebSocket('ws://localhost:3000') websocket.onopen = () => { console.log('Connected to WebSocket server') link = new RPCLink({ websocket }) } websocket.onclose = () => { console.log('Connection closed, attempting to reconnect...') setTimeout(connectWebSocket, 3000) } websocket.onerror = (error) => { console.error('WebSocket error:', error) } } connectWebSocket()packages/server/src/adapters/bun-ws/rpc-handler.test.ts (2)
5-11: Avoid usingundefined as anyfor typingThe test setup initializes
signalwithundefined as any, which bypasses TypeScript's type checking. Consider using a more precise type definition or initializing it in a type-safe way.- signal = undefined as any + signal = null as unknown as AbortSignalAlternatively, you could type the variable with
AbortSignal | nulland handle the null case appropriately in your tests.
38-45: Consider simplifying the binary message encodingThe file_request_message object creation is more complex than necessary by manually encoding the message. Since
encodeRequestMessageis already being used and can return a string that could be encoded to a buffer, consider simplifying this.- const file_request_message = { - buffer: new TextEncoder().encode(await encodeRequestMessage(19, MessageType.REQUEST, { - url: new URL('orpc:/file'), - body: { json: 'input' }, - headers: {}, - method: 'POST', - }) as string), - } + const encoded = await encodeRequestMessage(19, MessageType.REQUEST, { + url: new URL('orpc:/file'), + body: { json: 'input' }, + headers: {}, + method: 'POST', + }) as string + const file_request_message = { + buffer: new TextEncoder().encode(encoded), + }packages/client/src/adapters/websocket/rpc-link.ts (1)
16-22: Consider providing more explanation for the 'orpc:/' URLThe constructor passes 'orpc:/' as the URL to the super constructor, but it's not immediately clear why this particular value is used or what it represents.
Consider adding a comment explaining the purpose of this URL scheme:
constructor(options: experimental_RPCLinkOptions<T>) { const linkClient = new LinkWebsocketClient(options) + // Using 'orpc:/' as a base URL since actual connection details + // are handled by the WebSocket client rather than URL parameters super(linkClient, { ...options, url: 'orpc:/' }) }packages/client/src/adapters/websocket/link-websocket-client.ts (3)
13-23: Consider adding error event handling and connection management.The constructor sets up message and close event handlers, but there's no handling for connection errors or connection state management.
constructor(options: experimental_LinkWebsocketClientOptions) { this.peer = new ClientPeer(options.websocket.send.bind(options.websocket)) options.websocket.addEventListener('message', (event) => { this.peer.message(event.data) }) options.websocket.addEventListener('close', () => { this.peer.close() }) + + options.websocket.addEventListener('error', (event) => { + // Handle WebSocket errors + console.error('WebSocket error:', event) + }) }
25-28: The Promise.resolve wrapping is redundant for an already resolved value.In
body: () => Promise.resolve(response.body), thePromise.resolveis unnecessary ifresponse.bodyis already a value and not a promise.However, if the intention is to ensure consistent asynchronous behavior regardless of whether
response.bodyis already available, this approach is valid. You might want to add a comment explaining this design choice.
10-12: Consider adding a method to explicitly close the connection.The class lacks a method to explicitly close the WebSocket connection, which would be useful for clean resource management.
export class experimental_LinkWebsocketClient<T extends ClientContext> implements StandardLinkClient<T> { private readonly peer: ClientPeer + private readonly websocket: Pick<WebSocket, 'addEventListener' | 'send' | 'close'> constructor(options: experimental_LinkWebsocketClientOptions) { + this.websocket = options.websocket this.peer = new ClientPeer(options.websocket.send.bind(options.websocket)) // ... } + close(): void { + if ('close' in this.websocket) { + (this.websocket as WebSocket).close() + } + this.peer.close() + } }packages/server/src/adapters/crossws/rpc-handler.test.ts (2)
13-25: Consider separating handler declarations for better test isolation.The handler is currently declared once for all tests. For better test isolation, consider declaring handlers within each test or in a beforeEach block.
8-11: Avoid usingas anyfor type assertions.Using
signal = undefined as anyis a type safety escape hatch. Consider creating a proper mock object or using a more type-safe approach.- signal = undefined as any + signal = new AbortController().signalpackages/server/src/adapters/websocket/rpc-handler.test.ts (1)
8-11: Consider using a proper AbortSignal mock.Using
signal = undefined as anyis a type-safety escape hatch. For better type safety:- signal = undefined as any + signal = new AbortController().signalpackages/shared/src/queue.ts (2)
23-45: Minor memory-safety concern: staleitemsmay accumulate
push()appends toitemswhen no consumer is waiting. However, a queue may remain open indefinitely, allowing unbounded growth if the consumer never pulls.
Although the caller is responsible for balancingpush/pull, it may be worth:
- Documenting that
itemsis unbounded and must be drained, or- Exposing an optional
maxBufferSizewith back-pressure (reject or overwrite) semantics.This helps avoid silent memory leaks in long-running servers.
76-88: Shadowed variable name slightly reduces clarityInside
close({ id, reason }), theforEach((pendingPulls, id) => …)callback parameter shadows the outerid. While technically safe, it hurts readability and can confuse debugging.- this.pendingPulls.forEach((pendingPulls, id) => { + this.pendingPulls.forEach((pendingPulls, queueId) => { pendingPulls.forEach(([, reject]) => { - reject(reason ?? new Error(`[AsyncIdQueue] Queue[${id}] was closed or aborted while waiting for pulling.`)) + reject(reason ?? new Error(`[AsyncIdQueue] Queue[${queueId}] was closed or aborted while waiting for pulling.`)) }) })A small rename avoids the shadow.
packages/standard-server-peer/src/event-iterator.ts (3)
7-11: Expose proper generic signature for consumers
toEventIteratorcurrently returnsAsyncGeneratorwithout type parameters, so consumers lose type safety onvalueandreturn. Declaring the generics keeps the strong typing propagated:-export function toEventIterator( +export function toEventIterator<T = unknown, TReturn = unknown>( queue: AsyncIdQueue<EventIteratorPayload>, id: number, cleanup: CreateAsyncIteratorObjectCleanupFn, -): AsyncGenerator { +): AsyncGenerator<T, TReturn, unknown> {This matches the runtime behaviour while preserving developer ergonomics.
66-72: Preserve unknown error informationWhen
iterator.next()throws something other thanErrorEvent, the original error object is discarded (data: undefined). This hides diagnostics upstream. Consider forwarding it:- data: err instanceof ErrorEvent ? err.data : undefined, + data: err instanceof ErrorEvent ? err.data : err,Callers who care can still pattern-match on the payload type.
82-90: Silent failure initerator.return()can mask issuesErrors inside
iterator.return()are swallowed. While preventing double-faults is good, completely ignoring failures makes debugging tough. At minimum emit a console/debug log, or wrap incauseso upstream can inspect:try { await iterator.return?.() } catch (err) { logger?.warn('iterator.return() failed', err) }This keeps the flow silent in production but traceable when needed.
packages/standard-server-peer/src/event-iterator.test.ts (2)
249-255: Silence Biome’snoUnsafeFinallywithout throwingThrowing inside
finallyintentionally tests cleanup-silencing, but Biome flags it as unsafe. A targeted directive keeps intent while passing lint:-finally { - isCleanupCalled = true - - // eslint-disable-next-line no-unsafe-finally - throw new Error('this should silence ignored') -} +/* biome-ignore lint/correctness/noUnsafeFinally */ +finally { + isCleanupCalled = true + throw new Error('this should silence ignored') +}Alternatively, assert via a side-effect rather than a throw if the exception itself isn’t required.
🧰 Tools
🪛 Biome (1.9.4)
[error] 253-253: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
1-5: Explicitly import thevitest utility for clarity
viis available globally in Vitest, but an explicit import (import { vi } from 'vitest') improves editor intellisense and avoids surprises if globals are disabled in the config.packages/server/package.json (1)
43-62: Confirm sub-path export parity & tree-shaking friendlinessGreat to see the new adapter entry points added. Two small verifications:
Ensure each path listed here also exists in the root-level
"exports"map (they do 👍), and that the relative paths actually get emitted by the build.Consider marking seldom-used adapters (
crossws,bun-ws) as"sideEffects": falsein their individualpackage.jsons so bundlers can tree-shake unused code.Otherwise looks good.
packages/standard-server-peer/src/client.ts (1)
30-36: Consider surfacingencodeRequestMessagefailures upstream
encodeRequestMessagereturns a promise that can reject (e.g. blob→arrayBuffer issues).
Right now the rejection is silently swallowed inside the wrapper and will only surface through the outersend()call if it manages to execute. Surfacing the error earlier (or wrapping it in a richer error type) would greatly simplify debugging transport-layer issues.packages/standard-server-peer/src/codec.ts (2)
266-303: Avoiddeleteon hot-path objectsUsing
deleteon plain objects de-opts V8’s hidden-class and hurts performance.
A tiny micro-optimisation here pays off becauseprepareBodyAndHeadersForSerialization()is on the critical path for every message.- delete headers['content-type'] - delete headers['content-disposition'] + headers['content-type'] = undefined + headers['content-disposition'] = undefinedIf preserving enumerable-key order matters, consider
const {['content-type']: _, ['content-disposition']: __, ...rest} = headersinstead.🧰 Tools
🪛 Biome (1.9.4)
[error] 273-273: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 274-274: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
317-328: Delimiter choice is good – document maximum JSON size
0xFFis indeed impossible in valid UTF-8. However, because the algorithm scans the entire buffer withUint8Array.indexOf(), very large JSON headers could still trigger O(N) behaviour on every frame.
Consider documenting (or enforcing) a practical upper limit for header-JSON size (e.g. 8 KiB) to protect against DoS vectors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/content/.vitepress/assets/nodejs-logo-icon.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (78)
apps/content/.vitepress/config.ts(3 hunks)apps/content/docs/adapters/http.md(1 hunks)apps/content/docs/adapters/websocket.md(1 hunks)apps/content/docs/client/rpc-link.md(1 hunks)apps/content/docs/comparison.md(1 hunks)apps/content/docs/getting-started.md(1 hunks)apps/content/docs/integrations/bun.md(0 hunks)apps/content/docs/integrations/cloudflare-workers.md(0 hunks)apps/content/docs/integrations/deno.md(0 hunks)apps/content/docs/integrations/elysia.md(1 hunks)apps/content/docs/integrations/express.md(1 hunks)apps/content/docs/integrations/fetch-server.md(0 hunks)apps/content/docs/integrations/hono.md(1 hunks)apps/content/docs/integrations/nextjs.md(1 hunks)apps/content/docs/integrations/node.md(0 hunks)apps/content/docs/integrations/nuxt.md(1 hunks)apps/content/docs/integrations/react-native.md(1 hunks)apps/content/docs/integrations/remix.md(1 hunks)apps/content/docs/integrations/solid-start.md(1 hunks)apps/content/docs/integrations/svelte-kit.md(1 hunks)apps/content/docs/integrations/tanstack-start.md(1 hunks)apps/content/docs/plugins/strict-get-method.md(1 hunks)apps/content/docs/rpc-handler.md(2 hunks)packages/client/package.json(2 hunks)packages/client/src/adapters/standard/rpc-link.test.ts(2 hunks)packages/client/src/adapters/websocket/index.ts(1 hunks)packages/client/src/adapters/websocket/link-websocket-client.ts(1 hunks)packages/client/src/adapters/websocket/rpc-link.test.ts(1 hunks)packages/client/src/adapters/websocket/rpc-link.ts(1 hunks)packages/openapi/src/adapters/node/openapi-handler.ts(1 hunks)packages/server/package.json(3 hunks)packages/server/src/adapters/bun-ws/handler.ts(1 hunks)packages/server/src/adapters/bun-ws/index.ts(1 hunks)packages/server/src/adapters/bun-ws/rpc-handler.test.ts(1 hunks)packages/server/src/adapters/bun-ws/rpc-handler.ts(1 hunks)packages/server/src/adapters/crossws/handler.ts(1 hunks)packages/server/src/adapters/crossws/index.ts(1 hunks)packages/server/src/adapters/crossws/rpc-handler.test.ts(1 hunks)packages/server/src/adapters/crossws/rpc-handler.ts(1 hunks)packages/server/src/adapters/fetch/rpc-handler.test.ts(1 hunks)packages/server/src/adapters/fetch/rpc-handler.ts(1 hunks)packages/server/src/adapters/node/rpc-handler.test.ts(1 hunks)packages/server/src/adapters/node/rpc-handler.ts(1 hunks)packages/server/src/adapters/standard/rpc-handler.test.ts(0 hunks)packages/server/src/adapters/standard/rpc-handler.ts(1 hunks)packages/server/src/adapters/websocket/handler.ts(1 hunks)packages/server/src/adapters/websocket/index.ts(1 hunks)packages/server/src/adapters/websocket/rpc-handler.test.ts(1 hunks)packages/server/src/adapters/websocket/rpc-handler.ts(1 hunks)packages/server/src/adapters/ws/handler.ts(1 hunks)packages/server/src/adapters/ws/index.ts(1 hunks)packages/server/src/adapters/ws/rpc-handler.test.ts(1 hunks)packages/server/src/adapters/ws/rpc-handler.ts(1 hunks)packages/shared/src/function.test.ts(2 hunks)packages/shared/src/function.ts(1 hunks)packages/shared/src/id.test.ts(1 hunks)packages/shared/src/id.ts(1 hunks)packages/shared/src/index.ts(1 hunks)packages/shared/src/iterator.test.ts(2 hunks)packages/shared/src/iterator.ts(1 hunks)packages/shared/src/queue.test.ts(1 hunks)packages/shared/src/queue.ts(1 hunks)packages/standard-server-peer/.gitignore(1 hunks)packages/standard-server-peer/README.md(1 hunks)packages/standard-server-peer/package.json(1 hunks)packages/standard-server-peer/src/client.test.ts(1 hunks)packages/standard-server-peer/src/client.ts(1 hunks)packages/standard-server-peer/src/codec.test.ts(1 hunks)packages/standard-server-peer/src/codec.ts(1 hunks)packages/standard-server-peer/src/event-iterator.test.ts(1 hunks)packages/standard-server-peer/src/event-iterator.ts(1 hunks)packages/standard-server-peer/src/index.ts(1 hunks)packages/standard-server-peer/src/server.test.ts(1 hunks)packages/standard-server-peer/src/server.ts(1 hunks)packages/standard-server-peer/src/types.ts(1 hunks)packages/standard-server-peer/tests/e2e.test.ts(1 hunks)packages/standard-server-peer/tsconfig.json(1 hunks)tsconfig.json(1 hunks)
💤 Files with no reviewable changes (6)
- apps/content/docs/integrations/cloudflare-workers.md
- apps/content/docs/integrations/fetch-server.md
- packages/server/src/adapters/standard/rpc-handler.test.ts
- apps/content/docs/integrations/deno.md
- apps/content/docs/integrations/bun.md
- apps/content/docs/integrations/node.md
🧰 Additional context used
🧬 Code Graph Analysis (15)
packages/shared/src/function.test.ts (1)
packages/shared/src/function.ts (1)
sequential(18-28)
packages/server/src/adapters/ws/rpc-handler.ts (3)
packages/server/src/context.ts (1)
Context(1-1)packages/server/src/router.ts (1)
Router(12-17)packages/server/src/adapters/standard/rpc-handler.ts (2)
StandardRPCHandlerOptions(10-11)StandardRPCHandler(13-22)
packages/server/src/adapters/websocket/rpc-handler.ts (3)
packages/server/src/context.ts (1)
Context(1-1)packages/server/src/router.ts (1)
Router(12-17)packages/server/src/adapters/standard/rpc-handler.ts (2)
StandardRPCHandlerOptions(10-11)StandardRPCHandler(13-22)
packages/client/src/adapters/websocket/rpc-link.ts (1)
packages/client/src/adapters/standard/rpc-link.ts (2)
StandardRPCLinkOptions(11-12)StandardRPCLink(14-22)
packages/server/src/adapters/crossws/rpc-handler.ts (3)
packages/server/src/context.ts (1)
Context(1-1)packages/server/src/router.ts (1)
Router(12-17)packages/server/src/adapters/standard/rpc-handler.ts (2)
StandardRPCHandlerOptions(10-11)StandardRPCHandler(13-22)
packages/server/src/adapters/bun-ws/rpc-handler.test.ts (2)
packages/server/src/builder.ts (2)
handler(273-280)os(336-352)packages/standard-server-peer/src/codec.ts (2)
encodeRequestMessage(92-136)decodeResponseMessage(223-264)
packages/server/src/adapters/bun-ws/rpc-handler.ts (6)
packages/server/src/adapters/crossws/rpc-handler.ts (1)
experimental_RPCHandler(13-17)packages/server/src/adapters/ws/rpc-handler.ts (1)
experimental_RPCHandler(13-17)packages/server/src/adapters/websocket/rpc-handler.ts (1)
experimental_RPCHandler(13-17)packages/server/src/context.ts (1)
Context(1-1)packages/server/src/router.ts (1)
Router(12-17)packages/server/src/adapters/standard/rpc-handler.ts (2)
StandardRPCHandlerOptions(10-11)StandardRPCHandler(13-22)
packages/standard-server-peer/src/types.ts (5)
packages/standard-server-peer/src/client.ts (1)
message(102-141)packages/standard-server-peer/src/server.ts (1)
message(49-79)packages/server/src/adapters/bun-ws/handler.ts (1)
message(21-51)packages/server/src/adapters/crossws/handler.ts (1)
message(18-48)packages/shared/src/index.ts (1)
Promisable(15-15)
packages/server/src/adapters/websocket/handler.ts (5)
packages/server/src/context.ts (1)
Context(1-1)packages/server/src/adapters/standard/handler.ts (1)
StandardHandler(50-146)packages/shared/src/args.ts (2)
MaybeOptionalOptions(1-3)resolveMaybeOptionalOptions(5-7)packages/server/src/adapters/standard/utils.ts (2)
FriendlyStandardHandleOptions(4-6)resolveFriendlyStandardHandleOptions(8-13)packages/standard-server-peer/src/server.ts (2)
ServerPeer(18-129)response(81-109)
packages/shared/src/iterator.ts (2)
packages/shared/src/function.ts (1)
sequential(18-28)packages/shared/src/value.ts (1)
value(5-14)
packages/standard-server-peer/src/codec.test.ts (2)
packages/standard-server/src/types.ts (1)
StandardHeaders(1-3)packages/standard-server-peer/src/codec.ts (5)
encodeRequestMessage(92-136)decodeRequestMessage(138-178)encodeResponseMessage(180-221)decodeResponseMessage(223-264)isEventIteratorHeaders(305-307)
packages/shared/src/id.test.ts (1)
packages/shared/src/id.ts (1)
SequentialIdGenerator(1-12)
packages/shared/src/queue.test.ts (1)
packages/shared/src/queue.ts (1)
AsyncIdQueue(6-104)
packages/standard-server-peer/src/codec.ts (7)
packages/standard-server/src/event-iterator/meta.ts (1)
EventMeta(7-7)packages/standard-server/src/types.ts (4)
StandardRequest(13-24)StandardResponse(34-41)StandardBody(5-11)StandardHeaders(1-3)packages/standard-server-peer/src/types.ts (1)
EncodedMessage(3-3)packages/shared/src/array.ts (1)
toArray(1-3)packages/standard-server/src/utils.ts (2)
getFilenameFromContentDisposition(15-26)generateContentDisposition(4-13)packages/shared/src/iterator.ts (1)
isAsyncIteratorObject(3-9)packages/shared/src/json.ts (1)
stringifyJSON(9-12)
packages/standard-server-peer/src/event-iterator.ts (4)
packages/shared/src/queue.ts (1)
AsyncIdQueue(6-104)packages/standard-server-peer/src/codec.ts (1)
EventIteratorPayload(15-19)packages/shared/src/iterator.ts (2)
CreateAsyncIteratorObjectCleanupFn(11-13)createAsyncIteratorObject(15-82)packages/shared/src/object.ts (1)
isTypescriptObject(44-46)
🪛 LanguageTool
apps/content/docs/integrations/remix.md
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ation Remix is a full stack JavaScript framework for building web a...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
apps/content/docs/integrations/solid-start.md
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...Start](https://start.solidjs.com/) is a full stack JavaScript framework for building web a...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
apps/content/docs/integrations/nuxt.md
[uncategorized] ~8-~8: A comma might be missing here.
Context: ...on top of Nitro server a lightweight, high-performance Node.js...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
packages/standard-server-peer/README.md
[misspelling] ~33-~33: Possible spelling mistakes found.
Context: ...tack Query (React, Vue, Solid, Svelte), Pinia Colada, and more. - 🚀 Server Actions: Ful...
(EN_MULTITOKEN_SPELLING_TWO)
[style] ~51-~51: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... your API or implement API contract. - [@orpc/client](https://www.npmjs.com/package/@...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~52-~52: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... API on the client with type-safety. - [@orpc/react](https://www.npmjs.com/package/@o...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~53-~53: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...with React and React Server Actions. - [@orpc/react-query](https://www.npmjs.com/pack...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~54-~54: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...test/docs/framework/react/overview). - [@orpc/vue-query](https://www.npmjs.com/packag...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~55-~55: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...latest/docs/framework/vue/overview). - [@orpc/solid-query](https://www.npmjs.com/pack...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~56-~56: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...test/docs/framework/solid/overview). - [@orpc/svelte-query](https://www.npmjs.com/pac...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~57-~57: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...est/docs/framework/svelte/overview). - [@orpc/vue-colada](https://www.npmjs.com/packa...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[misspelling] ~57-~57: Possible spelling mistakes found.
Context: ...ge/@orpc/vue-colada): Integration with Pinia Colada. - [@or...
(EN_MULTITOKEN_SPELLING_TWO)
[style] ~58-~58: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ada](https://pinia-colada.esm.dev/). - [@orpc/openapi](https://www.npmjs.com/package/...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~59-~59: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...I specs and handle OpenAPI requests. - [@orpc/zod](https://www.npmjs.com/package/@orp...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~60-~60: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...tps://zod.dev/) doesn't support yet. - [@orpc/valibot](https://www.npmjs.com/package/...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...rom Valibot. - [@orpc/arktype](https://www.npmjs.com/package/...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 GitHub Check: codecov/patch
packages/server/src/adapters/bun-ws/index.ts
[warning] 2-2: packages/server/src/adapters/bun-ws/index.ts#L2
Added line #L2 was not covered by tests
packages/server/src/adapters/ws/index.ts
[warning] 2-2: packages/server/src/adapters/ws/index.ts#L2
Added line #L2 was not covered by tests
packages/server/src/adapters/crossws/index.ts
[warning] 2-2: packages/server/src/adapters/crossws/index.ts#L2
Added line #L2 was not covered by tests
packages/server/src/adapters/websocket/index.ts
[warning] 2-2: packages/server/src/adapters/websocket/index.ts#L2
Added line #L2 was not covered by tests
packages/client/src/adapters/websocket/index.ts
[warning] 2-2: packages/client/src/adapters/websocket/index.ts#L2
Added line #L2 was not covered by tests
packages/standard-server-peer/src/event-iterator.ts
[warning] 94-94: packages/standard-server-peer/src/event-iterator.ts#L94
Added line #L94 was not covered by tests
🪛 markdownlint-cli2 (0.17.2)
packages/standard-server-peer/README.md
71-71: Images should have alternate text (alt text)
null
(MD045, no-alt-text)
🪛 Biome (1.9.4)
packages/standard-server-peer/src/client.test.ts
[error] 297-297: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
packages/standard-server-peer/src/event-iterator.test.ts
[error] 253-253: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
[error] 303-303: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
packages/standard-server-peer/src/server.test.ts
[error] 451-451: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
packages/standard-server-peer/src/codec.ts
[error] 24-24: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 30-30: void is confusing outside a return type or a type parameter.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 273-273: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 274-274: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (109)
packages/openapi/src/adapters/node/openapi-handler.ts (1)
10-12: Update JSDoc reference to HTTP Adapter Docs
The documentation link has been correctly updated to point to the new HTTP Adapter guide, aligning with the broader docs restructuring.packages/standard-server-peer/tsconfig.json (1)
1-19: tsconfig for standard-server-peer looks good
The configuration properly extends the base lib, specifies the requiredlibtargets, includes onlysrc, and excludes tests/benchmarks. References tostandard-serverandsharedensure correct project linkage.tsconfig.json (1)
15-19: Add new package references
The new composite project references forstandard-server,standard-server-fetch,standard-server-node, andstandard-server-peerhave been appended, ensuring they’re included in the TypeScript build graph.apps/content/docs/integrations/svelte-kit.md (1)
8-8: Update integration doc link
The inline reference has been switched to the HTTP Adapter guide, keeping consistency with other framework integration docs.apps/content/docs/integrations/solid-start.md (1)
8-8: Update integration doc link
The link now points to the unified HTTP Adapter documentation, matching the overall documentation reorganization.🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...Start](https://start.solidjs.com/) is a full stack JavaScript framework for building web a...(EN_COMPOUND_ADJECTIVE_INTERNAL)
apps/content/docs/integrations/remix.md (1)
8-8: Updated reference to HTTP Adapter guide
The documentation now correctly points to the new unified HTTP Adapter guide (/docs/adapters/http), centralizing integration instructions.🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ation Remix is a full stack JavaScript framework for building web a...(EN_COMPOUND_ADJECTIVE_INTERNAL)
apps/content/docs/integrations/elysia.md (1)
8-8: Updated reference to HTTP Adapter guide
The Elysia integration doc now points to the consolidated HTTP Adapter guide, maintaining consistency across runtimes.apps/content/docs/integrations/express.md (1)
8-8: Updated reference to HTTP Adapter guide
The Express.js integration guide now references the unified HTTP Adapter documentation, aligning with the overall docs reorganization.apps/content/docs/integrations/hono.md (1)
8-8: Updated reference to HTTP Adapter guide
The Hono integration documentation has been updated to reference the new HTTP Adapter guide, ensuring a consistent user experience.apps/content/docs/integrations/tanstack-start.md (1)
8-8: Updated reference to HTTP Adapter guide
The TanStack Start integration now correctly links to the HTTP Adapter documentation, reflecting the new centralized adapter structure.apps/content/docs/integrations/react-native.md (1)
8-8: Update reference to HTTP Adapter
The new reference to the centralized HTTP Adapter guide improves discoverability and aligns with the overall documentation restructuring.apps/content/docs/plugins/strict-get-method.md (1)
11-11: Clarify default plugin behavior for HTTP Adapter
The note thatStrictGetMethodPluginis now enabled by default for the HTTP Adapter accurately reflects the updatedRPCHandlerimplementations.packages/server/src/adapters/websocket/index.ts (1)
1-2: Add tests for aggregator exports
The index file re-exportshandlerandrpc-handler, but static analysis indicates these lines aren’t covered by tests. Please add unit tests to verify the module’s public API exports to maintain full coverage.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 2-2: packages/server/src/adapters/websocket/index.ts#L2
Added line #L2 was not covered by testsapps/content/docs/integrations/nextjs.md (1)
8-8: Update Next.js guide to reference HTTP Adapter
Replacing runtime-specific guides with the centralized HTTP Adapter link streamlines the integration docs and keeps them consistent across ecosystems.apps/content/docs/comparison.md (1)
37-37: Confirm WebSocket support linkThe entry for WebSockets now points to the new adapter docs at
/docs/adapters/websocket, which aligns with the introduced WebSocket adapter support. Please verify that this link is valid and that the table renders correctly in the comparison page.packages/shared/src/index.ts (2)
5-5: Export new ID utilitiesRe-exporting the
./idmodule centralizes theSequentialIdGeneratorin the shared package and makes it available to consumers. This change is well-aligned with the package’s design.
10-10: Export new AsyncIdQueueRe-exporting the
./queuemodule surfaces theAsyncIdQueueutility for other packages. This integration is consistent and facilitates easier imports across the codebase.packages/client/src/adapters/standard/rpc-link.test.ts (2)
16-16: Configuration option removal aligns with API changes.The
strictGetMethodPluginEnabled: falseoption has been removed from the StandardRPCHandler instantiation. This aligns with changes in the handler implementation where this configuration was refactored.
51-51: Configuration option removal aligns with API changes.Similar to the earlier instance, the
strictGetMethodPluginEnabled: falseoption has been removed from this StandardRPCHandler instantiation as well.packages/server/src/adapters/crossws/index.ts (1)
1-2: Clean index file for Crossws adapter exports.This index file follows the standard pattern of re-exporting functionality from component modules, providing a clean entry point for the Crossws adapter.
The static analysis warning about line 2 not being covered by tests can be considered a false positive, as index files typically don't require direct test coverage when the modules they export are already tested.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 2-2: packages/server/src/adapters/crossws/index.ts#L2
Added line #L2 was not covered by testspackages/client/src/adapters/websocket/index.ts (1)
1-2: Clean index file for WebSocket client adapter exports.This index file provides a single entry point for the WebSocket client adapter functionality, following the standard pattern of re-exporting from component modules.
Similar to the Crossws index file, the static analysis warning about line 2 not being covered by tests can be considered a false positive for index files.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 2-2: packages/client/src/adapters/websocket/index.ts#L2
Added line #L2 was not covered by testsapps/content/docs/getting-started.md (1)
111-111: Documentation link update reflects the new adapter-based organization.The link has been updated from
/docs/integrations/nodeto/docs/adapters/http, and the description of supported runtimes has been simplified. This change aligns with the broader reorganization of documentation from runtime-specific integration guides to adapter-based documentation.packages/standard-server-peer/src/index.ts (1)
1-5: Well-structured barrel file ✅The index file follows the standard pattern for re-exporting modules, making the package's API surface clean and accessible. This allows consumers to import from a single entry point rather than from individual files.
apps/content/docs/rpc-handler.md (2)
14-17: Clear adapter-specific documentation warningThe added warning clearly informs users that this documentation specifically targets the HTTP Adapter, which is important for setting the right context.
94-98: Improved clarity on security plugins and their applicabilityThe updated text and new table format provide better clarity about which security plugins are enabled by default, which adapters they apply to, and how to configure them.
packages/server/src/adapters/node/rpc-handler.test.ts (1)
7-10: Good test setup with well-defined route configurationThe handler now clearly defines two routes with different method constraints, which is perfect for testing the StrictGetMethodPlugin behavior.
packages/shared/src/function.test.ts (1)
16-53: Well-structured tests for the sequential functionThese tests thoroughly validate the sequential execution behavior, covering both normal cases and error scenarios. The tests effectively demonstrate that:
- Calls are executed in order regardless of their completion timing
- The sequence continues even when an error occurs in one of the calls
packages/standard-server-peer/.gitignore (1)
1-26: LGTM! Comprehensive .gitignore for the new packageThe .gitignore file includes all standard patterns for Node.js/TypeScript projects, properly handling generated folders, build artifacts, logs, and temporary files. This will help keep the repository clean.
packages/server/src/adapters/ws/rpc-handler.ts (2)
1-5: Good organization of importsThe imports are well-organized, separating types from implementations and internal from external imports. This is clean and easy to follow.
7-12: Well-documented with appropriate linksThe documentation comments provide good context with links to relevant documentation. This helps users understand where to find more information about RPC handlers and WebSocket adapters.
packages/standard-server-peer/src/types.ts (1)
1-7: Well-defined types for peer communicationThese type definitions provide a clear foundation for peer-to-peer communication. The
EncodedMessageunion type appropriately handles different data formats that might be used in WebSocket or IPC communication, and theEncodedMessageSendFninterface provides flexibility by allowing both synchronous and asynchronous message sending.apps/content/.vitepress/config.ts (3)
3-3: Good addition of icon loaderAdding the
localIconLoaderis appropriate for supporting custom icons in the documentation.
86-93: Well-structured documentation reorganizationThe new "Adapters" section in the sidebar properly reflects the architectural changes introduced in this PR, making it clear that HTTP and WebSocket are now primary integration methods.
234-234: Good addition of Node.js iconAdding a custom Node.js icon enhances the visual documentation and maintains consistency with the existing Cloudflare icon.
packages/standard-server-peer/package.json (2)
15-26: Well-configured exportsThe exports configuration correctly defines both TypeScript types and JavaScript module entry points, following best practices for modern ESM packages.
30-38: Dependencies and build scripts look goodThe dependencies are correctly specified to use workspace references, and the build scripts follow the project's conventions.
packages/server/src/adapters/fetch/rpc-handler.test.ts (3)
5-8: Effective handler configuration with named routesThe update to use named handlers with explicit method configurations is well-structured. The
pinghandler is correctly configured with the GET method route, while theponghandler has no method restriction, providing good test coverage for both scenarios.
10-17: Clean test refactoringThe test has been properly updated to verify the successful response from the
pingendpoint, maintaining the original test functionality while aligning with the new handler structure.
19-25: Great test for default plugin behaviorThis new test effectively verifies that the
StrictGetMethodPluginis enabled by default, ensuring that requests to handlers without explicit GET method permissions are rejected with a 405 status. This helps document and validate the expected behavior.packages/server/src/adapters/websocket/rpc-handler.ts (2)
7-12: Clear documentation with helpful linksThe JSDoc comment provides good context with appropriate links to related documentation.
13-16:Details
❓ Verification inconclusive
Verify the NoInfer type utility
The implementation properly composes the
WebsocketHandlerwith aStandardRPCHandler, following a good adapter pattern. However, theNoInfer<StandardRPCHandlerOptions<T>>type is used without being clearly defined or imported in the visible code.Make sure the
NoInfertype utility is properly defined or imported in your codebase, as it's not a standard TypeScript utility type.
🏁 Script executed:
#!/bin/bash # Check if NoInfer is defined in the codebase rg -t ts "type\s+NoInfer" --no-heading # Or check for imports of NoInfer rg -t ts "import.*NoInfer" --no-headingLength of output: 82
#!/bin/bash # Broad search for all occurrences of "NoInfer" across the repository rg -n "NoInfer" --no-heading
Verify
NoInfertype utility import or definitionI searched the codebase and didn’t find any definition or import of
NoInfer. Please ensure that:
NoInferis defined in your codebase (e.g., in a sharedtypesorutilsfile),- or imported from an external library (for example,
type-festor your in-house utilities).Locations to check:
packages/server/src/adapters/websocket/rpc-handler.ts– add or correct the import ofNoInferabove the class definition.packages/server/src/adapters/crossws/rpc-handler.ts (2)
7-12: Consistent documentation across adaptersThe documentation matches the WebSocket adapter documentation, maintaining consistency across the WebSocket-based adapters.
13-16:Details
✅ Verification successful
Consistent adapter implementation pattern
This implementation follows the same pattern as the WebSocket adapter, promoting code consistency and maintainability across different WebSocket library integrations.
The same note about the
NoInfertype applies here - ensure it's properly defined or imported in your codebase.
🏁 Script executed:
#!/bin/bash # Check if there are any runtime-specific differences needed between the adapters grep -A 3 "extends.*Handler<T>" packages/server/src/adapters/*/rpc-handler.tsLength of output: 3530
Crossws adapter consistency verified
- The
experimental_RPCHandlerinpackages/server/src/adapters/crossws/rpc-handler.tsmirrors the same constructor and inheritance pattern as thebun-ws,websocket, andwsadapters, so no runtime-specific adjustments are needed.- Confirm that
NoInferandStandardRPCHandlerOptionsare correctly imported in this file.packages/client/package.json (3)
37-42: LGTM - WebSocket adapter export configuration looks good.The configuration properly exposes the WebSocket adapter's TypeScript definitions and JavaScript implementations, following the same pattern as the other adapters.
49-50: LGTM - Source exports correctly configured.The source exports are properly set up to point to the WebSocket adapter implementation.
63-64: LGTM - Dependencies added for WebSocket adapter support.The addition of
@orpc/standard-server-peeras a dependency is necessary to support the WebSocket adapter implementation which relies on the peer communication interface provided by this package.packages/client/src/adapters/websocket/rpc-link.test.ts (3)
1-4: LGTM - Good imports for testing the WebSocket RPC link.The imports correctly bring in the necessary dependencies from the standard-server-peer package for message encoding/decoding and the experimental RPCLink implementation.
5-24: LGTM - Effective test setup with WebSocket mocking.The test setup correctly mocks the WebSocket interface with event listeners and the send method, which is essential for testing the RPC link's behavior without actual network communication.
43-49: LGTM - WebSocket closure test.The test correctly verifies that the client rejects promises when the WebSocket connection is closed.
packages/shared/src/queue.test.ts (10)
1-11: LGTM - Good test setup for AsyncIdQueue.The test setup correctly imports the AsyncIdQueue class and prepares a fresh instance before each test.
12-22: LGTM - Good validation of queue opening requirement.These tests correctly verify that the queue enforces the requirement for queues to be opened before operations can be performed on them.
24-31: LGTM - Proper FIFO ordering test.This test effectively verifies that the queue maintains FIFO (First In, First Out) ordering of items.
33-43: LGTM - Good async waiting test.This test correctly verifies the more complex async behavior where pulls are registered before pushes occur, ensuring the queue properly resolves waiting pulls when items become available.
45-54: LGTM - Queue isolation verification.This test properly verifies that different queue IDs maintain isolated queues that don't interfere with each other.
56-68: LGTM - Comprehensive queue closure testing.The test correctly verifies that closing a queue prevents further operations and properly updates the queue's state.
70-87: LGTM - Default rejection reason testing.This test correctly verifies that pending pulls are rejected with the default reason when their queue is closed.
89-103: LGTM - Custom rejection reason testing.This test properly verifies that a custom error reason can be provided when closing queues and that it's used when rejecting pending pulls.
105-134: LGTM - Comprehensive queue state testing.This test thoroughly verifies the
isOpenmethod andlengthproperty functionality, ensuring they accurately reflect the queue's state as queues are opened and closed.
1-135: Excellent test coverage for a critical utility class.The AsyncIdQueue test suite is very thorough, covering all the key functionality including queue lifecycle, FIFO ordering, isolation between queues, error handling, and state management. This level of coverage is important as this utility appears to be foundational for the peer communication mechanisms in the WebSocket adapters.
packages/standard-server-peer/README.md (5)
1-24: LGTM - Well-structured package header and introduction.The header with logo, badges, and introduction provide a clear overview of the package and its place in the oRPC ecosystem.
28-41: LGTM - Comprehensive feature highlights.The feature highlights section effectively communicates the key capabilities of the oRPC framework, which helps users understand the broader context in which this package operates.
🧰 Tools
🪛 LanguageTool
[misspelling] ~33-~33: Possible spelling mistakes found.
Context: ...tack Query (React, Vue, Solid, Svelte), Pinia Colada, and more. - 🚀 Server Actions: Ful...(EN_MULTITOKEN_SPELLING_TWO)
43-62: LGTM - Good documentation links and package overview.The documentation link and package list provide users with clear navigation to more detailed resources and related packages.
🧰 Tools
🪛 LanguageTool
[style] ~51-~51: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... your API or implement API contract. - [@orpc/client](https://www.npmjs.com/package/@...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~52-~52: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... API on the client with type-safety. - [@orpc/react](https://www.npmjs.com/package/@o...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~53-~53: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...with React and React Server Actions. - [@orpc/react-query](https://www.npmjs.com/pack...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~54-~54: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...test/docs/framework/react/overview). - [@orpc/vue-query](https://www.npmjs.com/packag...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~55-~55: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...latest/docs/framework/vue/overview). - [@orpc/solid-query](https://www.npmjs.com/pack...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~56-~56: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...test/docs/framework/solid/overview). - [@orpc/svelte-query](https://www.npmjs.com/pac...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~57-~57: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...est/docs/framework/svelte/overview). - [@orpc/vue-colada](https://www.npmjs.com/packa...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[misspelling] ~57-~57: Possible spelling mistakes found.
Context: ...ge/@orpc/vue-colada): Integration with Pinia Colada. - [@or...(EN_MULTITOKEN_SPELLING_TWO)
[style] ~58-~58: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ada](https://pinia-colada.esm.dev/). - [@orpc/openapi](https://www.npmjs.com/package/...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~59-~59: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...I specs and handle OpenAPI requests. - [@orpc/zod](https://www.npmjs.com/package/@orp...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~60-~60: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...tps://zod.dev/) doesn't support yet. - [@orpc/valibot](https://www.npmjs.com/package/...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...rom Valibot. - [@orpc/arktype](https://www.npmjs.com/package/...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
63-65: LGTM - Clear package description.The description clearly explains the purpose of the package: "Provides a standard interface for oRPC server adapters to interact with connected peers across various transports (e.g., WebSocket, IPC)."
1-77: LGTM - Well-structured README for the new package.The README provides a comprehensive overview of both the oRPC ecosystem and the specific purpose of the standard-server-peer package. It clearly communicates that this package provides a standard interface for server adapters to interact with connected peers across various transports like WebSocket and IPC, which aligns well with the PR objectives.
🧰 Tools
🪛 LanguageTool
[misspelling] ~33-~33: Possible spelling mistakes found.
Context: ...tack Query (React, Vue, Solid, Svelte), Pinia Colada, and more. - 🚀 Server Actions: Ful...(EN_MULTITOKEN_SPELLING_TWO)
[style] ~51-~51: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... your API or implement API contract. - [@orpc/client](https://www.npmjs.com/package/@...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~52-~52: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... API on the client with type-safety. - [@orpc/react](https://www.npmjs.com/package/@o...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~53-~53: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...with React and React Server Actions. - [@orpc/react-query](https://www.npmjs.com/pack...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~54-~54: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...test/docs/framework/react/overview). - [@orpc/vue-query](https://www.npmjs.com/packag...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~55-~55: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...latest/docs/framework/vue/overview). - [@orpc/solid-query](https://www.npmjs.com/pack...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~56-~56: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...test/docs/framework/solid/overview). - [@orpc/svelte-query](https://www.npmjs.com/pac...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~57-~57: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...est/docs/framework/svelte/overview). - [@orpc/vue-colada](https://www.npmjs.com/packa...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[misspelling] ~57-~57: Possible spelling mistakes found.
Context: ...ge/@orpc/vue-colada): Integration with Pinia Colada. - [@or...(EN_MULTITOKEN_SPELLING_TWO)
[style] ~58-~58: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ada](https://pinia-colada.esm.dev/). - [@orpc/openapi](https://www.npmjs.com/package/...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~59-~59: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...I specs and handle OpenAPI requests. - [@orpc/zod](https://www.npmjs.com/package/@orp...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~60-~60: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...tps://zod.dev/) doesn't support yet. - [@orpc/valibot](https://www.npmjs.com/package/...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~61-~61: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...rom Valibot. - [@orpc/arktype](https://www.npmjs.com/package/...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.17.2)
71-71: Images should have alternate text (alt text)
null(MD045, no-alt-text)
packages/server/src/adapters/node/rpc-handler.ts (4)
5-5: Clean import addition for StrictGetMethodPlugin.The addition of the StrictGetMethodPlugin import supports the new option to conditionally enable/disable this security feature.
9-16: Well-structured type definition with clear documentation.The RPCHandlerOptions type effectively composes multiple option interfaces and adds a well-documented strictGetMethodPluginEnabled option with an explicit default value.
22-22: Documentation link properly updated.The link has been updated to reference the HTTP Adapter Docs instead of Node Integration Docs, aligning with the reorganized documentation structure mentioned in the PR summary.
25-30: Well-implemented plugin conditional logic.The constructor correctly implements the conditional addition of the StrictGetMethodPlugin based on the option value, using nullish coalescing to maintain the default enabled behavior while allowing explicit disabling.
packages/server/src/adapters/fetch/rpc-handler.ts (3)
5-5: Clean import addition for StrictGetMethodPlugin.The addition of the StrictGetMethodPlugin import supports the new option to conditionally enable/disable this security feature.
9-16: Well-structured type definition with clear documentation.The RPCHandlerOptions type effectively composes multiple option interfaces and adds a well-documented strictGetMethodPluginEnabled option with an explicit default value.
25-30: Well-implemented plugin conditional logic.The constructor correctly implements the conditional addition of the StrictGetMethodPlugin based on the option value, using nullish coalescing to maintain the default enabled behavior while allowing explicit disabling.
packages/server/src/adapters/standard/rpc-handler.ts (1)
14-14: Simplified constructor with appropriate default options.The constructor has been properly simplified to default the options parameter to an empty object. This change aligns with the architectural refinement to remove StrictGetMethodPlugin from the standard handler and delegate it to specific adapters.
packages/standard-server-peer/tests/e2e.test.ts (3)
7-30: Robust client/server peer setup with good test coverage considerations.The test setup effectively creates a ClientPeer and ServerPeer that communicate asynchronously, with the server echoing back client requests. The randomized encoding variations (alternating between string and binary formats) are a great approach to increase test coverage.
33-78: Comprehensive data type serialization testing.The tests thoroughly validate serialization/deserialization across a wide range of data types in multiple contexts (flat, nested, complex structures). This approach ensures the peer communication handles all supported data formats correctly.
80-118: Well-structured event iteration test.The event iteration test effectively validates async generator handling, including event metadata preservation and generator completion. The assertions are thorough, checking both values and metadata for each yielded item.
packages/shared/src/iterator.ts (1)
48-65: Potential race ifreturn/throw/disposeinvoked while an asyncnextis pending
nextcalls are serialized viasequential, butreturn,throw, and disposal bypass that wrapper and may run concurrently with an in-flightnext.
Although the single-threaded JS model eliminates true parallelism, overlapping micro-tasks can still lead to the cleanup block executing twice if both branches read!isExecuteCompletebefore one sets it.For full robustness, you could reuse
sequentialfor these termination paths or protect theisExecuteCompletecheck with an atomic micro-task queue (e.g. using the same wrapper).packages/server/src/adapters/ws/rpc-handler.test.ts (1)
1-142: Well-structured tests covering key WebSocket handler scenariosThis test suite comprehensively covers the important test cases for the WebSocket RPC handler, including successful calls, abort signal handling, connection closure, and error handling. The mocking approach and assertions are appropriate.
packages/server/src/adapters/websocket/handler.ts (1)
9-36: Clean implementation leveraging ServerPeer abstractionThe WebSocket handler is well-designed, using the ServerPeer abstraction to handle message encoding/decoding and properly managing WebSocket connection lifecycle with appropriate event listeners.
packages/server/src/adapters/crossws/handler.ts (2)
25-35: Efficient handling of different message formatsThe code properly handles different message formats (Blob vs other) when sending responses. This provides good compatibility with various WebSocket implementations.
10-58: Well-designed resource management with WeakMapThe use of WeakMap for tracking ServerPeer instances is a good design choice, allowing for proper garbage collection when peers are no longer referenced. The explicit close method ensures proper cleanup of resources.
apps/content/docs/adapters/websocket.md (1)
1-140: Comprehensive and clear documentation of WebSocket adaptersThe documentation provides excellent coverage of the available WebSocket adapters for both server and client, with clear code examples for each environment. The tables clearly identify the target platforms for each adapter.
apps/content/docs/adapters/http.md (1)
1-154: Well-structured documentation with comprehensive examples!The new HTTP adapter documentation provides clear, consistent examples across multiple platforms (Node.js, Bun, Cloudflare Workers, Deno) with appropriate handling of matched and unmatched routes. The code samples follow a consistent pattern and show proper error handling. The client section appropriately demonstrates the Fetch adapter configuration with URL and optional headers.
I particularly like how you've organized the content with code groups to allow developers to easily find examples relevant to their platform.
packages/server/src/adapters/bun-ws/rpc-handler.test.ts (5)
56-69: The success test flow looks goodThis test properly verifies the success path for JSON payloads. It checks the response structure including status, headers, and body content.
71-88: The binary success test looks goodThis test properly verifies handling of binary data responses. It checks the correct encoding and content type headers as well as the actual data content.
90-104: Good abort signal test implementationThe test properly verifies that abort signals are propagated to the handler and that no response is sent when aborted.
106-119: Good connection close test implementationThe test properly verifies that closing a connection aborts any pending requests and cleans up resources.
121-134: Good error path testing for unmatched proceduresThe test properly verifies that requests for non-existent procedures return a 404 status with an appropriate error message.
packages/server/src/adapters/bun-ws/handler.ts (2)
13-19: Good use of WeakMap for efficient resource managementUsing a WeakMap to track peers is an excellent choice as it allows garbage collection when WebSocket references are no longer used, preventing memory leaks.
53-60: Good resource cleanup in the close methodThe close method properly cleans up resources by closing the peer and removing it from the map.
packages/client/src/adapters/websocket/rpc-link.ts (2)
7-8: Good interface design excluding HTTP-specific optionsThe options interface properly extends the standard options while excluding HTTP-specific properties. This makes the API cleaner and more intuitive for WebSocket usage.
10-15: Excellent documentation with appropriate linksThe JSDoc comment provides clear explanations and includes links to relevant documentation. This helps developers understand the purpose of the class and where to find more information.
packages/client/src/adapters/websocket/link-websocket-client.ts (1)
6-8: Good use of the experimental prefix for API that might evolve.The
experimental_prefix clearly indicates that this API is subject to change, which is good practice for features that are still being refined.packages/server/src/adapters/crossws/rpc-handler.test.ts (4)
65-77: LGTM! Good test for successful RPC execution.The test properly verifies the successful response flow, checking both the message ID type and payload structure.
99-113: LGTM! Comprehensive test for abort signal functionality.The test effectively verifies that the abort signal works correctly by checking the aborted state before and after sending the abort message.
115-128: LGTM! Good validation of WebSocket close behavior.The test properly verifies that closing the WebSocket connection aborts any in-flight requests.
130-143: LGTM! Good test for handling unmatched procedures.The test properly verifies the 404 response for unmatched procedure paths.
packages/standard-server-peer/src/codec.test.ts (6)
4-8: Excellent stress test for large header handling.Creating a 10MB headers object is a good way to test the codec's ability to handle large payloads. This ensures the serialization and deserialization process can handle extreme cases.
10-49: LGTM! Well-structured tests for basic message types.The tests for
ABORT_SIGNALandEVENT_ITERATORmessage types are thorough and well-organized, checking both encoding output types and decoding result structures.
51-59: Excellent use of parameterized tests for HTTP methods and URLs.Using
describe.eachto test multiple URL formats and HTTP methods is an efficient way to ensure comprehensive coverage while keeping the test code DRY.
126-238: LGTM! Comprehensive tests for FormData encoding/decoding.The tests properly verify that FormData objects with text and file entries are correctly serialized and deserialized, including proper handling of content types.
374-406: Good performance test for large payload handling.Testing with a 10MB payload ensures the codec can handle large messages without timeouts or memory issues. The increased timeout setting is appropriate for this case.
777-794: LGTM! Good validation of isEventIteratorHeaders utility.The tests thoroughly verify the conditions under which headers should or should not be identified as event iterator headers.
packages/server/src/adapters/websocket/rpc-handler.test.ts (4)
21-33: Good approach for capturing event listeners with mock functions.Using mock functions to capture event listeners is an effective way to test the handler's interaction with the WebSocket API.
58-71: LGTM! Good test for successful message processing.The test properly verifies the handler can process a request and return a successful response with the expected payload.
73-94: LGTM! Good test for abort signal handling.The test effectively verifies that abort signals are properly processed and affect in-flight requests.
111-124: LGTM! Good test for handling unmatched procedures.The test correctly verifies that requests to non-existent procedures return 404 responses with the appropriate error message.
packages/server/package.json (1)
84-87: Peer- & dev-dependency duplication – double-check install experience
crosswsandwsare declared in bothpeerDependenciesanddevDependencies.
That is valid, but keep in mind:
- Consumers must install them manually (because they’re peers).
- The dev entry pins a version (
^0.3.4,^8.18.1) – if these diverge from the>=peer range, CI might pass while consumers hit version-mismatch warnings.Consider adopting identical semver ranges (e.g.
">=8.18.1 <9"in both places) or usingoptionalDependenciesinstead ofpeerDependenciesif you want automatic installation.packages/standard-server-peer/src/client.ts (1)
143-155:close()does not guarantee queue draining when invoked withid
AsyncIdQueue.close({id})typically flushes any waitingpull()promises with a rejection/reason.
Double-check that yourAsyncIdQueueimplementation actually propagates thereasonyou pass, otherwise callers might receive a generic “queue closed” error and lose the original context.packages/standard-server-peer/src/server.ts (1)
49-63:decodeRequestMessagemay createFile,Responsein Node runtimes
new File(...)and the fetchResponseconstructor are only globally available in modern browsers or Node ≥ 20 with experimental globals.
Since the package targets multiple runtimes (Web, Bun, Deno, and Node), please verify Node ≤ 18 compatibility or polyfill availability, otherwise decoding binary bodies will throw at runtime.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/standard-server-peer/src/client.test.ts (1)
38-46:⚠️ Potential issue
expect(...).resolves / .rejectsstill not awaited – assertions may run after the test finishesThe exact same issue was raised in the previous review:
expect(promise).resolves|rejects…returns a Promise that must beawait-ed (or returned from a non-async test).
Without this, the assertion executes after Vitest/Jest marks the test complete, leading to false-positives and flakiness.Example fix:
- expect(peer.request(baseRequest)).resolves.toEqual(baseResponse) + await expect(peer.request(baseRequest)).resolves.toEqual(baseResponse)Please apply this to every occurrence highlighted in the ranges above.
(If you prefer, return the expectation instead of awaiting in non-async tests.)Also applies to: 49-57, 92-99, 117-123, 141-151, 174-182, 198-206, 221-233, 249-253, 265-270, 301-307, 426-435, 467-475, 506-514, 529-535, 541-544
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/openapi/src/adapters/fetch/openapi-handler.ts(1 hunks)packages/server/src/adapters/fetch/rpc-handler.ts(1 hunks)packages/standard-server-peer/src/client.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/openapi/src/adapters/fetch/openapi-handler.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/src/adapters/fetch/rpc-handler.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/standard-server-peer/src/client.test.ts
[error] 297-297: Unsafe usage of 'throw'.
'throw' in 'finally' overwrites the control flow statements inside 'try' and 'catch'.
(lint/correctness/noUnsafeFinally)
This adapter can use for websocket, IPC, ...
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests
Chores