Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
41 changes: 35 additions & 6 deletions src/bundler/bundle_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4748,9 +4748,11 @@ pub mod bv2_impl {
msgs: vec![msg],
..Default::default()
};
let loader =
this.graph.input_files.items_loader()[load.source_index.get() as usize];
dev.handle_parse_task_failure(
crate::Error::Plugin,
load.bake_graph(),
failure_graph(loader, load.parse_task().known_target),
source.path.key_for_incremental_graph(),
&raw const temp_log,
this,
Expand Down Expand Up @@ -5513,6 +5515,7 @@ pub mod bv2_impl {

self.dynamic_import_entry_points = ArrayHashMap::new();
let mut html_files: ArrayHashMap<Index, ()> = ArrayHashMap::new();
let mut failed_css_imported_on_server: ArrayHashMap<Index, ()> = ArrayHashMap::new();

// Separate non-failing files into two lists: JS and CSS
let js_reachable_files: &[Index] = 'reachable_files: {
Expand Down Expand Up @@ -5618,6 +5621,12 @@ pub mod bv2_impl {
(*parts_col.add(record.source_index.get() as usize)).len()
} == 0
{
// Failed: not a CSS root, not a module dependency.
record.path.is_disabled = true;
Comment thread
robobun marked this conversation as resolved.
Resolved
if target != Target::Browser {
failed_css_imported_on_server
.put(record.source_index, ())?;
}
record.source_index = Index::INVALID;
continue;
}
Expand Down Expand Up @@ -5824,6 +5833,7 @@ pub mod bv2_impl {
chunks,
css_file_list: core::mem::take(&mut start.css_entry_points),
html_files,
failed_css_imported_on_server,
},
)
.map_err(|_| AllocError)
Expand Down Expand Up @@ -6504,7 +6514,10 @@ pub mod bv2_impl {
// SAFETY: log lives in DevServer/transpiler, disjoint from `self.graph`.
let log: &mut bun_ast::Log = unsafe {
&mut *std::ptr::from_mut::<bun_ast::Log>(
self.log_for_resolution_failures(source.path.text, bake_graph),
self.log_for_resolution_failures(
source.path.text,
failure_graph(loader, target),
),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
)
};

Expand Down Expand Up @@ -6532,7 +6545,8 @@ pub mod bv2_impl {
dev.track_resolution_failure(
source.path.text,
import_record.path.text,
ctx.target.bake_graph(), // use the source file target not the altered one
// The source file target, not the altered one.
failure_graph(loader, ctx.target),
loader,
)
.expect("oom");
Expand Down Expand Up @@ -6710,8 +6724,10 @@ pub mod bv2_impl {
// blocks an assertion failure because the DevServer
// reserves the HTML file's spot in IncrementalGraph for the
// route definition.
let log =
self.log_for_resolution_failures(source.path.text, bake_graph);
let log = self.log_for_resolution_failures(
source.path.text,
failure_graph(loader, target),
);
log.add_range_error_fmt(
Some(source),
import_record.range,
Expand Down Expand Up @@ -7590,10 +7606,12 @@ pub mod bv2_impl {
[err.source_index.get() as usize]
.path
.text;
let loader = this.graph.input_files.items_loader()
[err.source_index.get() as usize];
dev_server
.handle_parse_task_failure(
err.err,
err.target.bake_graph(),
failure_graph(loader, err.target),
abs_path,
&raw const err.log,
std::ptr::from_mut(this),
Expand Down Expand Up @@ -8003,6 +8021,17 @@ pub mod bv2_impl {
pub chunks: &'a mut [Chunk],
pub css_file_list: ArrayHashMap<Index, CssEntryPointMeta>,
pub html_files: ArrayHashMap<Index, ()>,
/// Stylesheets that failed to build and that a server file imports.
pub failed_css_imported_on_server: ArrayHashMap<Index, ()>,
}

/// CSS lives in the client graph, whichever target imported it.
pub(crate) fn failure_graph(loader: Loader, target: options::Target) -> bake::Graph {
if loader == Loader::Css {
bake::Graph::Client
} else {
target.bake_graph()
}
}

pub(crate) fn generate_unique_key() -> u64 {
Expand Down
11 changes: 11 additions & 0 deletions src/runtime/bake/DevServer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4079,6 +4079,17 @@ pub(super) fn finalize_bundle(
}
}

// A failed stylesheet gets a server-side entry too, so its route has an edge to it.
for index in result.failed_css_imported_on_server.keys() {
let key = ctx.sources[index.get() as usize]
.path
.key_for_incremental_graph();
if dev.client_graph.get_file_index(key).is_some() {
dev.server_graph
.insert_css_file_on_server(&mut ctx, *index, key)?;
Comment thread
robobun marked this conversation as resolved.
Resolved
}
}

for chunk in html_chunks_mut.iter_mut() {
let index = bun_ast::Index::init(chunk.entry_point.source_index());
let bundler::CompileResult::Html {
Expand Down
14 changes: 14 additions & 0 deletions src/runtime/bake/dev_server/incremental_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1154,6 +1154,20 @@ impl<const SIDE: bake::Side> IncrementalGraph<SIDE> {
if goal == TraceDependencyGoal::StopAtBoundary {
return Ok(());
}
} else if goal == TraceDependencyGoal::NoStop {
// A failure reaches the server-side importers. A rebuild does not: same asset URL.
let key = bun_ptr::RawSlice::new(
&*self.bundled_files.keys()[file_index.get() as usize],
);
// SAFETY: cross-graph sibling access; `server_graph` disjoint.
let server_graph = unsafe { &mut (*dev).server_graph };
if let Some(index) = server_graph.get_file_index(key.slice()) {
if server_graph.bundled_files.values()[index.get() as usize].kind
== FileKind::Css
{
server_graph.trace_dependencies(index, gts, goal, index)?;
}
}
Comment thread
robobun marked this conversation as resolved.
Resolved
}
}
}
Expand Down
158 changes: 157 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 @@ -705,3 +705,159 @@ function extractCssUrl(backgroundImage: string): string {
}
return url[2];
}

// A stylesheet that a framework route imports has a node in the server graph. A failure of the
// stylesheet has to reach the route through that node: the stylesheet has no importer in the
// client graph.
devTest("framework route with a stylesheet that fails to build", {
framework: minimalFramework,
files: {
"routes/index.ts": `
import "../one.css";
export default (req, meta) => Response.json(meta.styles);
`,
"one.css": `.one { color: red; }`,
},
async test(dev) {
const before: string[] = await dev.fetch("/").json();
expect(before).toHaveLength(1);
expect((await dev.fetch(before[0])).status).toBe(200);

// The route answers with the error page, like an html route whose stylesheet fails. The page
// lists the failure once: the rebuild of the route parses the stylesheet again, with the
// target of the route, and that must not add a second entry.
await dev.write("one.css", `.one { color }`, { errors: null });
{
await using c = await dev.client("/", { errors: ["one.css:1:14: error: Unexpected end of input"] });
}

await dev.write("one.css", `.one { color: #00f; }`);
const after: string[] = await dev.fetch("/").json();
expect(after).toEqual(before);
const stylesheet = await dev.fetch(after[0]);
expect(await stylesheet.text()).toContain("#00f");
expect(stylesheet.status).toBe(200);
},
});

devTest("framework route with a stylesheet whose import fails to build", {
framework: minimalFramework,
files: {
"routes/index.ts": `
import "../one.css";
export default (req, meta) => Response.json(meta.styles);
`,
"one.css": `@import "./two.css";\n.one { color: red; }`,
"two.css": `.two { color: red; }`,
},
async test(dev) {
const before: string[] = await dev.fetch("/").json();
expect(before).toHaveLength(1);
await dev.fetch(before[0]).expect.toContain(".two");

// The trace reaches the route through "one.css", not through the file that failed.
await dev.write("two.css", `.two { color }`, { errors: null });
const during = await dev.fetch("/");
expect(await during.text()).toContain("Build Failed");
expect(during.status).toBe(500);

await dev.write("two.css", `.two { color: #00f; }`);
const after: string[] = await dev.fetch("/").json();
expect(after).toEqual(before);
expect((await dev.fetch(after[0])).status).toBe(200);
},
});

devTest("framework route edited while its stylesheet fails to build", {
framework: minimalFramework,
files: {
"routes/index.ts": `
import "../one.css";
export default (req, meta) => Response.json(meta.styles);
`,
"one.css": `.one { color: red; }`,
},
async test(dev) {
const before: string[] = await dev.fetch("/").json();
expect(before).toHaveLength(1);

// The server bundle of the route must not depend on a module for the failed stylesheet.
// previously: error: Failed to load bundled module 'one.css'
await dev.write("one.css", `.one { color }`, { errors: null });
await dev.write(
"routes/index.ts",
`
import "../one.css";
export default (req, meta) => Response.json(["edited", ...meta.styles]);
`,
{ errors: null },
);
expect((await dev.fetch("/")).status).toBe(500);

await dev.write("one.css", `.one { color: #00f; }`);
expect(await dev.fetch("/").json()).toEqual(["edited", ...before]);
},
});

devTest("framework route whose stylesheet fails to build at the first request", {
framework: minimalFramework,
files: {
"routes/index.ts": `
import "../one.css";
export default (req, meta) => Response.json(meta.styles);
`,
"one.css": `.one { color }`,
},
async test(dev) {
const error = "one.css:1:14: error: Unexpected end of input";
{
await using c = await dev.client("/", { errors: [error] });
}

// The stylesheet never built, so the route learns of it only through the server graph.
await dev.write("one.css", `.one { color: red; }`);
const styles: string[] = await dev.fetch("/").json();
expect(styles).toHaveLength(1);
await dev.fetch(styles[0]).expect.toContain("red");

await dev.write("one.css", `.one { color }`, { errors: null });
{
await using c = await dev.client("/", { errors: [error] });
}

await dev.write("one.css", `.one { color: #00f; }`);
expect(await dev.fetch("/").json()).toEqual(styles);
await dev.fetch(styles[0]).expect.toContain("#00f");
},
});

devTest("framework route with a stylesheet whose import does not resolve", {
framework: minimalFramework,
files: {
"routes/index.ts": `
import "../one.css";
export default (req, meta) => Response.json(meta.styles);
`,
"one.css": `.one { color: red; }`,
},
async test(dev) {
const before: string[] = await dev.fetch("/").json();
expect(before).toHaveLength(1);

// A resolution failure of the stylesheet is reported like a syntax error: once, and it clears.
const error = 'one.css:1:1: error: Could not resolve: "./missing.css"';
await dev.write("one.css", `@import "./missing.css";\n.one { color: red; }`, { errors: null });
{
await using c = await dev.client("/", { errors: [error] });
}

await dev.write("one.css", `.one { color: #00f; }`);
expect(await dev.fetch("/").json()).toEqual(before);
await dev.fetch(before[0]).expect.toContain("#00f");

await dev.write("one.css", `@import "./missing.css";\n.one { color: red; }`, { errors: null });
{
await using c = await dev.client("/", { errors: [error] });
}
},
});
Loading