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
2 changes: 2 additions & 0 deletions src/bundler/bundle_v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2701,6 +2701,7 @@ pub mod bv2_impl {
dev.track_resolution_failure(
&import_record.source_file,
&import_record.specifier,
import_record.kind,
target.bake_graph(),
self.graph.input_files.items_loader()
[import_record.importer_source_index as usize],
Expand Down Expand Up @@ -6796,6 +6797,7 @@ pub mod bv2_impl {
dev.track_resolution_failure(
source.path.text,
import_record.path.text,
import_record.kind,
ctx.target.bake_graph(), // use the source file target not the altered one
loader,
)
Expand Down
2 changes: 1 addition & 1 deletion src/bundler/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ bun_dispatch::link_interface! {
fn finalize_bundle(bv2: *mut bundle_v2::BundleV2<'_>, result: *mut bundle_v2::DevServerOutput<'_>) -> Result<(), crate::Error>;
fn handle_parse_task_failure(err: crate::Error, graph: bake_types::Graph, abs_path: &[u8], log: *const bun_ast::Log, bv2: *mut bundle_v2::BundleV2<'_>) -> Result<(), crate::Error>;
fn put_or_overwrite_asset(path: *const (), contents: &[u8], content_hash: u64) -> Result<(), crate::Error>;
fn track_resolution_failure(import_source: &[u8], specifier: &[u8], renderer: bake_types::Graph, loader: bun_ast::Loader) -> Result<(), crate::Error>;
fn track_resolution_failure(import_source: &[u8], specifier: &[u8], kind: bun_ast::ImportKind, renderer: bake_types::Graph, loader: bun_ast::Loader) -> Result<(), crate::Error>;
fn is_file_cached(abs_path: &[u8], side: bake_types::Graph) -> Option<bake_types::CacheEntry>;
fn asset_hash(abs_path: &[u8]) -> Option<u64>;
fn current_bundle_start_data() -> *mut ();
Expand Down
1 change: 1 addition & 0 deletions src/runtime/bake/DevServer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5052,6 +5052,7 @@ impl DevServer {
self.directory_watchers.track_resolution_failure(
abs_path,
paths::basename(abs_path),
bun_ast::ImportKind::Stmt,
Comment thread
robobun marked this conversation as resolved.
Resolved
bake::Graph::Client,
Loader::Html,
)?;
Expand Down
62 changes: 44 additions & 18 deletions src/runtime/bake/dev_server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -442,25 +442,40 @@ impl HotReloadEvent {
while let Some(index) = it {
// Note: reshaped for borrowck — re-index per iteration instead of
// holding `dep` ref across resolver call + appendFile + freeDependencyIndex.
let (source_file_path, specifier, next) = {
let (source_file_path, specifier, import_kind, next) = {
let dep = &dev.directory_watchers.dependencies[index as usize];
(dep.source_file_path, &raw const *dep.specifier, dep.next)
(
dep.source_file_path,
&raw const *dep.specifier,
dep.import_kind,
dep.next,
)
};
it = next;

// `specifier` points into the dep's owned `Box<[u8]>`, which is
// not mutated until after `resolve` returns.
// SAFETY: see `Dep` doc — neither slice is mutated mid-resolve.
let resolved = unsafe { dev.server_transpiler.assume_init_mut() }
.resolver
.resolve(
bun_paths::resolve_path::dirname::<bun_paths::platform::Auto>(
source_file_path.slice(),
),
unsafe { &*specifier },
bun_ast::ImportKind::Stmt,
)
.is_ok();
let resolved = {
// SAFETY: the dep owns this slice, and nothing frees it in this block.
let specifier: &[u8] = unsafe { &*specifier };
// SAFETY: server_transpiler is initialized before any event fires.
let resolved = unsafe { dev.server_transpiler.assume_init_mut() }
.resolver
.resolve(
bun_paths::resolve_path::dirname::<bun_paths::platform::Auto>(
source_file_path.slice(),
),
specifier,
import_kind,
)
.is_ok();
bun_core::scoped_log!(
DevServer,
"DirectoryWatchStore retry({}, {}) = {}",
bun_core::fmt::quote(specifier),
bstr::BStr::new(import_kind.label()),
resolved,
);
resolved
};

if resolved {
// this resolution result is not preserved as passing it
Expand Down Expand Up @@ -1095,13 +1110,16 @@ pub(crate) mod directory_watch_store {
pub(crate) source_file_path: bun_ptr::RawSlice<u8>,
/// The specifier that failed. Allocated memory.
pub(crate) specifier: Box<[u8]>,
/// The import kind the bundler resolved `specifier` with. The retry resolves with it too.
pub(crate) import_kind: bun_ast::ImportKind,
}
impl Default for Dep {
fn default() -> Self {
Dep {
next: None,
source_file_path: bun_ptr::RawSlice::EMPTY,
specifier: Box::default(),
import_kind: bun_ast::ImportKind::Stmt,
}
}
}
Expand Down Expand Up @@ -1140,10 +1158,10 @@ bun_bundler::link_impl_DevServerHandle! {
let blob = crate::webcore::blob::Any::from_owned_slice(contents.to_vec());
(*this).put_or_overwrite_asset(path, blob, content_hash).map_err(Into::into)
},
track_resolution_failure(import_source, specifier, renderer, loader) => {
track_resolution_failure(import_source, specifier, kind, renderer, loader) => {
(*this)
.directory_watchers
.track_resolution_failure(import_source, specifier, renderer, loader)
.track_resolution_failure(import_source, specifier, kind, renderer, loader)
.map_err(Into::into)
},
is_file_cached(abs_path, side) => {
Expand Down Expand Up @@ -1219,6 +1237,7 @@ impl DirectoryWatchStore {
&mut self,
import_source: &[u8],
specifier: &[u8],
kind: bun_ast::ImportKind,
renderer: Graph,
loader: bun_ast::Loader,
) -> Result<(), bun_alloc::AllocError> {
Expand All @@ -1230,6 +1249,10 @@ impl DirectoryWatchStore {
if !bun_paths::is_absolute(import_source) {
return Ok(());
}
// With the "./" prefix the resolver removes the query of "?v=2" and resolves the directory.
if kind.is_from_css() && specifier[0] == b'?' {
return Ok(());
}

match loader {
Loader::Tsx | Loader::Ts | Loader::Jsx | Loader::Js => {
Expand Down Expand Up @@ -1283,7 +1306,7 @@ impl DirectoryWatchStore {
}
};

match self.insert(dir, owned_file_path, specifier) {
match self.insert(dir, owned_file_path, specifier, kind) {
Ok(()) => Ok(()),
Err(DirectoryWatchInsertError::Ignore) => Ok(()), // ignoring watch errors.
Err(DirectoryWatchInsertError::OutOfMemory) => Err(bun_alloc::AllocError),
Expand All @@ -1297,6 +1320,7 @@ impl DirectoryWatchStore {
dir_name_to_watch: &[u8],
file_path: bun_ptr::RawSlice<u8>,
specifier: &[u8],
kind: bun_ast::ImportKind,
) -> Result<(), DirectoryWatchInsertError> {
debug_assert!(!specifier.is_empty());
// TODO: watch the parent dir too.
Expand Down Expand Up @@ -1341,6 +1365,7 @@ impl DirectoryWatchStore {
next: prev_first,
source_file_path: file_path,
specifier: specifier_cloned,
import_kind: kind,
});
self.watches.values_mut()[gop_index].first_dep = dep;
return Ok(());
Expand Down Expand Up @@ -1434,6 +1459,7 @@ impl DirectoryWatchStore {
next: None,
source_file_path: file_path,
specifier: specifier_cloned,
import_kind: kind,
});
self.watches.values_mut()[gop_index] = directory_watch_store::Entry {
dir: fd,
Expand Down
190 changes: 189 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 @@ -646,6 +646,194 @@ devTest("css import before create", {
await dev.fetch("/").expect.toContain("HELLO");
},
});
// The resolver has rules that only apply to a CSS import kind: it removes a
// ?query or #fragment, and it tries "./other" only as "./other.css". The retry
// for a missing file uses the import kind that the bundler used.
const retriedWithCssKind: {
name: string;
/** The stylesheet that index.html links. */
stylesheet?: string;
css: string;
error: string;
create: [file: string, contents: string];
/** A file that a JS import of the same specifier resolves to. */
decoy?: [file: string, contents: string];
plugin?: boolean;
/** Text in the stylesheet that the server sends. The test client drops `@layer` and hashes class names. */
served?: string[];
}[] = [
{
name: "url with query",
css: `body { background-image: url(./bun.png?v=2); }`,
error: 'styles.css:1:26: error: Could not resolve: "./bun.png?v=2"',
create: ["bun.png", imageFixtures.bun],
},
{
name: "url with fragment",
css: `body { background-image: url(./bun.png#icon); }`,
error: 'styles.css:1:26: error: Could not resolve: "./bun.png#icon"',
create: ["bun.png", imageFixtures.bun],
},
{
name: "bare url with empty query and fragment",
css: `body { background-image: url(bun.png?#iefix); }`,
error: 'styles.css:1:26: error: Could not resolve: "bun.png?#iefix". Maybe you need to "bun install"?',
create: ["bun.png", imageFixtures.bun],
},
{
name: "url with query after a plugin declines it",
css: `body { background-image: url(./bun.png?v=2); }`,
error: 'styles.css:1:26: error: Could not resolve: "./bun.png?v=2"',
create: ["bun.png", imageFixtures.bun],
plugin: true,
},
{
name: "@import with query and fragment",
css: `@import "./other.css?v=3#c";`,
error: 'styles.css:1:1: error: Could not resolve: "./other.css?v=3#c"',
create: ["other.css", `body { color: red; }`],
},
{
name: "@import with fragment",
css: `@import "./other.css#frag";`,
error: 'styles.css:1:1: error: Could not resolve: "./other.css#frag"',
create: ["other.css", `body { color: red; }`],
},
{
name: "conditional @import with query",
css: `@import "./other.css?v=2" layer(base);`,
error: 'styles.css:1:1: error: Could not resolve: "./other.css?v=2"',
create: ["other.css", `body { color: red; }`],
served: ["@layer base", "color: red"],
},
{
name: "@import without extension",
css: `@import "./other";`,
error: 'styles.css:1:1: error: Could not resolve: "./other"',
create: ["other.css", `body { color: red; }`],
decoy: ["other.ts", `export {};`],
},
{
name: "composes with query",
stylesheet: "styles.module.css",
css: `.a { composes: b from "./other.module.css?v=2"; }`,
error: 'styles.module.css:1:22: error: Could not resolve: "./other.module.css?v=2"',
create: ["other.module.css", `.b { color: red; }`],
served: ["color: red"],
},
{
name: "bare composes with query",
stylesheet: "styles.module.css",
css: `.a { composes: b from "other.module.css?v=2"; }`,
error: 'styles.module.css:1:22: error: Could not resolve: "other.module.css?v=2". Maybe you need to "bun install"?',
create: ["other.module.css", `.b { color: red; }`],
served: ["color: red"],
},
];
for (const { name, stylesheet = "styles.css", css, error, create, decoy, plugin, served } of retriedWithCssKind) {
devTest(`css ${name} before create`, {
files: {
"index.html": emptyHtmlFile({
styles: [stylesheet],
body: `
<div>HELLO</div>
`,
}),
[stylesheet]: css,
...(decoy && { [decoy[0]]: decoy[1] }),
...(plugin && {
"bunfig.toml": `
[serve.static]
plugins = ["./decline-plugin.ts"]
`,
"decline-plugin.ts": `
export default {
name: "decline-plugin",
setup(build) {
build.onResolve({ filter: /bun\\.png/ }, () => undefined);
},
};
`,
}),
},
async test(dev) {
await using c = await dev.client("/", { errors: [error] });
// A change in the watched directory that does not create the file sends nothing.
await c.expectNoWebSocketActivity(async () => {
await dev.write("unrelated.txt", "x", { errors: null });
await dev.delete("unrelated.txt", { errors: null });
});
await c.expectReload(async () => {
await dev.write(create[0], create[1]);
});
if (served) {
const href = (await (await dev.fetch("/")).text()).match(/href="(\/_bun\/asset\/[^"]+\.css)"/)?.[1];
assert(href);
const text = await (await dev.fetch(href)).text();
for (const expected of served) expect(text).toContain(expected);
} else if (create[0] === "bun.png") {
const backgroundImage = await c.style("body").backgroundImage;
assert(backgroundImage);
// The printed url() keeps a fragment. The page does not send it when it loads the image.
await dev.fetch(extractCssUrl(backgroundImage).replace(/#.*$/, "")).expectFile(imageFixtures.bun);
} else {
await c.style("body").color.expect.toBe("red");
}
await dev.fetch("/").expect.toContain("HELLO");
},
});
}
devTest("css retry does not rebuild on an unrelated change", {
files: {
"index.html": emptyHtmlFile({
styles: ["query-only.css", "builtin.css", "styles.css"],
body: `
<div>HELLO</div>
`,
}),
// "./" resolves to this file, so "./?v=2" without its query would resolve.
"index.css": `/* index */`,
"query-only.css": `body { background-image: url(?v=2); }`,
// The resolver of the server resolves "bun". The retry must ask for "./bun".
"builtin.css": `div { background-image: url(bun); }`,
"styles.css": `p { background-image: url(./bun.png?v=2); }`,
},
async test(dev) {
const queryOnly = 'query-only.css:1:26: error: Could not resolve: "?v=2". Maybe you need to "bun install"?';
const builtin = 'builtin.css:1:25: error: Browser build cannot url() Bun builtin: "bun"';
const missing = 'styles.css:1:23: error: Could not resolve: "./bun.png?v=2"';
await using c = await dev.client("/", { errors: [builtin, queryOnly, missing] });
await c.expectNoWebSocketActivity(async () => {
await dev.write("unrelated.txt", "x", { errors: null });
await dev.delete("unrelated.txt", { errors: null });
});
await dev.write("bun.png", imageFixtures.bun, { errors: [builtin, queryOnly] });
},
});
devTest("framework route stylesheet url with query before create", {
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);

await dev.write("one.css", `.one { background-image: url(./bun.png?v=2); }`, { errors: null });
await dev.write("bun.png", imageFixtures.bun, { errors: null });

const after: string[] = await dev.fetch("/").json();
expect(after).toHaveLength(1);
const stylesheet = await dev.fetch(after[0]);
expect(await stylesheet.text()).toContain("background-image");
expect(stylesheet.status).toBe(200);
},
});
devTest("css import before create project relative", {
files: {
"html/index.html": emptyHtmlFile({
Expand Down
Loading
Loading