Fix Bun.serve to respect development flag for sourcemap generation #24542
Conversation
When development: false is set in Bun.serve, sourcemaps should not be
generated or included in the bundled output. Previously, sourcemaps were
always generated with config.source_map = .linked regardless of the
development mode.
This change makes sourcemap generation conditional:
- development: false -> config.source_map = .none
- development: true -> config.source_map = .linked
Added comprehensive tests to verify:
- Sourcemaps are not included when development: false
- Sourcemaps are included when development: true
- Sourcemaps are included when development: { hmr: false }
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <[email protected]>
|
Updated 4:30 PM PT - Nov 9th, 2025
❌ Your commit
🧪 To try this PR locally: bunx bun-pr 24542That installs a local version of the PR into your bun-24542 --bun |
WalkthroughModified source map configuration in the HTML bundler to conditionally set source maps based on environment (production or development). Added a comprehensive test suite with three test cases and corresponding server fixture files to validate source map behavior in different configurations. Changes
Suggested reviewers
Pre-merge checks✅ Passed checks (2 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
test/js/bun/http/bun-serve-html-sourcemaps.test.ts (2)
177-212: Same issues: unused variable.Line 191: The
processvariable is assigned but never used, same as inwaitForProductionServer.
214-249: Same issues: unused variable.Line 228: The
processvariable is assigned but never used, same as in the other helper functions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
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 (5)
src/bun.js/api/server/HTMLBundle.zig(1 hunks)test/js/bun/http/bun-serve-html-sourcemaps.test.ts(1 hunks)test/js/bun/http/fixtures/serve-dev-no-hmr.js(1 hunks)test/js/bun/http/fixtures/serve-development.js(1 hunks)test/js/bun/http/fixtures/serve-production.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Files:
src/bun.js/api/server/HTMLBundle.zig
src/{**/js_*.zig,bun.js/api/**/*.zig}
📄 CodeRabbit inference engine (.cursor/rules/registering-bun-modules.mdc)
Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions
Files:
src/bun.js/api/server/HTMLBundle.zig
src/bun.js/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy
Files:
src/bun.js/api/server/HTMLBundle.zig
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
When adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
src/**/*.zig: Use Zig private fields with the # prefix for encapsulation (e.g., struct { #foo: u32 })
Prefer Decl literals for initialization (e.g., const decl: Decl = .{ .binding = 0, .value = 0 };)
Place @import statements at the bottom of the file (formatter will handle ordering)
Files:
src/bun.js/api/server/HTMLBundle.zig
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/js/bun/http/fixtures/serve-development.jstest/js/bun/http/bun-serve-html-sourcemaps.test.tstest/js/bun/http/fixtures/serve-dev-no-hmr.jstest/js/bun/http/fixtures/serve-production.js
test/js/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place JavaScript and TypeScript tests under test/js/
Files:
test/js/bun/http/fixtures/serve-development.jstest/js/bun/http/bun-serve-html-sourcemaps.test.tstest/js/bun/http/fixtures/serve-dev-no-hmr.jstest/js/bun/http/fixtures/serve-production.js
test/js/bun/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Files:
test/js/bun/http/fixtures/serve-development.jstest/js/bun/http/bun-serve-html-sourcemaps.test.tstest/js/bun/http/fixtures/serve-dev-no-hmr.jstest/js/bun/http/fixtures/serve-production.js
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/js/bun/http/fixtures/serve-development.jstest/js/bun/http/bun-serve-html-sourcemaps.test.tstest/js/bun/http/fixtures/serve-dev-no-hmr.jstest/js/bun/http/fixtures/serve-production.js
🧠 Learnings (26)
📓 Common learnings
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/sourcemap.test.ts : sourcemap.test.ts should verify source map correctness in dev
Learnt from: CR
Repo: oven-sh/bun PR: 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
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/bundler/**/* : Place bundler/transpiler/CSS/bun build tests under test/bundler/
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/css.test.ts : css.test.ts should contain CSS bundling tests in dev mode
Learnt from: CR
Repo: oven-sh/bun PR: 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
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev-and-prod.ts : Use devAndProductionTest to run the same test in both dev and production; these tests must not attempt to test hot reloading
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/esm.test.ts : esm.test.ts should cover ESM feature behavior in development mode
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/html.test.ts : html.test.ts should contain tests relating to HTML files themselves
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Learnt from: CR
Repo: oven-sh/bun PR: 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
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 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:
src/bun.js/api/server/HTMLBundle.zigtest/js/bun/http/fixtures/serve-development.jstest/js/bun/http/bun-serve-html-sourcemaps.test.tstest/js/bun/http/fixtures/serve-dev-no-hmr.jstest/js/bun/http/fixtures/serve-production.js
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Update src/bun.js/bindings/generated_classes_list.zig to include new classes
Applied to files:
src/bun.js/api/server/HTMLBundle.zig
📚 Learning: 2025-10-04T09:52:49.414Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-10-04T09:52:49.414Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use process.platform and process.arch for platform detection (rely on inlining/dead-code elimination)
Applied to files:
src/bun.js/api/server/HTMLBundle.zig
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/sourcemap.test.ts : sourcemap.test.ts should verify source map correctness in dev
Applied to files:
src/bun.js/api/server/HTMLBundle.zigtest/js/bun/http/bun-serve-html-sourcemaps.test.ts
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS<ClassName> and re-export toJS/fromJS/fromJSDirect
Applied to files:
src/bun.js/api/server/HTMLBundle.zig
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Applied to files:
src/bun.js/api/server/HTMLBundle.zig
📚 Learning: 2025-10-19T02:52:37.412Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/tsconfig.json:1-15
Timestamp: 2025-10-19T02:52:37.412Z
Learning: In the Bun repository, packages under packages/ (e.g., bun-otel) can follow a TypeScript-first pattern where package.json exports point directly to .ts files (not compiled .js files). Bun natively runs TypeScript, so consumers import .ts sources directly and receive full type information without needing compiled .d.ts declaration files. For such packages, adding "declaration": true or "outDir" in tsconfig.json is unnecessary and would break the export structure.
<!-- [remove_learning]
ceedde95-980e-4898-a2c6-40ff73913664
Applied to files:
src/bun.js/api/server/HTMLBundle.zig
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/css.test.ts : css.test.ts should contain CSS bundling tests in dev mode
Applied to files:
src/bun.js/api/server/HTMLBundle.zigtest/js/bun/http/bun-serve-html-sourcemaps.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
Repo: oven-sh/bun PR: 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:
src/bun.js/api/server/HTMLBundle.zigtest/js/bun/http/fixtures/serve-development.jstest/js/bun/http/bun-serve-html-sourcemaps.test.tstest/js/bun/http/fixtures/serve-dev-no-hmr.jstest/js/bun/http/fixtures/serve-production.js
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev-and-prod.ts : Use devAndProductionTest to run the same test in both dev and production; these tests must not attempt to test hot reloading
Applied to files:
src/bun.js/api/server/HTMLBundle.zigtest/js/bun/http/bun-serve-html-sourcemaps.test.tstest/js/bun/http/fixtures/serve-production.js
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 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/js/bun/http/fixtures/serve-development.jstest/js/bun/http/bun-serve-html-sourcemaps.test.tstest/js/bun/http/fixtures/serve-dev-no-hmr.jstest/js/bun/http/fixtures/serve-production.js
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
Repo: oven-sh/bun PR: 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/js/bun/http/fixtures/serve-development.jstest/js/bun/http/bun-serve-html-sourcemaps.test.tstest/js/bun/http/fixtures/serve-dev-no-hmr.jstest/js/bun/http/fixtures/serve-production.js
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
Repo: oven-sh/bun PR: 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/js/bun/http/fixtures/serve-development.jstest/js/bun/http/bun-serve-html-sourcemaps.test.tstest/js/bun/http/fixtures/serve-dev-no-hmr.jstest/js/bun/http/fixtures/serve-production.js
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/client-fixture.mjs : Client subprocess logic (page load, IPC, error/reload coordination) should live in client-fixture.mjs and be driven by Client
Applied to files:
test/js/bun/http/fixtures/serve-development.jstest/js/bun/http/fixtures/serve-dev-no-hmr.jstest/js/bun/http/fixtures/serve-production.js
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 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} : Do not use node:fs APIs in tests; mutate files via dev.write, dev.patch, and dev.delete
Applied to files:
test/js/bun/http/fixtures/serve-development.jstest/js/bun/http/fixtures/serve-dev-no-hmr.jstest/js/bun/http/fixtures/serve-production.js
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 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/js/bun/http/fixtures/serve-development.jstest/js/bun/http/bun-serve-html-sourcemaps.test.tstest/js/bun/http/fixtures/serve-dev-no-hmr.js
📚 Learning: 2025-10-19T02:44:46.354Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.
Applied to files:
test/js/bun/http/fixtures/serve-development.jstest/js/bun/http/bun-serve-html-sourcemaps.test.tstest/js/bun/http/fixtures/serve-production.js
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/esm.test.ts : esm.test.ts should cover ESM feature behavior in development mode
Applied to files:
test/js/bun/http/fixtures/serve-development.js
📚 Learning: 2025-09-02T05:33:37.517Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 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/js/bun/http/fixtures/serve-development.jstest/js/bun/http/fixtures/serve-dev-no-hmr.jstest/js/bun/http/fixtures/serve-production.js
📚 Learning: 2025-10-19T04:38:37.720Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/examples/basic.ts:10-10
Timestamp: 2025-10-19T04:38:37.720Z
Learning: In packages/bun-otel/bun-sdk.ts, BunSDK.start() is intentionally synchronous (returns void) unlike NodeSDK.start() which is async. This is because BunSDK performs resource detection synchronously in the constructor using detectResourcesSync, while NodeSDK performs async resource detection during start(). The start() method only performs synchronous operations (setting context manager, propagator, creating tracer provider, calling installBunNativeTracing), so an async signature would be misleading.
Applied to files:
test/js/bun/http/fixtures/serve-development.jstest/js/bun/http/fixtures/serve-production.js
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/html.test.ts : html.test.ts should contain tests relating to HTML files themselves
Applied to files:
test/js/bun/http/bun-serve-html-sourcemaps.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 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/js/bun/http/bun-serve-html-sourcemaps.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 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/js/bun/http/bun-serve-html-sourcemaps.test.ts
📚 Learning: 2025-10-19T04:55:33.099Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.
Applied to files:
test/js/bun/http/fixtures/serve-dev-no-hmr.jstest/js/bun/http/fixtures/serve-production.js
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
Repo: oven-sh/bun PR: 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/js/bun/http/fixtures/serve-dev-no-hmr.js
🧬 Code graph analysis (4)
test/js/bun/http/fixtures/serve-development.js (2)
test/js/bun/http/fixtures/serve-dev-no-hmr.js (3)
server(1-11)files(14-14)routes(15-15)test/js/bun/http/fixtures/serve-production.js (3)
server(1-9)files(12-12)routes(13-13)
test/js/bun/http/bun-serve-html-sourcemaps.test.ts (4)
test/harness.ts (2)
tempDirWithFiles(259-266)bunExe(102-105)test/js/bun/http/fixtures/serve-dev-no-hmr.js (1)
server(1-11)test/js/bun/http/fixtures/serve-development.js (1)
server(1-9)test/js/bun/http/fixtures/serve-production.js (1)
server(1-9)
test/js/bun/http/fixtures/serve-dev-no-hmr.js (2)
test/js/bun/http/fixtures/serve-development.js (3)
server(1-9)files(12-12)routes(13-13)test/js/bun/http/fixtures/serve-production.js (3)
server(1-9)files(12-12)routes(13-13)
test/js/bun/http/fixtures/serve-production.js (2)
test/js/bun/http/fixtures/serve-dev-no-hmr.js (3)
server(1-11)files(14-14)routes(15-15)test/js/bun/http/fixtures/serve-development.js (3)
server(1-9)files(12-12)routes(13-13)
🔇 Additional comments (9)
src/bun.js/api/server/HTMLBundle.zig (1)
282-290: LGTM! Sourcemap configuration now respects development flag.The conditional sourcemap assignment correctly fixes the bug where sourcemaps were generated in production. The logic is clear:
- Production (
!is_development): No sourcemaps (.none)- Development: Linked sourcemaps (
.linked)test/js/bun/http/fixtures/serve-production.js (3)
1-9: LGTM! Production server configuration is correct.The server is properly configured with
development: falseand an ephemeral port for testing.
11-22: Consider adding error handling for dynamic imports.If any module import fails at line 15, the error would propagate uncaught. For production test reliability, consider wrapping the import loop in try-catch.
That said, if this is acceptable behavior for test fixtures (failing fast on import errors), the current implementation is fine.
24-27: LGTM! IPC communication pattern is correct.Sending port and hostname back to the parent process enables the test to connect to the spawned server.
test/js/bun/http/fixtures/serve-dev-no-hmr.js (1)
13-26: Consider error handling for dynamic imports.Same as serve-production.js: if import fails at line 17, the error propagates uncaught. Consider adding error handling if test reliability requires it.
test/js/bun/http/bun-serve-html-sourcemaps.test.ts (4)
7-51: LGTM! Production sourcemap test validates the fix correctly.The test properly verifies that production builds (
development: false) do not generate sourcemaps:
- No
sourceMappingURLcomment in JS- No
SourceMapheaderThe use of
await usingfor subprocess cleanup is appropriate.
53-97: LGTM! Development sourcemap test correctly validates sourcemap generation.The test properly verifies that development builds (
development: true) generate sourcemaps:
sourceMappingURLcomment is present in JSSourceMapheader is present
99-137: LGTM! Dev-no-HMR test correctly validates sourcemaps are still generated.The test properly verifies that
development: { hmr: false }still generates sourcemaps since it's considered development mode. The comment at line 134 clearly explains the intent.
140-175: Unused variable and refactoring opportunity.Line 154: The
processvariable is assigned but never used. The subprocess is correctly returned via theipccallback, but the variable name is misleading.Consider either:
- Removing the assignment:
Bun.spawn({...})- Or renaming to indicate it's intentionally unused:
const _process = ...Additionally, all three
waitFor*Serverfunctions (lines 140-249) are nearly identical, differing only in the fixture filename. Consider extracting a shared helper:async function waitForServer( fixtureName: string, dir: string, entryPoints: Record<string, string>, ): Promise<{ subprocess: Subprocess; port: number; hostname: string; }> { const defer = Promise.withResolvers<{ subprocess: Subprocess; port: number; hostname: string; }>(); Bun.spawn({ cmd: [bunExe(), join(import.meta.dir, "fixtures", fixtureName)], env: { ...bunEnv, NODE_ENV: undefined, }, cwd: dir, stdio: ["inherit", "inherit", "inherit"], ipc(message, subprocess) { subprocess.send({ files: entryPoints }); defer.resolve({ subprocess, port: message.port, hostname: message.hostname, }); }, }); return defer.promise; } // Then: const waitForProductionServer = (dir, ep) => waitForServer("serve-production.js", dir, ep); const waitForDevelopmentServer = (dir, ep) => waitForServer("serve-development.js", dir, ep); const waitForDevNoHMRServer = (dir, ep) => waitForServer("serve-dev-no-hmr.js", dir, ep);⛔ Skipped due to learnings
Learnt from: markovejnovic Repo: oven-sh/bun PR: 24417 File: test/js/bun/spawn/spawn.test.ts:903-918 Timestamp: 2025-11-06T00:58:23.965Z Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.Learnt from: CR Repo: oven-sh/bun PR: 0 File: .cursor/rules/dev-server-tests.mdc:0-0 Timestamp: 2025-08-30T00:09:39.100Z Learning: Applies to test/bake/client-fixture.mjs : Client subprocess logic (page load, IPC, error/reload coordination) should live in client-fixture.mjs and be driven by ClientLearnt from: Jarred-Sumner Repo: oven-sh/bun PR: 24491 File: test/js/bun/transpiler/declare-global.test.ts:17-17 Timestamp: 2025-11-08T04:06:33.198Z Learning: In Bun test files, `await using` with Bun.spawn() is the preferred pattern for spawned processes regardless of whether they are short-lived or long-running. Do not suggest replacing `await using proc = Bun.spawn(...)` with `const proc = Bun.spawn(...); await proc.exited;`.Learnt from: CR Repo: oven-sh/bun PR: 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 harnessLearnt from: CR Repo: oven-sh/bun PR: 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 pipesLearnt from: CR Repo: oven-sh/bun PR: 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 testsLearnt from: CR Repo: oven-sh/bun PR: 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 harnessLearnt from: Jarred-Sumner Repo: oven-sh/bun PR: 24082 File: test/cli/test/coverage.test.ts:60-112 Timestamp: 2025-10-26T01:32:04.844Z Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.Learnt from: CR Repo: oven-sh/bun PR: 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} : Do not use node:fs APIs in tests; mutate files via dev.write, dev.patch, and dev.deleteLearnt from: CR Repo: oven-sh/bun PR: 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
| let server = Bun.serve({ | ||
| port: 0, | ||
| development: { | ||
| hmr: false, | ||
| }, | ||
| async fetch(req) { | ||
| return new Response("Hello World", { | ||
| status: 404, | ||
| }); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! Dev server with HMR disabled is configured correctly.
Using development: { hmr: false } correctly tests the scenario where development mode is enabled but HMR is disabled.
Note: This fixture file is nearly identical to serve-production.js and serve-development.js. Consider extracting a shared factory function to reduce duplication if more fixtures are added in the future.
🤖 Prompt for AI Agents
test/js/bun/http/fixtures/serve-dev-no-hmr.js lines 1-11: this fixture is nearly
identical to serve-production.js and serve-development.js; extract a shared
factory function (e.g., createServeFixture(options)) in a new helper module
under test/js/bun/http/fixtures/helpers.js that accepts the differing options
(port, development.hmr, status, etc.), update the three fixture files to import
and call that factory with their specific config, and ensure tests continue to
export the Bun.serve instance or function the tests expect; keep behavior
unchanged and only move duplicated object construction into the helper.
| let server = Bun.serve({ | ||
| port: 0, | ||
| development: true, | ||
| async fetch(req) { | ||
| return new Response("Hello World", { | ||
| status: 404, | ||
| }); | ||
| }, | ||
| }); | ||
|
|
||
| process.on("message", async message => { | ||
| const files = message.files || {}; | ||
| const routes = {}; | ||
| for (const [key, value] of Object.entries(files)) { | ||
| routes[key] = (await import(value)).default; | ||
| } | ||
|
|
||
| server.reload({ | ||
| static: routes, | ||
| development: true, | ||
| }); | ||
| }); | ||
|
|
||
| process.send({ | ||
| port: server.port, | ||
| hostname: server.hostname, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
LGTM! Development server fixture is configured correctly.
The fixture properly sets development: true for testing sourcemap generation in full development mode.
Note: This is the third nearly-identical fixture file. The three fixtures (serve-production.js, serve-development.js, serve-dev-no-hmr.js) differ only in their development configuration. Consider refactoring to a parameterized fixture factory if maintenance becomes an issue:
// fixtures/serve-factory.js
export function createServerFixture(developmentConfig) {
let server = Bun.serve({
port: 0,
development: developmentConfig,
async fetch(req) {
return new Response("Hello World", { status: 404 });
},
});
process.on("message", async message => {
const files = message.files || {};
const routes = {};
for (const [key, value] of Object.entries(files)) {
routes[key] = (await import(value)).default;
}
server.reload({
static: routes,
development: developmentConfig,
});
});
process.send({
port: server.port,
hostname: server.hostname,
});
}🤖 Prompt for AI Agents
In test/js/bun/http/fixtures/serve-development.js lines 1-27: this fixture is
nearly identical to serve-production.js and serve-dev-no-hmr.js differing only
by the development flag; refactor by extracting the shared server code into a
new factory module (e.g., fixtures/serve-factory.js) that exports a
createServerFixture(developmentConfig) function, update this file to import that
factory and invoke it with development: true, and update the other two fixtures
to call the same factory with their respective boolean values so the duplicated
code is removed and maintenance is centralized.
Summary
Fixes the bug where sourcemaps were being generated and included in Bun.serve output even when
development: falsewas set.According to the official Bun documentation:
And for production mode:
The documentation clearly states that sourcemaps should only be included when
development: true, but the implementation was generating sourcemaps regardless of the development flag.Changes
src/bun.js/api/server/HTMLBundle.zig:282-290to conditionally setconfig.source_mapbased on the development mode:development: false→config.source_map = .none(no sourcemaps)development: true→config.source_map = .linked(sourcemaps with comments)Test Plan
Added comprehensive tests in
test/js/bun/http/bun-serve-html-sourcemaps.test.ts:USE_SYSTEM_BUN=1(confirms bug exists)bun bd test(confirms fix works)development: falsedoes NOT include sourcemap comments or SourceMap headersdevelopment: trueDOES include sourcemap comments and SourceMap headersdevelopment: { hmr: false }DOES include sourcemap comments (still in dev mode)All tests pass:
🤖 Generated with Claude Code