include only necessary files in npm package

[deepsweet]

$ tree node_modules/less -I node_modules -L 2

node_modules/less
├── bin/
│   └── lessc*
├── dist/
│   ├── less.js
│   └── less.min.js
├── lib/
│   ├── less/
│   ├── less-browser/
│   ├── less-node/
│   ├── less-rhino/
│   └── source-map/
├── test/
│   ├── browser/
│   ├── css/
│   ├── data/
│   ├── less/
│   ├── less-bom/
│   ├── plugins/
│   ├── rhino/
│   ├── sourcemaps/
│   ├── copy-bom.js
│   ├── index.js
│   ├── less-test.js
│   └── modify-vars.js
├── .jscsrc
├── .jshintrc
├── .npmignore
├── .travis.yml
├── CHANGELOG.md
├── CONTRIBUTING.md
├── Gruntfile.js
├── LICENSE
├── README.md
├── appveyor.yml
├── bower.json
├── browser.js
├── build.gradle
├── gradlew*
├── gradlew.bat
├── index.js
└── package.json

$ du -sh node_modules/less/test/

2.9M node_modules/less/test/

Loading unnecessary files is one of the reasons npm installs take so darn long. Anton Rudeshko very well put it in his article:

Speaking in terms of object-oriented design, your npm package should be highly cohesive and loosely coupled. Your package should do only one thing and do it well.

Corey Butler also wrote a nice article about why it's important to minimize module footprints, including test suite:

In the rare situation a developer actually wants to run your test suite on their own local computer, they'll likely clone or fork it.

Also see NPM docs for details about files field in package.json.

[lukeapage]

Plugins use some of the test folder files..
Ideally that could be pulled out to a seperate module.
I think you might get away with only js files directly under test though.
What about the readme and package.json, do they get included by default?

[deepsweet]

Certain files are always included, regardless of settings:

• package.json
• README (and its variants)
• CHANGELOG (and its variants)
• LICENSE / LICENCE

[deepsweet]

So, we should publish only .js from test/ dir, right?

[lukeapage]

Yes.. see the line and comment in the deleted npmignore.

[deepsweet]

only test/*.js, but not test/**/*.js? I'm going to update this PR.

[lukeapage]

Yes I think that should be ok. Someone should test the core plugins before merging this.

[ljharb]

All files are necessary - as I explained in tape-testing/tape#260. Please don't exclude files for what is npm's problem to solve, not package authors'.

[deepsweet]

3 MB of test fixtures are not necessary for using this package. NPM package is not a clone of your repo.

[ljharb]

Using a whitelist in package.json is very dangerous - it makes it very easy to accidentally create or rename a file in your package, and forget to add it to the whitelist.

If you want to blacklist test fixtures, great - do that with npmignore. That's a special case.

[lukeapage]

Given the pr author is including whole folders and given less' folder structure, i dont think we should suffer this problem.

[deepsweet]

it makes it very easy to accidentally create or rename a file in your package, and forget to add it to the whitelist

or you can accidentally rename a file and forget to change main field in package.json.

[ljharb]

If you do that, your tests will fail, because require('./') will hit the "main" field. It's very difficult to debug or even notice issues caused by a difference between "your filesystem" and "what's on npm".

[deepsweet]

Blacklists might be more dangerous since you can forget to exclude something and introduce a potential security threat if there is a sensitive information. If you forgot to include something into whitelist, at worst you'll have to release a patch version.

[deepsweet]

If you want to blacklist test fixtures, great - do that with npmignore. That's a special case.

why that's a special case? all tests without their fixtures are broken.

[ljharb]

Sure, you'd need to then edit your tests so that they skip the tests requiring fixtures, when fixtures aren't available. It's a tradeoff of that added complexity versus the size of the fixtures.

[seven-phases-max]

Referencing #2617, #2208 etc.

To be honest I can't see any reason to include just test/*.js w/o other tests files. We either do include the tests as a whole or we just don't. After all (not counting the mentioned proper "move to separate repo/package" method that requires further efforts), if you're a plugin developer and you're going to use the core tests scripts (and in general you don't) then it should not be a problem to install the library directly via github/git.
Personally I'm fine with either approach (with or without tests), but somewhat against the potentially confusing partial test/ installation as it would be exactly what we usually call "half-pregnant" :)
(Strictly speaking it's even more harsh: If one who installed the npm package is supposed to be able to develop/build the project, the tests should be included, and if he can't then the package should be stripped down to just lib and bin folders w/o any additional garbage-from-a-general-user-point-of-view files/folders. I.e. "descriminating files/folders basing on their size" is just a mismove. It's the package purpose/definition that should solely determine what's included and what's not).