-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Bloated zip on npm #834
Comments
Moving this issue to help as it is a topic regarding all repos/packages |
|
Thanks for the insight. I generally understand the issue but may lack in the open source development experience to fully understand. The reasoning for not using the files property in the package.json seems incorrect to me:
Using typescript results in a known input and output location. And using a glob pattern on one (or maybe both) of those locations is all that's needed. The license and readme are automatically included. Are there other artifacts that would need to be in npm that I'm omitting? |
I got the point, and I must say that I'm pro reducing the package size tbh but:
Are you suggesting migrating ~60 repositories to typescript? The big deal to re-open the discussion with the whole team is: |
No, sorry. The way I phrased my rebuttal has more to do with how I code and was probably confusing. However, supporting type definitions in the repository requires a known file structure to allow type completion to work (e.g. using the The point being that the file structure, though inconsistent between fastify repos, is still well organized, so glob patterns could be leveraged with confidence. Some fastify repos could use
A self-complete package artifact on NPM in 2300 seems duplicative to me if you already have the self-complete source in GitHub. The previous discussion covers the list of pros very well, I think. I resurfaced this discussion purely because I thought it would be an easy win and because this isn't just a problem for fastify. Too many packages in NPM include vestigial artifacts and if everyone does it, it can make installing dependencies much more burdensome. Where fastify is renown in the javascript/nodejs community and developers may look to it for an example, more care should be given to code leanly. |
They could also learn how to use the source code already on their systems. At least a few of us use the included assets when debugging things. |
I'm not sure what you mean. Why would I need |
To be honest I also prefer only having active code in the npm packages because nothing is more annoying than installing node modules with burger king internet. Also happened that i could reconstruct the codebase of some abandoned npm packages because it was containing more than the active code (mainly tests and docs). |
The latter is why we include all the relevant artifacts in our releases. |
I'm okay with backups, but NPM probably is not the best place for them IMO. And if @Uzlopak's scenario is based on repos that he doesn't maintain, then it makes sense but fastify doesn't fit that scenario either. |
An npm package is forever. I have plenty examples, here is one: https://www.npmjs.com/package/outpipe The only though I got is:
In any case, it is more work |
I still see no gain in saving that files space or bandwidth especially we are not talking about If anyone do consider the size, they have plenty of way to remove it.
|
If you're really interested in backups, GitHub recommends using backup utilities on GitHub marketplace. Just for reference, fastify weekly downloads currently add up to 2TB As much as I hope you'll reconsider, keeping fastify stable, maintainable, and long-living is a great goal regardless of how its achieved. I'm happy to continue to respond as needed, but I think I'll just leave this in your hands now. Thanks all. |
To sum up the pros and cons of having non-production files in the npm package Pros:
Cons:
Solutions:
|
Solutions: |
Prerequisites
Issue
NIT: The package on NPM includes tests, examples, and .github in the download which unnecessarily increases the size of the package. Not a huge deal but an easy fix.
The text was updated successfully, but these errors were encountered: