Switches the build to ESBuild #2362
Conversation
| // esprima is only needed for parsing !!js/function, which isn't part of the FAILSAFE_SCHEMA. | ||
| // Unfortunately, js-yaml declares it as a hard dependency and requires the entire module, | ||
| // which causes webpack to add 0.13 MB of unused code to the bundle. | ||
| // Fortunately, js-yaml wraps the require call inside a try / catch block, so we can just ignore it. | ||
| // Reference: https://github.com/nodeca/js-yaml/blob/34e5072f43fd36b08aaaad433da73c10d47c41e5/lib/js-yaml/type/js/function.js#L15 | ||
| new webpack.IgnorePlugin({ | ||
| resourceRegExp: /^esprima$/, | ||
| contextRegExp: /js-yaml/, | ||
| }), |
There was a problem hiding this comment.
This is most likely why the size went up, would be good if we could still exclude that
There was a problem hiding this comment.
Esprima isn't in the bundle. I suspect it's because ESBuild uses the browser version of js-yaml rather than the Node one, so Esprima isn't requested. I don't remember what the problem was with the browser version - things seem to work 🤔
There was a problem hiding this comment.
We shouldn't let it use the browser version of anything as we can pretty much be certain those are polyfilled into oblivion
Good question, I'm not sure yet tbh 🤔 I would probably tend to prefer using the exact same transpiler on both branches, if only to make sure that they have very similar behaviours (can matter with things like initialization order, etc, or simply supported features). What do you think? |
| }), | ||
| new webpack.BannerPlugin({ | ||
| entryOnly: true, | ||
| banner: `#!/usr/bin/env node\n/* eslint-disable */`, |
There was a problem hiding this comment.
Shouldn't we continue telling eslint to ignore the bundle? (and add prettier-ignore as well)
Agreed, using the same one makes the most sense. The per-file compilation overhead is caused by esbuild launching as a new external process for each sync compiler call. (async can share a single background process) evanw/esbuild#590 and evanw/esbuild#612 are about using a worker_thread to talk to a shared esbuild process for all sync compiler calls. It still needs some work. I haven't had much time for open-source over the last month. I suppose, alternatively, the worker_thread stuff could be implemented as a third-party library that wraps esbuild. But that adds extra complexity to yarn's tooling. |
|
Have you seen webpack's persistent-caching? Not sure that build time would be the same as esbuild, but I think it was an option too. |
|
@7rulnik Would be interesting to check, I'm probably doing something wrong - after upgrading from Webpack 5.1 to 5.13, the build time went from 28s to 39s, and the cache didn't appear in |
|
@arcanis I can check it tomorrow. I tried it in my work project and build time went from 12 to 2 minutes |
|
@arcanis I debugged a bit webpack caching by adding this options: cache: {
type: `filesystem`,
},
infrastructureLogging: {
debug: /webpack\.cache/,
},and got two errors: So seems that you were looking in the wrong place for the cache. But more importantly, it was never created because of Then I tried to move webpack config to plain |
|
The build fails on Windows but since the CI doesn't build on Windows it wont catch it. The output also shows that we're not handling failures correctly as it still prints the "success" path cc @evanw |
|
As a quick status update, this is currently blocked on evanw/esbuild#687 |
What's the problem this PR addresses?
It used to take 28s to build the CLI. With this diff it becomes 6s (on the other hand, went 1.62MB -> 1.92MB).
How did you fix it?
Replaced the bundle building by an esbuild-based config. It only uses the wasm version, so it's a bit slower than it could, but this way we don't have to deal with postinstall scripts.
Checklist