bundler: drop import attributes from import() calls that code splitting points at a chunk

src/ast/import_record.rs

@@ -72,6 +72,13 @@ bitflags::bitflags! {
 
         const WAS_ORIGINALLY_REQUIRE = 1 << 9;
 
+        /// Code splitting repointed this `import()` from the file it named at
+        /// the JavaScript chunk built for that file. The options object written
+        /// on the `import()` described the original file, so the printer
+        /// leaves it out. Not set when the chunk is the file's own CSS or HTML
+        /// output, which the options still describe.
+        const POINTS_TO_JS_CHUNK = 1 << 10;
+
         /// If true, this import can be removed if it's unused
         const IS_EXTERNAL_WITHOUT_SIDE_EFFECTS = 1 << 11;
 

src/bundler/linker_context/computeCrossChunkDependencies.rs

@@ -1,6 +1,7 @@
 use crate::bun_renamer as renamer;
 use crate::mal_prelude::*;
 use bun_alloc::ArenaVecExt as _;
+use bun_ast::ImportRecordFlags;
 use bun_collections::{ArrayHashMap, VecExt};
 
 use crate::LinkerContext;
@@ -86,9 +87,9 @@ pub(crate) fn compute_cross_chunk_dependencies(
 struct CrossChunkDependencies<'a, 'bump> {
     chunk_meta: &'a mut [ChunkMeta],
     // `BackRef` — the same `[Chunk]` slice is also iterated mutably by
-    // the caller's sequential `walk` loop; `walk` only reads `chunks[other].unique_key`
-    // (disjoint from the per-iteration `&mut Chunk`). The slice outlives the struct
-    // (caller stack frame).
+    // the caller's sequential `walk` loop; `walk` only reads
+    // `chunks[other].{unique_key,content}` (disjoint from the per-iteration
+    // `&mut Chunk`). The slice outlives the struct (caller stack frame).
     chunks: bun_ptr::BackRef<[Chunk]>,
     parts: &'a [bun_ast::PartList<'bump>],
     import_records: &'a mut [bun_ast::import_record::List<'bump>],
@@ -117,8 +118,8 @@ struct CrossChunkDependencies<'a, 'bump> {
 impl<'a, 'bump> CrossChunkDependencies<'a, 'bump> {
     // Called once per chunk from the sequential loop above. Writes:
     // `self.chunk_meta[chunk_index]` (per-chunk disjoint),
-    // `self.import_records[source_index][rec].{path,source_index}` (per-chunk
-    // disjoint via `chunk.files_with_parts_in_chunk`),
+    // `self.import_records[source_index][rec].{path,source_index,loader,flags}`
+    // (per-chunk disjoint via `chunk.files_with_parts_in_chunk`),
     // `symbols.assign_chunk_index(ref)` (Relaxed atomic store to
     // `Symbol.chunk_index: AtomicU32`; per-symbol-ref disjoint by chunk
     // membership — debug-asserted in `assign_chunk_index`).
@@ -170,12 +171,21 @@ impl<'a, 'bump> CrossChunkDependencies<'a, 'bump> {
                     {
                         let other_chunk_index =
                             entry_point_chunk_indices[import_record.source_index.get() as usize];
+                        let other_chunk = &_chunks[other_chunk_index as usize];
                         // Slice copy (fat pointer):
                         // `path.text` borrows the chunk's
                         // `unique_key` backing buffer (`LinkerContext.unique_key_buf`),
                         // which outlives the link pass.
-                        import_record.path.text = _chunks[other_chunk_index as usize].unique_key;
+                        import_record.path.text = other_chunk.unique_key;
                         import_record.source_index = Index::INVALID;
+                        // A CSS or HTML entry point's chunk is the .css/.html output
+                        // itself, which the import's attributes still describe.
+                        if other_chunk.content.is_javascript() {
+                            import_record.loader = None;
+                            import_record
+                                .flags
+                                .insert(ImportRecordFlags::POINTS_TO_JS_CHUNK);
+                        }
 
                         // Track this cross-chunk dynamic import so we make sure to
                         // include its hash when we're calculating the hashes of all

src/js_printer/lib.rs

@@ -2711,7 +2711,9 @@ pub(crate) mod __gated_printer {
                 self.print_string_literal_utf8(path.pretty, false);
             }
 
-            if !import_options.is_missing() {
+            if !import_options.is_missing()
+                && !record.flags.contains(ImportRecordFlags::POINTS_TO_JS_CHUNK)
+            {
                 self.print_whitespacer(ws!(b", "));
                 self.print_expr(import_options, Level::Comma, ExprFlagSet::empty());
             }

test/bundler/bundler_splitting.test.ts

@@ -328,6 +328,211 @@ describe("bundler", () => {
     },
   });
 
+  // A dynamic import the linker points at a JavaScript chunk must not keep the
+  // import attributes the user wrote for the original file: the runtime would
+  // otherwise try to load the chunk with the attribute's loader.
+  itBundled("splitting/DynamicImportWithAttributeToChunk", {
+    files: {
+      "/entry.ts": `
+        const { default: data } = await import("./data.json", { with: { type: "json" } });
+        console.log(data.answer);
+      `,
+      "/data.json": `{ "answer": 42 }`,
+    },
+    splitting: true,
+    outdir: "/out",
+    target: "bun",
+    onAfterBundle(api) {
+      expect(api.readFile("/out/entry.js")).toMatch(/import\("\.\/data-[a-z0-9]+\.js"\)/);
+    },
+    run: {
+      file: "/out/entry.js",
+      stdout: "42",
+    },
+  });
+
+  // The attribute is what selects the loader for these files (the extension
+  // does not), so each chunk holds the file parsed with that loader, and the
+  // attribute has nothing left to do at runtime.
+  itBundled("splitting/DynamicImportAttributesToChunkAllLoaders", {
+    files: {
+      "/entry.ts": `
+        const json = await import("./data.notjson", { assert: { type: "json" } });
+        const text = await import("./note.md", { with: { type: "text" } });
+        const toml = await import("./config", { with: { type: "toml" } });
+        const file = await import("./asset.bin", { with: { type: "file" } });
+        console.log(
+          json.default.answer,
+          JSON.stringify(text.default),
+          toml.default.name,
+          /^\\.\\/asset-[a-z0-9]+\\.bin$/.test(file.default),
+        );
+      `,
+      "/data.notjson": `{ "answer": 42 }`,
+      "/note.md": `# hello`,
+      "/config": `name = "from toml"`,
+      "/asset.bin": `binary`,
+    },
+    splitting: true,
+    outdir: "/out",
+    onAfterBundle(api) {
+      const entry = api.readFile("/out/entry.js");
+      expect(entry).toMatch(/import\("\.\/data-[a-z0-9]+\.js"\)/);
+      expect(entry).toMatch(/import\("\.\/note-[a-z0-9]+\.js"\)/);
+      expect(entry).toMatch(/import\("\.\/config-[a-z0-9]+\.js"\)/);
+      expect(entry).toMatch(/import\("\.\/asset-[a-z0-9]+\.js"\)/);
+      expect(entry).not.toContain("type:");
+    },
+    run: {
+      file: "/out/entry.js",
+      stdout: '42 "# hello" from toml true',
+    },
+  });
+
+  // The options do not have to be an object literal at the import() site.
+  itBundled("splitting/DynamicImportOptionsVariableToChunk", {
+    files: {
+      "/entry.ts": `
+        const options = { with: { type: "json" } };
+        const { default: data } = await import("./data.json", options);
+        console.log(data.answer);
+      `,
+      "/data.json": `{ "answer": 42 }`,
+    },
+    splitting: true,
+    outdir: "/out",
+    onAfterBundle(api) {
+      expect(api.readFile("/out/entry.js")).toMatch(/import\("\.\/data-[a-z0-9]+\.js"\)/);
+    },
+    run: {
+      file: "/out/entry.js",
+      stdout: "42",
+    },
+  });
+
+  // The import() lives in a module shared by two entry points, so the rewrite
+  // happens in a shared chunk rather than in an entry point's chunk.
+  itBundled("splitting/DynamicImportAttributesToChunkFromSharedChunk", {
+    files: {
+      "/a.ts": `
+        import { load } from "./shared";
+        console.log("a", await load());
+      `,
+      "/b.ts": `
+        import { load } from "./shared";
+        console.log("b", await load());
+      `,
+      "/shared.ts": `
+        export async function load() {
+          const { default: data } = await import("./data.json", { with: { type: "json" } });
+          return data.answer;
+        }
+      `,
+      "/data.json": `{ "answer": 42 }`,
+    },
+    entryPoints: ["/a.ts", "/b.ts"],
+    splitting: true,
+    outdir: "/out",
+    onAfterBundle(api) {
+      const outputs = readdirSync(api.outdir).map(name => readFileSync(join(api.outdir, name), "utf8"));
+      expect(outputs.filter(code => /import\("\.\/data-[a-z0-9]+\.js"\)/.test(code))).toHaveLength(1);
+      expect(outputs.filter(code => code.includes("type:"))).toHaveLength(0);
+    },
+    run: [
+      { file: "/out/a.js", stdout: "a 42" },
+      { file: "/out/b.js", stdout: "b 42" },
+    ],
+  });
+
+  // One options object is shared by both arms of a conditional import(). The
+  // arm that stays external keeps it; the arm pointed at a chunk drops it.
+  itBundled("splitting/ConditionalDynamicImportExternalAndChunk", {
+    files: {
+      "/entry.ts": `
+        const useExternal = process.argv.length > 100;
+        const { default: data } = await import(useExternal ? "external-data" : "./data.json", { with: { type: "json" } });
+        console.log(data.answer);
+      `,
+      "/data.json": `{ "answer": 42 }`,
+    },
+    external: ["external-data"],
+    splitting: true,
+    outdir: "/out",
+    onAfterBundle(api) {
+      expect(api.readFile("/out/entry.js")).toMatch(
+        /import\("external-data", \{ with: \{ type: "json" \} \}\) : import\("\.\/data-[a-z0-9]+\.js"\)/,
+      );
+    },
+    run: {
+      file: "/out/entry.js",
+      stdout: "42",
+    },
+  });
+
+  itBundled("splitting/DynamicImportAttributesToChunkMinified", {
+    files: {
+      "/entry.ts": `
+        const { default: data } = await import("./data.json", { with: { type: "json" } });
+        console.log(data.answer);
+      `,
+      "/data.json": `{ "answer": 42 }`,
+    },
+    splitting: true,
+    outdir: "/out",
+    minifyWhitespace: true,
+    minifySyntax: true,
+    onAfterBundle(api) {
+      expect(api.readFile("/out/entry.js")).toMatch(/import\("\.\/data-[a-z0-9]+\.js"\)/);
+    },
+    run: {
+      file: "/out/entry.js",
+      stdout: "42",
+    },
+  });
+
+  // The attributes still belong on an import() that stays external: it loads
+  // the file the user named, not a chunk.
+  itBundled("splitting/ExternalDynamicImportKeepsAttributes", {
+    files: {
+      "/entry.ts": `
+        const { default: data } = await import("./data.json", { with: { type: "json" } });
+        console.log(data.answer);
+      `,
+    },
+    external: ["*.json"],
+    splitting: true,
+    outdir: "/out",
+    runtimeFiles: {
+      "/out/data.json": `{ "answer": 42 }`,
+    },
+    onAfterBundle(api) {
+      api.expectFile("/out/entry.js").toContain('import("./data.json", { with: { type: "json" } })');
+    },
+    run: {
+      file: "/out/entry.js",
+      stdout: "42",
+    },
+  });
+
+  // A dynamically imported stylesheet is pointed at its CSS output, not at a
+  // JavaScript chunk, so the attribute still describes what gets loaded.
+  itBundled("splitting/DynamicImportToCssChunkKeepsAttribute", {
+    files: {
+      "/entry.ts": `
+        export const sheet = import("./styles.css", { with: { type: "css" } });
+      `,
+      "/styles.css": `.a { color: red; }`,
+    },
+    splitting: true,
+    outdir: "/out",
+    target: "browser",
+    onAfterBundle(api) {
+      expect(api.readFile("/out/entry.js")).toMatch(
+        /import\("\.\/styles-[a-z0-9]+\.css", \{ with: \{ type: "css" \} \}\)/,
+      );
+    },
+  });
+
   // N same-named cross-chunk exports must get unique aliases in O(N) total
   // (ExportRenamer::next_renamed_name). Debug/ASAN builds blow past the 15s
   // cap with far fewer files than release, hence the scaled N.