Skip to content

Commit

Permalink
fix #2558: remove esbuild-wasm package nesting
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Sep 22, 2022
1 parent a333130 commit 758d4e1
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 23 deletions.
8 changes: 8 additions & 0 deletions .gitignore
Expand Up @@ -6,6 +6,14 @@
/deno/
/esbuild
/github/
/npm/@esbuild/android-arm/esbuild.wasm
/npm/@esbuild/android-arm/exit0.js
/npm/@esbuild/android-arm/wasm_exec_node.js
/npm/@esbuild/android-arm/wasm_exec.js
/npm/esbuild-android-64/esbuild.wasm
/npm/esbuild-android-64/exit0.js
/npm/esbuild-android-64/wasm_exec_node.js
/npm/esbuild-android-64/wasm_exec.js
/npm/esbuild-wasm/browser.js
/npm/esbuild-wasm/esbuild.wasm
/npm/esbuild-wasm/esm/
Expand Down
14 changes: 14 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,19 @@
# Changelog

## Unreleased

* Fix an obscure npm package installation issue with `--omit=optional` ([#2558](https://github.com/evanw/esbuild/issues/2558))

The previous release introduced a regression with `npm install esbuild --omit=optional` where the file `node_modules/.bin/esbuild` would no longer be present after installation. That could cause any package scripts which used the `esbuild` command to no longer work. This release fixes the regression so `node_modules/.bin/esbuild` should now be present again after installation. This regression only affected people installing esbuild using `npm` with either the `--omit=optional` or `--no-optional` flag, which is a somewhat unusual situation.

More details:

The reason for this regression is due to some obscure npm implementation details. Since the Go compiler doesn't support trivial cross-compiling on certain Android platforms, esbuild's installer installs a WebAssembly shim on those platforms instead. In the previous release I attempted to simplify esbuild's WebAssembly shims to depend on the `esbuild-wasm` package instead of including another whole copy of the WebAssembly binary (to make publishing faster and to save on file system space after installation). However, both the `esbuild` package and the `esbuild-wasm` package provide a binary called `esbuild` and it turns out that adding `esbuild-wasm` as a nested dependency of the `esbuild` package (specifically `esbuild` optionally depends on `@esbuild/android-arm` which depends on `esbuild-wasm`) caused npm to be confused about what `node_modules/.bin/esbuild` is supposed to be.

It's pretty strange and unexpected that disabling the installation of optional dependencies altogether would suddenly cause an optional dependency's dependency to conflict with the top-level package. What happens under the hood is that if `--omit=optional` is present, npm attempts to uninstall the `esbuild-wasm` nested dependency at the end of `npm install` (even though the `esbuild-wasm` package was never installed due to `--omit=optional`). This uninstallation causes `node_modules/.bin/esbuild` to be deleted.

After doing a full investigation, I discovered that npm's handling of the `.bin` directory is deliberately very brittle. When multiple packages in the dependency tree put something in `.bin` with the same name, the end result is non-deterministic/random. What you get in `.bin` might be from one package, from the other package, or might be missing entirely. The workaround suggested by npm is to just avoid having two packages that put something in `.bin` with the same name. So this was fixed by making the `@esbuild/android-arm` and `esbuild-android-64` packages each include another whole copy of the WebAssembly binary, which works because these packages don't put anything in `.bin`.

## 0.15.8

* Fix JSX name collision edge case ([#2534](https://github.com/evanw/esbuild/issues/2534))
Expand Down
10 changes: 7 additions & 3 deletions Makefile
Expand Up @@ -299,10 +299,10 @@ platform-unixlike: version-go
node scripts/esbuild.js "$(NPMDIR)/package.json" --version
CGO_ENABLED=0 GOOS="$(GOOS)" GOARCH="$(GOARCH)" go build $(GO_FLAGS) -o "$(NPMDIR)/bin/esbuild" ./cmd/esbuild

platform-android-x64:
platform-android-x64: platform-wasm
node scripts/esbuild.js npm/esbuild-android-64/package.json --version

platform-android-arm:
platform-android-arm: platform-wasm
node scripts/esbuild.js npm/@esbuild/android-arm/package.json --version

platform-android-arm64:
Expand Down Expand Up @@ -533,7 +533,9 @@ validate-build:
# This checks that the published binaries are bitwise-identical to the locally-build binaries
validate-builds:
git fetch --all --tags && git checkout "v$(ESBUILD_VERSION)"
@$(MAKE) --no-print-directory TARGET=platform-android-arm SCOPE=@esbuild/ PACKAGE=android-arm SUBPATH=esbuild.wasm validate-build
@$(MAKE) --no-print-directory TARGET=platform-android-arm64 PACKAGE=esbuild-android-arm64 SUBPATH=bin/esbuild validate-build
@$(MAKE) --no-print-directory TARGET=platform-android-x64 PACKAGE=esbuild-android-64 SUBPATH=esbuild.wasm validate-build
@$(MAKE) --no-print-directory TARGET=platform-darwin-arm64 PACKAGE=esbuild-darwin-arm64 SUBPATH=bin/esbuild validate-build
@$(MAKE) --no-print-directory TARGET=platform-darwin-x64 PACKAGE=esbuild-darwin-64 SUBPATH=bin/esbuild validate-build
@$(MAKE) --no-print-directory TARGET=platform-freebsd-arm64 PACKAGE=esbuild-freebsd-arm64 SUBPATH=bin/esbuild validate-build
Expand All @@ -557,12 +559,14 @@ validate-builds:

clean:
rm -f esbuild
rm -f npm/esbuild-wasm/esbuild.wasm npm/esbuild-wasm/wasm_exec.js npm/esbuild-wasm/wasm_exec_node.js npm/esbuild-wasm/exit0.js
rm -f npm/esbuild-wasm/esbuild.wasm npm/esbuild-wasm/wasm_exec*.js npm/esbuild-wasm/exit0.js
rm -f npm/esbuild-windows-32/esbuild.exe
rm -f npm/esbuild-windows-64/esbuild.exe
rm -f npm/esbuild-windows-arm64/esbuild.exe
rm -f npm/esbuild/install.js
rm -rf npm/@esbuild/linux-loong64/bin
rm -rf npm/esbuild-android-64/bin npm/esbuild-android-64/esbuild.wasm npm/esbuild-android-64/wasm_exec*.js npm/esbuild-android-64/exit0.js
rm -rf npm/esbuild-android-arm/bin npm/esbuild-android-arm/esbuild.wasm npm/esbuild-android-arm/wasm_exec*.js npm/esbuild-android-arm/exit0.js
rm -rf npm/esbuild-android-arm64/bin
rm -rf npm/esbuild-darwin-64/bin
rm -rf npm/esbuild-darwin-arm64/bin
Expand Down
2 changes: 0 additions & 2 deletions npm/@esbuild/android-arm/bin/esbuild

This file was deleted.

3 changes: 0 additions & 3 deletions npm/@esbuild/android-arm/package.json
Expand Up @@ -8,9 +8,6 @@
"engines": {
"node": ">=12"
},
"dependencies": {
"esbuild-wasm": "0.15.8"
},
"os": [
"android"
],
Expand Down
2 changes: 0 additions & 2 deletions npm/esbuild-android-64/bin/esbuild

This file was deleted.

3 changes: 0 additions & 3 deletions npm/esbuild-android-64/package.json
Expand Up @@ -8,9 +8,6 @@
"engines": {
"node": ">=12"
},
"dependencies": {
"esbuild-wasm": "0.15.8"
},
"os": [
"android"
],
Expand Down
23 changes: 13 additions & 10 deletions scripts/esbuild.js
Expand Up @@ -208,6 +208,19 @@ module.exports = ${JSON.stringify(exit0Map, null, 2)};

// Join with the asynchronous WebAssembly build
await goBuildPromise;

// Also copy this into the WebAssembly shim directories
for (const dir of [
path.join(repoDir, 'npm', '@esbuild', 'android-arm'),
path.join(repoDir, 'npm', 'esbuild-android-64'),
]) {
fs.mkdirSync(path.join(dir, 'bin'), { recursive: true })
fs.writeFileSync(path.join(dir, 'wasm_exec.js'), wasm_exec_js);
fs.writeFileSync(path.join(dir, 'wasm_exec_node.js'), wasm_exec_node_js);
fs.writeFileSync(path.join(dir, 'exit0.js'), exit0Code);
fs.copyFileSync(path.join(npmWasmDir, 'bin', 'esbuild'), path.join(dir, 'bin', 'esbuild'));
fs.copyFileSync(path.join(npmWasmDir, 'esbuild.wasm'), path.join(dir, 'esbuild.wasm'));
}
}

const buildDenoLib = (esbuildPath) => {
Expand Down Expand Up @@ -293,19 +306,9 @@ exports.removeRecursiveSync = path => {
const updateVersionPackageJSON = pathToPackageJSON => {
const version = fs.readFileSync(path.join(path.dirname(__dirname), 'version.txt'), 'utf8').trim()
const json = JSON.parse(fs.readFileSync(pathToPackageJSON, 'utf8'))
let changed = false

if (json.version !== version) {
json.version = version
changed = true
}

if ('dependencies' in json && 'esbuild-wasm' in json.dependencies && json.dependencies['esbuild-wasm'] !== version) {
json.dependencies['esbuild-wasm'] = version
changed = true
}

if (changed) {
fs.writeFileSync(pathToPackageJSON, JSON.stringify(json, null, 2) + '\n')
}
}
Expand Down

0 comments on commit 758d4e1

Please sign in to comment.