Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/ast/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@
ImportKind::Dynamic => b"dynamic-import",
ImportKind::RequireResolve => b"require-resolve",
ImportKind::At => b"import-rule",
ImportKind::AtConditional => b"",
ImportKind::AtConditional => b"import-rule",

Check notice on line 97 in src/ast/lib.rs

View check run for this annotation

Claude / Claude Code Review

Plugin onResolve args.kind still wrong for conditional CSS @import — sibling $ImportKindIdToLabel table not updated

Pre-existing (not introduced here), but the same "`AtConditional` labelled wrong" class this PR fixes is still present in the sibling table the description dismissed as unaffected: `enums.ImportKind` in `src/codegen/replacements.ts:120-130` generates `$ImportKindIdToLabel`, which `BundlerPlugin.ts:402` indexes with the raw Rust discriminant, so a plugin `onResolve` for `@import "x" supports(...)` (discriminant 7) still receives `args.kind === "url-token"` (and `url()`→`"internal"`, `Composes`/`H
Comment thread
claude[bot] marked this conversation as resolved.
ImportKind::Url => b"url-token",
ImportKind::Composes => b"composes",
ImportKind::Internal => b"internal",
Expand All @@ -112,7 +112,7 @@
ImportKind::Dynamic => b"import()",
ImportKind::RequireResolve => b"require.resolve()",
ImportKind::At => b"@import",
ImportKind::AtConditional => b"",
ImportKind::AtConditional => b"@import",
ImportKind::Url => b"url()",
ImportKind::Internal => b"<bun internal>",
ImportKind::Composes => b"composes",
Expand Down
23 changes: 1 addition & 22 deletions src/jsc/ResolveMessage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,27 +35,6 @@ impl Default for ResolveMessage {
}
}

/// `ImportKind.label()` — the canonical table lives in
/// `bun_ast::ImportKind::label`, but
/// `bun_ast::MetadataResolve.import_kind` is the type-only `bun_ast::ImportKind`.
/// Replicate the table here verbatim.
fn import_kind_label(kind: ImportKind) -> &'static [u8] {
match kind {
ImportKind::EntryPointRun => b"entry-point-run",
ImportKind::EntryPointBuild => b"entry-point-build",
ImportKind::Stmt => b"import-statement",
ImportKind::Require => b"require-call",
ImportKind::Dynamic => b"dynamic-import",
ImportKind::RequireResolve => b"require-resolve",
ImportKind::At => b"import-rule",
ImportKind::AtConditional => b"",
ImportKind::Url => b"url-token",
ImportKind::Composes => b"composes",
ImportKind::Internal => b"internal",
ImportKind::HtmlManifest => b"html_manifest",
}
}

/// Host-agnostic bare-specifier check for Node ESM error shaping. Must not vary by host:
/// relative, separator-led, and ASCII-letter drive forms are path-like; everything else is a
/// package. Unlike `bun_paths::is_absolute`, the drive byte must be alphabetic.
Expand Down Expand Up @@ -479,7 +458,7 @@ impl ResolveMessage {
pub fn get_import_kind(this: &Self, global: &JSGlobalObject) -> JsResult<JSValue> {
Ok(match &this.msg.metadata {
bun_ast::Metadata::Resolve(resolve) => {
ZigString::init(import_kind_label(resolve.import_kind)).to_js(global)
ZigString::init(resolve.import_kind.label()).to_js(global)
}
_ => ZigString::init(b"").to_js(global),
})
Expand Down
47 changes: 47 additions & 0 deletions test/bundler/bun-build-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,53 @@ describe("Bun.build", () => {
}
});

test("css @import resolve errors report importKind 'import-rule' with and without import conditions", async () => {
using dir = tempDir("bun-build-api-css-import-kind", {
"entry.css": `
@import "./missing-plain.css";
@import "./missing-conditional.css" supports(display: grid);
@import "bun:sqlite";
@import "bun:sqlite" supports(display: grid);
`,
});

const build = await Bun.build({
entrypoints: [join(String(dir), "entry.css")],
target: "browser",
throw: false,
});

expect(build.success).toBe(false);
for (const log of build.logs) expect(log).toBeInstanceOf(ResolveMessage);
const logs = (build.logs as ResolveMessage[]).toSorted((a, b) => a.position!.line - b.position!.line);
expect(logs.map(({ specifier, importKind, message }) => ({ specifier, importKind, message }))).toEqual([
{
specifier: "./missing-plain.css",
importKind: "import-rule",
message: 'Could not resolve: "./missing-plain.css"',
},
{
specifier: "./missing-conditional.css",
importKind: "import-rule",
message: 'Could not resolve: "./missing-conditional.css"',
},
{
specifier: "bun:sqlite",
importKind: "import-rule",
message: `Browser build cannot @import Bun builtin: "bun:sqlite". When bundling for Bun, set target to 'bun'`,
},
{
specifier: "bun:sqlite",
importKind: "import-rule",
message: `Browser build cannot @import Bun builtin: "bun:sqlite". When bundling for Bun, set target to 'bun'`,
},
]);
expect(JSON.parse(JSON.stringify(logs[1]))).toMatchObject({
specifier: "./missing-conditional.css",
importKind: "import-rule",
});
});

test("returns output files", async () => {
Bun.gc(true);
const build = await Bun.build({
Expand Down
34 changes: 34 additions & 0 deletions test/bundler/metafile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,40 @@ describe("bundler metafile", () => {
expect(outputPaths).toContain(dynamicImport!.path);
});

test("metafile tracks css @import imports as import-rule, with or without import conditions", async () => {
using dir = tempDir("metafile-css-import-rule-test", {
// Reached through a JS entry because a CSS entrypoint currently contributes nothing to metafile.inputs.
"entry.js": `import "./styles.css";`,
"styles.css": `
@import "./plain.css";
@import "./conditional.css" supports(display: grid);
@import "./media.css" screen;
@import "./layered.css" layer(base) supports(display: grid) screen;
.foo { color: red; }
`,
"plain.css": `.plain { color: blue; }`,
"conditional.css": `.conditional { display: grid; }`,
"media.css": `.media { color: green; }`,
"layered.css": `.layered { color: black; }`,
});

const result = await Bun.build({
entrypoints: [`${dir}/entry.js`],
metafile: true,
});

expect(result.success).toBe(true);
const inputs = (result.metafile as Metafile).inputs;
const stylesKey = Object.keys(inputs).find(path => path.endsWith("styles.css"))!;
expect(stylesKey).toBeDefined();
expect(inputs[stylesKey].imports.map(({ kind, original }) => ({ kind, original }))).toEqual([
{ kind: "import-rule", original: "./plain.css" },
{ kind: "import-rule", original: "./conditional.css" },
{ kind: "import-rule", original: "./media.css" },
{ kind: "import-rule", original: "./layered.css" },
]);
});

test("metafile includes cssBundle for CSS outputs", async () => {
using dir = tempDir("metafile-css-bundle-test", {
"entry.js": `import "./styles.css"; console.log("styled");`,
Expand Down