-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
Reduce package size #16
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As written, this excludes way more than just the changelog - tests must always be included in the published package.
While this is a perfectly fine stance to hold (your package after all!), I'm not sure I understand why a published package should include tests since consumers wouldn't be running tests for packages (most test dependencies would be dev dependencies, no?).
Yeah, I considered that, but I thought it was better to allow-list than deny-list certain files. This package is used by a lot of other packages, so reducing the package size of this one package down will reduce packaging size and distribution bandwidth of the overall npm ecosystem a lot, so seems worth having the smallest reasonable package size. I'll update to liberally use |
Including the Unpacked size: 45.5 kB -> 12.3 kB -> 22.6 kB |
Also, it’s not package authors’ responsibility to manage package size and distribution bandwidth, it’s npm’s, and npm has chosen not to consider this a priority (by implementing separate “full” vs “minimal” tarballs for a single package) |
Man, I'm learning so much here: I can't find any reference about this anywhere either (my Google-fu is weak tonight...) and when I install Maybe the minimal change that we can both agree to here is excluding the changelog? But then again, if it's not the package author's responsibility, we can also just close this PR as won't fix, no hard feelings. 🤝 |
Latest changes: Unpacked size: 45.5 kB -> 12.3 kB -> 22.6 kB -> 23.3 kB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m fine with this change, thanks.
c0c85c6
to
4044e7f
Compare
Avoids bundling large files like the changelog into the distributed package.
Unpacked size: 45.5 kB -> 12.3 kB
Packaged size: 15.7 kB -> 5 kB
Before:
After:
After including
tests
back in the package contents: