Skip to content

docs: fix examples that do not run or type-check as written - #41734

Open
robobun wants to merge 3 commits into
mainfrom
robobun/508f4b6d/docs-ledger-29257
Open

robobun wants to merge 3 commits into
mainfrom
robobun/508f4b6d/docs-ledger-29257

Conversation

@robobun

@robobun robobun commented Sep 6, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • Several docs examples fail as written on 1.4.2 and canary. The ESM bytecode example passes a top-level outfile to Bun.build, which is not a BuildConfig key. It is ignored, the index to folder-name fallback runs, and the build fails with failed to rename ... ENOTEMPTY.
  • docs/test/runtime-behavior.mdx documents bun test --hot and bun test --frozen-lockfile. The test runner only re-runs under --watch (test_command.rs:2653), and --frozen-lockfile is an install-family flag. Both run once and exit 0 with no signal.
  • docs/runtime/sql.mdx lists PGUSERNAME with fallbacks USER, USERNAME. The adapter reads PG_USER || PGUSER || USER (src/js/internal/sql/shared.ts:1956). Two guides call blob.stream(1024) to set a chunk size, which neither type-checks (TS2554) nor changes the chunk size. Three guides read .name off JSON5.parse / YAML.parse / XML.parse, which return unknown / XML.Document (TS2339).

Fix

  • Move outfile under compile: { outfile }, the form docs/bundler/executables.mdx already uses. Swap the loader map example to text / file (see Notes on dataurl).
  • In the docs/bundler/esbuild.mdx migration table, mark outfile as not supported and drop it from the write row (folded in from docs: put outfile under compile in the Bun.build bytecode example #37473). Probe on 1.4.3: Bun.build({ entrypoints: ["./in.js"], outfile: "./named.js" }) returns outputs of ["./in.js"] and writes no named.js. compile: true with no outdir still writes the executable to disk.
  • Drop the --hot and --frozen-lockfile lines. List the auto-install flags the runtime reads (--prefer-offline, --install=fallback, --no-install).
  • Fix the Postgres table to PGUSER and add the PG_* aliases. Replace blob.stream(1024) with manual chunking. Cast the parse() results to the expected shape.
  • Verified: each changed TS block type-checks with tsc 6.0.2 and packages/bun-types, and runs with the stated output. prettier --check passes on the touched files.

Background

  • Bun.build reads outfile only inside the compile object (src/runtime/api/JSBundler.rs:405). bun-types declares it only on CompileBuildOptions.
  • bun test shares the runtime argument table, so --hot parses. The test command installs a HotReloader but only enters the watch loop for HotReload::Watch.
Notes

no test proof · iteration 0 · docs-only change; test-proof not applicable

- bundler/index.mdx: move `outfile` under `compile` in the ESM bytecode
  example. A top-level `outfile` is not a BuildConfig key and is ignored.
- bundler/index.mdx: use `text` and `file` in the loader map example.
  The `dataurl` and `base64` loaders are accepted but emit an empty
  string today, and are not in the `Loader` type.
- test/runtime-behavior.mdx: remove `bun test --hot` and
  `bun test --frozen-lockfile`. The test runner only re-runs under
  `--watch`, and `--frozen-lockfile` is an install flag. List the
  auto-install flags the runtime does read.
- runtime/sql.mdx: the Postgres user variable is `PGUSER` (also
  `PG_USER`, then `USER`). `PGUSERNAME` and `USERNAME` are never read.
  Document the `PG_*` aliases for the other rows.
- guides/binary: `blob.stream(1024)` does not set a 1024-byte chunk size
  and does not type-check. Show manual chunking instead.
- guides/runtime/import-{json5,yaml,xml}.mdx: `parse()` returns
  `unknown` (or `XML.Document`), so the property accesses on the next
  lines did not type-check. Cast the result to the expected shape.
@coderabbitai

coderabbitai Bot commented Sep 6, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 1a26c9de-1bf7-4b57-bec5-df4c0005389d

📥 Commits

Reviewing files that changed from the base of the PR and between d316760 and cfe059c.

📒 Files selected for processing (9)
  • docs/bundler/esbuild.mdx
  • docs/bundler/index.mdx
  • docs/guides/binary/buffer-to-readablestream.mdx
  • docs/guides/binary/typedarray-to-readablestream.mdx
  • docs/guides/runtime/import-json5.mdx
  • docs/guides/runtime/import-xml.mdx
  • docs/guides/runtime/import-yaml.mdx
  • docs/runtime/sql.mdx
  • docs/test/runtime-behavior.mdx

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.


Walkthrough

Changes

Documentation updates

Layer / File(s) Summary
Bundler option examples
docs/bundler/esbuild.mdx, docs/bundler/index.mdx
Documents supported output options, updated loader examples, and the object form of compile.outfile.
ReadableStream chunking examples
docs/guides/binary/buffer-to-readablestream.mdx, docs/guides/binary/typedarray-to-readablestream.mdx
Explains Bun-selected chunk sizes and shows manual chunking with ReadableStream slices.
Typed runtime parser examples
docs/guides/runtime/import-json5.mdx, docs/guides/runtime/import-xml.mdx, docs/guides/runtime/import-yaml.mdx
Adds explicit TypeScript result shapes for JSON5, XML, and YAML parsing examples.
Runtime and database behavior
docs/runtime/sql.mdx, docs/test/runtime-behavior.mdx
Documents PostgreSQL environment-variable aliases and precedence, plus installation and watch-mode flags.

Suggested reviewers: dylan-conway

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to cfe05

The updated examples and reference material correct documented runtime behavior without introducing an identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: fixing documentation examples that fail to run or type-check as written.
Description check ✅ Passed The description explains the problems, lists the documentation fixes, and includes verification details. It does not use the template headings exactly, but it provides the required information and is …

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

@github-actions github-actions Bot added the claude label Sep 6, 2026
@robobun

robobun commented Sep 6, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 7:42 AM PT - Sep 9th, 2026

✅ @robobun, your commit cfe059c818cd17cddeb05dae0289d5478816c04b passed in Build #113568! 🎉


🧪   To try this PR locally:

bunx bun-pr 41734

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

bun-41734 --bun

@robobun

robobun commented Sep 6, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review. Docs-only change, no build needed.

How each row was verified on 1.4.3 canary (d316760):

  • Bun.build({ outfile, compile: true }): top-level outfile ignored, compile: { outfile } produces ./mycli.
  • docs/bundler/esbuild.mdx outfile and write rows (folded in from docs: put outfile under compile in the Bun.build bytecode example #37473): Bun.build({ entrypoints: ["./in.js"], outfile: "./named.js" }) on 1.4.3 returns outputs of ["./in.js"] and writes no named.js. compile: true with no outdir writes the executable to disk.
  • bun test --hot / --frozen-lockfile: run once, exit 0. --watch stays resident.
  • PGUSERNAME=alice resolves to postgres, PGUSER=bob to bob, PG_USER=carol to carol.
  • new Blob([new Uint8Array(5000)]).stream(1024) yields one 5000-byte chunk.
  • Each changed TS block type-checks with tsc 6.0.2 and packages/bun-types, and prints the stated output.

@claude claude 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.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread docs/runtime/sql.mdx
Comment thread docs/guides/binary/typedarray-to-readablestream.mdx Outdated

@claude claude 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.

LGTM — thanks for fixing the chunk-size examples; the 64 KB / 16 KB split now demonstrates actual chunking. The open PASSWORD note on the Postgres table is optional and non-blocking.

What was reviewed:

  • Re-checked both ReadableStream guides after 8ccfbfe: 64 KB buffer with 16 KB slices yields four enqueues, loop terminates and closes.
  • Postgres env-var precedence against src/js/internal/sql/shared.ts — PG_* before PG* and USER fallback are accurate as documented.
  • Bun.build compile.outfile placement, --hot/--frozen-lockfile removal, and the unknown return-type casts on JSON5/YAML/XML.parse — each matches current source and types.
Extended reasoning...

Overview

This is a docs-only correction pass across eight .mdx files that fixes examples which do not run or type-check on current Bun: moving outfile under compile in the ESM-bytecode Bun.build snippet, swapping the loader example off dataurl (which currently emits an empty string), replacing the removed Blob.stream(chunkSize) overload with a manual ReadableStream that enqueues .subarray() slices, adding as casts to the Bun.JSON5/YAML/XML.parse() guides so they type-check against the unknown/generic-document return types, correcting the Postgres env-var table (PGUSER not PGUSERNAME, PG_* aliases, precedence note), and removing bun test --hot/--frozen-lockfile from the runtime-behavior page in favor of the auto-install flags the runtime actually reads.

Security risks

None. The diff touches only rendered documentation under docs/; no source, build config, CI, or type declarations are modified. The examples are illustrative snippets with no credential-shaped literals or unsafe patterns introduced.

Level of scrutiny

Low-to-moderate, per .claude/docs/landing-prs.md "Docs, types, and comments": the bar is that every published claim is verifiable against the implementation and every snippet runs as written. I previously left two optional inline comments; commit 8ccfbfe addressed the chunk-size one (both guides now use a 64 KB buffer with 16 KB chunks, so the loop visibly produces four slices). The other — the Postgres table omitting the PASSWORD fallback that shared.ts:1974 reads — remains open but is an optional completeness nit; the rewritten table is still strictly more accurate than the base version, which listed nonexistent PGUSERNAME/USERNAME.

Other factors

Exit reason was dry_streak, so the hunt completed without new findings. CODEOWNERS has no entry for docs/, and there are no outstanding CHANGES_REQUESTED reviews from other reviewers in the timeline. The Postgres precedence claim ("PG_* wins over PG*") matches env.PG_PASSWORD || env.PGPASSWORD || ... ordering in src/js/internal/sql/shared.ts. Given the change is mechanical, self-contained, and the one remaining note is non-blocking, approving is appropriate.

Bun.build reads outfile only inside the compile options object
(CompileOptions::from_js in src/runtime/api/JSBundler.rs). A top-level
outfile is ignored. Update the outfile and write rows to say so.

@claude claude 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.

LGTM — the follow-up commits addressed the chunk-size example and the new esbuild.mdx rows check out against source.

What was reviewed:

  • outfile in the JS API table: CompileOptions::from_js (src/runtime/api/JSBundler.rs:282-296, 405) reads outfile from the compile sub-object only, and bun-types declares it on CompileBuildOptions (bun.d.ts:3682) — top-level Bun.build({ outfile }) is indeed ignored.
  • Chunk-size examples now use 64 KB data with 16 KB chunks, so the loop demonstrably produces four slices.
  • The earlier optional note about the PASSWORD env fallback (shared.ts:1974) still stands but is a completeness nit, not a correctness error in what's documented.
Extended reasoning...

Overview

Docs-only accuracy sweep across nine .mdx pages. Since the last review round, one new commit (cfe059c8) landed, updating two rows in the docs/bundler/esbuild.mdx JavaScript API comparison table: outfile is now marked "n/a / Not supported" for Bun.build() (pointing to outdir + naming or compile.outfile), and the write row drops the outfile reference. The earlier commit 8ccfbfe9 addressed the chunk-size feedback by bumping the sample buffers to 64 KB with a 16 KB chunk size in both buffer-to-readablestream.mdx and typedarray-to-readablestream.mdx.

Security risks

None. Documentation only; no runtime code, types, build configuration, or CI changes. No security-adjacent examples were introduced.

Level of scrutiny

Low-to-moderate, per the "Docs, types, and comments" guidance: verify claims against the implementation on main. I traced the new outfile claim through src/runtime/api/JSBundler.rs — CompileOptions::from_js binds object to the compile sub-value (line 296) before reading "outfile" at line 405, and the top-level Config::from_js (line 447 onward) has no outfile read. packages/bun-types/bun.d.ts places outfile?: string on CompileBuildOptions (line 3682), not BuildConfig. The write row change is a direct consequence. The CLI table (separate section, untouched) still documents --outfile, which is correct since the CLI does accept it — the change is properly scoped to the JS API table.

Other factors

No CODEOWNERS entry covers docs/. No third-party CHANGES_REQUESTED reviews are outstanding. The one prior optional finding not acted on — the PGPASSWORD row omitting the PASSWORD generic-env fallback that shared.ts:1974 reads — was already posted, is a minor completeness gap (the documented vars are all correct), and doesn't warrant blocking a docs-accuracy PR. Exit reason was dry_streak with zero findings this run.

This branch has not been deployed

No deployments
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.

2 participants