Re-enable unref in Deno

CHANGELOG.md

@@ -1,5 +1,17 @@
 # Changelog
 
+## 0.20.3
+
+* Re-enable `unref` behaviour for Deno ([#3701](https://github.com/evanw/esbuild/issues/3701))
+
+    Version 0.20.0 of esbuild changed how the esbuild child process is run in esbuild's API for Deno. Previously it used `Deno.run` but that API is being removed in favor of `Deno.Command`. As part of this change, esbuild is now calling the new `unref` function on esbuild's long-lived child process, which is supposed to allow Deno to exit when your code has finished running even though the child process is still around (previously you had to explicitly call esbuild's `stop()` function to terminate the child process for Deno to be able to exit).
+
+    In version 0.20.2 of esbuild, this change was reverted because it was discovered that this change could result in the Deno process exiting when with an error with message `error: Promise resolution is still pending but the event loop has already resolved`, if the `stop()` function was called when there were no other ref'd pending async operations scheduled in the process.
+
+    It has now been determined that this was caused by a bug in the implementation of esbuild's `stop()` function. This function did not `ref` the child process again before waiting for it to exit. This meant that when a user explicitly called `stop()`, a promise was returned that was backed only by `unref`'d async operations. If the user then awaited this promise with no other operations pending, the event loop would starve, and the process would exit with the error message seen above. This has been fixed by re-`ref`ing the child process before waiting for it to exit.
+
+    This version of esbuild fixes this bug, and returns the `unref` behaviour to the behaviour present in esbuild's 0.20.0 release.
+
 ## 0.20.2
 
 * Support TypeScript experimental decorators on `abstract` class fields ([#3684](https://github.com/evanw/esbuild/issues/3684))

lib/deno/mod.ts

@@ -192,6 +192,8 @@ type SpawnFn = (cmd: string, options: {
   read(): Promise<Uint8Array | null>
   close(): Promise<void> | void
   status(): Promise<{ code: number }>
+  unref(): void
+  ref(): void
 }
 
 // Deno ≥1.40
@@ -209,6 +211,11 @@ const spawnNew: SpawnFn = (cmd, { args, stdin, stdout, stderr }) => {
     write: bytes => writer.write(bytes),
     read: () => reader.read().then(x => x.value || null),
     close: async () => {
+      // Ref the child process again, so that a user calling `close()` can await
+      // the returned promise without the event loop starving because there are
+      // no more ref'd async tasks.
+      child.ref()
+
       // 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
       // closed during the test" even though we kill the child process.
@@ -233,6 +240,8 @@ const spawnNew: SpawnFn = (cmd, { args, stdin, stdout, stderr }) => {
       await child.status
     },
     status: () => child.status,
+    unref: () => child.unref(),
+    ref: () => child.ref(),
   }
 }
 
@@ -273,6 +282,8 @@ const spawnOld: SpawnFn = (cmd, { args, stdin, stdout, stderr }) => {
       child.close()
     },
     status: () => child.status(),
+    unref: () => { },
+    ref: () => { },
   }
 }
 
@@ -328,12 +339,20 @@ const ensureServiceIsRunning = (): Promise<Service> => {
       })
       readMoreStdout()
 
+      let refCount = 0
+      child.unref() // Allow Deno to exit when esbuild is running
+
+      const refs: common.Refs = {
+        ref() { if (++refCount === 1) child.ref(); },
+        unref() { if (--refCount === 0) child.unref(); },
+      }
+
       return {
         build: (options: types.BuildOptions) =>
           new Promise<types.BuildResult>((resolve, reject) => {
             service.buildOrContext({
               callName: 'build',
-              refs: null,
+              refs,
               options,
               isTTY,
               defaultWD,
@@ -345,7 +364,7 @@ const ensureServiceIsRunning = (): Promise<Service> => {
           new Promise<types.BuildContext>((resolve, reject) =>
             service.buildOrContext({
               callName: 'context',
-              refs: null,
+              refs,
               options,
               isTTY,
               defaultWD,
@@ -356,7 +375,7 @@ const ensureServiceIsRunning = (): Promise<Service> => {
           new Promise<types.TransformResult>((resolve, reject) =>
             service.transform({
               callName: 'transform',
-              refs: null,
+              refs,
               input,
               options: options || {},
               isTTY,
@@ -389,7 +408,7 @@ const ensureServiceIsRunning = (): Promise<Service> => {
           new Promise((resolve, reject) =>
             service.formatMessages({
               callName: 'formatMessages',
-              refs: null,
+              refs,
               messages,
               options,
               callback: (err, res) => err ? reject(err) : resolve(res!),
@@ -399,7 +418,7 @@ const ensureServiceIsRunning = (): Promise<Service> => {
           new Promise((resolve, reject) =>
             service.analyzeMetafile({
               callName: 'analyzeMetafile',
-              refs: null,
+              refs,
               metafile: typeof metafile === 'string' ? metafile : JSON.stringify(metafile),
               options,
               callback: (err, res) => err ? reject(err) : resolve(res!),

lib/shared/types.ts

@@ -664,20 +664,24 @@ export let version: string
 
 // Call this function to terminate esbuild's child process. The child process
 // is not terminated and re-created after each API call because it's more
-// efficient to keep it around when there are multiple API calls.
+// efficient to keep it around when there are multiple API calls. This child
+// process normally exits automatically when the parent process exits, so you
+// usually don't need to call this function.
 //
-// In node this happens automatically before the parent node process exits. So
-// you only need to call this if you know you will not make any more esbuild
-// API calls and you want to clean up resources.
-//
-// Unlike node, Deno lacks the necessary APIs to clean up child processes
-// automatically. You must manually call stop() in Deno when you're done
-// using esbuild or Deno will continue running forever.
+// One reason you might want to call this is if you know you will not make any
+// more esbuild API calls and you want to clean up resources (since the esbuild
+// child process takes up some memory even when idle).
 //
 // Another reason you might want to call this is if you are using esbuild from
 // within a Deno test. Deno fails tests that create a child process without
 // killing it before the test ends, so you have to call this function (and
-// await the returned promise) in every Deno test that uses esbuild.
+// await the returned promise) in every Deno test that starts esbuild.
+//
+// You may also start esbuild once at the top level of your test suite (by
+// calling `initialize()`) instead of starting and stopping the esbuild process
+// for every test. This will not interfere with the resource sanitizer, and will
+// improve the efficiency of your tests because the esbuild process can be
+// reused between tests. 
 export declare function stop(): Promise<void>
 
 // Note: These declarations exist to avoid type errors when you omit "dom" from