Upgrade Webpack from 5.0.0-alpha.17 to 5.0.0-beta.28

[guybedford]

This upgrades Webpack 5 from 5.0.0-alpha.17 to 5.0.0-alpha.23 which is enough to get the now-cli to build with ncc.

The upgrade path is quite difficult due to the use of experimental and internal APIs as well as the fact that there is a lot of movement on these alpha releases.

If CI passes here I'm going to attempt to progressively upgrade as far as I can as that would obviously be preferable.

//cc @styfle

[styfle]

l converted this PR to a draft so we don't accidentally merge

[guybedford]

Ok it looks like 5.0.0-beta.8 is about as far as we can go for now, since beta.9 enables tree shaking which seems to break some relocation stuff and I'm not sure how to turn it off.

[guybedford]

I believe the tests should be passing now, ready for review.

[styfle]

I got a few warnings when compiling Vercel CLI using ncc from this branch. Is this expected?

ncc: Version 0.23.0
ncc: Compiling file index.js
<w> [webpack.cache.PackFileCacheStrategy] Restoring pack failed from /var/folders/v7/6kzxdgb93x328tk9pg633s200000gn/T/ncc-cache/ncc_6c640045e9.pack: TypeError: this.unserializable.add is not a function
<w> (during deserialization of webpack/lib/cache/PackFileCacheStrategy Pack)
ncc: Using typescript@3.9.3 (local user-provided)
(node:81518) [DEP_WEBPACK_MODULE_ERRORS] DeprecationWarning: Module.errors was removed (use getErrors instead)

[styfle]

@guybedford I also noticed that fsevents.node used to be in the dist folder and now it is not.

Likely because this message is no longer printed:

ncc: Module directory "/Users/styfle/Code/vercel/vercel/node_modules/chokidar/lib" attempted to require "fsevents" but could not be resolved, assuming external.

Update

I made sure to run both before/after with the minify option and both excluded fsevents.node 😌

But I did notice that the source maps became 2x larger 🤔

[guybedford]

@styfle thanks for looking into the details here:

• The logging shouldn't be coming through there, that is very odd as I've explicitly disabled the logging warnings.
• The deprecation warnings seem unavoidable unfortunately. Let me see if I can dig into those cases further.
• I've updated the source map option to reduce it slightly more, but I think the size is actually correct since the file is 4M, the original sources being 6.5M seems right to me, so I suspect this may have been a former bug in missing source maps that was fixed.

[guybedford]

Finally managed to get it all the way to the latest beta.28 depending on the PR at vercel/webpack-asset-relocator-loader#96 as well here.

[guybedford]

I've published and updated to webpack-asset-relocator-loader@0.8.0 here now.

This should then be good to release as a new major. @styfle I will leave the rest to you here now, and just let me know if you spot any issues further.

[styfle]

@guybedford I tried this PR on Vercel CLI and compared the time it takes too bootup and print the version. It is 2x slower than the bundle using ncc 0.18.5, even after several runs.

Version 0.18.5

time node packages/now-cli/dist/index.js --version  0.76s user 0.10s system 106% cpu 0.803 total
time node packages/now-cli/dist/index.js --version  0.79s user 0.09s system 103% cpu 0.848 total
time node packages/now-cli/dist/index.js --version  0.77s user 0.10s system 109% cpu 0.789 total

This PR

time node packages/now-cli/dist/index.js --version  1.35s user 0.13s system 114% cpu 1.293 total
time node packages/now-cli/dist/index.js --version  1.34s user 0.14s system 107% cpu 1.378 total
time node packages/now-cli/dist/index.js --version  1.36s user 0.14s system 114% cpu 1.309 total

[guybedford]

@sokra it seems like we have quite a significant performance drop here on the Webpack 5 upgrade path, despite disabling a lot of the tree-shaking functionality as discussed. Are there any other options that might help here?

[guybedford]

@styfle could you clarify if these numbers are before or after running ncc cache clean or doing multiple runs to fill the cache?

[styfle]

@guybedford Those are three consecutive runs. I actually did many more but three got the point across that it was slower. I didn't do ncc cache clean, however.

[guybedford]

@styfle a version of these benchmarks with a ncc cache clean before each run would indicate if the problem lies with the caching or with the actual operation.

[guybedford]

@styfle there are a few things that are very important to ensure the correct function here, which you may not have done:

• ncc cache clean must be run between each version change
• --external update-notifier reduces the startup time significantly
• also, as per the build script, node_modules/fsevents should be removed if present

With the above, I tried four different runs here:

1. Currently used ncc@: 7695KB, startup 490ms
2. Latest ncc@0.23.0 (with bug): 7695KB, startup 490ms
3. This Webpack upgrade: 7840KB, startup 505ms
4. This Webpack upgrade with tree shaking options enabled: 7799KB, startup 504ms

The above does show a slight slowdown with the latest Webpack 5 upgrade here, but not one that seems too significant to me. I would assume this difference comes down to subtle interop / wrapper function changes in Webpack, but perhaps Tobias can share more here.

I've gone ahead and reenabled the treeshaking options.

The remaining practical question, unless Tobias has further insights, is just whether a 3% increase in startup time is a genuine concern and if we want to not upgrade either project based on that. Personally I don't see it as significant, but it's certainly not my decision.

[styfle]

@guybedford The startup time I mentioned earlier is not about ncc itself but about vercel, the code compiled by ncc.

1. ncc cache clean didn't help, I'm still seeing 1.2s - 1.3s startup time for vercel
2. --external update-notifier is already used as seen here: https://github.com/vercel/vercel/blob/bd706beba5c8022a41e78ad6b3c303f3a6726272/packages/now-cli/scripts/build.ts#L52-L57
3. fsevents is not included, please ignore this part since it was a typo on my end

To reproduce the slowdown, please try the following:

## Build NCC
cd /tmp
git clone https://github.com/vercel/ncc
cd ncc
git checkout webpack-upgrade
yarn
yarn build
/tmp/ncc/dist/ncc/cli.js cache clean

## Build Vercel CLI with the local NCC
cd /tmp
git clone https://github.com/vercel/vercel
cd vercel
sed -i "" "s|@zeit/ncc|/tmp/ncc/dist/ncc/cli.js|g" ./packages/now-cli/scripts/build.ts
yarn
yarn build

## Run Vercel CLI and time it
time ./packages/now-cli/dist/index.js --version

[guybedford]

@styfle the numbers are above are exactly for the Vercel CLI build.

[guybedford]

(that is those sizes and ms counts are for ./dist/index.js --version in the CLI)

[guybedford]

Which version of Node.js are you running?

[styfle]

I'm using Node 12.18.3

[guybedford]

@styfle I dug into this and it looks like the source map is significantly reducing the execution time in Node.js - if you build without the sourcemap, the execution time reduces to 500ms. I would advise turning off the sourcemap entirely for this reason, possibly turning off minification as the bandwidth saving isn't really worth it there either.

[guybedford]

(that is - because the source map size increased, the execution time increased!)

[styfle]

Amazing work, thanks! 🤩

[styfle]

This made ncc much smaller 👍

• ncc@0.23.0
• ncc@0.24.0

[3cp]

I got an error in compiled result with v0.24.0, but v0.23.0 works fine. I have not found any related issue in vercel/ncc, so I am not sure why it failed. But clearly v0.23.0 works.

ghiscoding/excel-builder.js#1

The error:
ghiscoding/aurelia-slickgrid-demos#2 (comment)

[guybedford]

@3cp if you can share the exact replication steps as a new issue that would help.

[3cp]

@guybedford here is a demo for the bug. https://github.com/3cp/ncc-bug

I think this is technically a webpack bug. But it can be easily bypassed if ncc avoid the names of webpack global vars.

[3cp]

New issue created.