Wasm build doesn't work on Windows

[arcanis]

Running a build on Windows doesn't work when using the esbuild-wasm package. It works fine when using esbuild (but we'd prefer to avoid that since it requires a postinstall script).

Repro:

git clone git@github.com:yarnpkg/berry && cd berry
git checkout mael/esbuild-2
yarn build:cli

Error:

error: Invalid path: C:\berry\packages\yarnpkg-cli\bundles\yarn.js
    at failureErrorWithLog (C:\berry\.yarn\cache\esbuild-wasm-npm-0.8.33-0589ac914f-8d0542a70c.zip\node_modules\esbuild-wasm\lib\main.js:969:15)
    at buildResponseToResult (C:\berry\.yarn\cache\esbuild-wasm-npm-0.8.33-0589ac914f-8d0542a70c.zip\node_modules\esbuild-wasm\lib\main.js:767:32)
    at C:\berry\.yarn\cache\esbuild-wasm-npm-0.8.33-0589ac914f-8d0542a70c.zip\node_modules\esbuild-wasm\lib\main.js:819:20
    at handleIncomingPacket (C:\berry\.yarn\cache\esbuild-wasm-npm-0.8.33-0589ac914f-8d0542a70c.zip\node_modules\esbuild-wasm\lib\main.js:566:9)

[evanw]

Ah, I think I understand why. It looks like perhaps Go's path internals are locked to Unix-style paths when using WebAssembly. Specifically, building with WebAssembly sets GOOS=js and I think Go's path/filepath module only uses Windows-style paths when GOOS=windows. I clearly haven't ever tried this before. This appears to be a shortcoming of Go. I'll ask around to see what I should do here. I'd certainly like for this to work.

FWIW the WebAssembly version of esbuild is mainly intended to be used in the browser where running a native executable is impossible. The native version is intended to be used outside of the browser. The native version is around 10x faster than the WebAssembly version for various reasons. I'm not saying you have to switch away from the WebAssembly version, especially if I can figure out how to fix this problem. I'm just telling you that it'll go a lot faster if you do.

[arcanis]

Yep - our Webpack build was 28s, with esbuild-wasm it's 5s, and with esbuild it's 750ms. Still, not having to run yarn install when cloning the repo or switching between branches is a significant advantage, so for now we'll likely stay on the wasm version.

Fwiw I'm also thinking about adding proper builtin support for multi-platform packages into Yarn - similar to what you're mimicking with the "download the right package on-demand" postinstall script, but more integrated with the package manager infrastructure. If that works out native builds could be a viable option.

[kzc]

Off topic... comment from the cross linked yarn PR above:

Switches the build to ESBuild

It used to take 28s to build the CLI. With this diff it becomes 6s (on the other hand, went 1.62MB -> 1.92MB).

The development iteration speed improvement is fantastic and certainly justifies the change, but I wonder what accounts for the 300KB bundle size increase with esbuild compared to webpack+terser. An optimization opportunity for esbuild perhaps?

[evanw]

The development iteration speed improvement is fantastic and certainly justifies the change, but I wonder what accounts for the 300KB bundle size increase with esbuild compared to webpack+terser. An optimization opportunity for esbuild perhaps?

When I read the thread it looked like this might be more about which code was selected for bundling instead of the result of minifier optimizations: yarnpkg/berry#2362 (comment). Not sure if that's still current or not though.

[arcanis]

When I read the thread it looked like this might be more about which code was selected for bundling instead of the result of minifier optimizations: yarnpkg/berry#2362 (comment). Not sure if that's still current or not though.

I've managed to bring the size down to 1.83MB (don't remember how), but I couldn't go lower. The size seems the same whether I have platform: "node" set or not (in both cases esprima is missing from the bundle, as wanted).

[evanw]

I took a look at fixing this and it looks like this is an inherently buggy scenario. I've been trying all day and I still haven't gotten it working yet, but I've already hit these issues:

1. path/filepath: WebAssembly uses *nix conventions on Windows golang/go#43768: Go's path/filepath module is broken on Windows in WebAssembly.

   To fix this, I can fork Go's path/filepath module and include both the Unix and Windows implementations and then switch between them at run-time.

2. fs.readSync throws EOF Error instead of returning 0 (Windows, if STDIN is a pipe) nodejs/node#35997: Node's fs.read throws an error at the end of a pipe on Windows, meaning passing stdin to esbuild with WebAssembly is broken on Windows.

   I should be able to monkey-patch fs.read to swallow the incorrect error on Windows.

3. tiered WebAssembly compilation broken nodejs/node#36616: Node's unfortunate handling of WebAssembly compilation can make node temporarily hang on exit, which means the WebAssembly-powered esbuild command often takes 10x longer than necessary.

   You can almost do process.kill(process.pid) except the exit code will be non-zero, which isn't right. So I was thinking of trying a native node extension that calls _exit(0) instead.

I'm hoping that these are the main issues with the esbuild-wasm package on Windows.

[arcanis]

I seem to remember that the error is triggered by a Abs validation in esbuild - would it be possible to disable those checks in wasm mode, and just assume that all paths are always absolute?

[kzc]

So I was thinking of trying a native node extension that calls _exit(0) instead.

I would advise against this. Native compilation defeats the purpose of using wasm on NodeJS. It doesn't work on all platforms, like the older version of Mac OS I use which cannot build fsevents natively.

[kzc]

After reviewing 9ea2076 I can report that npm/esbuild-wasm/exit0/darwin-x64-LE.node works correctly on mac OS 10.9. The prebuilt zero dependency wasm-napi-exit0 dynamic libraries are a good workaround.

[evanw]

I would advise against this. Native compilation defeats the purpose of using wasm on NodeJS. It doesn't work on all platforms, like the older version of Mac OS I use which cannot build fsevents natively.

I figured it's harmless if it doesn't work for whatever reason, because node is exiting anyway and any errors running the thing are ignored. Worst case node will just exit many seconds later as usual.

Edit: Here are the times on my end for running the WebAssembly esbuild --version command:

	Before	After	Improvement
macOS 10.15	2.900s	0.153s	19x faster
Windows 10 VM	16.164s	0.397s	41x faster

[kzc]

Worst case node will just exit many seconds later as usual

On my Mac I see a roughly 0.4s improvement with esbuild-wasm with 9ea2076...

before 9ea2076:

$ /usr/bin/time npm/esbuild-wasm/bin/esbuild --version
0.8.34
        0.62 real         1.88 user         0.19 sys

$ echo 'console.log(1 + 2)' | /usr/bin/time npm/esbuild-wasm/bin/esbuild --minify
console.log(1+2);
        0.60 real         1.86 user         0.20 sys

after 9ea2076:

$ /usr/bin/time npm/esbuild-wasm/bin/esbuild --version
0.8.34
        0.22 real         0.68 user         0.06 sys

$ echo 'console.log(1 + 2)' | /usr/bin/time npm/esbuild-wasm/bin/esbuild --minify
console.log(1+2);
        0.23 real         0.69 user         0.07 sys

[kzc]

macOS 10.15 2.900s 0.153s 19x faster
Windows 10 VM 16.164s 0.397s 41x faster

It's hard to believe this NodeJS bug has gone unnoticed given its performance on recent OS versions. Wasm modules must not be widely used.

[evanw]

Ugh, another problem: nodejs/node#24550. Unicode output doesn't work on Windows with fs.write, only with console.log. This also affects Go's WebAssembly support because they use fs.write.

Edit: I filed this as golang/go#43917.

[evanw]

All of the fixes in this thread have now been released in version 0.8.35. There may yet be more bugs because node+Go+WASM+Windows seems to be a particularly buggy combination, but I've worked around all of the bugs I'm aware of. It's ready for another try.

[kzc]

See 93423e5#commitcomment-46371890

[merceyz]

Can confirm that yarnpkg/berry#2362 builds on Windows without crashing, however the bundle it produces is broken as the output is ~800kb smaller when built on Windows compared to Ubuntu. Due to the .node files it also counts as native and has to be unzipped which defeats part of the reason to use the WASM version.

[evanw]

the bundle it produces is broken as the output is ~800kb smaller when built on Windows compared to Ubuntu

You need to update your plugin to handle Windows-style paths with backslashes:

diff --git a/packages/yarnpkg-builder/sources/commands/build/bundle.ts b/packages/yarnpkg-builder/sources/commands/build/bundle.ts
index 4b619f1b..224b3d69 100644
--- a/packages/yarnpkg-builder/sources/commands/build/bundle.ts
+++ b/packages/yarnpkg-builder/sources/commands/build/bundle.ts
@@ -104,7 +104,7 @@ export default class BuildBundleCommand extends Command {
         const valLoader: Plugin = {
           name: `val-loader`,
           setup(build) {
-            build.onLoad({filter: /\/getPluginConfiguration\.ts$/}, async args => ({
+            build.onLoad({filter: /[\/\\]getPluginConfiguration\.ts$/}, async args => ({
               contents: valLoad(args.path, {modules, plugins}),
               loader: `default`,
             }));

Due to the .node files it also counts as native and has to be unzipped which defeats part of the reason to use the WASM version.

I'm going to try hiding the .node files inside a JavaScript file to hide them from Yarn. I assume that should avoid this problem.

[merceyz]

You need to update your plugin to handle Windows-style paths with backslashes:

Ah, of course, thanks 👍

I'm going to try hiding the .node files inside a JavaScript file to hide them from Yarn. I assume that should avoid this problem.

I suppose you'd need to write them to a temp folder before calling require on them but yes

[evanw]

The latest version 0.8.36 doesn't contain .node files anymore.

[evanw]

It looks like the N-API change isn't working out. I'm seeing this on the Windows tests for yarnpkg/berry#2362, which I assume is from esbuild's N-API hack:

#
# Fatal error in , line 0
# unreachable code
#
#
#
#FailureMessage Object: 0000006028EFF460
 1: 00007FF79AB9021F napi_wrap+109311
 2: 00007FF79AAC48DF std::basic_ostream<char,std::char_traits<char> >::operator<<+57151
 3: 00007FF79B6FD442 V8_Fatal+162
 4: 00007FF79B241C7C v8::internal::ItemParallelJob::NumberOfTasks+3292
 5: 00007FF79B241ECD v8::internal::ItemParallelJob::NumberOfTasks+3885
 6: 00007FF79B242533 v8::internal::ItemParallelJob::NumberOfTasks+5523
 7: 00007FF79B280052 v8::internal::ItemParallelJob::Task::RunInternal+18
 8: 00007FF79AAC729D std::basic_ostream<char,std::char_traits<char> >::operator<<+67837
 9: 00007FF79ABD47ED uv_poll_stop+557
10: 00007FF79B9A0D70 v8::internal::compiler::RepresentationChanger::Uint32OverflowOperatorFor+398512
11: 00007FF9E3537974 BaseThreadInitThunk+20
12: 00007FF9E567A0B1 RtlUserThreadStart+33

Looks like I should revert the N-API change. No idea why this is happening but this is probably a sign that it wasn't meant to be. I really wish node's WebAssembly support wasn't so incredibly slow. I guess maybe there's nothing robust I can do to make it faster.

[merceyz]

It's interesting that only node 14 fails while node 12 and 15 on Windows are just fine

[evanw]

Yeah that's strange. Just to check: have you ever seen this kind of error on any of your CI runs before?

[merceyz]

Not as far as I can remember, we're re-running the tests now to see if it was a one time thing

[arcanis]

A rebuild seems to have worked. I think we can try to land it and see whether it's stable.

Still, I'm curious about something: why do you need _exit rather than just process.exit? It seems to be a valid workaround that wouldn't require NAPI, as per nodejs/node#36616 (comment)

[kzc]

nodejs/node#36616 (comment) mentioned:

It took ~11 seconds for process.exit() to be honoured.