fix: handle builtin node deps for NPM packages (#105)

lucacasonato · Jan 22, 2024 · 2655c6d · 2655c6d
1 parent 5d47221
commit 2655c6d
Show file tree

Hide file tree

Showing 2 changed files with 113 additions and 1 deletion.
diff --git a/mod_test.ts b/mod_test.ts
@@ -411,6 +411,27 @@ Deno.test("npm specifiers global resolver - @oramacloud/client", async (t) => {
   });
 });
 
+Deno.test("npm specifiers global resolver - typo-js", async (t) => {
+  await testLoader(t, ["native"], async (esbuild, loader) => {
+    if (esbuild === PLATFORMS.wasm) return;
+    const res = await esbuild.build({
+      ...DEFAULT_OPTS,
+      plugins: [...denoPlugins({ loader })],
+      bundle: true,
+      entryPoints: ["npm:typo-js"],
+    });
+    assertEquals(res.warnings, []);
+    assertEquals(res.errors, []);
+    assertEquals(res.outputFiles.length, 1);
+    const output = res.outputFiles[0];
+    assertEquals(output.path, "<stdout>");
+    assert(!output.text.includes(`npm:`));
+    const dataURL = `data:application/javascript;base64,${btoa(output.text)}`;
+    const { default: Typo } = await import(dataURL);
+    assertEquals(typeof Typo, "function");
+  });
+});
+
 Deno.test("npm specifiers local resolver - preact", async (t) => {
   await testLoader(t, LOADERS, async (esbuild, loader) => {
     if (esbuild === PLATFORMS.wasm) return;
@@ -504,7 +525,7 @@ Deno.test("npm specifiers local resolver - @preact/signals", async (t) => {
   });
 });
 
-Deno.test("npm specifiers local resolver - @preact/signals", async (t) => {
+Deno.test("npm specifiers local resolver - @oramacloud/client", async (t) => {
   await testLoader(t, LOADERS, async (esbuild, loader) => {
     if (esbuild === PLATFORMS.wasm) return;
     const entryPoint =
@@ -535,6 +556,35 @@ Deno.test("npm specifiers local resolver - @preact/signals", async (t) => {
   });
 });
 
+Deno.test("npm specifiers local resolver - typo-js", async (t) => {
+  await testLoader(t, LOADERS, async (esbuild, loader) => {
+    if (esbuild === PLATFORMS.wasm) return;
+    const tmp = Deno.makeTempDirSync();
+    if (loader === "portable") {
+      new Deno.Command(Deno.execPath(), {
+        args: ["cache", "--node-modules-dir", "npm:typo-js"],
+        cwd: tmp,
+      }).outputSync();
+    }
+    const res = await esbuild.build({
+      ...DEFAULT_OPTS,
+      plugins: [...denoPlugins({ loader, nodeModulesDir: true })],
+      bundle: true,
+      absWorkingDir: tmp,
+      entryPoints: ["npm:typo-js"],
+    });
+    assertEquals(res.warnings, []);
+    assertEquals(res.errors, []);
+    assertEquals(res.outputFiles.length, 1);
+    const output = res.outputFiles[0];
+    assertEquals(output.path, "<stdout>");
+    assert(output.text.includes(`require("fs")`));
+    const dataURL = `data:application/javascript;base64,${btoa(output.text)}`;
+    const { default: Typo } = await import(dataURL);
+    assertEquals(typeof Typo, "function");
+  });
+});
+
 Deno.test("remote http redirects are de-duped", async (t) => {
   await testLoader(t, LOADERS, async (esbuild, loader) => {
     const res = await esbuild.build({

diff --git a/src/plugin_deno_loader.ts b/src/plugin_deno_loader.ts
@@ -78,6 +78,59 @@ export const DEFAULT_LOADER: typeof LOADERS[number] =
     ? "portable"
     : "native";
 
+const BUILTIN_NODE_MODULES = new Set([
+  "assert",
+  "assert/strict",
+  "async_hooks",
+  "buffer",
+  "child_process",
+  "cluster",
+  "console",
+  "constants",
+  "crypto",
+  "dgram",
+  "diagnostics_channel",
+  "dns",
+  "dns/promises",
+  "domain",
+  "events",
+  "fs",
+  "fs/promises",
+  "http",
+  "http2",
+  "https",
+  "module",
+  "net",
+  "os",
+  "path",
+  "path/posix",
+  "path/win32",
+  "perf_hooks",
+  "process",
+  "punycode",
+  "querystring",
+  "repl",
+  "readline",
+  "stream",
+  "stream/consumers",
+  "stream/promises",
+  "stream/web",
+  "string_decoder",
+  "sys",
+  "test",
+  "timers",
+  "timers/promises",
+  "tls",
+  "tty",
+  "url",
+  "util",
+  "util/types",
+  "v8",
+  "vm",
+  "worker_threads",
+  "zlib",
+]);
+
 /**
  * The Deno loader plugin for esbuild. This plugin will load fully qualified
  * `file`, `http`, `https`, and `data` URLs.
@@ -178,6 +231,15 @@ export function denoLoaderPlugin(
         args: esbuild.OnResolveArgs,
       ): Promise<esbuild.OnResolveResult | null | undefined> {
         if (args.namespace === "file" && args.pluginData === IN_NODE_MODULES) {
+          if (
+            BUILTIN_NODE_MODULES.has(args.path) ||
+            BUILTIN_NODE_MODULES.has("node:" + args.path)
+          ) {
+            return {
+              path: args.path,
+              external: true,
+            };
+          }
           if (nodeModulesDir) {
             const result = await build.resolve(args.path, {
               kind: args.kind,