fix #3917: running esbuild cli with deno

evanw · evanw · commit b50044303b34 · 2024-09-21T18:31:37.000-04:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,12 @@
 
     Previously a `tsconfig.json` file that `extends` another file in a package with an `exports` map could cause a stack overflow when Yarn's Plug'n'Play resolution was active. This edge case should work now starting with this release.
 
+* Work around more issues with Deno 1.31+ ([#3917](https://github.com/evanw/esbuild/pull/3917))
+
+    This version of Deno broke the `stdin` and `stdout` properties on command objects for inherited streams, which matters when you run esbuild's Deno module as the entry point (i.e. when `import.meta.main` is `true`). Previously esbuild would crash in Deno 1.31+ if you ran esbuild like that. This should be fixed starting with this release.
+
+    This fix was contributed by [@Joshix-1](https://github.com/Joshix-1).
+
 ## 0.23.1
 
 * Allow using the `node:` import prefix with `es*` targets ([#3821](https://github.com/evanw/esbuild/issues/3821))
diff --git a/Makefile b/Makefile
@@ -69,6 +69,7 @@ test-deno: esbuild platform-deno
 	@echo '✅ deno tests passed' # I couldn't find a Deno API for telling when tests have failed, so I'm doing this here instead
 	deno eval 'import { transform, stop } from "file://$(shell pwd)/deno/mod.js"; console.log((await transform("1+2")).code); stop()' | grep "1 + 2;"
 	deno eval 'import { transform, stop } from "file://$(shell pwd)/deno/wasm.js"; console.log((await transform("1+2")).code); stop()' | grep "1 + 2;"
+	deno run -A './deno/mod.js' # See: https://github.com/evanw/esbuild/pull/3917
 
 test-deno-windows: esbuild platform-deno
 	ESBUILD_BINARY_PATH=./esbuild.exe deno test --allow-run --allow-env --allow-net --allow-read --allow-write --no-check scripts/deno-tests.js
diff --git a/lib/deno/mod.ts b/lib/deno/mod.ts
@@ -203,11 +203,12 @@ const spawnNew: SpawnFn = (cmd, { args, stdin, stdout, stderr }) => {
     stdout,
     stderr,
   }).spawn()
-  const writer = child.stdin.getWriter()
-  const reader = child.stdout.getReader()
+  // Note: Need to check for "piped" in Deno ≥1.31.0 to avoid a crash
+  const writer = stdin === 'piped' ? child.stdin.getWriter() : null
+  const reader = stdout === 'piped' ? child.stdout.getReader() : null
   return {
-    write: bytes => writer.write(bytes),
-    read: () => reader.read().then(x => x.value || null),
+    write: writer ? bytes => writer.write(bytes) : () => Promise.resolve(),
+    read: reader ? () => reader.read().then(x => x.value || null) : () => Promise.resolve(null),
     close: async () => {
       // We can't call "kill()" because it doesn't seem to work. Tests will
       // still fail with "A child process was opened during the test, but not
@@ -223,8 +224,8 @@ const spawnNew: SpawnFn = (cmd, { args, stdin, stdout, stderr }) => {
       // we can do.
       //
       // See this for more info: https://github.com/evanw/esbuild/pull/3611
-      await writer.close()
-      await reader.cancel()
+      if (writer) await writer.close()
+      if (reader) await reader.cancel()
 
       // Wait for the process to exit. The new "kill()" API doesn't flag the
       // process as having exited because processes can technically ignore the
