Coder Social home page Coder Social logo

Comments (50)

exogen avatar exogen commented on May 14, 2024 4

I strongly feel this should be revisited. There is no reason to include the tests, and if you are installing es5-ext as a dependency, they are not even possible to run because the devDependencies won't be there. It is not advisable to include 1.7MB of unrunnable extra files for your users. Issues like this are why people complain that npm is slow and node_modules is huge. 99% of your installs will be from people who don't even know what es5-ext is or care that it exists, it's merely somewhere in their dependency tree. Be respectful of your users' install time and disk space.

from es5-ext.

MarcDiethelm avatar MarcDiethelm commented on May 14, 2024 3

Development of what is the question. If you want to develop/contribute to a node module you use git for that (you need to commit/push, right?) and everything you need for that will be there, tests, docs and whatnot (except stuff listed in .gitignore).

However if I use your package as a dependency of my own project, I will install it with npm. The consumers of your package really have no use for all the files that you need for your development. Production is not the issue per se, but serves to illustrate that having all these files in an environment where a package is "merely consumed" produces unfortunate overhead.

.npmignore only becomes relevant when publishing a package. It does not impact the development of your package.

from es5-ext.

tmcw avatar tmcw commented on May 14, 2024 2

Another vote for excluding tests via .npmignore or the files parameter. One of my projects currently gets a megabyte bump because a dependency of a dependency includes es5-ext. I think that the wasted time & space for CI, users, and servers outweighs the narrow and unlikely case that someone wants to develop on a npm package install rather than a git checkout.

from es5-ext.

alasdairhurst avatar alasdairhurst commented on May 14, 2024 2

19 months later, I come across this.

  1. The dependency is both 3, 5 and 6 levels deep in my application's dependency tree
  2. As other people have said, while someone may want to test a package, NPM isn't for that. Once I test your module and include it, and someone else uses my module, they probably don't want to test es5-ext alongside all my other dependencies as well as theirs. I don't need to ship your development tools with my product.
  3. If I did want to test your module, i wouldn't go to node_modules/es5-tempalte-strings/node_modules/es6-iterator/node_modules/d/node_modules/es5-ext/ to test it out (on the worst case where npm 2 tree structures are still used). I'd just go to npmjs, find your git repo and clone it.

I notice you mention that npmjs should decide, but i have to agree with them. It's up to the developer of a package what is important to be shipped for the functionality of the package. That doesn't necessarily mean that "test" should always be included if the developer wants to, but if the core functionality of the module depended on "test" for whatever reason, then it could be included. In this case, it's just for the edge case of those one or two people who dive into node_modules, do an npm install on a package and run test, or download the tarball and test it. The other 3.2 MILLION weekly downloaders of the package don't give a rats arse. That's 156GB of bandwidth a week, just for your tests folder. (not including other files like .editorconfig)

@medikoo after over 2 years, has your stance changed on this subject? The scale of the distribution of your module is HUGE and this has a big impact in the size of so many applications with it as a transient dependency. Not saying it's the only issue, but it's part of a bigger problem overall (and more often not intentional on the part of the developer).

from es5-ext.

alasdairhurst avatar alasdairhurst commented on May 14, 2024 1

npm v3+ definitely improved this situation, but some people ARE stuck on npm2, others use different non-flat methods to distribute and bundle. As a community we're just trying to see this package improved further. It's not like the project is dead. - any of us can make a PR for you to merge and release when you get the opportunity. (in fact we have been, and they've been closed many many times).

So disk size is a problem, not used bandwidth?

Both. Every little helps, and with a package as widespread as this one it will be a big improvement in the long run.

from es5-ext.

mathiasbynens avatar mathiasbynens commented on May 14, 2024

Instead of .npmignore (blacklist) please use the package.json files property (whitelist).

from es5-ext.

MarcDiethelm avatar MarcDiethelm commented on May 14, 2024

Ah, interesting suggestion. I'm currently opening issues at many projects that bloat my dependencies, I will amend those issues with your suggestion. Thanks @mathiasbynens.

from es5-ext.

akre54 avatar akre54 commented on May 14, 2024

I came here to ask the same thing. @medikoo mind doing this for a few of your other projects too? The tests and extra files take up a ton of space on my machine.

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

In my understanding npm is in first place utility that you use in development process and not for deployment on production (I personally never used npm to deploy anything on production).

So while tests play very important role in development, to me naturally they should be kept as part of a package.

The only case where I understand npm as a installer of production package, is when global install is involved, but as far I know there's no way to tell to npm to provide shrinked version of package for global case, and as es5-ext doesn't expose anything for global install, it's not a case here.

If you use npm for direct installs on production, and for some reason tests take ton of space, I suggest you to just run following command afterwards:

find . -name "test" -exec rm -rf {} \;

from es5-ext.

MarcDiethelm avatar MarcDiethelm commented on May 14, 2024

Let me give you some background of why I opened this ticket...
I started analyzing my dependencies because the installed/deployed size of my own projects was growing and growing. I went after the largest modules and looked at what actually makes them so large. Well, I quickly found out that there are plenty of files in your installed module that I will never use.

(Note that since all these never require-d files are installed with my dependencies, they are subsequently also commited to my repo. All the users of my framework/server also get and deploy all these files they will never use.)

es5-ext turned out to be the largest of my dependencies with clearly identifiable resources that can be safely omitted from publishing to npm.

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

Development of what is the question. If you want to develop/contribute to a node module you use git for

In most cases I'm considering other module as a dependency for one that I'm developing, this is the most common use case I see for npm install.
When I'm a developer of given module, then of course I work directly on a git repository.

Well, I quickly found out that there are plenty of files in your installed module that I will never use.

If you worry about final size of your project. You shouldn't in first place deploy it via npm, but instead with prepared bundle, that will contain all the dependencies. Then in pre-step you can easily do any cleanup (remove tests, documentation, minify files etc.) of your dependencies to avoid bloat you mention.

See node_modules in git, this is the most common way to work with final projects. I wouldn't use npm for anything but development.

from es5-ext.

akre54 avatar akre54 commented on May 14, 2024

No. npm is first and foremost a package manager. Its job is to expose up-to-date, easily consumable, and distribution-ready modules from a repository for my code. As a consumer of your library (several steps removed) I have no need for files associated with its development. Tests and documentation have no place in a packaged file. Please remove these.

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

Tests and documentation have no place in a packaged file. Please remove these.

I think you should revise your idea of what npm is. I take at least 99% of packages published to npm doesn't provide what you request. Maybe take a hint from fact, that npm reports a warning when you try to work with package which doesn't include the documentation, and npm has configured dedicated command to run package tests.

from es5-ext.

akre54 avatar akre54 commented on May 14, 2024

That perhaps should've read "tests and documentation have no place in a consumed package file". npm test (and run-script) are useful for application code deployed to a server, where package.json is being used as a manifest file. It's not at all useful for a module to include tests. Local documentation is useful for the same reason a man page is useful, yet I have no need for your test suite nor unbuilt documentation in my dependency tree. Can you link me to a post supporting your position written by someone other than you?

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

@akre54 can you explain well a use case? What issue comes from a fact that test files are published with a package? Your server is running out of space cause of additional bloat within npm packages?

from es5-ext.

akre54 avatar akre54 commented on May 14, 2024

It's unnecessary bloat in my repositories and increases the size of my git slug. I use bower in a number of my projects and it starts getting huge fast on my machine when I have to keep so many huge dependences around (this is a problem with npm, sure, but it still sucks).

I'm not trying to pick on you, I'm just wondering why you're being stubborn about this. Can I ask the counter-question, why do you feel you need this in the distributed version of your repository?

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

So if I get it correctly, the issue is that bower seems unnecessarily huge, and es5-ext as one of its (deep) dependencies attributes largely to that (?) Or is it also something else.

Sorry, but I don't get where is the problem. I work on quite complex full stack (server & client) JS applications myself, there's hundreds of modules involved (including es5-ext) and never size of npm tarball was a concern.

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

I think this is very relevant: npm/npm#5264
It's where solution for this problem should be coined. I'll be happy to follow anything that's decided over there.

For now I would suggest to use tools like cruft

from es5-ext.

akre54 avatar akre54 commented on May 14, 2024

Just so we're clear. Mind stating your position once and for all about why you want tests and other extraneous files in the package? What does it give you?

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

@akre54 I explained that well, I find documentation and tests a valuable part of npm package. I see npm more as a utility through which you install dependencies during development, than utility through which you update application on production server.

Still, I'm open for discussion, and will track how discussion under npm/npm#5264 will evolve

from es5-ext.

wookieb avatar wookieb commented on May 14, 2024

https://github.com/KyleRoss/modclean

from es5-ext.

TrySound avatar TrySound commented on May 14, 2024

+1 for package.json files

from es5-ext.

zpao avatar zpao commented on May 14, 2024

Any chance you'd be open for reconsidering? While I know you aren't responsible for packages who depend on you, a fresh install of eslint results in 8 copies of es5-ext (I'm stuck on npm 2). Each of those carries 350+ test files at ~1.5MB. 8 copies means nearly 3000 extra files & 12MB. If I check that into source control (that's a requirement for this project), then I have a lot of extra junk.

[...] I find documentation and tests a valuable part of npm package. I see npm more as a utility through which you install dependencies during development, than utility through which you update application on production server.

It's been 18 months since you said this and I get the stance. I have found tests and docs valuable at times in my own use of packages. But increasingly production applications are using installed modules and every extraneous file and byte (especially in transitive dependencies) adds up.

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

@zpao Indeed that's the issue, but I'd say it's not es5-ext issue that it gets included 8 times for reason.
I take you have three options:

  • Devote all work needed to finally switch to npm3
  • For a meantime rely on tools like https://github.com/KyleRoss/modclean (altough I don't know how reliable they are).
  • Manually install es5-ext as top node_modules package. and then npm 2 wouldn't add it in nested folders (that's how I solve things on my side when using npm 2)

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

@exogen the problem is that you can't satisfy two groups of people that use NPM differently.

If I go the way you propose the other developers (from group I belong to) will have stuff they use removed.

See also statement from NPM on that: npm/npm#5264 (comment)

Anyway I monitor the subject, and hope some community and/or NPM at some point will find solid way to join both worlds

from es5-ext.

Siilwyn avatar Siilwyn commented on May 14, 2024

So I didn't find this open issue first... (#59)
I noticed this module being required multiple times too taking around 30MB in a small Electron app, just omitting the test directory would reduce the size by a factor of almost two. The suggestion of 'cleaning' the modules before releasing is unreliable unless specifically defining what to delete because of different names given to files in different modules. That's why the module should define what to publish to npm.

Regarding two groups I'm pretty sure the group of people not wanting tests inside their dependencies is bigger than the one who do want tests in their dependencies. Also if testing a package I usually clone the git repository as @MarcDiethelm mentioned too.

It's a shame NPM takes a neutral stance and doesn't care about it.

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

@alasdairhurst I'm focused on solving real problems. Can you (in concise and constructive way) describe one you have in your project and explain why do you assume that publishing es5-ext at v0.10.47 with stripped tests folder will solve it (real numbers please)

from es5-ext.

exogen avatar exogen commented on May 14, 2024

@medikoo You're obviously NOT focused on real problems, you're focused on arguing a losing position. You've closed several PRs that make this one line change for you and taken the time posting several argumentative comments over the course of two years on this topic. Users are literally telling you their problem, and you are being combative about it, like a jerk.

from es5-ext.

Siilwyn avatar Siilwyn commented on May 14, 2024

@medikoo chiming in too, so besides the waste of bandwidth. The real problem for me is it increases the size of my electron app by about 30MB which users experience with a longer download time when getting the app for the first time and on every update.

💡 Just an idea is to strip the tests with the next version and see if anybody opens an issue about missing tests in the package.

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

@medikoo chiming in too, so besides the waste of bandwidth. The real problem for me is it increases the size of my electron app by about 30MB

Hmm.. how is that possible? The test folder weights 1,7MB (uncompressed, and 146KB compressed) at this point.

And even if you want to cut off those 1,7MB, a better solution seems to intelligently bundle your production app with just modules that are actually used by it, as even from main content of es5-ext, I believe your app relies just on the fraction. I take in total you'd cut off 3MB or even more then (when es5-ext is concerned)

Is production version of your app meant to be installed via npm install -g ? If so I would strongly recommend preparing some bundled dependency free version, e.g. prettier makes great example of such approach.
It is the only way you can have production app both install and initialize real fast.

from es5-ext.

TrySound avatar TrySound commented on May 14, 2024

@medikoo I guess 30MB comes when this module is duplicated in node modules. So this is a problem for developer experience and bad internet connection.

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

@medikoo I guess

Again, I expect valid reports that show real proven numbers, not guesses.

If es5-ext takes 30MB of network, it means it's duplicated 75 times across node_modules (400KB is whole project tarball), and if it's indeed the case, I don't think it's a problem with es5-ext here.

from es5-ext.

Siilwyn avatar Siilwyn commented on May 14, 2024

Yes exactly what @TrySound mentions, it's because of duplicates. Anyway even if it would cut off only 1MB I would be happy, bandwidth doesn't come free you know. It's just a waste.

Btw. the app is not installed with npm, it is packed with electron-packager.

from es5-ext.

TrySound avatar TrySound commented on May 14, 2024

@medikoo I guessed because I saw this problem a few years ago. And I see it wasn't solved.

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

bandwidth doesn't come free you know. It's just a waste.

Given version of package version is downloaded once to your computer, repeated installs just take it from local cache (that's how npm and yarn works)

from es5-ext.

Siilwyn avatar Siilwyn commented on May 14, 2024

Still a waste of bandwidth initially, no?

Also for some exact numbers see this great comment from some time ago: #11 (comment)
Though this is without deduping yet.

from es5-ext.

TrySound avatar TrySound commented on May 14, 2024

@medikoo Tarball is not the only case for measurements. We are talking about unpacked size too. Duplicated packages are waisting of memory.

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

@medikoo Tarball is not the case for measurements. We are talking about unpacked size.

So disk size is a problem, not used bandwidth? Will you really notice if your project will weight 1MB less?

Mind most of the problems reported back then, came from highly inefficient way of how npm v2 installed packages, and real solution for that came with npm v3. Are you still stuck on npm v2 ?

from es5-ext.

exogen avatar exogen commented on May 14, 2024

All of us work on a lot of projects both open source and private. I have 70 MB worth of useless tests in my projects folder just from this package alone.

from es5-ext.

TrySound avatar TrySound commented on May 14, 2024

I'm not stuck on npm2. But there are different not flatten versions which produces duplications. There are also a lot of projects I develop in parallel so they does not share dependencies.

from es5-ext.

wookieb avatar wookieb commented on May 14, 2024

(real numbers please)

Let's revert that question. Is there a confirmed number of users that need tests in production package?

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

Let's revert that question. Is there a confirmed number of users that need tests in production package?

One of my side projects is a documentation app, that is meant to expose documentation and spec of packages (at least those I maintain). I don't want to rely on disconnected sources for that, so package tarball is expected to be the only source of information.

I believe in that idea want to push it further, and do not want to be stopped by accepting changes that are not proven to be a solution for any real problem.

from es5-ext.

wookieb avatar wookieb commented on May 14, 2024

by accepting changes that are not proven to be a solution for any real problem.

Removing tests from production packages is not a solution for any real problem? Is that what you mean?

One of my side projects is

1 / 4 110 000 (amount of downloads of es5-ext) = You've helped 0,0000243% of users.

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

Removing tests from production packages is not a solution for any real problem?

es5-ext as published on npm is in first place a bare source code of development package. In this form It is not expected to satisfy all possible production needs, but is expected to satisfy all development level needs.

Mind it's not a production application (to be installed via npm install -g), it's a utility belt a library that on its own in all cases will provide more that you need for given use case.

Therefore if your production needs, dictate that e.g. node_modules needs to be stripped out of not used content, minified, bundled.. or whatever else, just use proper tooling, improve your CI configuration (e.g. caching) etc.

I don't think you should expect dependencies to be published in shape that satisfy every possible production need.

from es5-ext.

wookieb avatar wookieb commented on May 14, 2024

I don't think you should expect dependencies to be published in shape that satisfy every possible production need.

Why most (if not every) of npm packages are removing tests and other files that are not necessary for production? Because removing them satisfied at least 4 000 000 / 4 110 000 = roughly 97% of users which far greater than 0,0000243% (speaking about real numbers).

but is expected to satisfy all development level needs.

That way you should not have devDependencies at all but rather place them in dependencies.

or whatever else, just use proper tooling, improve your CI configuration (e.g. caching) etc.

Which is far more complex than adding one line in .npmignore. Programming in node and its whole environment is tough enough already. We don't want to do extra job to satisfy 0,0000243% of users.

from es5-ext.

alasdairhurst avatar alasdairhurst commented on May 14, 2024

es5-ext as published on npm is in first place a bare source code of development package. In this form It is not expected to satisfy all possible production needs, but is expected to satisfy all development level needs.
I think we have different ideas on what counts as a package for production here.
By production we don't mean only packages that are intended to be installed by npm install -g or an explicit application for users, we mean something that is used as part of an application which production users/customers will download and install.

We're referring to the latter here. This may not always be something that is bundled and packaged up like vscode, vivaldi or some mobile apps which could handle some level of content stripping before shipping. Most of the time it's just going to be another module which has a dependency or transient dependency on es5-ext, and will download all the tests without the user knowing, and therefore bloating their installed package size.

Just note we're not trying to single you out here. There are a small number of other packages that publish a lot of their dev stuff too and i'd expect a large decrease in installed package size if every one of these modules ignored files which are never used in production code. The tests are never accessible or run by anything accessible by require('es5-ext').

One of my side projects is a documentation app, that is meant to expose documentation and spec of packages (at least those I maintain). I don't want to rely on disconnected sources for that, so package tarball is expected to be the only source of information.

That's fair enough, but it will only end up working for your packages, plus one or two others. The majority of developers follow a standard of keeping their published packages small and offering a simple link to their source repo in package.json for all the commit history, pull requests, discussions and development source. If you want to go as far as keeping what you have for purely documentation purposes if this git repo itself isn't enough for that, then a second, minified version of your packages wouldn't go unnoticed. (es5-ext.min)? Then see which one people would choose to depend on if they knew about both.

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

@alasdairhurst can you contact me over email (should be visible in my profile), with specific information on your case? (All *dependencies sections from your project's package.json will do, assuming you rely just on public packages).
It'll be helpful if I can reproduce problem you're facing locally.

from es5-ext.

Siilwyn avatar Siilwyn commented on May 14, 2024

I love the analogy with the devDependencies by @wookieb!
Also you can't actually run the tests without first cd-ing into this module's directory and installing the devDependencies. So by that logic it makes even less sense to include them.

from es5-ext.

medikoo avatar medikoo commented on May 14, 2024

Also you can't actually run the tests without first cd-ing into this module's directory and installing the devDependencies.

Yes, but as I tried to point above, installing package as a deep project dependency is not the only use case to work with package as published on npm.

And again I ask for some proof (test case or analysis) showing that (for cases where package is installed as a dependency) having test folder published with package is a real obstacle in 2018 (where we have npm v6 and yarn with optimal caching and deps distribution in place).

from es5-ext.

Siilwyn avatar Siilwyn commented on May 14, 2024

Yes, but as I tried to point above, installing package as a deep project dependency is not the only use case to work with package as published on npm.

However I think in chiefly all use-cases it is.

And again I ask for some proof (test case or analysis) showing that (for cases where package is installed as a dependency) having test folder published with package is a real obstacle in 2018 (where we have npm v6 and yarn with optimal caching and deps distribution in place).

Countless concrete numbers were already given above, regardless, for me it is a principle: respect the user's time and resources. Even if the difference is small it benefits the whole ecosystem.
Imagine the waste if every package would include their development files.

To move on why not to strip the tests with the next version and see if anybody opens an issue about missing tests in the package. Enough said, this discussion is getting tiresome for me.

from es5-ext.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.