bundler: store a path external that ends in a separator with and without it

src/bundler/options.rs

@@ -169,7 +169,14 @@ pub(crate) fn init_external_modules(
             let normalized = validate_path(log, cwd, external, b"external path");
 
             if !normalized.is_empty() {
-                result.abs_paths.insert(&normalized).expect("unreachable");
+                // The resolver's key for an import keeps the separator that the import ends in,
+                // so a path that ends in one is stored with and without it.
+                let trimmed =
+                    bun_paths::string_paths::without_trailing_slash_windows_path(&normalized);
+                result.abs_paths.insert(trimmed).expect("unreachable");
+                if trimmed.len() != normalized.len() {
+                    result.abs_paths.insert(&normalized).expect("unreachable");
+                }
             }
         }
     }

src/paths/resolve_path.rs

@@ -1552,7 +1552,7 @@ pub fn join_abs<'a, P: PlatformT>(cwd: &'a [u8], part: &[u8]) -> &'a [u8] {
 
 /// Convert parts of potentially invalid file paths into a single valid filpeath
 /// without querying the filesystem
-/// This is the equivalent of path.resolve
+/// Unlike `path.resolve`, the result keeps the platform's separator at the end of the last part.
 ///
 /// Returned path is stored in a temporary buffer. It must be copied if it needs to be stored.
 // result borrows the thread-local buffer ('static) OR returns `cwd`
@@ -1596,7 +1596,7 @@ pub fn join_abs_string_spill<'a, P: PlatformT>(
 
 /// Convert parts of potentially invalid file paths into a single valid filpeath
 /// without querying the filesystem
-/// This is the equivalent of path.resolve
+/// Unlike `path.resolve`, the result keeps the platform's separator at the end of the last part.
 ///
 /// Returned path is stored in a temporary buffer. It must be copied if it needs to be stored.
 pub fn join_abs_string_z<'a, P: PlatformT>(cwd: &'a [u8], parts: &[&[u8]]) -> &'a ZStr {

test/aot/cli/metafile.test.ts

@@ -190,18 +190,20 @@ describe.concurrent("--metafile with --aot", () => {
 import type { Shape } from "./shape";
 import { unused } from "./unused";
 import * as marked from "marked-external";
+import * as outside from "./outside";
 import test from "node:test";
 let missing: unknown;
 try {
   missing = require("./not-there");
 } catch {}
 const shape: Shape = { name: process.argv[2] ?? "b" };
 const chosen = require("./chosen/" + shape.name + ".ts");
-console.log(typeof marked, typeof test, typeof missing, chosen.name, import.meta.resolve("./resolved.ts").endsWith("resolved.ts"));
+console.log(typeof marked, typeof outside, typeof test, typeof missing, chosen.name, import.meta.resolve("./resolved.ts").endsWith("resolved.ts"));
 `,
           "marked-external.d.ts": `declare module "marked-external";\n`,
           "shape.ts": `export interface Shape {\n  name: string;\n}\n`,
           "unused.ts": `export const unused = 1;\n`,
+          "outside/index.ts": `export const outside = 1;\n`,
           "resolved.ts": `export const resolved = 1;\n`,
           "chosen/a.ts": `export const name = "a";\n`,
           "chosen/b.ts": `export const name = "b";\n`,
@@ -211,12 +213,14 @@ console.log(typeof marked, typeof test, typeof missing, chosen.name, import.meta
           await bun(
             // Only the metafile is read. An executable that loads code at run time would hold every built-in module.
             ...[folder, "build", "--compile", "--aot", "--no-builtin-modules", "--external", "marked-external"],
-            ...["--metafile=meta.json", "main.ts", "--outfile", "app"],
+            // A folder is external with a separator at the end of its path too.
+            ...["--external", "./outside/", "--metafile=meta.json", "main.ts", "--outfile", "app"],
           ),
         );
         const metafile = read(join(folder, "meta.json"));
         expect(Object.keys(metafile.inputs).sort()).toEqual(["chosen/a.ts", "chosen/b.ts", "main.ts", "resolved.ts"]);
         expect(metafile.inputs["main.ts"].imports.map(line).sort()).toEqual([
+          "import-statement ./outside external",
           "import-statement marked-external external",
           "import-statement node:test external",
           "require-call ./not-there external",

test/bundler/bundler_edgecase.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from "bun:test";
-import { bunEnv, bunExe, tempDir } from "harness";
+import { bunEnv, bunExe, isWindows, tempDir } from "harness";
 import { readdirSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
 import { unsupported } from "../aot/harness/commands";
@@ -1870,6 +1870,70 @@ describe("bundler", () => {
       api.expectFile("/out.js").toContain("h(");
     },
   });
+  // A path in `external` names the same file or folder with and without a separator at its end.
+  // Each row: the external, what the entry point imports, and whether those imports stay imports.
+  const externalPathRows: [id: string, external: string, specifiers: string[], kept: boolean][] = [
+    ["Folder", "./lib/", ["./lib"], true],
+    ["AbsoluteFolder", "{{root}}/lib/", ["./lib"], true],
+    ["TwoSeparators", "./lib//", ["./lib"], true],
+    ["File", "./lib/index.js/", ["./lib/index.js"], true],
+    ["ImportEndsInDot", "./lib/", ["./lib/."], true],
+    ["MissingTarget", "./absent/", ["./absent"], true],
+    ["ImportEndsInSeparator", "./lib/", ["./lib/"], true],
+    ["BothImportSpellings", "./lib/", ["./lib", "./lib/"], true],
+    // The match is on the whole path: a folder does not cover the files inside it.
+    ["FileInsideFolder", "./lib/", ["./lib/index.js"], false],
+    ["ParentFolder", "./", ["./lib"], false],
+  ];
+  if (isWindows) {
+    externalPathRows.push(["Backslashes", ".\\lib\\", ["./lib"], true]);
+  } else {
+    // An import by absolute path has a compare of its own in the resolver. On Windows that compare
+    // is on the import as written, which has no drive letter or has `/` where the list has `\`.
+    externalPathRows.push(
+      ["AbsoluteImport", "./lib/", ["{{root}}/lib", "{{root}}/lib/"], true],
+      ["RootedImport", "/api/", ["/api", "/api/"], true],
+    );
+  }
+  for (const backend of ["cli", "api"] as const) {
+    for (const [id, external, specifiers, kept] of externalPathRows) {
+      const imports = specifiers.map((specifier, i) => `import v${i} from ${JSON.stringify(specifier)};`);
+      itBundled(`edgecase/ExternalPathEndsInSeparator/${id}/${backend}`, {
+        backend,
+        files: {
+          "/entry.js": `${imports.join("\n")}\nconsole.log(${specifiers.map((_, i) => `v${i}`)});`,
+          "/lib/index.js": `export default "LIB";`,
+        },
+        external: [external],
+        onAfterBundle(api) {
+          const out = api.readFile("/out.js");
+          expect({
+            imports: out.match(/^import .+$/gm) ?? [],
+            bundlesLib: out.includes(`"LIB"`),
+          }).toEqual({
+            imports: kept ? imports.map(line => line.replaceAll("{{root}}", api.root)) : [],
+            bundlesLib: !kept,
+          });
+        },
+      });
+    }
+  }
+  // bunfig.toml gives `bun build` the same list.
+  itBundled("edgecase/ExternalPathEndsInSeparator/Bunfig", {
+    backend: "cli",
+    files: {
+      "/entry.js": `import v from "./lib";\nconsole.log(v);`,
+      "/lib/index.js": `export default "LIB";`,
+      "/bunfig.toml": `external = ["./lib/"]`,
+    },
+    onAfterBundle(api) {
+      const out = api.readFile("/out.js");
+      expect({
+        imports: out.match(/^import .+$/gm) ?? [],
+        bundlesLib: out.includes(`"LIB"`),
+      }).toEqual({ imports: [`import v from "./lib";`], bundlesLib: false });
+    },
+  });
   // `bun build --compile --aot` resolves entry points with a resolver of its own, to the same answers.
   const itCompiledAheadOfTime = (id: string, opts: BundlerTestInput) =>
     (unsupported ? itBundled.skip : itBundled)(`edgecase/aot/${id}`, {