medikoo / es5-ext

ECMAScript extensions (with respect to upcoming ECMAScript features)
ISC License
168 stars 81 forks source link

Create .npmignore #49

Closed jamiebuilds closed 8 years ago

mathiasbynens commented 8 years ago

Don’t you think the files property in package.json is a better approach? It acts as a safelist, whereas .npmignore is a blocklist.

medikoo commented 8 years ago

Duplicate of #10, #11, #29, #37 and #47

mathiasbynens commented 8 years ago

@medikoo How many duplicates will it take for you to reconsider your opinion from #10?

medikoo commented 8 years ago

@mathiasbynens it's not about count of duplicates, but about convincing arguments. Do you have any? I'm certainly willing to consider change having those.

wookieb commented 8 years ago

Having test in the package files is not necessary. I don't care about tests from node_modules since I can' event use them (requires to install dev packages).

mathiasbynens commented 8 years ago

I think the arguments have already been presented in #11.

Consumers of an npm module generally just want to require() and use it, not necessarily run tests. If they want to view/run the tests, they can clone the repo for the project instead (or, if they insist on using npm for it, use Git URLs in package.json).

Including the tests in the package bloats the download size unnecessarily for the common case.

medikoo commented 8 years ago

@wookieb I understand that you don't care, but there are devs which install packages in development, and want to run tests (e.g. that's how I use npm). I would prefer not to remove this possibility for them.

medikoo commented 8 years ago

Including the tests in the package bloats the download size unnecessarily for the common case.

Why is the download size the issue?

mathiasbynens commented 8 years ago

If my-project depends on es5-ext, every time someone npm installs my-project it downloads and stores the es5-ext tests as well. What’s the point? I certainly don’t want any of my dependencies wasting storage space by including their test suites. It’s not just about download size but also about storage space.

wookieb commented 8 years ago

For what purpose they would want to run tests on installed package? For fixing? Make a fork, fetch a repo and fix it.

Also good argument from @mathiasbynens above.

wookieb commented 8 years ago

Also think about it from the different perspective. How many developers do so? More than users that install your package? Doubt it. Why bother them with test files then?

medikoo commented 8 years ago

This issue is actually a result of npm not being flexible enough in addressing use cases it aims for.

e.g. we have a possibility to mark a dev only dependencies, but we don't have a possibility to mark dev only files. I take it's not been addressed, as it will require from npm to keep two versions of bundles per package, so most likely big architectural and costly change

For what purpose they would want to run tests on installed package?

Testing the installed package is first resort when you feel that package doesn't work as expected, and cloning a repo is not a same thing, as you want to test exactly the code that was shipped from npm.

Reasons I'm really reluctant:

So I have a feeling that dropping tests from npm package, doesn't really solve any problem

There's some interesting discussion here: https://github.com/npm/npm/issues/5264

Real solution for that problem might be introduction of some utility (possibly integrated with npm) that will allow us to receive stripped package version for production use. It may not only cut tests but also all other modules that dependant package is not using.

wookieb commented 8 years ago

Pure numbers. 1) People that would like to have "tests" in .npmignore: 8 2) People that do run tests of packages installed in "node_modules": 1 (i assume you know few more) 3) People that fetch tests but they don't need to: 3 815 208 * uniqueness factorm(amount of downloads from npm) Compare 3 and 2.

I agree that running tests on installed package might be useful but how often? How often do you need to do it in order to make sure everything works fine? If for some reason installed package doesn't work - Ask the developer to fix it and setup tests "correctly" on the CI. Much more than 3 800 000 times?

Also if you're asking how size of the package might affect people. Cases:

wookieb commented 8 years ago

BTW. That's why option "--prefer-source" for npm would be useful. https://getcomposer.org/doc/03-cli.md#install

medikoo commented 8 years ago

Compare 3 and 2.

Where this data comes from. Source?

Running several containers (each container need a copy of es5-ext)

What's a "container" ?

Builds on CI (time to download, size)

I run CI on all (a lot) projects that rely on es5-ext, I don't see any issues related to 'tests' being included with es5-ext. What are exactly the issues on your side? Can you pass me the numbers on how it will improve things on your side, if tests are not included with es5-ext.

I'm using phpstorm and I mark "node_modules" directory as "excluded".

If you don't need es5-ext tests in that project, you can remove the folder. And anyway what you describe looks more a thing for tuning in phpstorm, certainly not a reason to publish es5-ext without tests

Munter commented 8 years ago

If you want to run the tests of an installed package, why not use npm link like everyone else does? Package size concerns are not just for this package. It's a systemic issue that authors include too many things, especially tests, in their packages, resulting in literally millions of unused files. It's ramping up the install time, download time and wasting disk space. When your project is heavily depended upon you should be a good citizen and only package the part users need. Developers can clone and link

medikoo commented 8 years ago

When your project is heavily depended upon you should be a good citizen and only package the part users need

Sure, but that should be solved with tools like modclean. In case of es5-ext even if tests are stripped out, you still (in regular case) will end with tens of modules that its dependants don't use

mathiasbynens commented 8 years ago

Just because it can be solved with tools doesn’t mean that should be the go-to solution. By including everything in your package you’re forcing the 99% of developers that don’t need the tests to use modclean or something similar. Optimize for the common case.

mathiasbynens commented 7 years ago

npat was removed from npm in v4.0.0: https://github.com/npm/npm/releases/tag/v4.0.0#remove-npat

Emphasis mine:

REMOVE npat (BREAKING)

Let’s be real here — almost no one knows this feature ever existed, and it’s a vestigial feature of the days when the ideal for npm was to distribute full packages that could be directly developed on, even from the registry.

It turns out the npm community decided to go a different way: primarily publishing packages in a production-ready format, with no tests, build tools, etc. And so, we say goodbye to npat.

Publishing “packages in a production-ready format, with no tests, build tools, etc.” is now officially what npm is for.

medikoo commented 7 years ago

@mathiasbynens it's first time I hear about npat, so I'm not sure what change does it make. Still if they would decide to remove npm test then it would be clear signal they do not want npm to be used to run tests, and that would indeed change things.