Conversation
…oader.rs The public loader names are the snake_case variants of bun_ast::Loader. The Loader union in packages/bun-types/bun.d.ts and the three `type Loader =` snippets in the docs are hand-written copies of that list, and recent loaders landed with some copies stale: json5 and md never reached the union, xml never reached the docs snippets. Add test/internal/source-lints/loader-names.test.ts, which derives the public names from the enum minus an explicit, commented list of variants that are deliberately not in the union (base64, dataurl, bunsh, sqlite, sqlite_embedded), and asserts that LOADER_NAMES accepts each public name, that the bun.d.ts union equals the set, and that every `type Loader` snippet under docs/ equals it. Bring the copies up to date so the lint passes, and add bun.d.ts and docs/**/*.mdx to the source-lints workflow triggers, since the lint reads them.
|
Warning Review limit reached
Next review available in: 26 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 (8)
Comment |
|
Status: the lint in this PR fails against main's copies of the loader list ( |
There was a problem hiding this comment.
LGTM — docs/types sync plus a source-lint that keeps them synced; no runtime code changes.
What was reviewed
- Verified the new lint passes locally (4 tests, 17 expects) and that its enum/table regexes match
src/ast/loader.rsas written; the[^}]*bodies have no nested braces. - Confirmed the added names (
json5,md,xml) are realLoadervariants and thatdocs/runtime/transpiler.mdx's skippedtype Loaderis genuinely a different (JS-only) subset. - Checked
bun.d.tshas exactly onetype Loader =declaration and the docs glob finds exactly the three snippets the test expects. - Workflow path triggers added symmetrically to both
pushandpull_request; theloader.rshunk is comment-only.
Extended reasoning...
Overview
This PR adds a source-tree lint (test/internal/source-lints/loader-names.test.ts) that parses pub enum Loader and LOADER_NAMES from src/ast/loader.rs, derives the set of public loader names (enum variants minus a documented notPublic exclusion list), and asserts that the Loader string-literal union in packages/bun-types/bun.d.ts and every type Loader = snippet under docs/**/*.mdx equal that set. It also brings the currently-stale copies up to date (json5/md in bun.d.ts; json5/xml/md in three docs files), adds packages/bun-types/bun.d.ts and docs/**/*.mdx to the source-lints workflow's path filters, adds a pointer comment above the enum, and documents the path-filter requirement in the source-lints README.
Security risks
None. No runtime code is touched: the .rs hunk is a comment, the .d.ts hunk adds string-literal union members, the docs hunks are prose snippets, and the test only reads files from the repo tree. The workflow change only widens which paths trigger an existing read-only test job.
Level of scrutiny
Low. This is test + docs + type-declaration + CI-trigger territory with no compiled-code changes. The test follows the established pattern of the ~20 other regex-based lints already in test/internal/source-lints/. I ran it locally against the checked-out tree and it passes cleanly. The regex parsers are guarded against silent breakage (toBeGreaterThan(10) on the enum and table sizes, snippets.length > 0 on the docs scan, and an explicit assertion that the union body contains only string literals and |), so a future refactor of loader.rs or the docs that breaks the parser will fail loudly rather than pass vacuously.
Other factors
- Verified via grep that
bun.d.tshas exactly onetype Loader =(line 5555), that the three docs files listed are the only full-union snippets, and that the skippeddocs/runtime/transpiler.mdxdeclares the unrelated four-memberJavaScriptLoadersubset — the skip is correct. - The
snakeCasehelper correctly reproduces strum'ssnake_casefor every current variant (checkedSqliteEmbedded->sqlite_embedded,Json5->json5). - The
notPublicexclusions each carry a reason and are themselves asserted to name real variants, so they cannot go stale silently. - No prior human review comments; only bot noise (CodeRabbit rate-limit notice, robobun status).
|
Updated 5:05 AM PT - Aug 14th, 2026
❌ @robobun, your commit c10cd0a has some failures in 🧪 To try this PR locally: bunx bun-pr 38424That installs a local version of the PR into your bun-38424 --bun |
Problem
packages/bun-types/bun.d.ts(type Loader, line 5555) and into three docs snippets (docs/runtime/plugins.mdx,docs/bundler/plugins.mdx,docs/bundler/index.mdx), and nothing checks the copies againstpub enum Loaderinsrc/ast/loader.rs.json5(feat: add native JSON5 parser (Bun.JSON5) #26439) andmd(feat(md): Zig markdown parser with Bun.markdown API #26440) never reached the union,xml(Add Bun.XML (parse/stringify) and an .xml loader #37048) reached the union but none of the docs snippets. On main today the union is missingjson5andmd, and all three snippets are missingjson5,mdandxml, althoughbun build --loader .x:json5,Bun.build({ loader })andBuildArtifact.loaderall accept or produce those names on 1.4.0 (run in the details block).base64/dataurlto the union and todocs/bundler/index.mdx, not to the twoplugins.mdxsnippets.Fix
test/internal/source-lints/loader-names.test.tsderives the public names from the enum variants (strum's snake_case spelling, which is whatBuildArtifact.loaderreports) minusnotPublic, a commented list of the variants deliberately absent from the union:base64anddataurl(the bundler emits an empty module for them),bunsh(backsbun run x.sh),sqliteandsqlite_embedded(not declared yet; bundler: honor jsonc and sqlite_embedded in loader maps, accept every loader name in Bun.build #38247 adds them). It then asserts thatLOADER_NAMESaccepts each public name, that thebun.d.tsunion equals the set, and that everytype Loader =declaration found underdocs/**/*.mdxequals it (docs/runtime/transpiler.mdxis skipped: itstype Loaderis theJavaScriptLoadersubset). EachnotPublicentry must still name a variant, so the exclusions cannot go stale either.notPublicentry is removed, and removing the entry fails the docs check until every snippet lists it. Adding an enum variant fails all three checks until it is either listed everywhere or excluded with a reason. Each failure is a sorted-array diff naming the file.json5andmdinbun.d.ts,json5,xmlandmdin the three snippets. These hunks are the same lines as in bun-types: add json5 and md to the Loader union, document both loaders #38267 (and thebun.d.tshunk as in Delete the schema::api mirror types and bun_api; one loader numbering across Rust/C++/JS #37095), so the PRs merge cleanly in either order; bun-types: add json5 and md to the Loader union, document both loaders #38267 still carries the loader documentation sections, this PR only carries what the lint needs..github/workflows/source-lints.ymlgainspackages/bun-types/bun.d.tsanddocs/**/*.mdxas triggers, because the lint reads them and the workflow is path-filtered (the whole directory runs in about 12 s). The directory README notes that a lint has to add whatever it reads there. A comment next to the enum points at the lint.git stash push -- packages/or the unmodified docs) the lint fails with the diffs in the details block; with this PRbun test test/internal/source-lints/loader-names.test.tspasses (4 tests) andbun test test/internal/source-lints/is 150+ green. Simulated a new variant (three failures naming it), a stale exclusion (one failure), and bundler: honor jsonc and sqlite_embedded in loader maps, accept every loader name in Bun.build #38247's union hunk with its JSDoc member comment (union failure until the exclusions go, then docs failures until the snippets list it). No runtime code changes; thesrc/ast/loader.rshunk is a comment.Background
bun build --loader .ext:name,Bun.build({ loader: { ".ext": "name" } }), bunfig[loader]and pluginonLoadresults, and whatBuildArtifact.loader/OnLoadArgs.loaderreturn. TheLoaderunion in bun-types is the type of all of those.src/ast/loader.rsholds the enum (#[strum(serialize_all = "snake_case")], soSqliteEmbeddedis the namesqlite_embedded) andLOADER_NAMES, the tableLoader::from_stringaccepts. The table also has aliases (mjs,node,txt,markdown, ...), which is why the lint derives the list from the variants and only looks the canonical name up in the table.test/internal/source-lints/holds tests that only read the source tree;.buildkite/ci.mjsexcludes them from the binary lanes and.github/workflows/source-lints.ymlruns them against a released bun, so this needs no build. Delete the schema::api mirror types and bun_api; one loader numbering across Rust/C++/JS #37095 addsloader-numbering.test.tsto the same directory for the numeric discriminant copies (C header, Rust plugin crate); this lint covers the names and does not overlap with it.Lint output against main's copies, and the 1.4.0 runs behind the synced names
Why the excluded names are excluded, on 1.4.0 (
Bun.build({ loader: { ".sh": name }, target: "bun" })):