bun-types: type BuildMessage/ResolveMessage level and importKind as the strings the runtime reports - #38542
bun-types: type BuildMessage/ResolveMessage level and importKind as the strings the runtime reports#38542robobun wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 37 seconds 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 (9)
Comment |
|
Updated 8:45 PM PT - Aug 14th, 2026
✅ @robobun, your commit e1ea82a0a9b284d19eae62b45de365eb9126c8e7 passed in 🧪 To try this PR locally: bunx bun-pr 38542That installs a local version of the PR into your bun-38542 --bun |
|
Status: ready to merge from my side. Diff is 4dcef48 (e1ea82a on top is an empty CI retrigger).
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. Since packages/bun-types/ has a CODEOWNER, a human sign-off is still appropriate.
What was reviewed:
- Verified the new
levelunion matchesKind::string()(src/ast/lib.rs:555) andget_levelin both BuildMessage.rs and ResolveMessage.rs. - Verified
Bun.ImportKindnow covers every arm ofImportKind::label()(src/ast/lib.rs:88) except the intentionalAtConditional => ""gap noted in the description. - Checked the test refactor: the extracted
expectToTypecheckhelper preserves the existing mmap case verbatim, and the new case asserts runtime values before feeding them to tsc.
Extended reasoning...
Overview
This PR corrects three stale string-literal unions in packages/bun-types/: BuildMessage.level, ResolveMessage.level, and ResolveMessage.importKind. It also adds two missing members ("composes", "html_manifest") and one-line doc comments to Bun.ImportKind, updates the hand-maintained copy in docs/bundler/index.mdx, and adds a mirror comment on Kind::string() in src/ast/lib.rs. The test file extracts the existing single-file tsc helper into expectToTypecheck and adds a case that runs Bun.build to collect real level/importKind strings, then type-checks them against the packed declarations. A fixture pins the exact unions for the release-build LanguageService checks.
Verification against source
I confirmed against src/ast/lib.rs that Kind::string() returns exactly error|warn|note|debug|verbose and ImportKind::label() returns exactly the eleven strings now in Bun.ImportKind (plus "" for AtConditional, deliberately excluded per the description). ResolveMessage::get_level calls this.msg.kind.string() and get_import_kind calls import_kind_label(resolve.import_kind), which replicates label() verbatim, so the declarations are now accurate mirrors of runtime behavior.
Security risks
None. This is type declarations, a doc comment, an .mdx update, and a test. No runtime code paths change.
Level of scrutiny
Low-to-medium. The change is mechanical (aligning .d.ts with what the runtime already returns) and thoroughly tested — the new test would fail on the old declarations with TS2367/TS1360. However, packages/bun-types/ is CODEOWNER-gated, so per the approval guidelines I'm deferring rather than approving outright.
Other factors
The test refactor is clean: the mmap case is byte-equivalent through the new helper, pipes are drained concurrently, tempDir with using handles cleanup, and assertions check stdout/stderr before exit code. The docs hunk lists 11 importKind members while the old block listed 10, so the diff's +/- counts differ by one line — that's expected, not a mis-splice.
|
Note on the |
e692cdf to
760233c
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
760233c to
7aa6a94
Compare
…he strings the runtime reports
level is bun_ast::Kind::string(), so the runtime reports "warn" and "note",
never the "warning" / "info" the declarations listed. ResolveMessage.importKind
has been ImportKind::label() ("import-statement", "require-call", ...) since the
getter stopped using the enum tag names, but the declaration still listed the
tag names. Both declarations now reuse one type each: Bun.ImportKind, which
gains the "composes" label it was missing, and a new Bun.BuildMessageLevel.
The docs copies are updated to match.
ResolveMessage's getter used a verbatim copy of the label table; it now calls
ImportKind::label() directly. A source lint parses both tables out of
src/ast/lib.rs and fails when the bun-types unions or the docs snippets differ
from them (AtConditional and HtmlManifest are the listed exclusions), and the
source-lints workflow now also triggers on bun.d.ts and the docs so that lint
runs for declaration-only changes.
7aa6a94 to
4dcef48
Compare
2ea95c1 to
e1ea82a
Compare
|
Ignore the 2ea95c1 push: it was meant to be an empty CI retrigger (build 96666 lost its Windows lanes to agent provisioning failures) but picked up 21 unrelated files that were staged from a newer main. The branch is reset; e1ea82a is the empty retrigger on top of 4dcef48, and the diff is the same 9 files as before. The three bot threads on the stray file are resolved. |
There was a problem hiding this comment.
LGTM — the author addressed my earlier note on HtmlManifest (kept as a documented exclusion coordinated with #38621), and the diff is otherwise unchanged since. Verified ImportKind::label() in src/ast/lib.rs is byte-identical to the deleted private table, so get_import_kind returns the same strings; Kind::string() matches the new BuildMessageLevel union; and the source-lint's regex parsing lines up with the actual match arm shapes in lib.rs.
Extended reasoning...
Overview
This PR corrects long-standing drift between the runtime's reported strings and the TypeScript declarations for BuildMessage.level, ResolveMessage.level, and ResolveMessage.importKind. It touches: type declarations (bun.d.ts, globals.d.ts) to add Bun.BuildMessageLevel and "composes" to Bun.ImportKind and reference those aliases from the global classes; docs (docs/bundler/index.mdx) to match; src/jsc/ResolveMessage.rs to delete a private copy of the label table and call ImportKind::label() directly; a one-line doc comment in src/ast/lib.rs; a new source-lint test that parses the Rust tables and compares them to the declared unions and docs snippets; workflow paths: additions so the lint triggers on bun-types/docs edits; and a bun-types fixture exercising the corrected types.
Security risks
None. No auth, crypto, network, or user-input parsing paths are touched. The only compiled change is replacing a private static-string table with a call to the identical shared one.
Level of scrutiny
Low-to-medium. The runtime change is a pure dedup (I diffed the deleted import_kind_label arms against ImportKind::label() in src/ast/lib.rs:88-102 — byte-identical, including the AtConditional => "" and HtmlManifest => "html_manifest" arms). Everything else is types, docs, and a source-tree lint that reads files rather than executing the build. The lint's regex parsing is guarded by a self-check test that compares parsed arm keys against the enum variants, so a future refactor of lib.rs that breaks the regex fails loudly rather than silently passing.
Other factors
I previously flagged the HtmlManifest exclusion in notPublic. The author responded with a clear rationale (adding it now would leave a member that outlives its last producer once #38621 lands, and the lint could not detect that; leaving it out costs only the metafile kind typing for one narrow case, which was never typed before), updated the lint comment to present tense, and updated the description. That is a reasonable scope decision, not a correctness issue, and the thread is resolved. All other review threads (comment-cop nits, an accidental stray push) are resolved and the diff is back to the described 9 files. The bug-hunting pass found nothing this run. The change is self-contained, well-tested (source lint fails on main / passes here; bun-types fixture compiles the corrected comparisons), and low-risk.
1 participant
Problem
BuildMessage.levelandResolveMessage.levelare declared as"error" | "warning" | "info" | "debug" | "verbose"(packages/bun-types/globals.d.ts), but both getters returnbun_ast::Kind::string()(src/jsc/BuildMessage.rs and src/jsc/ResolveMessage.rsget_level; table in src/ast/lib.rs), whose values are"error" | "warn" | "note" | "debug" | "verbose". Solog.level === "warn"(the value that comes back) is a TS2367 error, whilelog.level === "warning"type-checks and is never true.ResolveMessage.importKindis declared as the old enum tag names ("stmt" | "require" | "require_resolve" | "at" | ...), but the getter has returnedImportKind::label()("import-statement" | "require-call" | "require-resolve" | "import-rule" | ...) since bundler tests and improveBun.buildreturn type #2833, the same change that added thelevelunions above. Same failure mode.Bun.ImportKindin bun.d.ts, the declared mirror ofImportKind::label(), is missing"composes", which aResolveMessageforcomposes: x from "./missing.css"reports.Fix
Bun.ImportKindgains"composes"(each member now documented), and a newBun.BuildMessageLevelalias holds theKind::string()arms.ResolveMessage.importKindand bothleveldeclarations reference those two types, so each union is declared once. The docs copies in docs/bundler/index.mdx are updated to match.ResolveMessage::get_import_kindnow callsImportKind::label()instead of its private copy of the table (the copy's justification was stale:MetadataResolve.import_kindis thebun_ast::ImportKindthat haslabel()). The table is identical, so the reported strings do not change (verified with the probe below on a debug build).Kind::string()andImportKind::label()arms out of src/ast/lib.rs and fails whenBun.BuildMessageLevel,Bun.ImportKind, or anyclass BuildMessage/class ResolveMessagesnippet in the docs differs from them. Two variants are listed as exclusions with their reasons:AtConditional(its label is"") andHtmlManifest(see below). Same shape as the Loader lint in Lint the loader name list in bun-types and the docs against src/ast/loader.rs #38424; the source-lints workflow and README hunks are identical to that PR's so the two merge in either order.BuildMessageLevelabsent,ImportKindmissing"composes"); with this change it passes. test/integration/bun-types/fixture/build.ts additionally compiles realistic usage (log.level === "warn",log.importKind === "require-call") and checks that the globals resolve to the two aliases, under both the DOM and non-DOM tsconfigs.bun bd test test/js/bun/resolve/resolve-error.test.ts test/bundler/bun-build-api.test.ts(74 pass) andbun test test/internal/source-lints/(151 pass).Not changed here
ImportKind::label()returns""for a conditional CSS@import, so that oneResolveMessagereportsimportKind: ""today. That is a runtime gap for a separate change; the lint documents it rather than adding""to the type."html_manifest"(theHtmlManifestvariant) is deliberately left out ofBun.ImportKind, so the metafilekindof the first server-side importer of an HTML file (the only place it is reported today; it is never aResolveMessage.importKind) stays untyped for now. bundler: keep the import kind of server-side HTML imports so import() and require() of the manifest link #38621 removes that rewrite but keeps the variant, so a member added now would outlive its last use and the lint could not tell. The lint lists the variant as an exclusion with that reason; if bundler: keep the import kind of server-side HTML imports so import() and require() of the manifest link #38621 is dropped, the entry gets deleted and the lint then demands the member.onResolveargs get the wrongkindfor CSS imports because the JS-side id table is hand-written; Delete the schema::api mirror types and bun_api; one loader numbering across Rust/C++/JS #37095 fixes that.notes,line,column,toJSON, theErrorbase); separate change.scanImports()returns, which is deliberately a subset ofImportKind(a JS scan cannot produce the CSS kinds), so it is left as is and not linted.Background
bun_ast::Msgwhosekindis abun_ast::Kind(Err,Warn,Note,Debug,Verbose).Bun.buildreturns each one wrapped as aBuildMessage, or aResolveMessagewhen it carries resolve metadata, andlevelisKind::string()of that kind.Noteis the kind of the entries in a message'snotes.bun_ast::ImportKindclassifies the import record being resolved.label()is its public string form, shared byResolveMessage.importKind, the metafilekindfield andTranspiler.scanImports();Bun.ImportKindis the TypeScript mirror of that table.Runtime probe (bun 1.4.0 and a debug build of this branch print the same rows)
Earlier revision of this PR
The first push fixed the same declarations but re-pinned the unions as literals in the bun-types fixture and added a
Bun.build+ tsc case to test/integration/bun-types/bun-types.test.ts. Self-review pointed out that the literal pins only compared TypeScript with TypeScript, that test/integration/bun-types is excluded from the Buildkite lanes (so theBun.buildhalf never ran against the build under test), that the tsc helper extraction collided with #37421, and that ResolveMessage.rs still carried its own copy of the label table. This revision replaces all of that with the source lint, the shared aliases and thelabel()call described above; bun-types.test.ts is no longer touched.[review] gate passed · iteration 2 · 9 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 2 passed · 0 rejected · iteration 2
evidence per changed file