Conversation
The list of accepted loader names in the bunfig [loader] section and in the --loader flag reference predates the yaml, json5, xml, md, html and sqlite loaders, all of which work when mapped from either place. It also listed base64 and dataurl, which currently produce an empty module in the bundler and fall back to the file loader at runtime, so they are removed until they are implemented. jsonc is left out on purpose: a bunfig or --loader mapping to it is currently parsed as plain json.
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
Comment |
|
Updated 4:55 AM PT - Aug 14th, 2026
❌ @robobun, your commit 0749b07 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 38266That installs a local version of the PR into your bun-38266 --bun |
|
Status: ready for review. Diff is How the stale entries were confirmed: mapped a made-up extension to every name in Related PRs, so the lists can be kept in step: #38270 updates the matching CI: the only red tests across the two runs so far ( |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — docs-only update, the new loader list matches LOADER_NAMES in src/ast/loader.rs and the exclusions (jsonc, sqlite_embedded, sh, base64, dataurl, aliases) are each justified in the PR description. The inline nit about Arguments.rs / bun.zsh is a worthwhile follow-up but non-blocking; the two files touched here are strictly more accurate than before.
Extended reasoning...
Overview
This PR updates two hand-maintained copies of the "valid loader names" list in the docs: docs/runtime/bunfig.mdx (the [loader] section) and docs/snippets/cli/run.mdx (the --loader flag). It replaces a stale 10–13 name list with 17 names that are actually accepted and functional today, and drops base64/dataurl which are accepted but stubbed. No source, test, or type-declaration files are touched.
Security risks
None. Pure documentation prose changes to two .mdx files.
Level of scrutiny
Low. Docs-only, no runtime behavior change, CI green. The main correctness question is whether the new list matches what the code accepts — I cross-checked it against LOADER_NAMES in src/ast/loader.rs:73-103 and every name in the new list is present there and maps to a distinct, working loader. The PR description's per-name verification table and its rationale for each excluded name (jsonc/sqlite_embedded collapse via to_api(), sh maps to file, base64/dataurl are stubs, mjs/cjs/etc. are aliases) all check out against the source.
Other factors
One nit was surfaced: the same stale 10-name list also lives in src/runtime/cli/Arguments.rs:137 (the bun --help text) and four sites in completions/bun.zsh. That's a valid "fix the whole class" observation per REVIEW.md, but it's explicitly a nit — the docs pages this PR touches are strictly correct after the change, and leaving --help stale is no worse than the status quo. It's fine as a follow-up. No prior human reviews or outstanding comments to address.
The zsh descriptions for --loader/-l carried the same stale "Valid loaders" list as the docs, and the bash completion only offered six of the names. Both now use the same list as docs/runtime/bunfig.mdx.
robobun
changed the title
|
Addressed the review nit in 3ee536a: the four |
There was a problem hiding this comment.
LGTM — thanks for picking up the zsh and bash completions too. The remaining Arguments.rs help string is noted as a separate follow-up, which is fine for a docs-scoped PR.
What was reviewed:
- Confirmed the 17-name list matches across all four files and lines up with
LOADER_NAMESper the PR's verification table. - Checked the bash
compgen -Wrefactor — the loop builds the same word-list shape as before, just longer; leading space is harmless. - Verified
bun.fish,spec.yaml, andbun-cli.jsoncarry no loader list, so no other completion sites are missed.
Extended reasoning...
Overview
Docs + shell-completion update to the list of valid --loader / bunfig [loader] names. Four files touched: two .mdx docs pages, completions/bun.zsh (four identical description-string substitutions), and completions/bun.bash (the -l|--loader compgen wordlist expanded from 6 to 17 names via a small for-loop). No runtime, bundler, or test code is changed.
Security risks
None. The changes are prose in docs and static word lists in completion scripts; nothing here reaches parsing, evaluation, or the network.
Level of scrutiny
Low. This is a documentation correction backed by empirical verification (the PR description tabulates the runtime and bun build behavior for every name in LOADER_NAMES). The completion-script edits are mechanical string replacements plus a trivial bash loop that produces the same ext:loader word-list shape the old one-liner did.
Other factors
My previous review flagged the stale list in completions/bun.zsh and src/runtime/cli/Arguments.rs. The author responded by updating both zsh and bash completions in commit 3ee536a2, and had already stated that Arguments.rs is being handled separately — reasonable scoping for a PR titled docs: since that file ships in the binary and needs a build. I grepped the remaining completion files (bun.fish, spec.yaml, bun-cli.json) and none carry a loader list, so nothing else is left behind in completions/. No CODEOWNERS entries cover docs/ or completions/.
There was a problem hiding this comment.
LGTM — thanks for pulling in the zsh and bash completion sites in 3ee536a.
What was reviewed:
- Cross-checked all 17 names against
LOADER_NAMESinsrc/ast/loader.rs; each maps to a distinctLoadervariant, and the omissions (jsonc,sqlite_embedded,sh,base64,dataurl, aliases) match the stated reasons. - Bash
--loadercompletion: theforloop +compgen -Wbuilds the same 17 candidates as before with the wider set; leading space inloadersis harmless to word-splitting. - Confirmed no other
--loaderlist lives undercompletions/(no fish entry); the remainingArguments.rshelp string is called out for a follow-up PR.
Extended reasoning...
Overview
Docs + shell-completion update to the list of loader names accepted by bunfig.toml [loader] and --loader. Four files touched: docs/runtime/bunfig.mdx (bullet list expanded from 13 → 17 entries, base64/dataurl removed), docs/snippets/cli/run.mdx (inline <code> list for --loader), completions/bun.zsh (four _arguments description strings), and completions/bun.bash (the -l|--loader candidate list rewritten from a hardcoded 6-entry string to a for loop over 17 names). No src/ changes.
Security risks
None. Pure documentation prose and shell-completion metadata; nothing executes with untrusted input, and the loader resolution code path itself is untouched.
Level of scrutiny
Low. The changes are mechanical string-list updates. I verified each of the 17 names against the LOADER_NAMES table in src/ast/loader.rs:73-103 — every listed name is present and maps to a distinct Loader variant. The zsh edits are description text inside [...] (no _arguments syntax change). The bash edit is a straightforward for loop building a whitespace-separated word list for compgen -W; the author reports sourcing the file and driving _bun_completions directly to confirm 17 candidates and correct prefix narrowing.
Other factors
A prior review pass flagged the sibling zsh/bash sites and Arguments.rs; the author addressed the completion files in 3ee536a and explicitly deferred the Arguments.rs help string to a separate PR (it ships in the binary and needs a build + help-output test), which is a reasonable split for a docs-only change. I grepped completions/ for any remaining loader list — nothing else carries one. The bug-hunting system found no issues on this revision.
|
The in-binary copy of this list (the |
The bash --loader branch only runs when ':' has been removed from COMP_WORDBREAKS (bash splits ".ext:name" at the colon by default, so prev is ":" rather than --loader), and completions/bun.bash is about to become generated output, so the list belongs in that generator instead. The bunfig list is described as the available loaders rather than the only accepted values: aliases such as mjs or txt are accepted too, they are just not listed.
|
Scope update after another pass over the siblings of this list: reverted the |
There was a problem hiding this comment.
LGTM — docs and zsh-completion description strings only; the 17 loader names match LOADER_NAMES in src/ast/loader.rs, and the earlier sibling-site nit was addressed (zsh in 3ee536a, Arguments.rs split out to #38270). The one remaining nit is prose in the PR description, not the diff.
Extended reasoning...
Overview
Updates three hand-maintained copies of the "valid loader names" list to reflect loaders added since the list was last touched (yaml, json5, xml, md, html, sqlite) and removes two stubs (base64, dataurl) that don't do what their names say today:
docs/runtime/bunfig.mdx— the[loader]section's bulleted listdocs/snippets/cli/run.mdx— the--loaderflag descriptioncompletions/bun.zsh— four--loader/-ldescription strings (inside[...]— display text only, not completion candidates)
No source, no runtime code, no tests changed. I cross-checked all 17 names against LOADER_NAMES in src/ast/loader.rs:73-103; every one is present, and the deliberate exclusions (jsonc, sqlite_embedded, sh, base64, dataurl, aliases) each have a stated reason in the PR body tied to how to_api() collapses them.
Security risks
None. Documentation prose and zsh help strings; nothing executable, nothing user-input-driven.
Level of scrutiny
Low. The zsh edits sit entirely inside _arguments description brackets, so they can't change what completes — only what the help line reads. The mdx changes are prose lists. No CODEOWNERS entry covers docs/ or completions/. The single CI failure (test/bake/deinitialization.test.ts segfault on Windows x64) has no plausible connection to a docs+completions change.
Other factors
This is my second pass. My earlier nit (same stale list in completions/bun.zsh and Arguments.rs) was addressed: the four zsh sites were updated in 3ee536a, and Arguments.rs was intentionally split out to #38270 with the reason stated in the description. 0749b07 then dropped the completions/bun.bash hunk (per its commit message, the branch only fires with a non-default COMP_WORDBREAKS and the file is about to become generated). The one finding this run is that the PR description still mentions the reverted bash change — that's description prose, not the diff, and doesn't affect what merges.
This branch has not been deployed
1 participant
Problem
docs/runtime/bunfig.mdx, sectionloader, lists the loader names a[loader]entry accepts asjsx, js, ts, tsx, css, file, json, toml, wasm, napi, base64, dataurl, text.docs/snippets/cli/run.mdxrepeats the list (minus css) for--loader, andcompletions/bun.zshcarries the same--loadersentence four times (these scripts are embedded in the binary and printed bybun completions).yaml,json5,xml,md,htmlandsqliteloaders. Mapping an extension to any of them from bunfig or--loaderworks today (checked on the current build, table below), but none of them is documented as a valid value.base64anddataurlare listed but not implemented: the name is accepted, then the bundler emits a module whose default export is""and the runtime falls back to thefileloader (returns the path).docs/bundler/esbuild.mdxalready describes them as not implemented.Fix
--loader:js, jsx, ts, tsx, json, toml, yaml, json5, xml, text, md, css, html, wasm, napi, sqlite, file. Same names, same order, in every copy.[loader]and--loader(bun run,bun build,bun test) resolve the value through the same code,bun_ast::Loader::from_stringfollowed byto_api()(src/bunfig/bunfig.rs,loader_resolverinsrc/runtime/cli/Arguments.rs), and each of the 17 names maps to a loader that does what the name says on both the runtime andbun buildpaths (table below). The bunfig page now says "The available loaders are" rather than claiming these are the only accepted spellings: aliases such asmjs,nodeortxtare accepted too, they are just not listed, as before.jsoncandsqlite_embedded: accepted, butto_api()collapses them intojsonandsqlite(src/options_types/bundle_enums.rs), so a jsonc file with comments fails to parse. bundler: honor jsonc and sqlite_embedded in loader maps, accept every loader name in Bun.build #38247 fixes that; when it lands, both names should be added to every copy of this list in one go.base64,dataurl: stubs as described above. bundler: implement dataurl and base64 loaders #36327 (bundler) and runtime + --no-bundle: implement base64/dataurl loaders #36334 (runtime) implement them; add them back with those.sh: mapped tofile; the loader docs already say it is not available in the runtime or bundler.src/runtime/cli/Arguments.rs:137, the--helptext: cli: list every working loader name in the --loader help text #38270 changes it to this exact list and adds a test that--helpprints it and that every listed name works via--loader. That test's array is the natural place to also assert the docs and zsh copies match, so the cross-check lives in one file once both PRs are in; this PR stays free of source changes.completions/bun.bash: its--loaderbranch only runs when:has been removed fromCOMP_WORDBREAKS(bash splits.ext:nameat the colon by default, soprevis:and the branch is skipped; checked by driving_bun_completionsdirectly), and completions: generate fish/bash from bun-cli.json #35443 turns the file into generated output ofmisctools/generate-shell-completions.ts. The longer list belongs in that generator. An earlier revision of this PR changed the file; reverted.dataurlexample underloaderindocs/bundler/index.mdxis being replaced separately.LOADER_NAMES(src/ast/loader.rs) from abunfig.toml, importing one fixture per extension at runtime and bundling one entry per extension withbun build --target bun, then repeating the runtime run with--loaderflags underbun runandbun test. Without any mapping every fixture imports as a path, so the results are attributable to the mapping.prettier --checkpasses on both docs pages.Background
jsonparses JSON into an object,textreturns the contents as a string,filereturns the path, and so on).[loader]in bunfig.toml andbun --loader .ext:nameoverride the extension to loader mapping, for the runtime and forbun build.LOADER_NAMESinsrc/ast/loader.rsis the table of spellings these settings accept (29 entries, including aliases). The chosen loader is converted withto_api()into the enum the bundler consumes, which is wherejsonc,sqlite_embeddedandshcurrently collapse intojson,sqliteandfile. The documented list is the subset of that table that behaves as named.Per-name results on the current build (each row is a fixture mapped to that loader name from bunfig.toml)
import()at runtime:bun build --target bun, then running the output:The same runtime import with
--loader .l_<name>:<name>flags instead of the bunfig gives the same results for every name, under bothbun runandbun test.Earlier revision
The first revision also expanded the candidate list in
completions/bun.bashfrom six names to the same seventeen. Reverted for the two reasons given above (the branch is unreachable with the defaultCOMP_WORDBREAKS, and #35443 regenerates the file from a template that still carries the six-name list).