Work around issues with Deno 1.31+ #3685
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 ofDeno.Command
. As part of this change, esbuild is now calling the newunref
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'sstop()
function to terminate the child process for Deno to be able to exit).However, this introduced a problem for Deno's testing API which now fails some tests that use esbuild with
error: Promise resolution is still pending but the event loop has already resolved
. It's unclear to me why this is happening. The call tounref
was recommended by someone on the Deno core team, and calling Node's equivalentunref
API has been working fine for esbuild in Node for a long time. It could be that I'm using it incorrectly, or that there's some reference counting and/or garbage collection bug in Deno's internals, or that Deno'sunref
just works differently than Node'sunref
. In any case, it's not good for Deno tests that use esbuild to be failing.In this PR, I am removing the call to
unref
to fix this issue. This means that you will now have to call esbuild'sstop()
function to allow Deno to exit, just like you did before esbuild version 0.20.0 when this regression was introduced.Note: This regression wasn't caught earlier because Deno doesn't seem to fail tests that have outstanding
setTimeout
calls, which esbuild's test harness was using to enforce a maximum test runtime. Adding asetTimeout
was allowing esbuild's Deno tests to succeed. So this regression doesn't necessarily apply to all people using tests in Deno.Fixes #3682