Delete the schema::api mirror types and bun_api; one loader numbering across Rust/C++/JS - #37095
Open
dylan-conway wants to merge 4 commits into
Open
Delete the schema::api mirror types and bun_api; one loader numbering across Rust/C++/JS#37095dylan-conway wants to merge 4 commits into
dylan-conway wants to merge 4 commits into
Claude / Claude Code Review
completed
Aug 7, 2026 in 26m 56s
Code review found 2 potential issues
Found 2 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | src/codegen/replacements.ts:113-120 |
$ImportKindIdToLabel derives empty-string label for ImportKind::AtConditional |
| 🟡 Nit | packages/bun-types/bun.d.ts:5550-5554 |
bun.d.ts type ImportKind not updated for newly-surfaced labels |
Annotations
Check warning on line 120 in src/codegen/replacements.ts
claude / Claude Code Review
$ImportKindIdToLabel derives empty-string label for ImportKind::AtConditional
`importKindLabel()` derives labels from `bun_ast::ImportKind::label()`, but `ImportKind::AtConditional` (discriminant 7, emitted for `@import "..." supports(...)`) has `label() => b""`, so the generated `$ImportKindIdToLabel[7] === ""` and `$ImportKindLabelToId` gains an empty-string key. An `onResolve` plugin whose filter matches such an import will see `args.kind === ""` (falsy, and not in the `ImportKind` d.ts union) — before this PR id 7 read as `"url-token"` (also wrong, but truthy). Since
Check warning on line 5554 in packages/bun-types/bun.d.ts
claude / Claude Code Review
bun.d.ts type ImportKind not updated for newly-surfaced labels
`type ImportKind` in this file (line 2745) wasn't updated even though `$ImportKindIdToLabel` now derives all 12 `bun_ast::ImportKind` labels — `onResolve` plugins and metafile imports can now surface `"composes"` and `"html_manifest"`, which the union doesn't include. The new comment at src/ast/lib.rs:79 says to keep this type in sync by hand, and `type Loader` was updated here, so this looks like an oversight.
Loading