implicit /+esm if no extension; fix /+esm conflict (#1165)

* implicit /+esm if no extension

* tidy imports

* no /+esm if trailing slash

* move trailing /+esm to leading /_esm

* prettier-ignore

* failing test case

* back to _esm.js

* inline fromJsDelivrPath

Author: mbostock

src/npm.ts

@@ -1,6 +1,6 @@
 import {existsSync} from "node:fs";
 import {mkdir, readFile, readdir, writeFile} from "node:fs/promises";
-import {dirname, join} from "node:path/posix";
+import {dirname, extname, join} from "node:path/posix";
 import type {CallExpression} from "acorn";
 import {simple} from "acorn-walk";
 import {rsort, satisfies} from "semver";
@@ -249,7 +249,16 @@ export async function resolveNpmImport(root: string, specifier: string): Promise
       ? "dist/echarts.esm.min.js"
       : "+esm"
   } = parseNpmSpecifier(specifier);
-  return `/_npm/${name}@${await resolveNpmVersion(root, {name, range})}/${path.replace(/\+esm$/, "_esm.js")}`;
+  const version = await resolveNpmVersion(root, {name, range});
+  return `/_npm/${name}@${version}/${
+    extname(path) || // npm:foo/bar.js
+    path === "" || // npm:foo/
+    path.endsWith("/") // npm:foo/bar/
+      ? path
+      : path === "+esm" // npm:foo/+esm
+      ? "_esm.js"
+      : path.replace(/(?:\/\+esm)?$/, "._esm.js") // npm:foo/bar or npm:foo/bar/+esm
+  }`;
 }
 
 /**
@@ -264,18 +273,34 @@ export async function resolveNpmImports(root: string, path: string): Promise<Imp
 
 /**
  * Given a local npm path such as "/_npm/[email protected]/_esm.js", returns the
- * corresponding npm specifier such as "[email protected]".
+ * corresponding npm specifier such as "[email protected]/+esm". For example:
+ *
+ * /_npm/[email protected]/_esm.js         → [email protected]/+esm
+ * /_npm/[email protected]/lite._esm.js    → [email protected]/lite/+esm
+ * /_npm/[email protected]/lite.js._esm.js → [email protected]/lite.js/+esm
  */
 export function extractNpmSpecifier(path: string): string {
   if (!path.startsWith("/_npm/")) throw new Error(`invalid npm path: ${path}`);
-  return path.replace(/^\/_npm\//, "").replace(/\/_esm\.js$/, "/+esm");
+  const parts = path.split("/"); // ["", "_npm", "[email protected]", "lite.js._esm.js"]
+  const i = parts[2].startsWith("@") ? 4 : 3; // test for scoped package
+  const namever = parts.slice(2, i).join("/"); // "[email protected]" or "@observablehq/[email protected]"
+  const subpath = parts.slice(i).join("/"); // "_esm.js" or "lite._esm.js" or "lite.js._esm.js"
+  return `${namever}/${subpath === "_esm.js" ? "+esm" : subpath.replace(/\._esm\.js$/, "/+esm")}`;
 }
 
 /**
  * Given a jsDelivr path such as "/npm/[email protected]/+esm", returns the corresponding
- * local path such as "/_npm/[email protected]/_esm.js".
+ * local path such as "/_npm/[email protected]/_esm.js". For example:
+ *
+ * /npm/[email protected]/+esm         → /_npm/[email protected]/_esm.js
+ * /npm/[email protected]/lite/+esm    → /_npm/[email protected]/lite._esm.js
+ * /npm/[email protected]/lite.js/+esm → /_npm/[email protected]/lite.js._esm.js
  */
 export function fromJsDelivrPath(path: string): string {
   if (!path.startsWith("/npm/")) throw new Error(`invalid jsDelivr path: ${path}`);
-  return path.replace(/^\/npm\//, "/_npm/").replace(/\/\+esm$/, "/_esm.js");
+  const parts = path.split("/"); // e.g. ["", "npm", "[email protected]", "lite", "+esm"]
+  const i = parts[2].startsWith("@") ? 4 : 3; // test for scoped package
+  const namever = parts.slice(2, i).join("/"); // "[email protected]" or "@observablehq/[email protected]"
+  const subpath = parts.slice(i).join("/"); // "+esm" or "lite/+esm" or "lite.js/+esm"
+  return `/_npm/${namever}/${subpath === "+esm" ? "_esm.js" : subpath.replace(/\/\+esm$/, "._esm.js")}`;
 }

test/npm-test.ts

@@ -1,6 +1,6 @@
 import assert from "node:assert";
-import {extractNpmSpecifier, getDependencyResolver, rewriteNpmImports} from "../src/npm.js";
-import {fromJsDelivrPath} from "../src/npm.js";
+import {extractNpmSpecifier, parseNpmSpecifier} from "../src/npm.js";
+import {fromJsDelivrPath, getDependencyResolver, resolveNpmImport, rewriteNpmImports} from "../src/npm.js";
 import {relativePath} from "../src/path.js";
 import {mockJsDelivr} from "./mocks/jsdelivr.js";
 
@@ -20,10 +20,62 @@ describe("getDependencyResolver(root, path, input)", () => {
   });
 });
 
+describe("parseNpmSpecifier(specifier)", () => {
+  it("parses the name", () => {
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array"), {name: "d3-array", range: undefined, path: undefined});
+  });
+  it("parses the name and range", () => {
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array@1"), {name: "d3-array", range: "1", path: undefined});
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array@latest"), {name: "d3-array", range: "latest", path: undefined});
+  });
+  it("parses the name and path", () => {
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array"), {name: "d3-array", range: undefined, path: undefined});
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array/foo"), {name: "d3-array", range: undefined, path: "foo"});
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array/foo/bar"), {name: "d3-array", range: undefined, path: "foo/bar"}); // prettier-ignore
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array/foo.js"), {name: "d3-array", range: undefined, path: "foo.js"});
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array/foo/bar.js"), {name: "d3-array", range: undefined, path: "foo/bar.js"}); // prettier-ignore
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array/+esm"), {name: "d3-array", range: undefined, path: "+esm"});
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array/foo.js/+esm"), {name: "d3-array", range: undefined, path: "foo.js/+esm"}); // prettier-ignore
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array/foo/bar.js/+esm"), {name: "d3-array", range: undefined, path: "foo/bar.js/+esm"}); // prettier-ignore
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array/foo/+esm"), {name: "d3-array", range: undefined, path: "foo/+esm"}); // prettier-ignore
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array/foo/bar/+esm"), {name: "d3-array", range: undefined, path: "foo/bar/+esm"}); // prettier-ignore
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array/"), {name: "d3-array", range: undefined, path: ""});
+  });
+  it("parses the name, version, and path", () => {
+    assert.deepStrictEqual(parseNpmSpecifier("d3-array@1/foo"), {name: "d3-array", range: "1", path: "foo"});
+  });
+});
+
+describe("resolveNpmImport(root, specifier)", () => {
+  mockJsDelivr();
+  const root = "test/input/build/simple";
+  it("implicitly adds /_esm.js for specifiers without an extension", async () => {
+    assert.strictEqual(await resolveNpmImport(root, "d3-array"), "/_npm/[email protected]/_esm.js");
+    assert.strictEqual(await resolveNpmImport(root, "d3-array/src"), "/_npm/[email protected]/src._esm.js");
+    assert.strictEqual(await resolveNpmImport(root, "d3-array/foo+bar"), "/_npm/[email protected]/foo+bar._esm.js");
+    assert.strictEqual(await resolveNpmImport(root, "d3-array/foo+esm"), "/_npm/[email protected]/foo+esm._esm.js");
+  });
+  it("replaces /+esm with /_esm.js or ._esm.js", async () => {
+    assert.strictEqual(await resolveNpmImport(root, "d3-array/+esm"), "/_npm/[email protected]/_esm.js");
+    assert.strictEqual(await resolveNpmImport(root, "d3-array/src/+esm"), "/_npm/[email protected]/src._esm.js");
+    assert.strictEqual(await resolveNpmImport(root, "d3-array/src/index.js/+esm"), "/_npm/[email protected]/src/index.js._esm.js"); // prettier-ignore
+  });
+  it("does not add /_esm.js if given a path with a file extension", async () => {
+    assert.strictEqual(await resolveNpmImport(root, "d3-array/src/index.js"), "/_npm/[email protected]/src/index.js");
+  });
+  it("does not add /_esm.js if given a path with a trailing slash", async () => {
+    assert.strictEqual(await resolveNpmImport(root, "d3-array/"), "/_npm/[email protected]/");
+    assert.strictEqual(await resolveNpmImport(root, "d3-array/src/"), "/_npm/[email protected]/src/");
+  });
+});
+
 describe("extractNpmSpecifier(path)", () => {
   it("returns the npm specifier for the given local npm path", () => {
     assert.strictEqual(extractNpmSpecifier("/_npm/[email protected]/_esm.js"), "[email protected]/+esm");
     assert.strictEqual(extractNpmSpecifier("/_npm/[email protected]/dist/d3.js"), "[email protected]/dist/d3.js");
+    assert.strictEqual(extractNpmSpecifier("/_npm/[email protected]/dist/d3.js._esm.js"), "[email protected]/dist/d3.js/+esm");
+    assert.strictEqual(extractNpmSpecifier("/_npm/[email protected]/lite._esm.js"), "[email protected]/lite/+esm");
+    assert.strictEqual(extractNpmSpecifier("/_npm/@uwdata/[email protected]/_esm.js"), "@uwdata/[email protected]/+esm");
   });
   it("throws if not given a local npm path", () => {
     assert.throws(() => extractNpmSpecifier("/npm/[email protected]/+esm"), /invalid npm path/);
@@ -35,6 +87,7 @@ describe("fromJsDelivrPath(path)", () => {
   it("returns the local npm path for the given jsDelivr path", () => {
     assert.strictEqual(fromJsDelivrPath("/npm/[email protected]/+esm"), "/_npm/[email protected]/_esm.js");
     assert.strictEqual(fromJsDelivrPath("/npm/[email protected]/dist/d3.js"), "/_npm/[email protected]/dist/d3.js");
+    assert.strictEqual(fromJsDelivrPath("/npm/[email protected]/dist/d3.js/+esm"), "/_npm/[email protected]/dist/d3.js._esm.js");
   });
   it("throws if not given a jsDelivr path", () => {
     assert.throws(() => fromJsDelivrPath("/_npm/[email protected]/_esm.js"), /invalid jsDelivr path/);