Skip to content

Commit

Permalink
fix stdin with wasm on windows (#687)
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Jan 26, 2021
1 parent caa4316 commit df92f96
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 16 deletions.
4 changes: 4 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ jobs:
if: matrix.os == 'ubuntu-latest'
run: make test-wasm-node

- name: WebAssembly API Tests (node)
if: matrix.os != 'ubuntu-latest'
run: node scripts/wasm-tests.js

- name: Sucrase Tests
if: matrix.os == 'ubuntu-latest'
run: make test-sucrase
Expand Down
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,16 @@

This option was unintentionally broken when the internal JavaScript web worker source code was moved from an inline function to a string in version 0.5.20. The regression has been fixed and the `worker: false` scenario now has test coverage.

* Add a hack for faster command-line execution for the WebAssembly module in certain cases
* Fix using stdin with the `esbuild-wasm` package on Windows ([#687](https://github.com/evanw/esbuild/issues/687))

The package `esbuild-wasm` has an `esbuild` command implemented using WebAssembly instead of using native code. It uses node's WebAssembly implementation and calls methods on node's `fs` module to access the file system.

Node has [an old bug ([nodejs/node#19831](https://github.com/nodejs/node/issues/19831), [nodejs/node#35997](https://github.com/nodejs/node/issues/35997)) where `fs.read` returns an EOF error at the end of stdin on Windows. This causes Go's WebAssembly implementation to panic when esbuild tries to read from stdin.

The workaround was to manually check for this case and then ignore the error in this specific case. With this release, it should now be possible to pipe something to the `esbuild` command on Windows.

* Add a hack for faster command-line execution for the WebAssembly module in certain cases

Node has an unfortunate bug where the node process is unnecessarily kept open while a WebAssembly module is being optimized: https://github.com/nodejs/node/issues/36616. This means cases where running `esbuild` should take a few milliseconds can end up taking many seconds instead.

The workaround is to force node to exit by ending the process early. This is done in one of two ways depending on the exit code. For non-zero exit codes (i.e. when there is a build error), the `esbuild` command now calls `process.kill(process.pid)` to avoid the hang.
Expand Down
17 changes: 6 additions & 11 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@ ESBUILD_VERSION = $(shell cat version.txt)
esbuild: cmd/esbuild/version.go cmd/esbuild/*.go pkg/*/*.go internal/*/*.go go.mod
go build "-ldflags=-s -w" ./cmd/esbuild

npm/esbuild-wasm/esbuild.wasm: cmd/esbuild/version.go cmd/esbuild/*.go pkg/*/*.go internal/*/*.go
cp "$(shell go env GOROOT)/misc/wasm/wasm_exec.js" npm/esbuild-wasm/wasm_exec.js
GOOS=js GOARCH=wasm go build -o npm/esbuild-wasm/esbuild.wasm ./cmd/esbuild

test:
make -j6 test-common

Expand All @@ -32,9 +28,9 @@ vet-go:
fmt-go:
go fmt ./cmd/... ./internal/... ./pkg/...

test-wasm-node: platform-wasm
test-wasm-node: esbuild
PATH="$(shell go env GOROOT)/misc/wasm:$$PATH" GOOS=js GOARCH=wasm go test ./internal/...
npm/esbuild-wasm/bin/esbuild --version
node scripts/wasm-tests.js

test-wasm-browser: platform-wasm | scripts/browser/node_modules
cd scripts/browser && node browser-tests.js
Expand Down Expand Up @@ -144,9 +140,8 @@ platform-linux-mips64le:
platform-linux-ppc64le:
make GOOS=linux GOARCH=ppc64le NPMDIR=npm/esbuild-linux-ppc64le platform-unixlike

platform-wasm: esbuild npm/esbuild-wasm/esbuild.wasm | scripts/node_modules
platform-wasm: esbuild | scripts/node_modules
cd npm/esbuild-wasm && npm version "$(ESBUILD_VERSION)" --allow-same-version
mkdir -p npm/esbuild-wasm/lib
node scripts/esbuild.js ./esbuild --wasm

platform-neutral: esbuild lib-typecheck | scripts/node_modules
Expand Down Expand Up @@ -459,7 +454,7 @@ demo-three-esbuild: esbuild | demo/three
du -h demo/three/esbuild/Three.esbuild.js*
shasum demo/three/esbuild/Three.esbuild.js*

demo-three-eswasm: npm/esbuild-wasm/esbuild.wasm | demo/three
demo-three-eswasm: platform-wasm | demo/three
rm -fr demo/three/eswasm
time -p ./npm/esbuild-wasm/bin/esbuild --bundle --summary --global-name=THREE \
--sourcemap --minify demo/three/src/Three.js --outfile=demo/three/eswasm/Three.eswasm.js
Expand Down Expand Up @@ -548,7 +543,7 @@ bench-three-esbuild: esbuild | bench/three
du -h bench/three/esbuild/entry.esbuild.js*
shasum bench/three/esbuild/entry.esbuild.js*

bench-three-eswasm: npm/esbuild-wasm/esbuild.wasm | bench/three
bench-three-eswasm: platform-wasm | bench/three
rm -fr bench/three/eswasm
time -p ./npm/esbuild-wasm/bin/esbuild --bundle --summary --global-name=THREE \
--sourcemap --minify bench/three/src/entry.js --outfile=bench/three/eswasm/entry.eswasm.js
Expand Down Expand Up @@ -803,7 +798,7 @@ bench-readmin-esbuild: esbuild | bench/readmin
du -h bench/readmin/esbuild/main.js*
shasum bench/readmin/esbuild/main.js*

bench-readmin-eswasm: npm/esbuild-wasm/esbuild.wasm | bench/readmin
bench-readmin-eswasm: platform-wasm | bench/readmin
rm -fr bench/readmin/eswasm
time -p ./npm/esbuild-wasm/bin/esbuild $(READMIN_ESBUILD_FLAGS) --outfile=bench/readmin/eswasm/main.js bench/readmin/repo/src/index.js
echo "$(READMIN_HTML)" > bench/readmin/eswasm/index.html
Expand Down
19 changes: 19 additions & 0 deletions npm/esbuild-wasm/bin/esbuild
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,24 @@ process.on('exit', code => {
}
});

// Node has another bug where using "fs.read" to read from stdin reads
// everything successfully and then throws an error, but only on Windows. Go's
// WebAssembly support uses "fs.read" so it hits this problem. This is a patch
// to try to work around the bug in node. This bug has been reported to node
// at least twice in https://github.com/nodejs/node/issues/35997 and in
// https://github.com/nodejs/node/issues/19831. This issue has also been
// reported to the Go project: https://github.com/golang/go/issues/43913.
const read = fs.read;
fs.read = function () {
const callback = arguments[5];
arguments[5] = function (err, count) {
if (count === 0 && err && err.code === 'EOF') {
arguments[0] = null;
}
return callback.apply(this, arguments);
};
return read.apply(this, arguments);
};

const argv = ['node', wasm_exec, esbuild_wasm].concat(process.argv.slice(2));
wrapper(require, require.main, Object.assign(Object.create(process), { argv }), { instantiate });
24 changes: 20 additions & 4 deletions scripts/esbuild.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const repoDir = path.dirname(__dirname)
const npmDir = path.join(repoDir, 'npm', 'esbuild')
const version = fs.readFileSync(path.join(repoDir, 'version.txt'), 'utf8').trim()

function buildNativeLib(esbuildPath) {
const buildNativeLib = (esbuildPath) => {
const libDir = path.join(npmDir, 'lib')
fs.mkdirSync(libDir, { recursive: true })

Expand Down Expand Up @@ -38,13 +38,26 @@ function buildNativeLib(esbuildPath) {
fs.writeFileSync(path.join(libDir, 'main.d.ts'), types_ts)
}

function buildWasmLib(esbuildPath) {
exports.buildWasmLib = async (esbuildPath) => {
// Asynchronously start building the WebAssembly module
const npmWasmDir = path.join(repoDir, 'npm', 'esbuild-wasm')
const goBuildPromise = new Promise((resolve, reject) => childProcess.execFile('go',
['build', '-o', path.join(npmWasmDir, 'esbuild.wasm'), path.join(repoDir, 'cmd', 'esbuild')],
{ cwd: repoDir, stdio: 'inherit', env: { ...process.env, GOOS: 'js', GOARCH: 'wasm' } },
err => err ? reject(err) : resolve()))

const libDir = path.join(npmWasmDir, 'lib')
const esmDir = path.join(npmWasmDir, 'esm')
fs.mkdirSync(libDir, { recursive: true })
fs.mkdirSync(esmDir, { recursive: true })

// Generate "npm/esbuild-wasm/wasm_exec.js"
const GOROOT = childProcess.execFileSync('go', ['env', 'GOROOT']).toString().trim();
fs.copyFileSync(
path.join(GOROOT, 'misc', 'wasm', 'wasm_exec.js'),
path.join(npmWasmDir, 'wasm_exec.js'),
);

// Generate "npm/esbuild-wasm/lib/main.js"
childProcess.execFileSync(esbuildPath, [
path.join(repoDir, 'lib', 'node.ts'),
Expand Down Expand Up @@ -112,6 +125,9 @@ function buildWasmLib(esbuildPath) {
].concat(minifyFlags), { cwd: repoDir }).toString()
fs.writeFileSync(path.join(esmDir, minify ? 'browser.min.js' : 'browser.js'), browserESM)
}

// Join with the asynchronous WebAssembly build
await goBuildPromise;
}

exports.buildBinary = () => {
Expand Down Expand Up @@ -172,8 +188,8 @@ exports.dirname = __dirname
// The main Makefile invokes this script before publishing
if (require.main === module) {
if (process.argv.indexOf('--wasm') >= 0) {
buildWasmLib(process.argv[2])
exports.buildWasmLib(process.argv[2])
} else {
buildNativeLib(process.argv[2])
exports.buildNativeLib(process.argv[2])
}
}
64 changes: 64 additions & 0 deletions scripts/wasm-tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
const { removeRecursiveSync, buildWasmLib } = require('./esbuild.js');
const child_process = require('child_process');
const assert = require('assert');
const path = require('path');
const fs = require('fs');

const tests = {
basicStdinTest({ testDir, esbuildPath }) {
const stdout = child_process.execFileSync('node', [
esbuildPath,
'--format=cjs',
], {
stdio: ['pipe', 'pipe', 'inherit'],
cwd: testDir,
input: `export default 1+2`,
}).toString();

// Check that the bundle is valid
const module = { exports: {} };
new Function('module', 'exports', stdout)(module, module.exports);
assert.deepStrictEqual(module.exports.default, 3);
},
};

function runTest({ testDir, esbuildPath, test }) {
try {
fs.mkdirSync(testDir, { recursive: true })
test({ testDir, esbuildPath })
return true
} catch (e) {
console.error(`❌ ${test.name} failed: ${e && e.message || e}`)
return false
}
}

async function main() {
// Generate the WebAssembly module
await buildWasmLib(path.join(__dirname, '..', process.platform === 'win32' ? 'esbuild.exe' : 'esbuild'));

const esbuildPath = path.join(__dirname, '..', 'npm', 'esbuild-wasm', 'bin', 'esbuild');
const testDir = path.join(__dirname, '.wasm-tests')

// Run all tests in serial because WebAssembly compilation is a CPU hog
let allTestsPassed = true;
for (const test in tests) {
if (!runTest({
testDir: path.join(testDir, test),
test: tests[test],
esbuildPath,
})) {
allTestsPassed = false;
}
}

if (!allTestsPassed) {
console.error(`❌ wasm-tests failed`)
process.exit(1)
} else {
console.log(`✅ wasm-tests passed`)
removeRecursiveSync(testDir)
}
}

main().catch(e => setTimeout(() => { throw e }))

0 comments on commit df92f96

Please sign in to comment.