diff --git a/src/bundler/Chunk.rs b/src/bundler/Chunk.rs index 8f0e8acd8a62..297edc1c8ffb 100644 --- a/src/bundler/Chunk.rs +++ b/src/bundler/Chunk.rs @@ -272,6 +272,24 @@ impl Chunk { self.entry_point.is_entry_point() } + /// Kind used to name and classify the chunk; a stylesheet's JS chunk is always `DynamicImport`. + 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. " &'static [u8] { diff --git a/src/bundler/LinkerContext.rs b/src/bundler/LinkerContext.rs index b42db03a24ed..a41ca72a711b 100644 --- a/src/bundler/LinkerContext.rs +++ b/src/bundler/LinkerContext.rs @@ -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; } } diff --git a/src/bundler/LinkerGraph.rs b/src/bundler/LinkerGraph.rs index bfb5fd0871f3..dba9995bc8fe 100644 --- a/src/bundler/LinkerGraph.rs +++ b/src/bundler/LinkerGraph.rs @@ -212,6 +212,9 @@ pub struct LinkerGraph<'a> { pub(crate) is_scb_bitset: BitSet, + /// Every `import()` target, including files that are also user-specified entry points. + 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 @@ -225,9 +228,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. // - `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 @@ -275,6 +278,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(), } } @@ -625,6 +629,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()) }; @@ -681,6 +686,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 @@ -978,7 +984,7 @@ 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 `dynamically_imported_files`). pub entry_point_kind: EntryPoint::Kind, /// If "entry_point_kind" is not ".none", this is the index of the diff --git a/src/bundler/linker_context/computeChunks.rs b/src/bundler/linker_context/computeChunks.rs index 8711e692f0ce..ea8123c6b980 100644 --- a/src/bundler/linker_context/computeChunks.rs +++ b/src/bundler/linker_context/computeChunks.rs @@ -188,8 +188,10 @@ 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 + if !this + .graph + .dynamically_imported_files + .is_set(source_index as usize) { continue; } @@ -567,7 +569,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(); @@ -597,8 +598,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 diff --git a/src/bundler/linker_context/generateChunksInParallel.rs b/src/bundler/linker_context/generateChunksInParallel.rs index fd5175c2aba4..52845ca1f7b2 100644 --- a/src/bundler/linker_context/generateChunksInParallel.rs +++ b/src/bundler/linker_context/generateChunksInParallel.rs @@ -436,8 +436,6 @@ pub(crate) fn generate_chunks_in_parallel( 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() @@ -446,9 +444,7 @@ pub(crate) fn generate_chunks_in_parallel( 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); @@ -1219,11 +1215,8 @@ pub(crate) fn generate_chunks_in_parallel( 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 = diff --git a/src/bundler/linker_context/writeOutputFilesToDisk.rs b/src/bundler/linker_context/writeOutputFilesToDisk.rs index 4e3d991dde76..19a0cccdf880 100644 --- a/src/bundler/linker_context/writeOutputFilesToDisk.rs +++ b/src/bundler/linker_context/writeOutputFilesToDisk.rs @@ -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 { diff --git a/test/bundler/bundler_splitting.test.ts b/test/bundler/bundler_splitting.test.ts index 14c66a1bd402..34aedcdca1fd 100644 --- a/test/bundler/bundler_splitting.test.ts +++ b/test/bundler/bundler_splitting.test.ts @@ -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 = { @@ -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: { @@ -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'); @@ -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", @@ -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')`, @@ -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": `