Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
22 changes: 22 additions & 0 deletions src/bundler/Chunk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,28 @@ impl Chunk {
self.entry_point.is_entry_point()
}

/// The entry point kind this chunk is named and classified by.
///
/// A stylesheet only gets a JS chunk because it is `import()`ed (see `compute_chunks`), so
/// that chunk is a dynamic import chunk even when the user also passed the stylesheet as an
/// entry point: the entry point name and kind belong to its CSS chunk.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub(crate) fn entry_point_kind(
&self,
linker_graph: &LinkerGraph<'_>,
) -> crate::entry_point::Kind {
if !self.entry_point.is_entry_point() {
return crate::entry_point::Kind::None;
}
let source_index = self.entry_point.source_index() as usize;
if matches!(self.content, Content::Javascript(_))
&& linker_graph.ast.items_css()[source_index].is_some()
{
debug_assert!(linker_graph.dynamically_imported_files.is_set(source_index));
return crate::entry_point::Kind::DynamicImport;
}
linker_graph.files.items_entry_point_kind()[source_index]
}

/// Returns the HTML closing tag that must be escaped when this chunk's content
/// is inlined into a standalone HTML file (e.g. "</script" for JS, "</style" for CSS).
pub(crate) fn closing_tag_for_content(&self) -> &'static [u8] {
Expand Down
6 changes: 5 additions & 1 deletion src/bundler/LinkerContext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2738,7 +2738,11 @@ impl<'a> LinkerContext<'a> {
}

// An import()ed stylesheet has its own JS chunk; walk its parts like an entry point's.
if ctx.entry_point_kinds[source_index as usize] != EntryPoint::Kind::DynamicImport {
if !self
.graph
.dynamically_imported_files
.is_set(source_index as usize)
{
return;
}
}
Expand Down
17 changes: 13 additions & 4 deletions src/bundler/LinkerGraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,11 @@ pub struct LinkerGraph<'a> {

pub(crate) is_scb_bitset: BitSet,

/// Every target of an `import()` (only populated with code splitting). Unlike
/// `File.entry_point_kind`, this also covers files the user passed as entry points, which
/// stay `UserSpecified`; a stylesheet in this set needs a JS chunk for the `import()` to load.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub(crate) dynamically_imported_files: BitSet,

/// This is for cross-module inlining of detected inlinable constants
// const_values: bun_ast::Ast::ConstValuesMap,
/// This is for cross-module inlining of TypeScript enum constants
Expand All @@ -225,9 +230,9 @@ pub struct LinkerGraph<'a> {
// - `bump: *const Arena` is a backref into `BundleV2`; the arena is frozen
// (no new allocations) for the duration of any worker-pool fan-out that
// holds `&LinkerGraph`.
// - `files_live` / `parts_live` / `is_scb_bitset` / `reachable_files` /
// `stable_source_indices` / `code_splitting` / `ts_enums` are populated
// before fan-out and only read by workers.
// - `files_live` / `parts_live` / `is_scb_bitset` / `dynamically_imported_files` /
// `reachable_files` / `stable_source_indices` / `code_splitting` / `ts_enums` are
// populated before fan-out and only read by workers.
Comment thread
robobun marked this conversation as resolved.
// - `ast` / `meta` / `files` columns that workers mutate are split out via
// `split_mut()` into disjoint `&mut [_]` *before* the pool runs (see
// `compute_cross_chunk_dependencies`); workers never reach those columns
Expand Down Expand Up @@ -275,6 +280,7 @@ impl Default for LinkerGraph<'_> {
reachable_files: Vec::new(),
stable_source_indices: Vec::new(),
is_scb_bitset: BitSet::default(),
dynamically_imported_files: BitSet::default(),
ts_enums: bun_ast::ast_result::TsEnumsMap::default(),
}
}
Expand Down Expand Up @@ -625,6 +631,7 @@ impl<'a> LinkerGraph<'a> {
self.files.set_capacity(sources.len())?;
self.files.zero();
self.files_live = BitSet::init_empty(sources.len())?;
self.dynamically_imported_files = BitSet::init_empty(sources.len())?;
// SAFETY: capacity reserved above; columns zeroed by `zero()`.
unsafe { self.files.set_len(sources.len()) };

Expand Down Expand Up @@ -681,6 +688,7 @@ impl<'a> LinkerGraph<'a> {

for &id in dynamic_import_entry_points {
debug_assert!(self.code_splitting); // this should never be a thing without code splitting
self.dynamically_imported_files.set(id as usize);

if entry_point_kinds[id as usize] != entry_point::Kind::None {
// You could dynamic import a file that is already an entry point
Expand Down Expand Up @@ -978,7 +986,8 @@ pub struct File {
/// This file is an entry point if and only if this is not ".none".
/// Note that dynamically-imported files are allowed to also be specified by
/// the user as top-level entry points, so some dynamically-imported files
/// may be ".user_specified" instead of ".dynamic_import".
/// may be ".user_specified" instead of ".dynamic_import"; see
/// `LinkerGraph.dynamically_imported_files` for the complete set.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub entry_point_kind: EntryPoint::Kind,

/// If "entry_point_kind" is not ".none", this is the index of the
Expand Down
13 changes: 7 additions & 6 deletions src/bundler/linker_context/computeChunks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,12 @@ pub(crate) fn compute_chunks(
};
}

// An import()ed stylesheet is loaded as a JS module, so it also needs a JS chunk.
if this.graph.files.items_entry_point_kind()[source_index as usize]
!= crate::EntryPoint::Kind::DynamicImport
// An import()ed stylesheet is loaded as a JS module, so it also needs a JS chunk
// (whether or not the user also passed the stylesheet as an entry point).
Comment thread
robobun marked this conversation as resolved.
Outdated
if !this
.graph
.dynamically_imported_files
.is_set(source_index as usize)
{
continue;
}
Expand Down Expand Up @@ -567,7 +570,6 @@ pub(crate) fn compute_chunks(
// Derived from `this_ptr` (raw) so it does not reborrow `*this` here — the column
// slices below hold disjoint immutable borrows into `this.graph`.
let bv2: &mut BundleV2 = unsafe { &mut *LinkerContext::bundle_v2_ptr(this_ptr) };
let kinds = this.graph.files.items_entry_point_kind();
let output_paths = this.graph.entry_points.items_output_path();
// re-borrow after `find_all_imported_parts_in_js_order` released `&mut this`.
let ast_targets = this.graph.ast.items_target();
Expand Down Expand Up @@ -597,8 +599,7 @@ pub(crate) fn compute_chunks(

if chunk.entry_point.is_entry_point()
&& (matches!(chunk.content, chunk::Content::Html)
|| (kinds[chunk.entry_point.source_index() as usize]
== crate::EntryPoint::Kind::UserSpecified
|| (chunk.entry_point_kind(&this.graph) == crate::EntryPoint::Kind::UserSpecified
&& !chunk.flags.contains(chunk::Flags::HAS_HTML_CHUNK)))
{
// Use fileWithTarget template if there are HTML imports and user hasn't manually set naming
Expand Down
11 changes: 2 additions & 9 deletions src/bundler/linker_context/generateChunksInParallel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,8 +436,6 @@ pub(crate) fn generate_chunks_in_parallel<const IS_DEV_SERVER: bool>(

writeln!(&mut msg, "Multiple files share the same output path")?;

let kinds = c.graph.files.items_entry_point_kind();

for (key, dup) in duplicates_map
.keys()
.iter()
Expand All @@ -446,9 +444,7 @@ pub(crate) fn generate_chunks_in_parallel<const IS_DEV_SERVER: bool>(
writeln!(&mut msg, " {}:", bstr::BStr::new(key))?;
for chunk in dup.sources.iter() {
if chunk.entry_point.is_entry_point() {
if kinds[chunk.entry_point.source_index() as usize]
== EntryPoint::Kind::UserSpecified
{
if chunk.entry_point_kind(&c.graph) == EntryPoint::Kind::UserSpecified {
entry_naming = Some(&chunk.template.data);
} else {
chunk_naming = Some(&chunk.template.data);
Expand Down Expand Up @@ -1219,11 +1215,8 @@ pub(crate) fn generate_chunks_in_parallel<const IS_DEV_SERVER: bool>(

let output_kind = if matches!(chunk.content, crate::chunk::Content::Css(_)) {
options::OutputKind::Asset
} else if chunk.entry_point.is_entry_point() {
c.graph.files.items_entry_point_kind()[chunk.entry_point.source_index() as usize]
.output_kind()
} else {
options::OutputKind::Chunk
chunk.entry_point_kind(&c.graph).output_kind()
};

let chunk_index =
Expand Down
5 changes: 1 addition & 4 deletions src/bundler/linker_context/writeOutputFilesToDisk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,11 +540,8 @@ pub(crate) fn write_output_files_to_disk(

let output_kind = if matches!(chunk.content, Content::Css(_)) {
options::OutputKind::Asset
} else if chunk.entry_point.is_entry_point() {
c.graph.files.items_entry_point_kind()[chunk.entry_point.source_index() as usize]
.output_kind()
} else {
options::OutputKind::Chunk
chunk.entry_point_kind(&c.graph).output_kind()
};

let chunk_index = output_files.insert_for_chunk(OutputFile::init(OutputFileInit {
Expand Down
126 changes: 118 additions & 8 deletions test/bundler/bundler_splitting.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import type { BuildArtifact } from "bun";
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, isASAN, isDebug, tempDir } from "harness";
import { readdirSync, readFileSync } from "node:fs";
import { join } from "node:path";
import { basename, join } from "node:path";
import { itBundled } from "./expectBundled";

const env = {
Expand All @@ -10,6 +11,12 @@ const env = {
BUN_FEATURE_FLAG_DISABLE_ASYNC_TRANSPILER: "1",
};

function outputKinds(outputs: BuildArtifact[]) {
return outputs
.map(output => ({ file: basename(output.path), kind: output.kind }))
.sort((a, b) => (a.file < b.file ? -1 : a.file > b.file ? 1 : 0));
}

describe("bundler", () => {
itBundled("splitting/DynamicImportCSSFile", {
files: {
Expand Down Expand Up @@ -414,12 +421,12 @@ describe("bundler", () => {
],
});

// A stylesheet that is both a user-specified entry point and import()ed only
// gets its CSS output (entry point kinds are exclusive), so the import() is
// still rewritten to the .css output (or, with --css-chunking, to whatever
// chunk is at index 0) instead of a JS chunk exporting the class map.
// A stylesheet that is both a user-specified entry point and import()ed keeps
// its entry-point-named CSS output and additionally gets the same hashed JS
// chunk an import()-only stylesheet gets, which is what the import() resolves
// to. Previously the stylesheet's entry point kind alone decided whether the
// JS chunk exists, so the import() was rewritten to the .css output.
itBundled("splitting/DynamicImportOfUserSpecifiedCSSEntryPoint", {
todo: true,
files: {
"/entry.js": `
const mod = await import('./styles.module.css');
Expand All @@ -430,8 +437,30 @@ describe("bundler", () => {
entryPoints: ["/entry.js", "/styles.module.css"],
splitting: true,
outdir: "/out",
metafile: true,
onAfterApiBundle(build) {
expect(outputKinds(build.outputs)).toEqual([
{ file: "entry.css", kind: "asset" },
{ file: "entry.js", kind: "entry-point" },
{ file: expect.stringMatching(/^styles\.module-[a-z0-9]+\.js$/), kind: "chunk" },
{ file: "styles.module.css", kind: "asset" },
]);
},
onAfterBundle(api) {
expect(api.readFile("/out/entry.js")).toMatch(/import\("\.\/styles\.module[^"]*\.js"\)/);
expect(api.readFile("/out/entry.js")).toMatch(/import\("\.\/styles\.module-[a-z0-9]+\.js"\)/);
expect(api.readFile("/out/styles.module.css")).toContain("color: red");

const { outputs } = JSON.parse(api.readFile("/metafile.json"));
const stylesheetOutputs = Object.keys(outputs).filter(file => outputs[file].entryPoint === "styles.module.css");
expect(stylesheetOutputs.map(file => basename(file)).sort()).toEqual([
expect.stringMatching(/^styles\.module-[a-z0-9]+\.js$/),
"styles.module.css",
]);
const chunk = stylesheetOutputs.find(file => file.endsWith(".js"))!;
expect(outputs[chunk]).toMatchObject({
exports: ["default", "foo"],
cssBundle: stylesheetOutputs.find(file => file.endsWith(".css")),
});
},
run: {
file: "/out/entry.js",
Expand All @@ -440,7 +469,34 @@ describe("bundler", () => {
},
});

// A stylesheet the user passes as an entry point still only produces CSS.
// The same build with the stylesheet listed first (so it has entry point id 0)
// and kept in memory, which classifies the outputs on a separate code path.
test("splitting/DynamicImportOfUserSpecifiedCSSEntryPointInMemory", async () => {
using dir = tempDir("splitting-user-css-entry-in-memory", {
"entry.js": `import('./styles.module.css').then(mod => console.log(mod.foo));`,
"styles.module.css": `.foo { color: red; }`,
});
const root = String(dir);

const build = await Bun.build({
entrypoints: [join(root, "styles.module.css"), join(root, "entry.js")],
splitting: true,
naming: { chunk: "[name]-[hash].[ext]" },
});
expect(build.logs).toEqual([]);
expect(outputKinds(build.outputs)).toEqual([
{ file: "entry.css", kind: "asset" },
{ file: "entry.js", kind: "entry-point" },
{ file: expect.stringMatching(/^styles\.module-[a-z0-9]+\.js$/), kind: "chunk" },
{ file: "styles.module.css", kind: "asset" },
]);

const entry = build.outputs.find(output => output.kind === "entry-point")!;
expect(await entry.text()).toMatch(/import\("\.\/styles\.module-[a-z0-9]+\.js"\)/);
});

// A stylesheet the user passes as an entry point without import()ing it
// anywhere still only produces CSS.
itBundled("splitting/UserSpecifiedCSSEntryPointHasNoJSChunk", {
files: {
"/entry.js": `console.log('entry')`,
Expand Down Expand Up @@ -504,6 +560,60 @@ describe("bundler", () => {
expect(runExit).toBe(0);
});

// The user-specified variant of the above: the stylesheet's own CSS chunk is
// the one shared with the importing entry point (named after whichever entry
// point comes first), and the import() must point at the stylesheet's JS chunk.
// It used to be rewritten to the shared CSS output when the stylesheet came
// first, and to the importing entry point itself (chunk 0) when it came second.
const stylesheetChunk = expect.stringMatching(/^styles\.module-[a-z0-9]+\.js$/);
test.each([
{ entryPoints: ["./entry.js", "./styles.module.css"], outputs: ["entry.css", "entry.js", stylesheetChunk] },
{ entryPoints: ["./styles.module.css", "./entry.js"], outputs: ["entry.js", stylesheetChunk, "styles.module.css"] },
])(
"splitting/DynamicImportOfUserSpecifiedCSSEntryPointWithCSSChunking $entryPoints",
async ({ entryPoints, outputs }) => {
using dir = tempDir("splitting-css-chunking-user-css-entry", {
"entry.js": `
import('./styles.module.css').then(mod => console.log(Object.keys(mod).join(','), /^foo_/.test(mod.foo)));
`,
"styles.module.css": `.foo { color: red; }`,
});
const root = String(dir);

await using build = Bun.spawn({
cmd: [bunExe(), "build", "--splitting", "--css-chunking", "--outdir", "out", ...entryPoints],
env: bunEnv,
cwd: root,
stdout: "pipe",
stderr: "pipe",
});
const [buildOut, buildErr, buildExit] = await Promise.all([
build.stdout.text(),
build.stderr.text(),
build.exited,
]);
expect(buildErr).toBe("");
expect(buildOut).toMatch(/styles\.module-[a-z0-9]+\.js\s+\S+ bytes\s+\(chunk\)/);
expect(buildExit).toBe(0);

expect(readdirSync(join(root, "out")).sort()).toEqual(outputs);
expect(readFileSync(join(root, "out", "entry.js"), "utf8")).toMatch(
/import\("\.\/styles\.module-[a-z0-9]+\.js"\)/,
);

await using run = Bun.spawn({
cmd: [bunExe(), join(root, "out", "entry.js")],
env,
stdout: "pipe",
stderr: "pipe",
});
const [runOut, runErr, runExit] = await Promise.all([run.stdout.text(), run.stderr.text(), run.exited]);
expect(runErr).toBe("");
expect(runOut).toBe("default,foo true\n");
expect(runExit).toBe(0);
},
);

itBundled("splitting/CircularDynamicImportsWithCSS", {
files: {
"/entry.js": `
Expand Down
Loading