Review requested:

• @nodejs/security-wg

I’m strongly -1 with this. I regularly have to debug Node.js JS layer. Minified JS almost means obfuscated JS. And I’m not sure about executing npx with an extra dependency. Not a fan of this change

@juanarbol Doesn't source maps fix that?

what are the numbers? how much does this improve the binary size?

@juanarbol Doesn't source maps fix that?

It is not about that only. When code is obfuscated, it could impact performance as well. V8 is a capricious engine. This is not a lib for a CDN, having a bit more of extra bytes is not that critical (only for distributions, no in runtime). And it the thing about about bundle size; that means we will have to distribute the source maps for the debugging.

what are the numbers? how much does this improve the binary size?

Not sure if that will actually have impact, everything in JS will go thru' the js2c task.

Again, not the biggest fan of this, it introduces npx with a lib, not sure that is reliable, the "compressed" js output will require fixing the "esbuild" version, I mean; something else to maintain. And I don't see any factual value on this.

Still -1 on this, but maybe people from @nodejs/build could give a more objective opinion about this.

@Kikobeats And congrats on the first contribution! Sorry for being that strict! You want to land something on a big project, we just needs to ensure we delivers the best -always-. Thanks for taking the time to contribute!

CC @nodejs/undici

I'm +1 if it's backed with data (benchmarks)

I am -1
I am pretty sure that you can compress the final executable, if you want to reduce the filesize. Under Windows you can use a so called PE-Packer. Dont know what is applicable for Linux or MacOS.
Obfuscating the code + the obligatory need then to somehow ship debug information via sourcemaps is imho annoying. Shipping a "node debug" with uncompressed deps could be unreliable, because the obfuscator/compressor could be transforming the code wrong.

I think the correct way to implement this is through having a build option called minify-js which only minifies when building for release.

Afaik undici already goes through a build step, idk if location are preserved

I think the correct way to implement this is through having a build option called minify-js which only minifies when building for release.

I agree, because in my opinion, if minification were to occur, it should happen during build, allowing the debug build to retain the unminified sources.

I'm skeptical of the usefulness of this. -1

-1

After a meeting yesterday this PR is shining in a different light. It is about startup time of a node instance. Using a dispatcher is not the solution.

What if we use esbuild as a bundler and not as minifier. Maybe removing comments. This should improve the hoisting.

Is that improving the startup performance significantly?

After a meeting yesterday this PR is shining in a different light. It is about startup time of a node instance. Using a dispatcher is not the solution.

What if we use esbuild as a bundler and not as minifier. Maybe removing comments. This should improve the hoisting.

Is that improving the startup performance significantly?

I wonder if resource mappings can be used to resolve this?

example:

find "deps/undici/src" -type f -name "*.js" | while read -r file; do
  npx esbuild "$file" --minify --sourcemap --outfile="$file" --allow-overwrite
done

What if we use esbuild as a bundler and not as minifier. Maybe removing comments. This should improve the hoisting.

This is what we're already doing?

@KhafraDev and I already faced this previously for some optimization attempts. I believe we are bundling already. There are more esbuild optimizations we could try (such as comment removal iirc), but there are nuances to investigate carefully.