Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
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
10 changes: 9 additions & 1 deletion src/bundler/ParseTask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1210,10 +1210,18 @@
debug_assert!(entry.ref_.inner_index() < extra.symbols.len() as u32);
}
}
// Bake builds emit every stylesheet into the client bundle, so
// minify with browser targets (vendor prefixing, selector
// downleveling) even when the importing graph is the server's.

Check warning on line 1215 in src/bundler/ParseTask.rs

View check run for this annotation

Claude / Claude Code Review

Same override needed at print-time: linker CSS sites still use server target in bake builds

The same "bake CSS is always browser-facing" override is also needed at print time: the four sibling `Targets::for_bundler_target(c.options.target)` sites in the linker (`generateCompileResultForCssChunk.rs:111,154,187` and `prepareCssAstsForChunk.rs:224`) still receive `Target::Bun` in bake builds, so print-time-decided transforms like CSS nesting downleveling and `light-dark()` fallbacks remain skipped for every bake stylesheet. This is pre-existing (not the server-vs-HTML inconsistency this P
Comment thread
robobun marked this conversation as resolved.
Resolved
Comment thread
robobun marked this conversation as resolved.
Resolved
let css_target = if topts.server_components || topts.has_dev_server() {
options::Target::Browser
} else {
topts.target
};

Check warning on line 1220 in src/bundler/ParseTask.rs

View check run for this annotation

Claude / Claude Code Review

Condition misses bake production builds with frameworks that omit serverComponents

The condition `topts.server_components || topts.has_dev_server()` doesn't cover all bake builds — a custom framework can define `fileSystemRouterTypes` without `serverComponents` (it's optional per `bake.d.ts:108`), and in a production `bun build --app` of that framework both flags are false, so server-imported CSS still minifies with `Target::Bun`. `topts.framework.is_some()` is the direct "is a bake build" predicate (unconditionally set at `bake_body.rs:1229`, documented in `options.rs` as "Se
Comment thread
robobun marked this conversation as resolved.
Resolved
if let Err(e) = css_ast.minify(
bump,
&bun_css::MinifyOptions {
targets: bun_css::Targets::for_bundler_target(topts.target),
targets: bun_css::Targets::for_bundler_target(css_target),
unused_symbols: Default::default(),
},
&extra,
Expand Down
30 changes: 29 additions & 1 deletion test/bake/dev/css.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// CSS tests concern bundling bugs with CSS files
import { expect } from "bun:test";
import assert from "node:assert";
import { devTest, emptyHtmlFile, imageFixtures } from "../bake-harness";
import { devTest, emptyHtmlFile, imageFixtures, minimalFramework } from "../bake-harness";

devTest("css file with syntax error does not kill old styles", {
files: {
Expand Down Expand Up @@ -692,6 +692,34 @@ devTest("css import before create project relative", {
},
});

devTest("css imported on the server gets browser-target processing", {
framework: minimalFramework,
files: {
"routes/index.ts": `
import "../styles.css";
export default function (req, meta) {
return new Response(JSON.stringify(meta.styles), {
headers: { "Content-Type": "application/json" },
});
}
`,
"styles.css": `
.box:fullscreen {
color: red;
}
`,
},
async test(dev) {
const styles = await (await dev.fetch("/")).json();
expect(styles).toHaveLength(1);
const css = await (await dev.fetch(styles[0])).text();
// Stylesheets are always served to the browser, so selector downleveling
// for browser targets must apply even when the importer is a server file.
expect(css).toContain(":-webkit-full-screen");
expect(css).toContain(":fullscreen");
},
});

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
function extractCssUrl(backgroundImage: string): string {
const url = backgroundImage.match(/url\((['"])(.*?)\1\)/);
if (!url) {
Expand Down
38 changes: 38 additions & 0 deletions test/bake/dev/production.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -594,4 +594,42 @@ export default function IndexPage() {
// Verify NO JavaScript imports are included in the HTML
expect(htmlContent).not.toContain('<script type="module"');
});

test("css imported on the server gets browser-target processing", async () => {
const dir = await tempDirWithBakeDeps("bake-production-server-css", {
"src/index.tsx": `export default { app: { framework: "react" } };`,
// Importing the stylesheet from a page puts it in the server graph, but
// the emitted CSS is still a browser asset, so selector downleveling for
// browser targets must apply.
"pages/index.tsx": `
import "../styles.css";
export default function IndexPage() {
return <div>Hello World</div>;
}`,
"styles.css": `
.box:fullscreen {
color: red;
}`,
"package.json": JSON.stringify({
"name": "test-app",
"version": "1.0.0",
"devDependencies": {
"react": "^18.0.0",
"react-dom": "^18.0.0",
},
}),
});

const { exitCode } = await Bun.$`${bunExe()} build --app ./src/index.tsx --outdir ./dist`
.cwd(dir)
.env(bunEnv)
.throws(false);
expect(exitCode).toBe(0);

const cssFiles = await Array.fromAsync(new Bun.Glob("**/*.css").scan(path.join(dir, "dist")));
expect(cssFiles).toHaveLength(1);
const css = await Bun.file(path.join(dir, "dist", cssFiles[0])).text();
expect(css).toContain(":-webkit-full-screen");
expect(css).toContain(":fullscreen");
});
});
Loading