bundler: do not rename a shared binding that an entry point also exports

src/bundler/linker_context/crossChunkNames.rs

@@ -32,16 +32,15 @@ fn cross_chunk_refs(chunks: &[Chunk]) -> Vec<Ref> {
 }
 
 /// Names no chunk may use for a cross-chunk binding: keywords and the like,
-/// every unbound or must-not-be-renamed name in any module scope of the
-/// bundle, and the entry points' own export names. A per-chunk renamer only
-/// avoids its own chunk's; a bundle-wide name has to avoid all of them.
+/// plus every unbound or must-not-be-renamed name in any module scope of the
+/// bundle. A per-chunk renamer only avoids its own chunk's; a bundle-wide
+/// name has to avoid all of them.
 fn reserved_names(
     c: &LinkerContext,
     chunks: &[Chunk],
 ) -> Result<StringHashMap<u32>, bun_alloc::AllocError> {
     let mut reserved = renamer::compute_initial_reserved_names(c.options.output_format)?;
     let scopes = c.graph.ast.items_module_scope();
-    let export_aliases = c.graph.meta.items_sorted_and_filtered_export_aliases();
     for chunk in chunks {
         if let Content::Javascript(js) = &chunk.content {
             for &source_index in js.files_in_chunk_order.iter() {
@@ -52,13 +51,6 @@ fn reserved_names(
                 );
             }
         }
-        // An entry point's own `export {}` names share its export namespace
-        // with the cross-chunk exports it may carry.
-        if chunk.entry_point.is_entry_point() {
-            for alias in export_aliases[chunk.entry_point.source_index() as usize].iter() {
-                reserved.put(alias, 1)?;
-            }
-        }
     }
     Ok(reserved)
 }
@@ -136,18 +128,12 @@ pub(crate) fn assign_minified(
     // Every chunk's renamer already reserved the keywords plus its own module
     // scopes' unbound / pinned names; a bundle-wide name avoids all of them.
     let mut reserved = StringHashMap::<u32>::default();
-    let export_aliases = c.graph.meta.items_sorted_and_filtered_export_aliases();
     for chunk in chunks.iter() {
         if let ChunkRenamer::Minify(r) = &chunk.renamer {
             for (name, _) in r.reserved_names().iter() {
                 reserved.put(name, 1)?;
             }
         }
-        if chunk.entry_point.is_entry_point() {
-            for alias in export_aliases[chunk.entry_point.source_index() as usize].iter() {
-                reserved.put(alias, 1)?;
-            }
-        }
     }
 
     // (total count, first-seen order) per binding; most used first. Each
@@ -234,10 +220,14 @@ pub(crate) fn assign_minified(
 }
 
 /// Writes the names into the cross-chunk `export {}` / `import {}` clause items.
-pub(crate) fn apply_to_clauses(c: &LinkerContext, chunks: &mut [Chunk]) {
+pub(crate) fn apply_to_clauses(
+    c: &LinkerContext,
+    chunks: &mut [Chunk],
+) -> Result<(), bun_alloc::AllocError> {
     if c.cross_chunk_names.is_empty() {
-        return;
+        return Ok(());
     }
+    let export_aliases = export_aliases_beside_entry_exports(c, chunks)?;
     for chunk in chunks.iter_mut() {
         let Content::Javascript(js) = &mut chunk.content else {
             continue;
@@ -253,9 +243,113 @@ pub(crate) fn apply_to_clauses(c: &LinkerContext, chunks: &mut [Chunk]) {
                 _ => continue,
             };
             for item in items {
-                item.alias =
-                    bun_ast::StoreStr::new(*c.cross_chunk_names.get(&item.name.ref_).unwrap());
+                let ref_ = item.name.ref_;
+                let alias = export_aliases
+                    .get(&ref_)
+                    .unwrap_or_else(|| c.cross_chunk_names.get(&ref_).unwrap());
+                item.alias = bun_ast::StoreStr::new(*alias);
             }
         }
     }
+    Ok(())
+}
+
+/// Aliases for the bindings an ESM entry chunk exports to other chunks under
+/// the name of one of its entry point's exports: `export { helper as helper2 }`,
+/// `import { helper2 as helper }`. An item the entry point already exports is dropped.
+fn export_aliases_beside_entry_exports(
+    c: &LinkerContext,
+    chunks: &mut [Chunk],
+) -> Result<bun_collections::HashMap<Ref, &'static [u8]>, bun_alloc::AllocError> {
+    let mut aliases: bun_collections::HashMap<Ref, &'static [u8]> = Default::default();
+    let entry_export_names = c.graph.meta.items_sorted_and_filtered_export_aliases();
+    let mut buf: Vec<u8> = Vec::new();
+    for chunk in chunks.iter_mut() {
+        if !chunk.entry_point.is_entry_point() {
+            continue;
+        }
+        let source_index = chunk.entry_point.source_index() as usize;
+        // A CommonJS entry point prints `export default require_x()` only.
+        if entry_export_names[source_index].is_empty()
+            || c.graph.meta.items_flags()[source_index].wrap == crate::WrapKind::Cjs
+        {
+            continue;
+        }
+        let Content::Javascript(js) = &mut chunk.content else {
+            continue;
+        };
+        let [stmt] = js.cross_chunk_suffix_stmts.as_mut_slice() else {
+            continue;
+        };
+        let bun_ast::StmtData::SExportClause(clause) = &mut stmt.data else {
+            continue;
+        };
+        let items = clause.items.slice_mut();
+
+        let mut taken = StringHashMap::<()>::default();
+        for name in entry_export_names[source_index].iter() {
+            taken.put(name, ())?;
+        }
+        // Cross-chunk names are unique, so a hit here is an entry export name.
+        let mut colliding: Vec<usize> = Vec::new();
+        for (i, item) in items.iter().enumerate() {
+            let name = *c.cross_chunk_names.get(&item.name.ref_).unwrap();
+            if taken.contains_key(name) {
+                colliding.push(i);
+            } else {
+                taken.put(name, ())?;
+            }
+        }
+
+        let mut dropped: Vec<usize> = Vec::new();
+        for i in colliding {
+            let ref_ = items[i].name.ref_;
+            let name = *c.cross_chunk_names.get(&ref_).unwrap();
+            if entry_export_binding(c, source_index, name).is_some_and(|exported| {
+                c.graph.symbols.follow(exported) == c.graph.symbols.follow(ref_)
+            }) {
+                dropped.push(i);
+                continue;
+            }
+            let mut tries = 1u32;
+            loop {
+                tries += 1;
+                buf.clear();
+                buf.extend_from_slice(name);
+                write!(&mut buf, "{tries}").expect("Vec<u8> write");
+                if !taken.contains_key(buf.as_slice()) {
+                    break;
+                }
+            }
+            taken.put(&buf, ())?;
+            aliases.insert(ref_, intern(c, &buf));
+        }
+        if !dropped.is_empty() {
+            let mut kept = 0;
+            let mut next_dropped = 0;
+            for i in 0..items.len() {
+                if dropped.get(next_dropped) == Some(&i) {
+                    next_dropped += 1;
+                } else {
+                    items.swap(kept, i);
+                    kept += 1;
+                }
+            }
+            clause.items.truncate(kept);
+        }
+    }
+    Ok(aliases)
+}
+
+/// The binding `generate_entry_point_tail_js` exports as `name`; `None` for a copy of a CommonJS property.
+fn entry_export_binding(c: &LinkerContext, source_index: usize, name: &[u8]) -> Option<Ref> {
+    let export = c.graph.meta.items_resolved_exports()[source_index].get(name)?;
+    let mut ref_ = export.data.import_ref;
+    if let Some(import) = c.graph.meta.items_imports_to_bind()[source_index].get(&ref_) {
+        ref_ = import.data.import_ref;
+    }
+    if c.graph.symbols.get_const(ref_)?.namespace_alias.is_some() {
+        return None;
+    }
+    Some(ref_)
 }

src/bundler/linker_context/generateChunksInParallel.rs

@@ -108,7 +108,7 @@ pub(crate) fn generate_chunks_in_parallel<const IS_DEV_SERVER: bool>(
                 c.worker_pool()
                     .each_ptr(ctx, LinkerContext::finish_js_renamer, chunks);
             }
-            crate::linker_context::cross_chunk_names::apply_to_clauses(c, chunks);
+            crate::linker_context::cross_chunk_names::apply_to_clauses(c, chunks)?;
         }
         debug!("  DONE {} renamers", chunks.len());
     }

test/bundler/bundler_splitting.test.ts

@@ -679,6 +679,174 @@ describe("bundler", () => {
     ],
   });
 
+  // An entry point's export names are not bindings. A binding shared across
+  // chunks keeps its name when an entry point exports something under that
+  // name: the same binding (index.js) or one of its own (other.js).
+  itBundled("splitting/CrossChunkNameMatchesEntryExportName", {
+    files: {
+      "/index.js": /* js */ `
+        export { ValidationError } from './errors.js'
+      `,
+      "/cli.js": /* js */ `
+        import { ValidationError, helper } from './errors.js'
+        console.log(new ValidationError('x').constructor.name, helper.name, helper())
+      `,
+      "/other.js": /* js */ `
+        export function helper() { return 'own' }
+      `,
+      "/errors.js": /* js */ `
+        export class ValidationError extends Error {}
+        export function helper() { return 'shared' }
+      `,
+      "/run.js": /* js */ `
+        const index = await import('./out/index.js')
+        const other = await import('./out/other.js')
+        console.log(Object.keys(index).join(), index.ValidationError.name)
+        console.log(Object.keys(other).join(), other.helper.name, other.helper())
+      `,
+    },
+    entryPoints: ["/index.js", "/cli.js", "/other.js"],
+    splitting: true,
+    outdir: "/out",
+    format: "esm",
+    onAfterBundle(api) {
+      const shared = api.readFile(
+        "/out/" + jsFilesIn(api).find(f => api.readFile("/out/" + f).includes("extends Error"))!,
+      );
+      expect(shared).toContain("class ValidationError extends Error");
+      expect(shared).toContain("function helper()");
+      for (const f of jsFilesIn(api)) {
+        const out = api.readFile("/out/" + f);
+        for (const clause of out.match(/(?:import|export)\s*\{[^}]*\}/g) ?? []) expect(clause).not.toContain(" as ");
+      }
+    },
+    run: [
+      { file: "/out/cli.js", stdout: "ValidationError helper shared" },
+      { file: "/run.js", stdout: "ValidationError ValidationError\nhelper helper own" },
+    ],
+  });
+
+  // Same under --minify-identifiers. index.js exports every one-letter name
+  // (listed in module namespace key order); the two shared bindings still get
+  // one-letter names, and each export of index.js is the function it names.
+  const oneLetterNames = [..."$ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz"];
+  itBundled("splitting/CrossChunkNameMatchesEntryExportNameMinified", {
+    files: {
+      "/index.js": `export { ${oneLetterNames.map((name, i) => `${i % 2 ? "second" : "first"} as ${name}`).join(", ")} } from './lib.js'`,
+      "/cli.js": /* js */ `
+        import { first, second } from './lib.js'
+        console.log(first(), second())
+      `,
+      "/lib.js": /* js */ `
+        export function first() { return 'first' }
+        export function second() { return 'second' }
+      `,
+      "/run.js": /* js */ `
+        const index = await import('./out/index.js')
+        console.log(Object.keys(index).join(''))
+        console.log(Object.values(index).map(fn => fn()[0]).join(''))
+      `,
+    },
+    entryPoints: ["/index.js", "/cli.js"],
+    splitting: true,
+    minifyIdentifiers: true,
+    outdir: "/out",
+    format: "esm",
+    onAfterBundle(api) {
+      const shared = jsFilesIn(api).find(f => api.readFile("/out/" + f).includes('"first"'))!;
+      api.expectFile("/out/" + shared).toMatch(/export\s*\{\s*[\w$],\s*[\w$]\s*\}/);
+    },
+    run: [
+      { file: "/out/cli.js", stdout: "first second" },
+      { file: "/run.js", stdout: oneLetterNames.join("") + "\n" + oneLetterNames.map((_, i) => "fs"[i % 2]).join("") },
+    ],
+  });
+
+  // One chunk can list both kinds of name in its `export {}` clause: d.js is in
+  // the chunk of a.js today (#18008), and b.js reads `foo` off the exports
+  // object of d.js. Where a binding and an export name collide there, the
+  // binding keeps its name and is exported under an alias.
+  for (const minifyIdentifiers of [false, true]) {
+    itBundled(`splitting/CrossChunkBindingKeepsNameBesideEntryExportName${minifyIdentifiers ? "Minified" : ""}`, {
+      files: {
+        "/a.js": /* js */ `
+          import * as ns from './pkg/d.js'
+          const own = ns
+          export { own as exports_d, own as helper, ${oneLetterNames.map(name => `own as ${name}`).join(", ")} }
+        `,
+        "/b.js": /* js */ `
+          import { foo } from './pkg/d.js'
+          import { helper } from './shared.js'
+          console.log(foo, helper()${minifyIdentifiers ? "" : ", helper.name"})
+        `,
+        "/c.js": /* js */ `
+          import { helper } from './shared.js'
+          console.log(helper())
+        `,
+        "/shared.js": `export function helper() { return 'shared' }`,
+        "/pkg/d.js": `export * from './c.cjs'`,
+        "/pkg/c.cjs": `module.exports['f' + 'oo'] = 123`,
+        "/pkg/package.json": `{ "name": "pkg", "sideEffects": false }`,
+        "/run.js": /* js */ `
+          const a = await import('./out/a.js')
+          console.log(a.exports_d.foo, a.helper.foo, a.s.foo, a.$.foo)
+        `,
+      },
+      entryPoints: ["/a.js", "/b.js", "/c.js"],
+      splitting: true,
+      minifyIdentifiers,
+      outdir: "/out",
+      format: "esm",
+      onAfterBundle(api) {
+        if (minifyIdentifiers) return;
+        const all = jsFilesIn(api)
+          .map(f => api.readFile("/out/" + f))
+          .join("\n");
+        expect(all).toContain("var exports_d = {}");
+        expect(all).toContain("function helper()");
+        expect(all).not.toMatch(/\b(?:exports_d|helper)\d+\s*[=(]/);
+        api.expectFile("/out/a.js").toMatch(/\bexports_d as exports_d2\b/);
+        api.expectFile("/out/b.js").toMatch(/\bexports_d2 as exports_d\b/);
+      },
+      run: [
+        { file: "/out/b.js", stdout: minifyIdentifiers ? "123 shared" : "123 shared helper" },
+        { file: "/out/c.js", stdout: "shared" },
+        { file: "/run.js", stdout: "123 123 123 123" },
+      ],
+    });
+
+    // The same, but every export of a.js is that one binding: no second item,
+    // b.js imports the export a.js already has.
+    itBundled(`splitting/CrossChunkImportUsesEntryExportOfSameBinding${minifyIdentifiers ? "Minified" : ""}`, {
+      files: {
+        "/a.js": /* js */ `
+          import * as ns from './pkg/d.js'
+          export { ns as exports_d, ${oneLetterNames.map(name => `ns as ${name}`).join(", ")} }
+        `,
+        "/b.js": /* js */ `
+          import { foo } from './pkg/d.js'
+          console.log(foo)
+        `,
+        "/pkg/d.js": `export * from './c.cjs'`,
+        "/pkg/c.cjs": `module.exports['f' + 'oo'] = 123`,
+        "/pkg/package.json": `{ "name": "pkg", "sideEffects": false }`,
+        "/run.js": /* js */ `
+          const a = await import('./out/a.js')
+          console.log(Object.keys(a).join(), a.exports_d.foo, a.s.foo)
+        `,
+      },
+      entryPoints: ["/a.js", "/b.js"],
+      splitting: true,
+      minifyIdentifiers,
+      outdir: "/out",
+      format: "esm",
+      run: [
+        { file: "/out/b.js", stdout: "123" },
+        { file: "/run.js", stdout: [...oneLetterNames, "exports_d"].sort().join() + " 123 123" },
+      ],
+    });
+  }
+
   // Direct eval keeps every name in its scope chain as written — including a
   // CommonJS-wrapped file's top level — so the bundle-wide namer must route
   // around those names, and the shared bindings such a file declares fall