Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esbuild error on WSL (Ubuntu 20.04.1 LTS - Windows 10) / via yarn 2 #467

Closed
rizrmd opened this issue Oct 17, 2020 · 13 comments · Fixed by #494
Closed

esbuild error on WSL (Ubuntu 20.04.1 LTS - Windows 10) / via yarn 2 #467

rizrmd opened this issue Oct 17, 2020 · 13 comments · Fixed by #494

Comments

@rizrmd
Copy link

rizrmd commented Oct 17, 2020

After creating fresh yarn 2 project. install esbuild as usual, and then run it:

/home/riz/Projects/test-esbuild/.yarn/unplugged/esbuild-npm-0.7.16-564ecde0e0/node_modules/esbuild/bin/esbuild:1
ELF
^

SyntaxError: Invalid or unexpected token
    at wrapSafe (internal/modules/cjs/loader.js:979:16)
    at Module._compile (internal/modules/cjs/loader.js:1027:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.external_module_.Module._load (/home/riz/Projects/test-esbuild/.pnp.js:4749:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
    at internal/main/run_main_module.js:17:47
@rizrmd
Copy link
Author

rizrmd commented Oct 17, 2020

Strangely it works when I install esbuild-windows-64, WSL 2 is supposed to run native linux binary...

@evanw
Copy link
Owner

evanw commented Oct 17, 2020

I don't use yarn myself, but it appears to me that however you are running it is causing the binary executable to be interpreted as JavaScript code.

I once came across a strange page on the yarn repo that seemed to indicate that yarn 2 is deliberately incompatible with binary executables: yarnpkg/berry#882. This seems kind of unbelievable to me so I'm not sure if it's true. But could that be the case here?

If so, it could be that you could get it to work by changing how you are invoking it perhaps (no ideas for what to try, sorry), or by using esbuild's JavaScript API instead of the command-line interface. You could also consider using a different package manager if that still doesn't work.

If not, you could try providing more details here about how exactly you are running esbuild in case that makes something stand out.

Edit: Actually it looks like a similar issue has already been filed here in the past: #237. That's where I learned about this from. That thread may also have some useful information.

@arcanis
Copy link

arcanis commented Oct 27, 2020

I once came across a strange page on the yarn repo that seemed to indicate that yarn 2 is deliberately incompatible with binary executables: yarnpkg/berry#882.

Correct - in practice exceedingly few packages directly use binaries in their bin fields, and doing so has various issues (such as requiring an heuristic to detect whether a file is binary or not, which is slow and/or pretty unreliable), so we deprecated this behaviour in the v2 release. Our recommendation in this kind of case would probably be to simply make the entry point a bridge to the binary you intend to execute. Something like this:

const {spawnSync} = require(`child_process`);
const {status} = spawnSync(`${__dirname}/file.exe`, [], {stdio: `inherit`});
process.exitCode = status || 1;

We could reconsider, but to be honest I believe this is the first package that got hurt by this behaviour in the past year, so I'm not convinced restoring the heuristic would be worth it. Would the bridge I shared be an acceptable solution on your side?

@evanw
Copy link
Owner

evanw commented Oct 28, 2020

Thanks for the context. I deliberately did not add a bridge file because node has additional startup overhead and I'm trying to keep esbuild as fast and lightweight as possible.

I don't think it's appropriate to slow down esbuild for everyone just because Yarn 2 doesn't support this. Other package managers support this fine (e.g. npm, pnpm, Yarn 1). Since npm is the official package manager for node, it's also my officially-supported package manager for esbuild. People can use other package managers if they want to but they may have issues to the extent that those package managers aren't compatible with npm/node. For example, esbuild will not include support for Yarn 2's custom path resolution since that's not part of how npm/node works.

I'll add a note to the docs about Yarn 2 not supporting invoking esbuild directly. As you said, it's easy enough to work around for people that need to use Yarn 2 for other reasons.

@evanw
Copy link
Owner

evanw commented Oct 28, 2020

One random idea I had: in the install script I could try to detect that esbuild is being installed via Yarn 2 and attempt to work around this issue by generating the shim only for Yarn 2. Is there a way to tell if a package is being installed by Yarn 2?

@arcanis
Copy link

arcanis commented Oct 28, 2020

Hm that should be possible - the $npm_config_user_agent environment variable can tell you what package manager is running, so a slight pattern matching can yield you the right result (just be careful to have this behavior for all 2+ versions, since we'll probably jump to 3.x early next year):

yarn/2.3.3 npm/? node/12.16.0 darwin x6

@merceyz
Copy link

merceyz commented Oct 28, 2020

I deliberately did not add a bridge file because node has additional startup overhead and I'm trying to keep esbuild as fast and lightweight as possible.

Just for my own curiosity: I see you do this on Windows, any specific reason for that?

@arcanis
Copy link

arcanis commented Oct 28, 2020

Btw, slightly off-topic, while I understand why you wouldn't want to add builtin support for PnP, do you think it would be possible to add a message if a bare identifier resolution fails, no resolver plugin is configured, and you detect the existence of a .pnp.* file? Something like this:

ESBuild only supports the native Node resolution by default. Please read the following document
to learn how to add support for Plug'n'Play installs: https://esbuild.org/some-url

This would avoid people having to Google-search the solution, since they'll have to solve this anyway one way or another 🤔

@rtsao
Copy link
Contributor

rtsao commented Oct 30, 2020

I wonder if berry could support native executables on an opt-in basis using an inexpensive heuristic such as file extension. It might be a somewhat strange and possibly confusing constraint, but at the same time it's likely better than the current hard requirement of JS bin entries. There'd be potentially less overhead and it might lessen the burden for packages like esbuild to conform to the requirements of berry.

@arcanis
Copy link

arcanis commented Oct 30, 2020

I'm not sure it would help since in this case the intent is to have either a JS or a native binary depending on the circumstances, so the extension would necessarily be the same 🤔

I'll keep this issue in mind, perhaps we'll find a way to safely relax this requirement. It got implemented extremely early on (part of the first thirteen commits of the new Yarn codebase), and while I don't exactly remember why I made it this way it was probably more accidental than really on purpose.

@evanw evanw closed this as completed in #494 Nov 3, 2020
@evanw
Copy link
Owner

evanw commented Nov 3, 2020

This should work now as of version 0.8.3.

@sezze
Copy link

sezze commented Jun 7, 2022

I appear to now have this exact issue with regular NPM. Otherwise same configuration and same output.

> esbuild ./js/index.ts --bundle --outfile=./wwwroot/js/dist/index.js --minify

ELF☻☺☺
^

SyntaxError: Invalid or unexpected token
    at Object.compileFunction (node:vm:352:18)
    at wrapSafe (node:internal/modules/cjs/loader:1033:15)
    at Module._compile (node:internal/modules/cjs/loader:1069:27)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12)
    at node:internal/main/run_main_module:17:47
npm run build:esbuild exited with code 1

I worked around it by creating a js wrapper with child_process.spawn, but that isn't ideal.

@arcanis
Copy link

arcanis commented Jun 17, 2023

I once came across a strange page on the yarn repo that seemed to indicate that yarn 2 is deliberately incompatible with binary executables: yarnpkg/berry#882. This seems kind of unbelievable to me so I'm not sure if it's true. But could that be the case here?

Hey @evanw, I was just thinking about this issue. Would you be interested if Yarn supported directly spawning binaries if their extension is .bin or .exe (or something else)? That heuristic wouldn't have negative perf impact on us, and should be relatively easy to implement for packages (since the file path doesn't have to match the exposed binary name anyway).

(Note that we can't require the .cjs / .js extension for JS files - extensionless JS bins are very common in the ecosystem, whereas native bins aren't as much - in part because Yarn 2+ doesn't support them very well at the moment)

[edit] We're also discussing internally whether we could just read the first few bytes and check for its magic number / shebang line. That would avoid you having to make any change at all, provided the perf overhead is reasonable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants