Skip to content

Commit

Permalink
fix #2720: adapt npm i --no-optional to scopes
Browse files Browse the repository at this point in the history
  • Loading branch information
evanw committed Dec 7, 2022
1 parent 1ae3ca6 commit a2a2dd0
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 3 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,13 @@
# Changelog

## Unreleased

* Fix a regression with `npm install --no-optional` ([#2720](https://github.com/evanw/esbuild/issues/2720))

Normally when you install esbuild with `npm install`, npm itself is the tool that downloads the correct binary executable for the current platform. This happens because of how esbuild's primary package uses npm's `optionalDependencies` feature. However, if you deliberately disable this with `npm install --no-optional` then esbuild's install script will attempt to repair the installation by manually downloading and extracting the binary executable from the package that was supposed to be installed.

The change in version 0.16.0 to move esbuild's nested packages into the `@esbuild/` scope unintentionally broke this logic because of how npm's URL structure is different for scoped packages vs. normal packages. It was actually already broken for a few platforms earlier because esbuild already had packages for some platforms in the `@esbuild/` scope, but I didn't discover this then because esbuild's integration tests aren't run on all platforms. Anyway, this release contains some changes to the install script that should hopefully get this scenario working again.

## 0.16.1

This is a hotfix for the previous release.
Expand Down
2 changes: 1 addition & 1 deletion lib/npm/node-install.ts
Expand Up @@ -197,7 +197,7 @@ function maybeOptimizePackage(binPath: string): void {
async function downloadDirectlyFromNPM(pkg: string, subpath: string, binPath: string): Promise<void> {
// If that fails, the user could have npm configured incorrectly or could not
// have npm installed. Try downloading directly from npm as a last resort.
const url = `https://registry.npmjs.org/${pkg}/-/${pkg}-${ESBUILD_VERSION}.tgz`;
const url = `https://registry.npmjs.org/${pkg}/-/${pkg.replace('@esbuild/', '')}-${ESBUILD_VERSION}.tgz`;
console.error(`[esbuild] Trying to download ${JSON.stringify(url)}`);
try {
fs.writeFileSync(binPath, extractFileFromTarGzip(await fetch(url), subpath));
Expand Down
4 changes: 2 additions & 2 deletions lib/npm/node-platform.ts
Expand Up @@ -99,7 +99,7 @@ function pkgForSomeOtherPlatform(): string | null {

export function downloadedBinPath(pkg: string, subpath: string): string {
const esbuildLibDir = path.dirname(require.resolve('esbuild'));
return path.join(esbuildLibDir, `downloaded-${pkg}-${path.basename(subpath)}`);
return path.join(esbuildLibDir, `downloaded-${pkg.replace('/', '-')}-${path.basename(subpath)}`);
}

export function generateBinPath(): { binPath: string, isWASM: boolean } {
Expand Down Expand Up @@ -261,7 +261,7 @@ for your current platform.`);
'node_modules',
'.cache',
'esbuild',
`pnpapi-${pkg}-${ESBUILD_VERSION}-${path.basename(subpath)}`,
`pnpapi-${pkg.replace('/', '-')}-${ESBUILD_VERSION}-${path.basename(subpath)}`,
);
if (!fs.existsSync(binTargetPath)) {
fs.mkdirSync(path.dirname(binTargetPath), { recursive: true });
Expand Down

0 comments on commit a2a2dd0

Please sign in to comment.