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

build: disable minification #569

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

isti115
Copy link

@isti115 isti115 commented Jun 4, 2024

There seems to be little need for minification, as this is a node based project never intended to be run in browsers. My reason to get rid of it would be to enable the usage of tools like patch-package on the library, which operate on the distributed source, and is practically incompatible with tsx in its current state.

@privatenumber
Copy link
Owner

Why is minification not benficial in Node?

@isti115
Copy link
Author

isti115 commented Jun 5, 2024

Thank you for considering my suggestion! While minified code might still have some minimal runtime advantages due to optimization heuristics, (even though the node team seems to be moving away from this approach, e.g. disabling function length based inlining), in my opinion the main benefit to be had is in the reduced bandwidth usage, and thus improved load times when serving web content, which doesn't apply here. Also, even if would be talking about a library that gets used in web pages, I'm of the opinion that minification should be left as a last step to the author of the application itself, not the library. (Or in case of a library that doesn't have to be bundled and can be included from a CDN separately, I think it's the best practice to provide a regular build as well as a minified version ending in .min.js, as many of the widely used libraries do.)

@privatenumber
Copy link
Owner

the main benefit to be had is in the reduced bandwidth usage, and thus improved load times when serving web content, which doesn't apply here

It does. The package needs to be downloaded. And since it gets stored to disk long-term, size probably matters more.

@nwalters512
Copy link
Contributor

My 2 cents: the ability to drop into node_modules and (easily) read/edit the code of my dependencies is very valuable. It means I can try out potential changes or fixes in the context of my project without having to first clone the source repository, install its dependencies, figure out how to build it, copy the build files into my project, and so on an so on. I can also more easily debug it, either by manually inserting console.log calls or by using an actual debugger. In both cases, dealing with minified source that bears little resemblance to the original code is very painful.

@gustavopch
Copy link

FWIW: #391

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 this pull request may close these issues.

4 participants