-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[1.3] Bun.serve({ websocket }) types
#20918
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
|
Updated 6:22 PM PT - Oct 6th, 2025
❌ @alii, your commit 742cbd0 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 20918That installs a local version of the PR into your bun-20918 --bun |
…i/serve-types-redo
…i/serve-types-redo
…erty for typing websocket data
|
Caution Review failedThe pull request is closed. WalkthroughIntroduces typed WebSocket data via websocket.config data instead of Bun.serve generics, adds a new serve.d.ts with typed Serve/WebSocket APIs, restructures bun.d.ts toward TLS/Socket, removes top-level TLS key/cert in favor of tls object, replaces Awaitable with MaybePromise in bake and tests, and updates docs and tests accordingly. Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (4)
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/bun-types/bun.d.ts (1)
4683-4694: Duplicate and conflicting Socket.exportKeyingMaterial signaturesThis method is declared twice with different parameter/return types; one returns Buffer, the other returns void. This will confuse consumers and may break overload resolution. Keep a single signature that returns Buffer, with a union for context if needed.
Apply:
@@ - exportKeyingMaterial(length: number, label: string, context: Buffer): Buffer; + exportKeyingMaterial(length: number, label: string, context: Buffer): Buffer; @@ - exportKeyingMaterial(length: number, label: string, context?: string | BufferSource): void; + // Remove this duplicate; if needed, merge into the signature above: + // exportKeyingMaterial(length: number, label: string, context?: string | BufferSource): Buffer;Also applies to: 4893-4896
docs/api/websockets.md (1)
117-135: Fix type mismatch in contextual data example (channelId can be null)URLSearchParams.get() returns string | null, but WebSocketData.channelId is typed as string. The example as written won’t type-check.
type WebSocketData = { createdAt: number; - channelId: string; + channelId: string; authToken: string; }; Bun.serve({ fetch(req, server) { const cookies = new Bun.CookieMap(req.headers.get("cookie")!); - server.upgrade(req, { + const url = new URL(req.url); + const channelId = url.searchParams.get("channelId") ?? ""; + server.upgrade(req, { // this object must conform to WebSocketData data: { createdAt: Date.now(), - channelId: new URL(req.url).searchParams.get("channelId"), + channelId, authToken: cookies.get("X-Token"), }, });Alternatively, loosen the type: channelId: string | null.
🧹 Nitpick comments (13)
test/harness.ts (1)
186-205: Optional: use recursive mkdir for nested directory trees in async path tooTo mirror the sync fix and avoid EEXIST on re-runs, make mkdir recursive.
Suggested change (context-only; outside changed hunk):
// inside makeTree(...) if (isDirectoryTree(contents)) { fs.mkdirSync(joined, { recursive: true }); makeTree(joined, contents); continue; }src/bake/bake.d.ts (1)
370-371: Confirm signature + docs: render now takes RouteMetadata (doc still says “routeModule”)The comment block mentions an opaque routeModule, but the signature is (request, routeMetadata). Update the doc or signature for consistency.
packages/bun-types/bun.d.ts (2)
2771-2854: TLSOptions: types OK; please confirm ALPNProtocols shapeThe additions (keyFile, certFile, ciphers, renegotiation limits) look good, and ca/cert/key now accept BufferSource arrays. One question: ALPNProtocols is typed as string | BufferSource; most APIs accept a list (string[]/Buffer[]). If Bun accepts multiple entries, widen to array; if not, document it.
If arrays are supported, adjust:
- ALPNProtocols?: string | BufferSource; + ALPNProtocols?: string | BufferSource | Array<string | BufferSource>;
1773-1779: Minor: duplicated "linked" in sourcemap unionThe type lists "linked" twice. Tidy the union.
- sourcemap?: "none" | "linked" | "inline" | "external" | "linked" | boolean; + sourcemap?: "none" | "linked" | "inline" | "external" | boolean;packages/bun-types/index.d.ts (1)
30-33: Fix wording in comment for clarityMinor grammar/clarity nits.
-// We must declare that `onmessage` exists globally since many of Node's declarations -// fallback to the globally available versions when, which Bun overrides. It detects -// if the symbols are available by looking for a declaration of `onmessage`. +// We declare a global `onmessage` so Node declarations can fall back to DOM globals when present. +// Type resolution detects availability by checking for a declaration of `onmessage`, +// which Bun overrides when lib DOM is available.docs/guides/websocket/pubsub.md (1)
10-10: Docs change LGTM; optional readability tweakCasting with {} as { username: string } works; consider naming a type for clarity in guides.
- // TypeScript: specify the type of ws.data like this - data: {} as { username: string }, + // TypeScript: specify the type of ws.data like this + type ChatData = { username: string }; + data: {} as ChatData,Also applies to: 20-22
docs/guides/websocket/context.md (1)
30-30: Fix stray brace in log statementThere’s an extra “}”.
- console.log(`Received ${message} from ${ws.data.socketId}}`); + console.log(`Received ${message} from ${ws.data.socketId}`);test/integration/bun-types/fixture/serve-types.test.ts (1)
25-27: Rename parameter to avoid shadowing “constructor”Prevents linter noise and improves clarity.
-function expectInstanceOf<T>(value: unknown, constructor: new (...args: any[]) => T): asserts value is T { - expect(value).toBeInstanceOf(constructor); +function expectInstanceOf<T>(value: unknown, Ctor: new (...args: any[]) => T): asserts value is T { + expect(value).toBeInstanceOf(Ctor); }packages/bun-types/serve.d.ts (5)
77-89: Allow SharedArrayBuffer wherever binary payloads are accepted (parity with Server.publish).Server.publish accepts SharedArrayBuffer; the per-connection send/publish variants should too for consistency.
Apply this diff:
- send(data: string | BufferSource, compress?: boolean): ServerWebSocketSendStatus; + send(data: string | BufferSource | SharedArrayBuffer, compress?: boolean): ServerWebSocketSendStatus; - sendBinary(data: BufferSource, compress?: boolean): ServerWebSocketSendStatus; + sendBinary(data: BufferSource | SharedArrayBuffer, compress?: boolean): ServerWebSocketSendStatus; - publish(topic: string, data: string | BufferSource, compress?: boolean): ServerWebSocketSendStatus; + publish(topic: string, data: string | BufferSource | SharedArrayBuffer, compress?: boolean): ServerWebSocketSendStatus; - publishBinary(topic: string, data: BufferSource, compress?: boolean): ServerWebSocketSendStatus; + publishBinary(topic: string, data: BufferSource | SharedArrayBuffer, compress?: boolean): ServerWebSocketSendStatus;Also applies to: 99-100, 151-152, 163-164, 175-176
186-193: Grammar: “Unsubscribes a client from the topic.”Small doc fix.
Apply this diff:
- * Unsubscribes a client to the topic. + * Unsubscribes a client from the topic.
438-444: Typos: duplicate “the”.Two occurrences.
Apply this diff:
- * Sets the the number of seconds to wait before timing out a connection + * Sets the number of seconds to wait before timing out a connectionAlso applies to: 756-762
308-316: Doc import fix: remove unused/non-exportedwebsocketsymbol.Prevents confusion;
serveis sufficient in the example.Apply this diff:
- * import { websocket, serve } from "bun"; + * import { serve } from "bun";
901-945: upgrade() conditional args look good; keep overload alternative in mind.The tuple-conditional rest arg is correct and enforces data when WebSocketData ≠ undefined. If tooling shows friction, consider two overloads as a fallback.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
.gitignore(1 hunks)docs/api/websockets.md(4 hunks)docs/guides/websocket/context.md(4 hunks)docs/guides/websocket/pubsub.md(2 hunks)docs/guides/websocket/simple.md(1 hunks)packages/bun-types/bun.d.ts(3 hunks)packages/bun-types/bun.ns.d.ts(0 hunks)packages/bun-types/deprecated.d.ts(1 hunks)packages/bun-types/index.d.ts(1 hunks)packages/bun-types/serve.d.ts(1 hunks)src/bake/bake.d.ts(5 hunks)test/harness.ts(1 hunks)test/integration/bun-types/fixture/index.ts(0 hunks)test/integration/bun-types/fixture/serve-types.test.ts(10 hunks)test/integration/bun-types/fixture/serve.ts(1 hunks)
💤 Files with no reviewable changes (2)
- packages/bun-types/bun.ns.d.ts
- test/integration/bun-types/fixture/index.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
packages/bun-types/index.d.tstest/integration/bun-types/fixture/serve.tssrc/bake/bake.d.tstest/harness.tstest/integration/bun-types/fixture/serve-types.test.tspackages/bun-types/serve.d.tspackages/bun-types/deprecated.d.tspackages/bun-types/bun.d.ts
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/integration/bun-types/fixture/serve.tstest/harness.tstest/integration/bun-types/fixture/serve-types.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/integration/bun-types/fixture/serve.tstest/harness.tstest/integration/bun-types/fixture/serve-types.test.ts
test/integration/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Place integration tests under
test/integration/
Files:
test/integration/bun-types/fixture/serve.tstest/integration/bun-types/fixture/serve-types.test.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.ts: Name test files*.test.tsand usebun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; useport: 0to get a random port
When spawning Bun in tests, usebunExe()andbunEnvfromharness
Preferasync/awaitin tests; for a single callback, usePromise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
UsetempDir/tempDirWithFilesfromharnessfor temporary files and directories in tests
For large/repetitive strings in tests, preferBuffer.alloc(count, fill).toString()over"A".repeat(count)
Import common test utilities fromharness(e.g.,bunExe,bunEnv,tempDirWithFiles,tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and usetoThrowfor synchronous errors
Usedescribeblocks for grouping,describe.eachfor parameterized tests, snapshots withtoMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach); track resources for cleanup inafterEach
Useusing/await usingwith Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Files:
test/integration/bun-types/fixture/serve-types.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must be placed under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or use custom random port functions
In tests, use normalizeBunSnapshot when asserting snapshots
Never write tests that merely assert absence of "panic" or "uncaught exception" in output
Avoid shell commands (e.g., find, grep) in tests; use Bun.Glob and built-ins instead
Prefer snapshot tests over exact stdout equality assertions
Files:
test/integration/bun-types/fixture/serve-types.test.ts
🧠 Learnings (19)
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
packages/bun-types/index.d.tstest/integration/bun-types/fixture/serve.tstest/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/integration/bun-types/fixture/serve.tstest/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/integration/bun-types/fixture/serve.tstest/harness.tstest/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Name test files `*.test.ts` and use `bun:test`
Applied to files:
test/integration/bun-types/fixture/serve.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Use `using`/`await using` with Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Applied to files:
test/integration/bun-types/fixture/serve.tstest/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/react-spa.test.ts : react-spa.test.ts should contain React SPA, react-refresh, and basic server component transform tests
Applied to files:
test/integration/bun-types/fixture/serve.tstest/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/*.test.ts : Write Dev Server and HMR tests in test/bake/dev/*.test.ts using devTest from the shared harness
Applied to files:
test/integration/bun-types/fixture/serve.tstest/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*-fixture.ts : Name test fixture files that are spawned by tests with the suffix `-fixture.ts`
Applied to files:
test/integration/bun-types/fixture/serve.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/integration/bun-types/fixture/serve.tstest/harness.tstest/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Import testing utilities (devTest, prodTest, devAndProductionTest, Dev, Client) from test/bake/bake-harness.ts
Applied to files:
src/bake/bake.d.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Use shared utilities from test/harness.ts where applicable
Applied to files:
test/harness.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Import common test utilities from `harness` (e.g., `bunExe`, `bunEnv`, `tempDirWithFiles`, `tmpdirSync`, platform checks, GC helpers)
Applied to files:
test/harness.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Use `tempDir`/`tempDirWithFiles` from `harness` for temporary files and directories in tests
Applied to files:
test/harness.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/bake-harness.ts : Provide and maintain shared test utilities: devTest, prodTest, devAndProductionTest, Dev, Client, and helpers in the harness
Applied to files:
test/harness.tstest/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/harness.tstest/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-02T05:33:37.517Z
Learnt from: Jarred-Sumner
PR: oven-sh/bun#22323
File: test/js/web/websocket/websocket-subprotocol.test.ts:74-75
Timestamp: 2025-09-02T05:33:37.517Z
Learning: In Bun's runtime, `await using` with Node.js APIs like `net.createServer()` is properly supported and should not be replaced with explicit cleanup. Bun has extended Node.js APIs with proper async dispose support.
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Never hardcode port numbers in tests; use `port: 0` to get a random port
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: Applies to test/**/*.test.{ts,tsx} : In tests, always use port: 0; do not hardcode ports or use custom random port functions
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Dev server tests should be thorough yet concise to validate hot-reloading robustness, correctness, and reliability
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
🧬 Code graph analysis (4)
test/integration/bun-types/fixture/serve.ts (1)
test/integration/bun-types/fixture/utilities.ts (1)
expectType(35-37)
test/integration/bun-types/fixture/serve-types.test.ts (2)
test/integration/bun-types/fixture/utilities.ts (1)
expectType(35-37)test/integration/bun-types/fixture/serve.ts (4)
message(18-20)message(35-37)message(52-54)message(67-69)
packages/bun-types/serve.d.ts (3)
src/js/internal/debugger.ts (1)
WebSocketHandler(310-325)test/integration/bun-types/fixture/serve-types.test.ts (13)
message(19-21)message(115-118)message(175-178)message(227-229)message(259-259)message(279-279)message(302-302)message(526-528)message(563-563)error(201-208)error(756-758)error(776-779)error(820-822)test/integration/bun-types/fixture/serve.ts (4)
message(18-20)message(35-37)message(52-54)message(67-69)
packages/bun-types/deprecated.d.ts (1)
packages/bun-release/src/platform.ts (1)
Platform(15-22)
🪛 Biome (2.1.2)
test/integration/bun-types/fixture/serve-types.test.ts
[error] 25-25: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
packages/bun-types/deprecated.d.ts
[error] 107-107: Shouldn't redeclare 'Platform'. Consider to delete it or rename it.
'Platform' is defined here:
(lint/suspicious/noRedeclare)
[error] 121-121: Shouldn't redeclare 'Architecture'. Consider to delete it or rename it.
'Architecture' is defined here:
(lint/suspicious/noRedeclare)
[error] 124-124: Shouldn't redeclare 'UncaughtExceptionListener'. Consider to delete it or rename it.
'UncaughtExceptionListener' is defined here:
(lint/suspicious/noRedeclare)
[error] 132-132: Shouldn't redeclare 'UnhandledRejectionListener'. Consider to delete it or rename it.
'UnhandledRejectionListener' is defined here:
(lint/suspicious/noRedeclare)
[error] 135-135: Shouldn't redeclare 'MultipleResolveListener'. Consider to delete it or rename it.
'MultipleResolveListener' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (19)
.gitignore (1)
192-192: Correct fix: make cmake/sources/*.txt ignoredRemoving the stray leading "-" activates the ignore rule as intended. No further action.
src/bake/bake.d.ts (4)
380-381: LGTM: prerender uses MaybePromiseTypes align with the repo-wide MaybePromise migration.
410-411: LGTM: getParams uses MaybePromiseConsistent with the new async/sync flexibility.
449-450: LGTM: Dev server hook returns MaybePromiseMatches other hook surfaces.
506-509: Rename propagation verified: no GenericServeOptions references remain. – The only match is the expected deprecated ServeOptions alias in packages/bun-types/deprecated.d.ts.packages/bun-types/bun.d.ts (2)
2762-2766: LGTM: ErrorLike expanded with code/errno/syscallUseful across spawn and TLS error surfaces.
2857-2871: LGTM: SocketAddress clarifiedDocstrings clarify client addr/port/family; types unchanged.
docs/guides/websocket/simple.md (1)
10-10: Docs change LGTMExample now matches the new per-connection typing model (no Bun.serve generic).
packages/bun-types/index.d.ts (2)
24-24: Good: add explicit dependency on serve.d.tsPrevents ordering issues for Serve/WS types.
33-33: Sanity-check passed for onmessage
UseLibDomIfAvailable<"onmessage", never>returnsneverwhen no lib.dom is loaded and the DOM’sonmessagetype when lib.dom is present.docs/api/websockets.md (2)
133-135: Nice: explicit typing for ws.data in handlersThis aligns the docs with the new per-connection typing model.
170-187: Docs change LGTMThe pub/sub example now correctly shows typed ws.data and upgrade data.
test/integration/bun-types/fixture/serve.ts (1)
5-22: Types-only fixture looks goodCovers upgrade data requiredness and ws.data typing across cases.
Also applies to: 24-39, 41-56, 58-71
docs/guides/websocket/context.md (1)
10-16: Docs changes LGTMExamples match the new upgrade-data and ws.data typing flow.
Also applies to: 23-25, 47-60, 65-67
packages/bun-types/deprecated.d.ts (1)
101-105: Good deprecation path for ServeOptionsAlias to Bun.Serve.Options<T, R> is clear.
test/integration/bun-types/fixture/serve-types.test.ts (1)
139-146: Type assertions in WS tests look correctHandlers validate ws/message/code/reason types under the new Serve typings.
Also applies to: 168-176, 181-188, 227-229
packages/bun-types/serve.d.ts (3)
548-560: Routes/Options typing design reads well.The XOR-based gating between hostname/port vs unix and fetch vs websocket+upgrade is clear and prevents invalid combos. Nice.
Also applies to: 562-583, 584-649, 789-794
1277-1280: serve() generics align with the single-argument goal.Defaulting R to
neverenablesBun.serve<WSData>(...)while retaining route typing. Matches the PR objectives.
534-536: Source of MaybePromise resolved MaybePromise is declared in packages/bun-types/bun.d.ts (at line 47), so the reference in serve.d.ts is valid.
| /** | ||
| * Batches `send()` and `publish()` operations, which makes it faster to send data. | ||
| * | ||
| * The `message`, `open`, and `drain` callbacks are automatically corked, so | ||
| * you only need to call this if you are sending messages outside of those | ||
| * callbacks or in async functions. | ||
| * | ||
| * @param callback The callback to run. | ||
| * @example | ||
| * ws.cork((ctx) => { | ||
| * ctx.send("These messages"); | ||
| * ctx.sendText("are sent"); | ||
| * ctx.sendBinary(new TextEncoder().encode("together!")); | ||
| * }); | ||
| */ | ||
| cork<T = unknown>(callback: (ws: ServerWebSocket<T>) => T): T; | ||
|
|
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.
🛠️ Refactor suggestion
Fix cork() generic shadowing and return typing.
The method’s generic T shadows ServerWebSocket’s data type and wrongly rebinds ws. Use a distinct type var for the callback result and keep ws bound to the connection’s T.
Apply this diff:
- cork<T = unknown>(callback: (ws: ServerWebSocket<T>) => T): T;
+ cork<R>(callback: (ws: ServerWebSocket<T>) => R): R;📝 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.
| /** | |
| * Batches `send()` and `publish()` operations, which makes it faster to send data. | |
| * | |
| * The `message`, `open`, and `drain` callbacks are automatically corked, so | |
| * you only need to call this if you are sending messages outside of those | |
| * callbacks or in async functions. | |
| * | |
| * @param callback The callback to run. | |
| * @example | |
| * ws.cork((ctx) => { | |
| * ctx.send("These messages"); | |
| * ctx.sendText("are sent"); | |
| * ctx.sendBinary(new TextEncoder().encode("together!")); | |
| * }); | |
| */ | |
| cork<T = unknown>(callback: (ws: ServerWebSocket<T>) => T): T; | |
| cork<R>(callback: (ws: ServerWebSocket<T>) => R): R; |
🤖 Prompt for AI Agents
In packages/bun-types/serve.d.ts around lines 205 to 221, the cork method
declares a generic T that shadows the ServerWebSocket<T> type parameter and
incorrectly rebinds ws and the return type; change the signature to use the
connection's T for the ws parameter and introduce a distinct generic for the
callback result (e.g., R), so the method becomes cork<R = unknown>(callback:
(ws: ServerWebSocket<T>) => R): R; ensuring ws keeps the interface's data type
and the callback result is correctly typed and returned.
| /** | ||
| * Called when the server receives an incoming message. | ||
| * | ||
| * If the message is not a `string`, its type is based on the value of `binaryType`. | ||
| * - if `nodebuffer`, then the message is a `Buffer`. | ||
| * - if `arraybuffer`, then the message is an `ArrayBuffer`. | ||
| * - if `uint8array`, then the message is a `Uint8Array`. | ||
| * | ||
| * @param ws The websocket that sent the message | ||
| * @param message The message received | ||
| */ | ||
| message(ws: ServerWebSocket<T>, message: string | Buffer): void | Promise<void>; | ||
|
|
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.
message() must reflect binaryType (include ArrayBuffer and Uint8Array).
Docs above state the runtime shape depends on binaryType, but the type only allows Buffer. This breaks typed usage when users set ws.binaryType = "arraybuffer" or "uint8array".
Apply this diff:
- message(ws: ServerWebSocket<T>, message: string | Buffer): void | Promise<void>;
+ message(
+ ws: ServerWebSocket<T>,
+ message: string | Buffer | ArrayBuffer | Uint8Array
+ ): void | Promise<void>;📝 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.
| /** | |
| * Called when the server receives an incoming message. | |
| * | |
| * If the message is not a `string`, its type is based on the value of `binaryType`. | |
| * - if `nodebuffer`, then the message is a `Buffer`. | |
| * - if `arraybuffer`, then the message is an `ArrayBuffer`. | |
| * - if `uint8array`, then the message is a `Uint8Array`. | |
| * | |
| * @param ws The websocket that sent the message | |
| * @param message The message received | |
| */ | |
| message(ws: ServerWebSocket<T>, message: string | Buffer): void | Promise<void>; | |
| /** | |
| * Called when the server receives an incoming message. | |
| * | |
| * If the message is not a `string`, its type is based on the value of `binaryType`. | |
| * - if `nodebuffer`, then the message is a `Buffer`. | |
| * - if `arraybuffer`, then the message is an `ArrayBuffer`. | |
| * - if `uint8array`, then the message is a `Uint8Array`. | |
| * | |
| * @param ws The websocket that sent the message | |
| * @param message The message received | |
| */ | |
| message( | |
| ws: ServerWebSocket<T>, | |
| message: string | Buffer | ArrayBuffer | Uint8Array | |
| ): void | Promise<void>; |
🤖 Prompt for AI Agents
In packages/bun-types/serve.d.ts around lines 363 to 375, the message handler
type only allows string or Buffer while the docs above state the runtime type
varies with ws.binaryType; update the message parameter union to include
ArrayBuffer and Uint8Array so it reads string | Buffer | ArrayBuffer |
Uint8Array, preserving the existing ServerWebSocket<T> first parameter and the
void | Promise<void> return type; this ensures typed handlers match
ws.binaryType = "arraybuffer" or "uint8array".
| | ((opts: { root: string }) => Bun.MaybePromise<string | Buffer | DirectoryTree>); | ||
| }; |
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.
🛠️ Refactor suggestion
makeTreeSync now mis-handles async DirectoryTree entries; add guard and proper typing
Changing DirectoryTree function entries to Bun.MaybePromise means the sync builder can receive Promises. Today it casts to string and will incorrectly treat a Promise as a directory, causing runtime failures. Add a Promise guard and support Buffer/DirectoryTree without unsafe casts.
Apply:
@@
export function makeTreeSyncFromDirectoryTree(base: string, tree: DirectoryTree) {
const isDirectoryTree = (value: string | DirectoryTree | Buffer): value is DirectoryTree =>
typeof value === "object" && value && typeof value?.byteLength === "undefined";
for (const [name, raw_contents] of Object.entries(tree)) {
- const contents = (typeof raw_contents === "function" ? raw_contents({ root: base }) : raw_contents) as string;
+ const maybe = typeof raw_contents === "function" ? raw_contents({ root: base }) : raw_contents;
+ if (maybe && typeof (maybe as any).then === "function") {
+ throw new Error("makeTreeSync() does not support async DirectoryTree entries. Use makeTree() instead.");
+ }
+ const contents = maybe as string | Buffer | DirectoryTree;
const joined = join(base, name);
if (name.includes("/")) {
const dir = dirname(name);
if (dir !== name && dir !== ".") {
fs.mkdirSync(join(base, dir), { recursive: true });
}
}
if (isDirectoryTree(contents)) {
- fs.mkdirSync(joined);
+ fs.mkdirSync(joined, { recursive: true });
makeTreeSync(joined, contents);
continue;
}
- fs.writeFileSync(joined, contents);
+ fs.writeFileSync(joined, contents as string | Buffer);
}
}| port: 3000, | ||
| fetch() { | ||
| return new Response("port as number"); | ||
| }, | ||
| }); |
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.
🛠️ Refactor suggestion
Avoid hardcoded ports in tests (use port: 0 / "0")
Hardcoding 3000/3001 can cause flakiness/EADDRINUSE in CI. Guidelines: always use port: 0.
test("port: number type", {
- port: 3000,
+ port: 0,
fetch() {
return new Response("port as number");
},
});
test("port: string type", {
- port: "3001",
+ port: "0",
fetch() {
return new Response("port as string");
},
});Also applies to: 617-622
🤖 Prompt for AI Agents
In test/integration/bun-types/fixture/serve-types.test.ts around lines 611-615
(and also update the similar block at 617-622), replace the hardcoded port
numbers (3000/3001) with port: 0 (or "0") so the OS assigns an available port to
avoid EADDRINUSE flakes in CI; update both fixtures to use port: 0 and ensure
any assertions or fetch logic that depended on the literal port use the server's
actual bound port (e.g., read it from the server instance) or remain
port-agnostic so tests continue to pass.
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: 0
🧹 Nitpick comments (1)
test/integration/bun-types/bun-types.test.ts (1)
444-448: Reduce brittleness by not pinning exact line/columnOptional: use an asymmetric matcher for the line to avoid churn when fixture lines move.
{ code: 2345, - line: "index.ts:322:29", + line: expect.stringMatching(/^index\.ts:\d+:\d+$/), message: "Argument of type '{ headers: { \"x-bun\": string; }; }' is not assignable to parameter of type 'number'.", },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/bun-types/deprecated.d.ts(1 hunks)test/integration/bun-types/bun-types.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/bun-types/deprecated.d.ts
🧰 Additional context used
📓 Path-based instructions (6)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/integration/bun-types/bun-types.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/integration/bun-types/bun-types.test.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.ts: Name test files*.test.tsand usebun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; useport: 0to get a random port
When spawning Bun in tests, usebunExe()andbunEnvfromharness
Preferasync/awaitin tests; for a single callback, usePromise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
UsetempDir/tempDirWithFilesfromharnessfor temporary files and directories in tests
For large/repetitive strings in tests, preferBuffer.alloc(count, fill).toString()over"A".repeat(count)
Import common test utilities fromharness(e.g.,bunExe,bunEnv,tempDirWithFiles,tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and usetoThrowfor synchronous errors
Usedescribeblocks for grouping,describe.eachfor parameterized tests, snapshots withtoMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach); track resources for cleanup inafterEach
Useusing/await usingwith Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Files:
test/integration/bun-types/bun-types.test.ts
test/integration/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Place integration tests under
test/integration/
Files:
test/integration/bun-types/bun-types.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must be placed under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or use custom random port functions
In tests, use normalizeBunSnapshot when asserting snapshots
Never write tests that merely assert absence of "panic" or "uncaught exception" in output
Avoid shell commands (e.g., find, grep) in tests; use Bun.Glob and built-ins instead
Prefer snapshot tests over exact stdout equality assertions
Files:
test/integration/bun-types/bun-types.test.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
test/integration/bun-types/bun-types.test.ts
🧠 Learnings (8)
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/integration/bun-types/bun-types.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Name test files `*.test.ts` and use `bun:test`
Applied to files:
test/integration/bun-types/bun-types.test.ts
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add a corresponding test case in test/v8/v8.test.ts that invokes checkSameOutput with the new function
Applied to files:
test/integration/bun-types/bun-types.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
test/integration/bun-types/bun-types.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/integration/bun-types/bun-types.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/integration/bun-types/bun-types.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Do not set explicit test timeouts; rely on Bun’s built-in timeouts
Applied to files:
test/integration/bun-types/bun-types.test.ts
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: Applies to test/**/*.test.{ts,tsx} : In tests, use normalizeBunSnapshot when asserting snapshots
Applied to files:
test/integration/bun-types/bun-types.test.ts
🔇 Additional comments (1)
test/integration/bun-types/bun-types.test.ts (1)
444-448: LGTM: updated TS2345 diagnostic locationThe shifted index.ts position matches fixture/type surface changes; expectation looks correct.
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: 0
♻️ Duplicate comments (1)
test/integration/bun-types/fixture/serve-types.test.ts (1)
613-625: Avoid hardcoded ports in tests.
🧹 Nitpick comments (1)
test/integration/bun-types/fixture/serve-types.test.ts (1)
299-301: Consider consistent undefined return style.While functionally equivalent, consider using
return undefined;for consistency with line 226, or usereturn;consistently throughout.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
test/integration/bun-types/fixture/serve-types.test.ts(11 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/integration/bun-types/fixture/serve-types.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/integration/bun-types/fixture/serve-types.test.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.ts: Name test files*.test.tsand usebun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; useport: 0to get a random port
When spawning Bun in tests, usebunExe()andbunEnvfromharness
Preferasync/awaitin tests; for a single callback, usePromise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
UsetempDir/tempDirWithFilesfromharnessfor temporary files and directories in tests
For large/repetitive strings in tests, preferBuffer.alloc(count, fill).toString()over"A".repeat(count)
Import common test utilities fromharness(e.g.,bunExe,bunEnv,tempDirWithFiles,tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and usetoThrowfor synchronous errors
Usedescribeblocks for grouping,describe.eachfor parameterized tests, snapshots withtoMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach); track resources for cleanup inafterEach
Useusing/await usingwith Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Files:
test/integration/bun-types/fixture/serve-types.test.ts
test/integration/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Place integration tests under
test/integration/
Files:
test/integration/bun-types/fixture/serve-types.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must be placed under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or use custom random port functions
In tests, use normalizeBunSnapshot when asserting snapshots
Never write tests that merely assert absence of "panic" or "uncaught exception" in output
Avoid shell commands (e.g., find, grep) in tests; use Bun.Glob and built-ins instead
Prefer snapshot tests over exact stdout equality assertions
Files:
test/integration/bun-types/fixture/serve-types.test.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
test/integration/bun-types/fixture/serve-types.test.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Use `using`/`await using` with Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Never hardcode port numbers in tests; use `port: 0` to get a random port
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: Applies to test/**/*.test.{ts,tsx} : In tests, always use port: 0; do not hardcode ports or use custom random port functions
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Import common test utilities from `harness` (e.g., `bunExe`, `bunEnv`, `tempDirWithFiles`, `tmpdirSync`, platform checks, GC helpers)
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/bake-harness.ts : Provide and maintain shared test utilities: devTest, prodTest, devAndProductionTest, Dev, Client, and helpers in the harness
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Use `using`/`await using` with Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Import testing utilities (devTest, prodTest, devAndProductionTest, Dev, Client) from test/bake/bake-harness.ts
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/*.test.ts : Write Dev Server and HMR tests in test/bake/dev/*.test.ts using devTest from the shared harness
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/react-spa.test.ts : react-spa.test.ts should contain React SPA, react-refresh, and basic server component transform tests
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-02T05:33:37.517Z
Learnt from: Jarred-Sumner
PR: oven-sh/bun#22323
File: test/js/web/websocket/websocket-subprotocol.test.ts:74-75
Timestamp: 2025-09-02T05:33:37.517Z
Learning: In Bun's runtime, `await using` with Node.js APIs like `net.createServer()` is properly supported and should not be replaced with explicit cleanup. Bun has extended Node.js APIs with proper async dispose support.
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Dev server tests should be thorough yet concise to validate hot-reloading robustness, correctness, and reliability
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
🧬 Code graph analysis (1)
test/integration/bun-types/fixture/serve-types.test.ts (2)
test/integration/bun-types/fixture/utilities.ts (1)
expectType(35-37)test/harness.ts (1)
isCI(26-26)
🪛 Biome (2.1.2)
test/integration/bun-types/fixture/serve-types.test.ts
[error] 26-26: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (9)
test/integration/bun-types/fixture/serve-types.test.ts (9)
17-25: LGTM! Clean type export for WebSocket data.The new default export properly demonstrates the typed WebSocket data pattern, aligning with the PR's objective to improve
Bun.serve({ websocket })types.
30-43: Good type refinements for the test harness.The updated generic constraints and
NoInferwrapper provide better type safety for server testing. The skip option addition improves test flexibility.
119-120: Proper WebSocket type assertion added.Good use of explicit type assertion to verify that WebSocket without data defaults to
undefined.
141-150: Excellent upgrade method type validation.This properly verifies the typed upgrade signature with optional data and headers, ensuring compile-time safety for WebSocket upgrades.
170-191: Well-structured WebSocket lifecycle handlers with typed data.The addition of proper type assertions for message types (
string | Buffer<ArrayBufferLike>), code/reason in close handler, and data access in drain handler provides comprehensive type coverage.
538-541: Good TypeScript error validation for mutually exclusive options.Proper use of
@ts-expect-errorto validate that TypeScript correctly prevents combiningunixandportoptions.Also applies to: 557-560
577-648: Comprehensive hostname and port configuration tests.Good coverage of various hostname configurations including IPv4, IPv6, localhost, and environment variable port handling.
684-685: Good CI-aware test skipping.Smart use of
isCIflag to skip potentially flaky IPv6-only tests in CI environments.
688-833: Thorough server configuration coverage.Excellent test coverage for all server configuration options including timeouts, body size limits, development mode, error handling, and combined configurations. The async error handler test at lines 779-797 is particularly valuable.
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: 0
🧹 Nitpick comments (4)
test/integration/bun-types/fixture/serve-types.test.ts (3)
25-27: Rename param to avoid restricted-name shadowing ("constructor").This triggers Biome’s noShadowRestrictedNames. Rename the parameter and usage.
-function expectInstanceOf<T>(value: unknown, constructor: new (...args: any[]) => T): asserts value is T { - expect(value).toBeInstanceOf(constructor); +function expectInstanceOf<T>(value: unknown, Ctor: new (...args: any[]) => T): asserts value is T { + expect(value).toBeInstanceOf(Ctor); }
770-773: Avoid arbitrary sleeps in tests.Use a microtask to assert async behavior without time-based flakiness.
- async error(error) { - await new Promise(resolve => setTimeout(resolve, 10)); + async error(error) { + await Promise.resolve(); return new Response(`Async error handled: ${error.message}`, { status: 503 }); },
56-63: Redundant stop when usingusing.
using server = Bun.serve(...)disposes on scope exit. The explicitawait server.stop(true)is likely unnecessary unless you specifically need the “close active connections” semantics.packages/bun-types/bun.d.ts (1)
2825-2867: TLSOptions: alignALPNProtocolswith de-facto Node semantics.Node-style options accept plural forms; single
string | BufferSourceis likely too narrow.- ALPNProtocols?: string | BufferSource; + ALPNProtocols?: string | string[] | BufferSource | BufferSource[];Also, when both path-based (
keyFile/certFile) and in-memory (key/cert) are provided, which takes precedence? Please document/clarify in types or docs to avoid ambiguity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/bun-types/bun.d.ts(3 hunks)test/harness.ts(1 hunks)test/integration/bun-types/fixture/serve-types.test.ts(10 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/harness.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
packages/bun-types/bun.d.tstest/integration/bun-types/fixture/serve-types.test.ts
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/integration/bun-types/fixture/serve-types.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/integration/bun-types/fixture/serve-types.test.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.ts: Name test files*.test.tsand usebun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; useport: 0to get a random port
When spawning Bun in tests, usebunExe()andbunEnvfromharness
Preferasync/awaitin tests; for a single callback, usePromise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
UsetempDir/tempDirWithFilesfromharnessfor temporary files and directories in tests
For large/repetitive strings in tests, preferBuffer.alloc(count, fill).toString()over"A".repeat(count)
Import common test utilities fromharness(e.g.,bunExe,bunEnv,tempDirWithFiles,tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and usetoThrowfor synchronous errors
Usedescribeblocks for grouping,describe.eachfor parameterized tests, snapshots withtoMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach); track resources for cleanup inafterEach
Useusing/await usingwith Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Files:
test/integration/bun-types/fixture/serve-types.test.ts
test/integration/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Place integration tests under
test/integration/
Files:
test/integration/bun-types/fixture/serve-types.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must be placed under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or use custom random port functions
In tests, use normalizeBunSnapshot when asserting snapshots
Never write tests that merely assert absence of "panic" or "uncaught exception" in output
Avoid shell commands (e.g., find, grep) in tests; use Bun.Glob and built-ins instead
Prefer snapshot tests over exact stdout equality assertions
Files:
test/integration/bun-types/fixture/serve-types.test.ts
🧠 Learnings (13)
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/bundle.test.ts : bundle.test.ts should contain DevServer-specific bundling tests
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/react-spa.test.ts : react-spa.test.ts should contain React SPA, react-refresh, and basic server component transform tests
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/*.test.ts : Write Dev Server and HMR tests in test/bake/dev/*.test.ts using devTest from the shared harness
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Never hardcode port numbers in tests; use `port: 0` to get a random port
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: Applies to test/**/*.test.{ts,tsx} : In tests, always use port: 0; do not hardcode ports or use custom random port functions
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Use `using`/`await using` with Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-20T03:39:41.748Z
Learnt from: pfgithub
PR: oven-sh/bun#22534
File: test/regression/issue/21830.fixture.ts:14-63
Timestamp: 2025-09-20T03:39:41.748Z
Learning: Bun's test runner supports async describe callbacks, unlike Jest/Vitest where describe callbacks must be synchronous. The syntax `describe("name", async () => { ... })` is valid in Bun.
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-09-02T05:33:37.517Z
Learnt from: Jarred-Sumner
PR: oven-sh/bun#22323
File: test/js/web/websocket/websocket-subprotocol.test.ts:74-75
Timestamp: 2025-09-02T05:33:37.517Z
Learning: In Bun's runtime, `await using` with Node.js APIs like `net.createServer()` is properly supported and should not be replaced with explicit cleanup. Bun has extended Node.js APIs with proper async dispose support.
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Dev server tests should be thorough yet concise to validate hot-reloading robustness, correctness, and reliability
Applied to files:
test/integration/bun-types/fixture/serve-types.test.ts
🧬 Code graph analysis (1)
test/integration/bun-types/fixture/serve-types.test.ts (2)
test/integration/bun-types/fixture/utilities.ts (1)
expectType(35-37)test/integration/bun-types/fixture/serve.ts (4)
message(18-20)message(35-37)message(52-54)message(67-69)
🪛 Biome (2.1.2)
test/integration/bun-types/fixture/serve-types.test.ts
[error] 25-25: Do not shadow the global "constructor" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (5)
packages/bun-types/bun.d.ts (2)
2775-2779:ErrorLikeaddition looks good.Matches common errno-style error shapes and is safe as a widening over
Error.
2869-2884:SocketAddressAPI shape LGTM.Clear, minimal, and consistent with other networking types.
test/integration/bun-types/fixture/serve-types.test.ts (3)
612-617: Avoid hardcoded ports in tests (use port: 0).Prevents EADDRINUSE flakes in CI. Same as prior bot comment.
test("port: number type", { - port: 3000, + port: 0, fetch() { return new Response("port as number"); }, });
619-624: Avoid hardcoded ports in tests (use "0").String variant should also be dynamic.
test("port: string type", { - port: "3001", + port: "0", fetch() { return new Response("port as string"); }, });
634-647: Env-driven port fallback should not be hardcoded.Use "0" as fallback to keep tests non-flaky.
{ - port: process.env.PORT || "3002", + port: process.env.PORT || "0", fetch() { return new Response("port from env"); }, },
|
Woohoo! 🙌 |
|
@chevcast 1.3 soon! |
What does this PR do?
Fixes #19659
Fixes #19246
Fixes #22948
Fixes the types for Bun.serve() when using
ServerWebSocket.dataHow did you verify your code works?
bun-types.test.ts & serve-types.test.ts updated with lots of variations/common patterns