Skip to content

Commit

Permalink
work around api deprecations in deno 1.40.x
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Jan 26, 2024
1 parent f8ec300 commit 3b499bf
Show file tree
Hide file tree
Showing 8 changed files with 163 additions and 44 deletions.
42 changes: 36 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,10 @@ jobs:
with:
node-version: 16

# The version of Deno is pinned because version 1.25.1 was causing test
# flakes due to random segfaults.
- name: Setup Deno 1.24.0
- name: Setup Deno 1.40.0
uses: denoland/setup-deno@main
with:
deno-version: v1.24.0
deno-version: v1.40.0

- name: Check out code into the Go module directory
uses: actions/checkout@v3
Expand Down Expand Up @@ -199,8 +197,8 @@ jobs:
make test-yarnpnp
esbuild-old-versions:
name: esbuild CI (old versions)
esbuild-old-go-version:
name: esbuild CI (old Go version)
runs-on: ubuntu-latest

steps:
Expand All @@ -221,3 +219,35 @@ jobs:

- name: make test-old-ts
run: make test-old-ts

esbuild-old-deno-version:
name: esbuild CI (old Deno version)
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest, macos-latest, windows-latest]

steps:
- name: Set up Go 1.x
uses: actions/setup-go@v3
with:
go-version: 1.20.12
id: go

# Make sure esbuild works with old versions of Deno. Note: It's important
# to test a version before 1.31.0, which introduced the "Deno.Command" API.
- name: Setup Deno 1.24.0
uses: denoland/setup-deno@main
with:
deno-version: v1.24.0

- name: Check out code into the Go module directory
uses: actions/checkout@v3

- name: Deno Tests (non-Windows)
if: matrix.os != 'windows-latest'
run: make test-deno

- name: Deno Tests (Windows)
if: matrix.os == 'windows-latest'
run: make test-deno-windows
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Changelog

## Unreleased

* Work around API deprecations in Deno 1.40.x ([#3609](https://github.com/evanw/esbuild/issues/3609))

Deno 1.40.0 introduced run-time warnings about certain APIs that esbuild uses. With this release, esbuild will work around these run-time warnings by using other APIs if they are present, and falling back to the original APIs otherwise. This should avoid the warnings without breaking compatibility with older versions of Deno.

Unfortunately, doing this introduces a breaking change. The other child process APIs seem to lack a way to synchronously terminate esbuild's child process, which causes Deno to now fail tests with a confusing error about a thing called `op_spawn_wait` in Deno's internals. To work around this, esbuild's `stop()` function has been changed to return a promise, and you now have to change `esbuild.stop()` to `await esbuild.stop()` in all of your Deno tests.

## 0.19.12

* The "preserve" JSX mode now preserves JSX text verbatim ([#3605](https://github.com/evanw/esbuild/issues/3605))
Expand Down
146 changes: 112 additions & 34 deletions lib/deno/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ export const analyzeMetafileSync: typeof types.analyzeMetafileSync = () => {
throw new Error(`The "analyzeMetafileSync" API does not work in Deno`)
}

export const stop = () => {
if (stopService) stopService()
export const stop = async () => {
if (stopService) await stopService()
}

let initializeWasCalled = false
Expand Down Expand Up @@ -178,63 +178,142 @@ interface Service {

let defaultWD = Deno.cwd()
let longLivedService: Promise<Service> | undefined
let stopService: (() => void) | undefined
let stopService: (() => Promise<void>) | undefined

// Declare a common subprocess API for the two implementations below
type SpawnFn = (cmd: string, options: {
args: string[]
stdin: 'piped' | 'inherit'
stdout: 'piped' | 'inherit'
stderr: 'inherit'
}) => {
stdin: {
write(bytes: Uint8Array): void
close(): void
}
stdout: {
read(): Promise<Uint8Array | null>
close(): void
}
close(): Promise<void> | void
status(): Promise<{ code: number }>
}

// Deno ≥1.40
const spawnNew: SpawnFn = (cmd, { args, stdin, stdout, stderr }) => {
const child = new Deno.Command(cmd, {
args,
cwd: defaultWD,
stdin,
stdout,
stderr,
}).spawn()
const writer = child.stdin.getWriter()
const reader = child.stdout.getReader()
return {
stdin: {
write: bytes => writer.write(bytes),
close: () => writer.close(),
},
stdout: {
read: () => reader.read().then(x => x.value || null),
close: () => reader.cancel(),
},
close: async () => {
// Note: This can throw with EPERM on Windows (happens in GitHub Actions)
child.kill()

// Without this, Deno will fail tests with some weird error about "op_spawn_wait"
await child.status
},
status: () => child.status,
}
}

// Deno <1.40
const spawnOld: SpawnFn = (cmd, { args, stdin, stdout, stderr }) => {
const child = Deno.run({
cmd: [cmd].concat(args),
cwd: defaultWD,
stdin,
stdout,
stderr,
})
const stdoutBuffer = new Uint8Array(4 * 1024 * 1024)
let writeQueue: Uint8Array[] = []
let isQueueLocked = false

// We need to keep calling "write()" until it actually writes the data
const startWriteFromQueueWorker = () => {
if (isQueueLocked || writeQueue.length === 0) return
isQueueLocked = true
child.stdin!.write(writeQueue[0]).then(bytesWritten => {
isQueueLocked = false
if (bytesWritten === writeQueue[0].length) writeQueue.shift()
else writeQueue[0] = writeQueue[0].subarray(bytesWritten)
startWriteFromQueueWorker()
})
}

return {
stdin: {
write: bytes => {
writeQueue.push(bytes)
startWriteFromQueueWorker()
},
close: () => child.stdin!.close(),
},
stdout: {
read: () => child.stdout!.read(stdoutBuffer).then(n => n === null ? null : stdoutBuffer.subarray(0, n)),
close: () => child.stdout!.close(),
},
close: () => child.close(),
status: () => child.status(),
}
}

let ensureServiceIsRunning = (): Promise<Service> => {
// This is a shim for "Deno.run" for newer versions of Deno
const spawn: SpawnFn = Deno.Command ? spawnNew : spawnOld

const ensureServiceIsRunning = (): Promise<Service> => {
if (!longLivedService) {
longLivedService = (async (): Promise<Service> => {
const binPath = await install()
const isTTY = Deno.isatty(Deno.stderr.rid)
const isTTY = Deno.stderr.isTerminal
? Deno.stderr.isTerminal() // Deno ≥1.40
: Deno.isatty(Deno.stderr.rid) // Deno <1.40

const child = Deno.run({
cmd: [binPath, `--service=${version}`],
cwd: defaultWD,
const child = spawn(binPath, {
args: [`--service=${version}`],
stdin: 'piped',
stdout: 'piped',
stderr: 'inherit',
})

stopService = () => {
stopService = async () => {
// Close all resources related to the subprocess.
child.stdin.close()
child.stdout.close()
child.close()
await child.close()
initializeWasCalled = false
longLivedService = undefined
stopService = undefined
}

let writeQueue: Uint8Array[] = []
let isQueueLocked = false

// We need to keep calling "write()" until it actually writes the data
const startWriteFromQueueWorker = () => {
if (isQueueLocked || writeQueue.length === 0) return
isQueueLocked = true
child.stdin.write(writeQueue[0]).then(bytesWritten => {
isQueueLocked = false
if (bytesWritten === writeQueue[0].length) writeQueue.shift()
else writeQueue[0] = writeQueue[0].subarray(bytesWritten)
startWriteFromQueueWorker()
})
}

const { readFromStdout, afterClose, service } = common.createChannel({
writeToStdin(bytes) {
writeQueue.push(bytes)
startWriteFromQueueWorker()
child.stdin.write(bytes)
},
isSync: false,
hasFS: true,
esbuild: ourselves,
})

const stdoutBuffer = new Uint8Array(4 * 1024 * 1024)
const readMoreStdout = () => child.stdout.read(stdoutBuffer).then(n => {
if (n === null) {
const readMoreStdout = () => child.stdout.read().then(buffer => {
if (buffer === null) {
afterClose(null)
} else {
readFromStdout(stdoutBuffer.subarray(0, n))
readFromStdout(buffer)
readMoreStdout()
}
}).catch(e => {
Expand Down Expand Up @@ -331,9 +410,8 @@ let ensureServiceIsRunning = (): Promise<Service> => {

// If we're called as the main script, forward the CLI to the underlying executable
if (import.meta.main) {
Deno.run({
cmd: [await install()].concat(Deno.args),
cwd: defaultWD,
spawn(await install(), {
args: Deno.args,
stdin: 'inherit',
stdout: 'inherit',
stderr: 'inherit',
Expand Down
1 change: 1 addition & 0 deletions lib/deno/wasm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const analyzeMetafileSync: typeof types.analyzeMetafileSync = () => {

export const stop = () => {
if (stopService) stopService()
return Promise.resolve()
}

interface Service {
Expand Down
1 change: 1 addition & 0 deletions lib/npm/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export const analyzeMetafileSync: typeof types.analyzeMetafileSync = () => {

export const stop = () => {
if (stopService) stopService()
return Promise.resolve()
}

interface Service {
Expand Down
1 change: 1 addition & 0 deletions lib/npm/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ export let analyzeMetafileSync: typeof types.analyzeMetafileSync = (metafile, op
export const stop = () => {
if (stopService) stopService()
if (workerThreadService) workerThreadService.stop()
return Promise.resolve()
}

let initializeWasCalled = false
Expand Down
2 changes: 1 addition & 1 deletion lib/shared/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -673,4 +673,4 @@ export let version: string
// 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.
export declare function stop(): void;
export declare function stop(): Promise<void>
6 changes: 3 additions & 3 deletions scripts/deno-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function test(name, backends, fn) {
await fn({ esbuild: esbuildNative, testDir })
await Deno.remove(testDir, { recursive: true }).catch(() => null)
} finally {
esbuildNative.stop()
await esbuildNative.stop()
}
})
break
Expand All @@ -51,7 +51,7 @@ function test(name, backends, fn) {
await fn({ esbuild: esbuildWASM, testDir })
await Deno.remove(testDir, { recursive: true }).catch(() => null)
} finally {
esbuildWASM.stop()
await esbuildWASM.stop()
}
})
break
Expand All @@ -65,7 +65,7 @@ function test(name, backends, fn) {
await fn({ esbuild: esbuildWASM, testDir })
await Deno.remove(testDir, { recursive: true }).catch(() => null)
} finally {
esbuildWASM.stop()
await esbuildWASM.stop()
}
})
break
Expand Down

0 comments on commit 3b499bf

Please sign in to comment.