Skip to content

fix: static routes with unicode now send correct Content-Type header#24818

Closed
robobun wants to merge 1 commit into
mainfrom
claude/fix-static-route-unicode
Closed

fix: static routes with unicode now send correct Content-Type header#24818
robobun wants to merge 1 commit into
mainfrom
claude/fix-static-route-unicode

Conversation

@robobun

@robobun robobun commented Nov 18, 2025

Copy link
Copy Markdown
Collaborator

Summary

Fixes #24817 - Static routes with unicode characters now correctly send Content-Type: text/plain;charset=utf-8 header.

Problem

When using static routes like '/path': new Response('▲'), the response was missing the Content-Type header. This caused browsers to interpret the response as Latin-1 instead of UTF-8, resulting in mojibake (garbled text) for unicode characters.

Solution

The fix ensures proper tracking of string-originated response bodies through the following changes:

  1. Body.Value.use() - When converting InternalBlob to Blob, now preserves the was_string flag by setting the blob's charset field (.all_ascii or .non_ascii) and content_type to "text/plain;charset=utf-8"

  2. Body.Value.use() - When converting WTFStringImpl to Blob, sets the appropriate charset and content type

  3. Blob.Any.wasString() - Updated to recognize blobs with .all_ascii or .non_ascii charset as string-originated (not just .all_ascii)

  4. Headers.from() - Now checks both hasContentTypeFromUser() and wasString() to determine if a Content-Type header should be added

  5. StaticRoute.fromJS() - Always calls Headers.from() with the body parameter, even when no explicit headers are set

Test Plan

Created test/regression/issue/24817.test.ts which tests:

  • Basic unicode character (▲) in both dynamic and static routes
  • Japanese characters (こんにちは世界)
  • Emoji (🎉🚀✨)
  • Explicit content-type override behavior

All tests pass with bun bd test test/regression/issue/24817.test.ts

🤖 Generated with Claude Code

@robobun

robobun commented Nov 18, 2025

Copy link
Copy Markdown
Collaborator Author
Updated 3:19 PM PT - Nov 18th, 2025

@RiskyMH, your commit 5c18529 has 4 failures in Build #31979 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 24818

That installs a local version of the PR into your bun-24818 executable, so you can run:

bun-24818 --bun

@coderabbitai

coderabbitai Bot commented Nov 18, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Preserve string-origin charset metadata when converting strings to Blobs, propagate blob ownership into Headers construction for static routes, and auto-add Content-Type for string-derived bodies. Adds a regression test validating Unicode responses and content-type behavior for static and dynamic routes.

Changes

Cohort / File(s) Summary
Blob charset & Body conversions
src/bun.js/webcore/Blob.zig, src/bun.js/webcore/Body.zig
Blob charset/wasString behavior adjusted: wasString now true for both .all_ascii and .non_ascii. Body→Blob conversion preserves was_string and sets the Blob charset and related metadata when creating Blobs from strings.
StaticRoute headers binding
src/bun.js/api/server/StaticRoute.zig
StaticRoute.fromJS now always passes the blob reference via .body into Headers.from in both branches, unifying header construction ownership and OOM/error paths.
Headers Content-Type decision
src/http/Headers.zig
Content-Type is now auto-added when the body has a user-provided content type OR the body originated from a string, if no existing Content-Type header exists; comments clarify the condition.
Regression test
test/regression/issue/24817.test.ts
New test exercising Unicode responses (Japanese, emoji) and verifying response text and Content-Type: text/plain;charset=utf-8 for static and dynamic routes, plus explicit content-type override case.
Docs & misc (non-functional/formatting)
docs/**, packages/bun-types/bun.d.ts, test/bundler/expectBundled.ts, CONTRIBUTING.md
Multiple documentation updates, a minor typo fix, and removal of a duplicate "linked" union member in type definitions and test helper types.

Possibly related PRs

Suggested reviewers

  • Jarred-Sumner
  • alii
  • cirospaciari

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Most changes are in-scope; however, documentation updates (docs/* files) and unrelated fixes (CONTRIBUTING.md typo, duplicate type unions) appear to be outside the scope of fixing #24817. Remove out-of-scope changes: documentation updates (docs/* files, docs/quickstart.mdx, etc.), CONTRIBUTING.md typo fix, and duplicate type union cleanups in bun.d.ts and expectBundled.ts. Focus the PR on #24817.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: static routes with unicode now send correct Content-Type header' directly and clearly describes the main change—adding proper Content-Type headers for unicode in static routes.
Description check ✅ Passed The PR description covers the problem, solution with implementation details, and test plan, though it follows a custom structure rather than the template's exact sections.
Linked Issues check ✅ Passed Code changes address the core requirement from #24817: static routes returning string bodies now send 'Content-Type: text/plain;charset=utf-8', and the regression test validates unicode handling for both dynamic and static routes.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • UTF-8: Entity not found: Issue - Could not find referenced Issue.

Comment @coderabbitai help to get the list of available commands and usage tips.

lots of recent changes aren't present, so this reapplies them
@robobun robobun force-pushed the claude/fix-static-route-unicode branch from 24d0781 to 5c18529 Compare November 18, 2025 22:29
@robobun robobun requested a review from alii as a code owner November 18, 2025 22:29
@robobun

robobun commented Nov 18, 2025

Copy link
Copy Markdown
Collaborator Author

Closing in favor of new PR with fixed commits

@robobun robobun closed this Nov 18, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/guides/http/file-uploads.mdx (1)

84-92: Enhanced example with validation and persistence.

The updated example now demonstrates proper form data validation and file persistence. The validation check for profilePicture prevents errors when the field is missing.

One note: The hardcoded filename "profilePicture.png" will be overwritten on subsequent uploads. For a production example, you might want to add a note about generating unique filenames (e.g., using timestamps or UUIDs), but this is acceptable for a basic tutorial.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 24d0781bd2f6198a0dc894d51b88c78213d81732 and 5c18529.

📒 Files selected for processing (38)
  • CONTRIBUTING.md (1 hunks)
  • docs/bundler/executables.mdx (2 hunks)
  • docs/bundler/index.mdx (1 hunks)
  • docs/docs.json (2 hunks)
  • docs/guides/html-rewriter/extract-links.mdx (2 hunks)
  • docs/guides/http/file-uploads.mdx (2 hunks)
  • docs/guides/install/add-peer.mdx (1 hunks)
  • docs/guides/runtime/cicd.mdx (2 hunks)
  • docs/guides/runtime/define-constant.mdx (2 hunks)
  • docs/guides/test/concurrent-test-glob.mdx (1 hunks)
  • docs/guides/test/spy-on.mdx (2 hunks)
  • docs/guides/websocket/context.mdx (4 hunks)
  • docs/guides/websocket/pubsub.mdx (2 hunks)
  • docs/guides/websocket/simple.mdx (2 hunks)
  • docs/pm/cli/info.mdx (1 hunks)
  • docs/pm/cli/install.mdx (4 hunks)
  • docs/pm/cli/link.mdx (1 hunks)
  • docs/pm/cli/publish.mdx (1 hunks)
  • docs/pm/overrides.mdx (3 hunks)
  • docs/pm/workspaces.mdx (2 hunks)
  • docs/project/contributing.mdx (3 hunks)
  • docs/quickstart.mdx (1 hunks)
  • docs/runtime/bunfig.mdx (2 hunks)
  • docs/runtime/html-rewriter.mdx (1 hunks)
  • docs/runtime/http/routing.mdx (1 hunks)
  • docs/runtime/http/websockets.mdx (3 hunks)
  • docs/runtime/module-resolution.mdx (1 hunks)
  • docs/runtime/redis.mdx (1 hunks)
  • docs/runtime/utils.mdx (1 hunks)
  • docs/test/configuration.mdx (1 hunks)
  • docs/test/reporters.mdx (1 hunks)
  • packages/bun-types/bun.d.ts (1 hunks)
  • src/bun.js/api/server/StaticRoute.zig (1 hunks)
  • src/bun.js/webcore/Blob.zig (2 hunks)
  • src/bun.js/webcore/Body.zig (2 hunks)
  • src/http/Headers.zig (1 hunks)
  • test/bundler/expectBundled.ts (1 hunks)
  • test/regression/issue/24817.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (41)
📚 Learning: 2025-11-14T16:07:01.064Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24719
File: docs/bundler/executables.mdx:527-560
Timestamp: 2025-11-14T16:07:01.064Z
Learning: In the Bun repository, certain bundler features like compile with code splitting (--compile --splitting) are CLI-only and not supported in the Bun.build() JavaScript API. Tests for CLI-only features use backend: "cli" flag (e.g., test/bundler/bundler_compile_splitting.test.ts). The CompileBuildConfig interface correctly restricts these with splitting?: never;. When documenting CLI-only bundler features, add a note clarifying they're not available via the programmatic API.

Applied to files:

  • docs/pm/cli/link.mdx
  • docs/pm/cli/info.mdx
  • docs/runtime/module-resolution.mdx
  • docs/bundler/index.mdx
  • docs/guides/runtime/define-constant.mdx
  • CONTRIBUTING.md
  • docs/pm/cli/install.mdx
  • docs/pm/cli/publish.mdx
  • docs/runtime/bunfig.mdx
  • docs/guides/runtime/cicd.mdx
  • docs/project/contributing.mdx
  • docs/bundler/executables.mdx
  • docs/guides/install/add-peer.mdx
  • packages/bun-types/bun.d.ts
  • test/bundler/expectBundled.ts
  • docs/quickstart.mdx
📚 Learning: 2025-09-12T22:30:48.490Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22613
File: src/cli/package_manager_command.zig:0-0
Timestamp: 2025-09-12T22:30:48.490Z
Learning: Commands like `bun info` and `bun whoami` that are promoted from pm-only subcommands to top-level commands use manual `bun.argv` parsing to detect direct invocation vs. pm invocation. This is different from regular commands that should use `ctx.positionals`.

Applied to files:

  • docs/pm/cli/info.mdx
  • docs/runtime/bunfig.mdx
📚 Learning: 2025-09-12T22:30:48.490Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22613
File: src/cli/package_manager_command.zig:0-0
Timestamp: 2025-09-12T22:30:48.490Z
Learning: Promoted pm subcommands like `bun info` and `bun whoami` use manual `bun.argv` parsing because they are "standalone commands" that bypass the normal command flow. They parse arguments manually starting from argv[2], skip flags, then delegate to the corresponding pm functionality (e.g., `bun info` calls `PmViewCommand.view()`). This pattern is different from regular commands that use `ctx.positionals`.

Applied to files:

  • docs/pm/cli/info.mdx
📚 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:

  • docs/pm/cli/info.mdx
  • docs/runtime/module-resolution.mdx
  • test/regression/issue/24817.test.ts
  • docs/test/configuration.mdx
  • docs/docs.json
  • docs/pm/cli/install.mdx
  • docs/runtime/bunfig.mdx
  • docs/guides/test/spy-on.mdx
  • docs/guides/runtime/cicd.mdx
  • docs/pm/workspaces.mdx
  • docs/test/reporters.mdx
  • docs/guides/test/concurrent-test-glob.mdx
  • docs/guides/install/add-peer.mdx
  • packages/bun-types/bun.d.ts
  • test/bundler/expectBundled.ts
  • docs/quickstart.mdx
📚 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:

  • docs/pm/cli/info.mdx
  • docs/runtime/module-resolution.mdx
  • docs/bundler/index.mdx
  • docs/guides/runtime/define-constant.mdx
  • docs/pm/cli/install.mdx
  • docs/runtime/bunfig.mdx
  • docs/pm/workspaces.mdx
  • docs/bundler/executables.mdx
  • docs/guides/install/add-peer.mdx
  • packages/bun-types/bun.d.ts
  • test/bundler/expectBundled.ts
  • docs/quickstart.mdx
📚 Learning: 2025-09-12T18:16:50.754Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.

Applied to files:

  • docs/runtime/module-resolution.mdx
📚 Learning: 2025-10-08T13:56:00.875Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: src/bun.js/api/BunObject.zig:2514-2521
Timestamp: 2025-10-08T13:56:00.875Z
Learning: For Bun codebase: prefer using `bun.path` utilities (e.g., `bun.path.joinAbsStringBuf`, `bun.path.join`) over `std.fs.path` functions for path operations.

Applied to files:

  • docs/runtime/module-resolution.mdx
📚 Learning: 2025-11-13T07:23:11.159Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 24668
File: docs/quickstart.mdx:181-197
Timestamp: 2025-11-13T07:23:11.159Z
Learning: In Bun.serve's routes configuration, imported HTML files can be assigned directly to routes without wrapping them in a function that returns a Response. For example, `import index from './index.html'` followed by `"/": index` in the routes object is valid Bun syntax for serving HTML content.

Applied to files:

  • docs/runtime/module-resolution.mdx
  • docs/runtime/http/routing.mdx
  • test/regression/issue/24817.test.ts
  • docs/guides/http/file-uploads.mdx
  • docs/quickstart.mdx
📚 Learning: 2025-10-26T01:32:04.844Z
Learnt 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.

Applied to files:

  • docs/runtime/module-resolution.mdx
  • docs/guides/runtime/define-constant.mdx
  • docs/test/configuration.mdx
  • docs/pm/cli/install.mdx
  • docs/guides/test/spy-on.mdx
  • docs/guides/runtime/cicd.mdx
  • docs/quickstart.mdx
📚 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:

  • docs/runtime/module-resolution.mdx
  • docs/guides/http/file-uploads.mdx
📚 Learning: 2025-08-31T09:08:12.104Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22279
File: test/js/web/structured-clone-fastpath.test.ts:12-12
Timestamp: 2025-08-31T09:08:12.104Z
Learning: In Bun, process.memoryUsage.rss() is called as a direct function, not as process.memoryUsage().rss like in Node.js. This is the correct API shape for Bun.

Applied to files:

  • docs/runtime/module-resolution.mdx
📚 Learning: 2025-10-01T21:59:54.571Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.

Applied to files:

  • src/bun.js/webcore/Body.zig
📚 Learning: 2025-10-18T20:50:47.750Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:366-373
Timestamp: 2025-10-18T20:50:47.750Z
Learning: In Bun's Zig codebase (src/bun.js/bindings/JSValue.zig), the JSValue enum uses `.null` (not `.js_null`) for JavaScript's null value. Only `js_undefined` has the `js_` prefix to avoid collision with Zig's built-in `undefined` keyword. The correct enum fields are: `js_undefined`, `null`, `true`, `false`, and `zero`.

Applied to files:

  • src/bun.js/webcore/Body.zig
  • CONTRIBUTING.md
  • docs/project/contributing.mdx
  • src/bun.js/api/server/StaticRoute.zig
📚 Learning: 2025-10-16T02:17:35.237Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/analytics.zig:15-21
Timestamp: 2025-10-16T02:17:35.237Z
Learning: In src/analytics.zig and similar files using bun.EnvVar boolean environment variables: the new EnvVar API for boolean flags (e.g., bun.EnvVar.do_not_track.get(), bun.EnvVar.ci.get()) is designed to parse and return boolean values from environment variables, not just check for their presence. This is an intentional design change from the previous presence-based checks using bun.getenvZ().

Applied to files:

  • src/bun.js/webcore/Body.zig
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun’s Zig code, `.js_undefined` is a valid and preferred JSValue literal for “undefined” (e.g., resolving JSPromise). Do not refactor usages to `jsc.JSValue.jsUndefined()`, especially in src/valkey/js_valkey_functions.zig unsubscribe().

Applied to files:

  • src/bun.js/webcore/Body.zig
📚 Learning: 2025-11-03T20:43:06.996Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24273
File: src/bun.js/test/snapshot.zig:19-19
Timestamp: 2025-11-03T20:43:06.996Z
Learning: In Bun's Zig codebase, when storing JSValue objects in collections like ArrayList, use `jsc.Strong.Optional` (not raw JSValue). When adding values, wrap them with `jsc.Strong.Optional.create(value, globalThis)`. In cleanup code, iterate the collection calling `.deinit()` on each Strong.Optional item before calling `.deinit()` on the ArrayList itself. This pattern automatically handles GC protection. See examples in src/bun.js/test/ScopeFunctions.zig and src/bun.js/node/node_cluster_binding.zig.

Applied to files:

  • src/bun.js/webcore/Body.zig
  • CONTRIBUTING.md
  • docs/project/contributing.mdx
📚 Learning: 2025-11-11T22:55:04.070Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 24571
File: src/css/selectors/parser.zig:908-916
Timestamp: 2025-11-11T22:55:04.070Z
Learning: In oven-sh/bun, CSS serialization uses an arena allocator. In src/css/selectors/parser.zig, functions like PseudoClass.toCss and PseudoElement.toCss intentionally do not call deinit on std.Io.Writer.Allocating, scratch buffers, or css.Printer because dest.allocator is an arena and these temporaries are reclaimed when the CSS pass completes. Only debug-only paths (e.g., DeclarationBlock.DebugFmt in src/css/declaration.zig) may explicitly deinit.

Applied to files:

  • src/bun.js/webcore/Body.zig
📚 Learning: 2025-09-03T05:09:24.272Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22335
File: src/http.zig:1376-1393
Timestamp: 2025-09-03T05:09:24.272Z
Learning: In bun's HTTP module, `bun.http.default_allocator` is the correct allocator to use for internal HTTP buffers like MutableString initialization for response_message_buffer and compressed_body. This allocator is initialized in HTTPThread.zig as `bun.http.default_arena.allocator()` and is used consistently across the HTTP module, separate from any local `default_allocator` variables.

Applied to files:

  • src/bun.js/webcore/Body.zig
📚 Learning: 2025-10-18T05:23:24.403Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.

Applied to files:

  • test/regression/issue/24817.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/regression/issue/24817.test.ts
  • docs/guides/websocket/context.mdx
  • docs/runtime/http/websockets.mdx
  • test/bundler/expectBundled.ts
  • docs/guides/websocket/simple.mdx
📚 Learning: 2025-10-17T01:21:35.060Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/bindings/BunIDLConvert.h:79-95
Timestamp: 2025-10-17T01:21:35.060Z
Learning: In JavaScriptCore (used by Bun), the `toBoolean()` method implements ECMAScript's ToBoolean abstract operation, which is side-effect-free and cannot throw exceptions. It does not execute arbitrary JavaScript or invoke user-defined methods like Symbol.toPrimitive. Exception handling is not needed after toBoolean() calls.

Applied to files:

  • docs/guides/runtime/define-constant.mdx
📚 Learning: 2025-11-06T02:56:40.024Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 23713
File: src/bun.js/test/Execution.zig:94-107
Timestamp: 2025-11-06T02:56:40.024Z
Learning: In Bun's test framework (bun:test), the `repeats` option follows the semantics where `repeats: N` means the test runs once initially, then repeats N additional times (if passing), for a total of N+1 runs. For example, `repeats: 2` results in 3 total runs (1 initial + 2 repeats). This matches the parallel semantics of `retry: N` (1 initial + N retries).

Applied to files:

  • docs/test/configuration.mdx
  • docs/test/reporters.mdx
📚 Learning: 2025-10-18T23:43:42.502Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23817
File: src/js/node/test.ts:282-282
Timestamp: 2025-10-18T23:43:42.502Z
Learning: In the Bun repository, the error code generation script (generate-node-errors.ts) always runs during the build process. When reviewing code that uses error code intrinsics like $ERR_TEST_FAILURE, $ERR_INVALID_ARG_TYPE, etc., do not ask to verify whether the generation script has been run or will run, as it is automatically executed.

Applied to files:

  • CONTRIBUTING.md
  • docs/project/contributing.mdx
📚 Learning: 2025-10-24T10:43:09.398Z
Learnt from: fmguerreiro
Repo: oven-sh/bun PR: 23774
File: src/install/PackageManager/updatePackageJSONAndInstall.zig:548-548
Timestamp: 2025-10-24T10:43:09.398Z
Learning: In Bun's Zig codebase, the `as(usize, intCast(...))` cast pattern triggers a Zig compiler bug that causes compilation to hang indefinitely when used in complex control flow contexts (loops + short-circuit operators + optional unwrapping). Avoid this pattern and use simpler alternatives like just `intCast(...)` if type casting is necessary.

Applied to files:

  • CONTRIBUTING.md
  • docs/project/contributing.mdx
📚 Learning: 2025-10-18T02:06:31.606Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 23796
File: src/bun.js/test/Execution.zig:624-625
Timestamp: 2025-10-18T02:06:31.606Z
Learning: In Zig files using bun.Output.scoped(), the visibility level `.visible` means logs are enabled by default in debug builds unless `BUN_DEBUG_QUIET_LOGS=1` is set; if that environment variable is set, the logs can be re-enabled with `BUN_DEBUG_<scope>=1`. Use `.visible` for logs that should appear by default in debug builds, and `.hidden` for logs that require explicit opt-in via `BUN_DEBUG_<scope>=1`.

Applied to files:

  • CONTRIBUTING.md
  • docs/project/contributing.mdx
📚 Learning: 2025-10-11T15:19:30.301Z
Learnt from: mastermakrela
Repo: oven-sh/bun PR: 19167
File: src/bun.js/api.zig:31-31
Timestamp: 2025-10-11T15:19:30.301Z
Learning: The learnings about `pub const js = JSC.Codegen.JS<ClassName>` and re-exporting toJS/fromJS/fromJSDirect apply to class-based Zig bindings with constructors and prototype methods (e.g., those with `new` constructors). They do NOT apply to simple namespace-style API objects like TOMLObject, YAMLObject, and CSVObject which expose static functions via a `create()` method that returns a plain JS object.

Applied to files:

  • CONTRIBUTING.md
  • docs/project/contributing.mdx
📚 Learning: 2025-09-04T04:16:49.322Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22391
File: src/bun.js/modules/NodeModuleModule.cpp:911-913
Timestamp: 2025-09-04T04:16:49.322Z
Learning: In the Bun codebase, LUT header files (like NodeModuleModule.lut.h) are automatically regenerated by the CI system during the build process, so developers do not need to manually regenerate them when editing LUT source blocks.

Applied to files:

  • CONTRIBUTING.md
  • docs/project/contributing.mdx
📚 Learning: 2025-10-16T17:32:03.074Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23710
File: src/install/PackageManager/PackageManagerOptions.zig:187-193
Timestamp: 2025-10-16T17:32:03.074Z
Learning: In Bun's codebase (particularly in files like src/install/PackageManager/PackageManagerOptions.zig), mixing bun.EnvVar.*.get() and bun.EnvVar.*.platformGet() for environment variable lookups is intentional and safe. The code is protected by compile-time platform checks (Environment.isWindows, etc.), and compilation will fail if the wrong function is used on the wrong platform. This pattern should not be flagged as a consistency issue.

Applied to files:

  • docs/pm/cli/install.mdx
📚 Learning: 2025-09-13T17:01:41.393Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 21459
File: docs/api/import-meta.md:0-0
Timestamp: 2025-09-13T17:01:41.393Z
Learning: For docs/api/import-meta.md, keep documentation entries concise and consistent with the existing style - brief descriptions with small inline examples, not extensive bullet points or detailed explanations.

Applied to files:

  • docs/pm/overrides.mdx
📚 Learning: 2025-10-17T20:50:58.644Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/Handlers.zig:154-159
Timestamp: 2025-10-17T20:50:58.644Z
Learning: In Bun socket configuration error messages (src/bun.js/api/bun/socket/Handlers.zig), use the user-facing JavaScript names "data" and "drain" instead of internal field names "onData" and "onWritable", as these are the names users see in the API according to SocketConfig.bindv2.ts.

Applied to files:

  • docs/guides/websocket/pubsub.mdx
  • docs/guides/websocket/context.mdx
  • docs/runtime/http/websockets.mdx
  • packages/bun-types/bun.d.ts
  • docs/guides/websocket/simple.mdx
📚 Learning: 2025-09-12T22:27:19.572Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22613
File: src/cli/package_manager_command.zig:0-0
Timestamp: 2025-09-12T22:27:19.572Z
Learning: In Bun's CLI architecture, commands that go through the standard Command.Context flow should use `ctx.positionals` (which excludes flags) rather than manually parsing `bun.argv`. Manual `bun.argv` parsing is only used for standalone commands like `bun info` that bypass the normal command routing.

Applied to files:

  • docs/runtime/bunfig.mdx
📚 Learning: 2025-10-08T13:48:02.430Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.

Applied to files:

  • docs/guides/test/spy-on.mdx
📚 Learning: 2025-11-06T00:58:23.965Z
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.

Applied to files:

  • docs/guides/test/spy-on.mdx
📚 Learning: 2025-11-08T04:06:33.198Z
Learnt 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;`.

Applied to files:

  • docs/guides/test/spy-on.mdx
📚 Learning: 2025-09-20T03:39:41.770Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: test/regression/issue/21830.fixture.ts:14-63
Timestamp: 2025-09-20T03:39:41.770Z
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:

  • docs/guides/test/spy-on.mdx
  • docs/test/reporters.mdx
  • docs/guides/test/concurrent-test-glob.mdx
  • docs/quickstart.mdx
📚 Learning: 2025-10-15T20:19:37.256Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:37.256Z
Learning: In the Bun codebase, FFI (Foreign Function Interface) code is compiled separately using TinyCC (tcc), which barely supports C99. Headers like src/bun.js/api/FFI.h and src/bun.js/api/ffi-stdbool.h are only used for FFI compilation with tcc, not with the main compiler. Therefore, C standard changes to the main Bun target do not affect FFI code compilation.

Applied to files:

  • docs/project/contributing.mdx
📚 Learning: 2025-10-15T20:19:38.580Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 23680
File: cmake/targets/BuildBun.cmake:822-822
Timestamp: 2025-10-15T20:19:38.580Z
Learning: In the Bun codebase, FFI is compiled with tcc (TinyCC), which barely supports C99. The headers `src/bun.js/api/FFI.h` and `src/bun.js/api/ffi-stdbool.h` are only used for FFI compilation with tcc, not for the main Bun target. Therefore, C23 compatibility concerns (such as bool/true/false keyword conflicts) do not apply to these FFI headers.

Applied to files:

  • docs/project/contributing.mdx
📚 Learning: 2025-10-18T01:49:31.037Z
Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23755
File: src/bun.js/api/bun/socket/SocketConfig.bindv2.ts:58-58
Timestamp: 2025-10-18T01:49:31.037Z
Learning: In Bun's bindgenv2 TypeScript bindings (e.g., src/bun.js/api/bun/socket/SocketConfig.bindv2.ts), the pattern `b.String.loose.nullable.loose` is intentional and not a duplicate. The first `.loose` applies to the String type (loose string conversion), while the second `.loose` applies to the nullable (loose nullable, treating all falsy values as null rather than just null/undefined).

Applied to files:

  • docs/guides/websocket/context.mdx
  • packages/bun-types/bun.d.ts
📚 Learning: 2025-10-19T04:55:27.213Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-88
Timestamp: 2025-10-19T04:55:27.213Z
Learning: In the Bun.telemetry._node_binding API for Node.js http.createServer compatibility, there are exactly two hooks: handleIncomingRequest(req, res) called once per request (returns a request ID), and handleWriteHead(res, statusCode) called once when headers are sent (explicit or implicit). The old handleRequestFinish hook no longer exists after architectural refactoring. Both hooks are tested in test/js/bun/http/node-telemetry.test.ts.

Applied to files:

  • docs/runtime/http/websockets.mdx
  • docs/guides/websocket/simple.mdx
📚 Learning: 2025-10-26T04:50:17.892Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24086
File: src/bun.js/api/server.zig:2534-2535
Timestamp: 2025-10-26T04:50:17.892Z
Learning: In src/bun.js/api/server.zig, u32 is acceptable for route indices and websocket_context_index fields, as the practical limit of 4 billion contexts will never be reached.

Applied to files:

  • src/bun.js/api/server/StaticRoute.zig
📚 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:

  • docs/quickstart.mdx
🪛 LanguageTool
docs/pm/cli/install.mdx

[style] ~314-~314: ‘merged together’ might be wordy. Consider a shorter alternative.
Context: ...ml If both are found, the results are merged together. Configuring withbunfig.toml` is opt...

(EN_WORDINESS_PREMIUM_MERGED_TOGETHER)


[style] ~454-~454: ‘Prior to’ might be wordy. Consider a shorter alternative.
Context: .../bun.com/blog/bun-lock-text-lockfile). Prior to Bun 1.2, the lockfile was binary and ca...

(EN_WORDINESS_PREMIUM_PRIOR_TO)

🔇 Additional comments (49)
docs/runtime/utils.mdx (1)

237-238: TOML syntax correction is appropriate, but this change appears unrelated to the PR's main objective.

The change from // to # comments is correct for TOML syntax—TOML uses # for comments, not //. However, this documentation fix seems disconnected from the PR's stated objective of fixing Unicode handling in static routes with Content-Type headers.

Clarify whether this documentation change is intentional and part of the Unicode fix PR, or if it was included by mistake.

docs/runtime/html-rewriter.mdx (1)

49-67: Documentation syntax fix: use HTML comment markers in HTML example.

This change correctly updates the code diff markers from JavaScript comment syntax (// [!code ...]) to proper HTML comment syntax (<!-- [!code ...] -->). Since this is an HTML example, using HTML comment syntax ensures technical accuracy and consistency with the language being documented. The added {/* prettier-ignore */} directive appropriately preserves the intentional formatting.

docs/runtime/module-resolution.mdx (2)

1-1: Major discrepancy: File does not match PR objectives.

This file contains documentation additions for NODE_PATH module resolution, but PR #24818's objectives describe fixing unicode handling in static routes with proper Content-Type headers. The provided file appears unrelated to the PR's stated goal of preserving charset metadata when converting strings to Blobs and adding appropriate Content-Type headers to static responses.

Please verify that the correct files were provided for review. The expected changes should involve runtime/core fixes to Body handling, Blob construction, and Headers initialization—not documentation of module resolution features.


186-207: NODE_PATH documentation section is technically accurate and well-placed.

The additions correctly document Bun's support for NODE_PATH as additional module resolution directories, with clear examples showing platform-specific delimiters (colon on Unix/macOS, semicolon on Windows). The section placement after the Node.js algorithm explanation but before package.json exports details is logical and improves documentation flow.

Code examples are clear and demonstrate both single and multiple path usage patterns.

docs/guides/websocket/pubsub.mdx (1)

12-12: WebSocket documentation update is clear and consistent.

The removal of the generic type parameter and introduction of per-connection data typing via the data property is well-documented. The inline TypeScript comment on line 22 helpfully explains the typing approach.

Also applies to: 22-24

docs/guides/websocket/simple.mdx (1)

12-12: WebSocket documentation update maintains consistency with related guides.

The changes follow the same pattern as pubsub.mdx, removing the generic type parameter and using the data property for per-connection typing.

Also applies to: 25-27

docs/guides/websocket/context.mdx (1)

12-12: WebSocket documentation consistently updated across both examples.

Both code examples in this file follow the established pattern of removing generic type parameters and using the data property for per-connection type specification. The TypeScript comments are clear and helpful.

Also applies to: 25-27, 49-49, 67-69

docs/runtime/http/websockets.mdx (3)

110-123: Documentation example clearly shows headers usage with upgrade.

The prettier-ignore comment and expanded headers example on lines 116-118 make the API usage clear. The code highlighting enhances readability.


129-129: Per-connection data typing documentation is clear and actionable.

Line 129 provides explicit, easy-to-follow guidance on the new typing pattern using the data property.


171-173: Context about API change rationale is helpful for users.

The Info box on lines 171-173 explains why the generic type parameter was removed (TypeScript limitation) and explicitly references the TypeScript issue, helping users understand the migration path.

docs/pm/workspaces.mdx (4)

33-33: Documentation changes: icon metadata additions.

Lines 33, 50, and 60 add icon="file-json" metadata to code fences. Assuming this metadata is used by the documentation rendering system, these changes are fine. However, ensure that:

  • The icon metadata is consistently applied (line 10 uses icon="folder-tree" but other JSON fences at lines 33, 50, 60 now have icon="file-json").
  • This is an intentional update aligned with recent documentation styling changes.

Also applies to: 50-50, 60-60


45-47: Glob pattern documentation enhancement.

The updated note correctly documents that Bun supports negative glob patterns (e.g., !**/excluded/**) and includes a link to the glob documentation. The phrasing is clear and accurate.


50-56: New workspace configuration example with negative patterns.

This example effectively demonstrates negative glob patterns in the workspaces field. It's a good complement to the earlier, simpler ["packages/*"] example. The example is clear and well-formatted.


1-100: This file is not part of this PR and should not be reviewed.

Verification confirms that docs/pm/workspaces.mdx is not modified in this PR. The actual changes are to runtime and build files including src/bun.js/api/server/StaticRoute.zig, src/bun.js/webcore/Blob.zig, src/bun.js/webcore/Body.zig, src/http/Headers.zig, and others. No documentation files (docs/*) are modified in this PR.

The review context provided an incorrect file. Please review the actual modified files instead.

Likely an incorrect or invalid review comment.

CONTRIBUTING.md (1)

204-204: Typo fix is correct.

Correcting "lgos" to "logs" improves documentation clarity.

docs/test/reporters.mdx (1)

44-52: New Dots Reporter section is well-positioned and clear.

The concise description and dual-syntax examples (--dots and --reporter=dots) align with the patterns used for other built-in reporters.

docs/pm/overrides.mdx (1)

8-8: Formatting directives and code annotations are consistent with documentation style.

The prettier-ignore directives and inline // [!code ++] markers improve readability for JSON code examples without altering content.

Also applies to: 15-17, 53-53, 60-62, 79-81

docs/guides/html-rewriter/extract-links.mdx (1)

40-40: Formatting and code annotations are well-applied.

The prettier-ignore directive and inline // [!code ++] markers clearly highlight the URL resolution logic without altering the try-catch exception handling pattern.

Also applies to: 50-56

docs/pm/cli/link.mdx (1)

46-57: New Unlinking section complements the existing link documentation.

The concise description, CLI example, and expected output follow the established pattern and provide clear guidance for unregistering local packages.

docs/guides/install/add-peer.mdx (1)

29-29: Code formatting adjustments improve readability and consistency.

The prettier-ignore directive and repositioned inline annotation clarify the peerDependenciesMeta structure without altering the JSON content.

Also applies to: 36-36

docs/guides/runtime/define-constant.mdx (1)

30-30: Code formatting enhances clarity of transpiler transformation examples.

The prettier-ignore directives and end-of-line annotation placement clarify the progression of constant folding and dead code elimination without changing the logic presented.

Also applies to: 32-32, 45-45, 47-47

docs/guides/test/spy-on.mdx (1)

26-26: Test example expansion provides practical spy usage patterns.

The expanded test demonstrates the complete spy workflow (verify initial state → invoke method → verify invocation count and arguments), providing a comprehensive example for developers learning the spyOn API.

Also applies to: 39-44

docs/guides/runtime/cicd.mdx (1)

18-23: LGTM! Valid YAML comment syntax.

The change from // [!code ++] to # [!code ++] correctly uses YAML's comment syntax. YAML does not support // style comments, so this ensures the documentation examples show valid YAML.

docs/pm/cli/install.mdx (1)

137-147: LGTM! Documentation formatting improvement.

The prettier-ignore directive and inline comment adjustments ensure proper rendering of the code example without unwanted formatting changes.

docs/quickstart.mdx (1)

222-237: LGTM! Clear tutorial progression.

The prettier-ignore directive and inline markers effectively highlight the incremental additions to package.json for the quickstart tutorial.

docs/docs.json (2)

191-191: LGTM! Navigation entry added.

The addition of /pm/cli/info to the Publishing & Analysis section is properly structured and consistent with the navigation format.


459-459: LGTM! Test guide navigation entry added.

The addition of /guides/test/concurrent-test-glob is properly placed in the Test Runner guides section.

test/bundler/expectBundled.ts (1)

218-218: LGTM! Type deduplication fix.

Removes the duplicate "linked" entry from the sourceMap union type, making the type definition cleaner and more explicit.

docs/pm/cli/publish.mdx (1)

92-98: LGTM! Clear documentation for new CLI flag.

The --tolerate-republish flag documentation clearly explains its purpose and use case. The exit code behavior change from 1 to 0 for existing package versions is well-documented, making it useful for CI/CD pipelines.

docs/bundler/index.mdx (1)

1379-1379: LGTM! Type deduplication fix.

Removes the duplicate "linked" entry from the sourcemap union type, consistent with the same fix in test/bundler/expectBundled.ts.

docs/runtime/http/routing.mdx (1)

284-287: No issues found—API change is correct and intentional.

Verification confirms that server.requestIP() returns a SocketAddress object with address, port, and family properties. The change from ${ip} to ${ip.address} is correct and necessary to access the IP address string from the returned object. The return type is properly documented in the type definitions as SocketAddress | null.

docs/pm/cli/info.mdx (1)

1-70: LGTM!

The documentation is clear, comprehensive, and well-structured. The examples cover the main use cases for the bun info command effectively.

src/bun.js/api/server/StaticRoute.zig (1)

150-156: LGTM!

The unified header construction ensures consistent behavior across both branches. By passing the body parameter to Headers.from in both the header-present and header-absent cases, the static route now properly supports Content-Type auto-detection for string-originated responses. The error handling is correctly mirrored from the if branch.

src/http/Headers.zig (1)

156-160: LGTM!

This is the core fix for the Unicode handling issue. The updated logic correctly auto-adds Content-Type: text/plain;charset=utf-8 for string-originated bodies, preventing browsers from misinterpreting UTF-8 bytes as Latin-1. The explanatory comments make the intent clear.

test/regression/issue/24817.test.ts (2)

5-51: LGTM!

Excellent test coverage for the Unicode handling fix. The test validates:

  • Basic Unicode (▲)
  • Japanese characters (こんにちは世界)
  • Emoji (🎉🚀✨)
  • Both dynamic and static routes
  • Correct Content-Type headers

The test structure is clean and uses the using keyword for proper resource cleanup.


53-69: LGTM!

This test ensures that explicit content-type headers are still respected and not overridden by the auto-detection logic. This is an important edge case that prevents regressions.

src/bun.js/webcore/Blob.zig (2)

4323-4332: LGTM!

The updated contentType() method correctly returns text/plain;charset=utf-8 for string-originated blobs, ensuring proper Unicode handling. The explanatory comment makes the logic clear.


4341-4341: LGTM!

The updated wasString() method now correctly identifies both ASCII and non-ASCII string-originated blobs. This fix is crucial - the previous implementation only recognized ASCII strings, which was the root cause of the Unicode handling bug for non-ASCII content.

docs/test/configuration.mdx (1)

187-233: LGTM!

The new test execution documentation is clear and comprehensive. It covers important test configuration options with practical examples and clear explanations of their behavior. The TOML configuration snippets are correctly formatted.

packages/bun-types/bun.d.ts (1)

1721-1791: Duplicate "linked" union member correctly removed from sourcemap type

The updated BuildConfigBase.sourcemap union now matches the documented modes without redundancy and preserves backwards compatibility at the type level.

docs/runtime/bunfig.mdx (1)

279-329: New test.concurrentTestGlob, test.onlyFailures, and test.reporter docs are clear and consistent

The examples and wording around concurrent glob patterns, failure-only output, and reporter configuration (dots/JUnit) read well and align with the rest of the test runner docs, including the dedicated guide.

docs/guides/test/concurrent-test-glob.mdx (1)

1-137: Concurrent test glob guide is coherent and complements bunfig docs

The naming convention, configuration examples (single glob and array of globs), code samples, and migration tips clearly explain how concurrentTestGlob affects execution while keeping sequential tests safe. It fits well with the new [test] options in bunfig.toml.

docs/bundler/executables.mdx (1)

88-97: Marking bun-windows-arm64 as struck-through appropriately signals lack of support

Using strikethrough in the targets table makes it obvious that a Windows arm64 target exists conceptually but is not currently supported, without removing it entirely from the matrix.

docs/project/contributing.mdx (3)

10-23: Nix-based setup section is clear and low-friction

The flake-based nix develop flow, including the CMAKE_SYSTEM_PROCESSOR export and bun bd usage, gives a good reproducible alternative to manual dependency installation.


73-112: sccache/AWS guidance is detailed and appropriately scoped to core developers

The instructions for installing sccache with S3 support, configuring AWS credentials, and debugging via sccache --show-stats are precise, and clearly marked as “Core Developers Only” for write access to the shared cache, which avoids confusing external contributors.


213-213: Debug logging description matches current Output.scoped usage

Describing BUN_DEBUG_<scope>=1 in terms of Output.scoped(.<scope>, .hidden) behavior reflects the current logging design (hidden-by-default scopes that can be selectively enabled, plus BUN_DEBUG_QUIET_LOGS for global suppression and BUN_DEBUG=<file>.log for redirection). This should help contributors reason about debug output correctly.

Based on learnings

src/bun.js/webcore/Body.zig (3)

757-775: LGTM! InternalBlob charset preservation is correct.

The logic properly preserves the was_string flag from InternalBlob by setting the blob's charset. Conditionally setting charset only when was_string is true ensures that only string-originated bodies are marked, which aligns with the fix objective to auto-add Content-Type for string responses.


781-790: LGTM! WTFStringImpl UTF-8 charset assignment is correct.

The unconditional charset assignment is appropriate since WTFStringImpl always represents a string. The ASCII detection correctly determines whether to use .all_ascii or .non_ascii, enabling wasString() to return true for proper Content-Type header handling.


791-800: LGTM! Latin1 fallback charset handling is consistent.

The Latin1 fallback branch correctly mirrors the UTF-8 path by setting charset unconditionally. Memory is safely owned (via dupe), and ASCII detection appropriately categorizes the charset for downstream Content-Type header logic.

Comment on lines +527 to 567
## Code splitting

Standalone executables support code splitting. Use `--compile` with `--splitting` to create an executable that loads code-split chunks at runtime.

```bash
bun build --compile --splitting ./src/entry.ts --outdir ./build
```

<CodeGroup>

```ts src/entry.ts icon="/icons/typescript.svg"
console.log("Entrypoint loaded");
const lazy = await import("./lazy.ts");
lazy.hello();
```

```ts src/lazy.ts icon="/icons/typescript.svg"
export function hello() {
console.log("Lazy module loaded");
}
```

</CodeGroup>

```bash terminal icon="terminal"
./build/entry
```

```txt
Entrypoint loaded
Lazy module loaded
```

---

## Unsupported CLI arguments

Currently, the `--compile` flag can only accept a single entrypoint at a time and does not support the following flags:

- `--outdir` — use `outfile` instead.
- `--splitting`
- `--outdir` — use `outfile` instead (except when using with `--splitting`).
- `--public-path`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Add an explicit note that --compile --splitting is CLI-only (not Bun.build)

The new “Code splitting” section and --outdir exception in the unsupported-flags list accurately describe bun build --compile --splitting behavior. Given that compile-time code splitting is not supported via the Bun.build() JavaScript API, it would be helpful to add a short note here (or a one-liner cross‑reference) that this feature is CLI-only, to avoid users assuming compile+splitting works in the programmatic API as well.

For example, after the first paragraph:

“Note: --compile --splitting is only available via the CLI; Bun.build() currently does not support compile with splitting.”

This keeps the docs aligned with the API surface and previous guidance about CLI‑only bundler features.

🤖 Prompt for AI Agents
In docs/bundler/executables.mdx around lines 527 to 567, the "Code splitting"
section lacks a clear note that compile-time code splitting via --compile
--splitting is CLI-only and not supported by the Bun.build() JavaScript API; add
a single-sentence note immediately after the first paragraph stating that
--compile --splitting is only available via the CLI and Bun.build() currently
does not support compile with splitting, and optionally add a short
cross-reference to the "Unsupported CLI arguments" section to reinforce the
distinction.

Comment on lines 30 to 44
```bash macOS (Homebrew)
$ brew install automake ccache cmake coreutils gnu-sed go icu4c libiconv libtool ninja pkg-config rust ruby
$ brew install automake cmake coreutils gnu-sed go icu4c libiconv libtool ninja pkg-config rust ruby sccache
```

```bash Ubuntu/Debian
$ sudo apt install curl wget lsb-release software-properties-common cargo ccache cmake git golang libtool ninja-build pkg-config rustc ruby-full xz-utils
$ sudo apt install curl wget lsb-release software-properties-common cargo cmake git golang libtool ninja-build pkg-config rustc ruby-full xz-utils
```

```bash Arch
$ sudo pacman -S base-devel ccache cmake git go libiconv libtool make ninja pkg-config python rust sed unzip ruby
$ sudo pacman -S base-devel cmake git go libiconv libtool make ninja pkg-config python rust sed unzip ruby
```

```bash Fedora
$ sudo dnf install cargo ccache cmake git golang libtool ninja-build pkg-config rustc ruby libatomic-static libstdc++-static sed unzip which libicu-devel 'perl(Math::BigInt)'
$ sudo dnf install cargo clang19 llvm19 lld19 cmake git golang libtool ninja-build pkg-config rustc ruby libatomic-static libstdc++-static sed unzip which libicu-devel 'perl(Math::BigInt)'
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Align sccache positioning between base dependencies and “Optional” section

In the macOS Homebrew command, sccache is now listed alongside required build dependencies, but later we introduce sccache under an “Optional: Install `sccache`” heading. That makes it ambiguous whether sccache is required for contributors or just a recommended optimization.

Consider either:

  • removing sccache from the main brew install line and keeping it only in the optional section, or
  • renaming the later heading to something like “Recommended: Configure `sccache`” and clarifying that it is expected for core contributors.

This keeps expectations consistent for new contributors.

🤖 Prompt for AI Agents
In docs/project/contributing.mdx around lines 30 to 44 the macOS Homebrew
install line includes sccache while the document later lists sccache under an
“Optional” section, causing ambiguity; either remove sccache from the main brew
install line so it only appears in the optional section, or keep it in the brew
line and rename the later heading to “Recommended: Configure `sccache`” (and
update text to state it’s recommended for performance/core contributors) so the
document consistently communicates whether sccache is required or optional.

Comment thread docs/runtime/bunfig.mdx
The `bun run` command can be configured under the `[run]` section. These apply to the `bun run` command and the `bun` command when running a file or executable or script.

Currently, `bunfig.toml` isn't always automatically loaded for `bun run` in a local project (it does check for a global `bunfig.toml`), so you might still need to pass `-c` or `-c=bunfig.toml` to use these settings.
Currently, `bunfig.toml` is only automatically loaded for `bun run` in a local project (it doesn't check for a global `.bunfig.toml`).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

Clarify interaction between global .bunfig.toml and [run] configuration

Earlier in this page we describe global .bunfig.toml support and global/local shallow-merge behavior, but this line states that bunfig.toml is “only automatically loaded for `bun run` in a local project (it doesn't check for a global `.bunfig.toml`).”

Consider tightening this to make it explicit whether:

  • global .bunfig.toml is still honored for other sections (e.g. [install], [test]), and
  • this limitation applies only to [run] settings.

That would avoid confusion for users who read the “Global vs. local” section and expect global [run] config to work the same way.


I'll verify this review comment by examining the documentation file and checking the actual behavior described.


🏁 Script executed:

#!/bin/bash
# Retrieve the documentation file and show context around line 606
if [ -f "docs/runtime/bunfig.mdx" ]; then
  echo "=== File: docs/runtime/bunfig.mdx (lines 580-630) ==="
  sed -n '580,630p' docs/runtime/bunfig.mdx
fi

Length of output: 1836


🏁 Script executed:

#!/bin/bash
# Search for "Global vs. local" or similar section in the file
if [ -f "docs/runtime/bunfig.mdx" ]; then
  echo "=== Searching for 'Global' or similar sections ==="
  grep -n -i "global" docs/runtime/bunfig.mdx | head -20
fi

Length of output: 1306


🏁 Script executed:

#!/bin/bash
# Get overall structure to understand sections
if [ -f "docs/runtime/bunfig.mdx" ]; then
  echo "=== File structure (headings) ==="
  grep -n "^#" docs/runtime/bunfig.mdx
fi

Length of output: 3544


Let me search for additional context about this limitation and any related documentation.


🏁 Script executed:

#!/bin/bash
# Look for any additional context around [run] section and global config
if [ -f "docs/runtime/bunfig.mdx" ]; then
  echo "=== Context around line 606 and [run] section ==="
  sed -n '600,650p' docs/runtime/bunfig.mdx
  echo ""
  echo "=== Full Global vs. local section ==="
  sed -n '10,20p' docs/runtime/bunfig.mdx
fi

Length of output: 2488


🏁 Script executed:

#!/bin/bash
# Search for any notes, TODOs, or caveats about [run] or global config
if [ -f "docs/runtime/bunfig.mdx" ]; then
  echo "=== Searching for relevant caveats or notes ==="
  grep -n -i "caveat\|note\|limitation\|global.*run\|run.*global" docs/runtime/bunfig.mdx
fi

Length of output: 186


Clarify scope of [run] global config limitation

Verification confirms the documentation contradiction. Line 19 states that "If both a global and local bunfig are detected, the results are shallow-merged," but line 606 states that bunfig.toml "doesn't check for a global .bunfig.toml" for bun run. This creates genuine ambiguity—readers cannot determine whether:

  • This limitation applies only to [run] while global config works for [install], [test], etc., or
  • Global config is not used for [run] at all (contradicting the earlier section)

Recommend tightening line 606 to explicitly state whether the limitation is scoped to the [run] section or applies more broadly.

🤖 Prompt for AI Agents
In docs/runtime/bunfig.mdx around line 606, the sentence saying `bunfig.toml`
"doesn't check for a global `.bunfig.toml`" conflicts with earlier guidance
about shallow-merging global and local bunfig; update line 606 to explicitly
state whether the limitation applies only to the `[run]` section or to bun run
and all commands. Change the sentence to specify scope (e.g., "For bun run, only
the local bunfig.toml is considered; global .bunfig.toml is not checked — other
commands like [install] and [test] still support global+local shallow-merge")
or, if the behavior is global, rephrase to say "bun run and other commands check
both local and global bunfig and perform a shallow merge," ensuring wording
matches the behavior described earlier.

Comment thread docs/runtime/redis.mdx
Comment on lines 51 to 55
By default, the client reads connection information from the following environment variables (in order of precedence):

- `REDIS_URL`
- `VALKEY_URL`
- If not set, defaults to `"redis://localhost:6379"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify VALKEY_URL environment variable is actually implemented and precedence order matches.

The documentation adds VALKEY_URL as a supported environment variable, but this change appears inconsistent with the stated PR objectives (fixing Unicode in static routes with Content-Type headers). Additionally, ensure that:

  1. The Redis client implementation actually checks for and supports VALKEY_URL
  2. The precedence order (REDIS_URL → VALKEY_URL → default) matches the actual implementation logic
  3. VALKEY_URL behavior mirrors REDIS_URL where applicable

🏁 Script executed:

#!/bin/bash
# Description: Verify VALKEY_URL environment variable support in Redis client implementation

# Search for VALKEY_URL references in the codebase
echo "=== Searching for VALKEY_URL in source code ==="
rg -nP 'VALKEY_URL' --type-list | head -20

# Search for Redis environment variable handling to confirm precedence logic
echo "=== Searching for Redis environment variable handling ==="
rg -nP -C 5 '(REDIS_URL|VALKEY_URL)' -g '!*.mdx' -g '!test' | head -40

Length of output: 3830


🏁 Script executed:

#!/bin/bash
# Verify the exact context around the default URL in both documentation and implementation

echo "=== Documentation context (docs/runtime/redis.mdx) ==="
rg -nP -B 5 -A 10 'REDIS_URL' docs/runtime/redis.mdx | head -40

echo -e "\n=== Full implementation context (js_valkey.zig around line 272) ==="
rg -nP -B 15 -A 5 'REDIS_URL.*orelse.*VALKEY_URL' src/valkey/js_valkey.zig

Length of output: 2247


Correct the default Redis URL in documentation — should be "valkey://localhost:6379" not "redis://localhost:6379"

VALKEY_URL is correctly implemented and the precedence order in the documentation matches the runtime behavior. However, the default URL is incorrect. The implementation (src/valkey/js_valkey.zig:275) defaults to "valkey://localhost:6379", but the documentation (line 55) states "redis://localhost:6379". This discrepancy will mislead users about actual defaults.

Additionally, the TypeScript types (packages/bun-types/redis.d.ts:58-59) list the precedence in reverse order (VALKEY_URL first, REDIS_URL second), which contradicts the actual implementation and this documentation.

🤖 Prompt for AI Agents
In docs/runtime/redis.mdx around lines 51 to 55, update the documented default
Redis URL from "redis://localhost:6379" to "valkey://localhost:6379" to match
the implementation at src/valkey/js_valkey.zig:275; also correct the precedence
ordering in packages/bun-types/redis.d.ts (lines ~58-59) so it lists REDIS_URL
first then VALKEY_URL to reflect actual runtime behavior. Ensure the text and
the type declarations both match the implementation's precedence and default
URL.

@robobun robobun mentioned this pull request Nov 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

unicode not working with static route

2 participants