Use files property in package.json

[kevva]

No description provided.

[sonewman]

Why would we not want any of the other files?

for starts the readme & license are invaluable, in addition there are zlib and fs files, which can be used by node v0.10 allowing them to utilise streams3.

I am not sure what the benefit is of doing this?

[kevva]

The readme is always included by default, but I could add the license if you want.

Why you say. Well, the obvious reason is to keep the size as small as possible. So many modules includes useless files causing the node_modules directory to grow super big. Especially since npm downloads duplicate sub modules for every module.

[sonewman]

I get your point, but actually this should be covered in the .npmignore

[kevva]

It's always preferable to include the files that are necessary as opposed to excluding the ones you don't need because noone seems to maintain their .npmignore and the list of files you really need is shorter 99% of the time. You for example, are including a float.patch file which is totally useless for the end user.

Plus, you get to get rid of a .file which always feels pretty nice.

[sonewman]

I don't know, it depends on the scenario if that is not ignoring the files. Yes the .patch file could most likely be excluded, but lets be honest it's 30kb.

I can definitely see how using the files property in the package.json would be useful to specify only a sub-set of what could exist in a repo.

As a side note I would 100% always include the licence files if there is one, working at a huge international corporation, we have to be extremely careful in ensuring that we are not violating licensing terms of any module (or nested dependencies) that we want to include in our code base.

[domenic]

license is also included by default.

In general +1 for whitelist instead of blacklist.

[kevva]

Oh, it is? Guess mine never is because I use license.md.

[domenic]

license.* is included by defualt as of npm 2.1.6

[kevva]

Ahh, must've missed.

@sonewman, yes, it's 30kb but so many packages depend on this package and because of how npm handles dependencies (npm dedupe pls) it's going to be installed multiple times.

[sonewman]

ahh, that is something i didn't know. Well in general I am not overly opinionated about either way.

merge?

[isaacs]

-1. I'm not convinced size is a problem here. Also, is including just lib enough, or doesn't it have to list each file?

[shinnn]

license.* is included by defualt as of npm 2.1.6

I'm using npm v2.2.0 but it doesn't download license file if it's not included in files field.

Example: npm install get-stdin

[kevva]

@isaacs, lib is enough. And yes, size is a problem here, especially with this module. Think of all gulp plugins that depends on through2 which depend on this module. They are all going to install this module individually.

@shinnn, I think it's when publishing, but I might be wrong.

[jonathanong]

it needs to be published with a newer version of npm for it to be included in the package. doesn't matter which version you're downloading the package with.

[sonewman]

files

The "files" field is an array of files to include in your project. If you name a folder in the array, then it will also include the files inside that folder. (Unless they would be ignored by another rule.)

You can also provide a ".npmignore" file in the root of your package, which will keep files from being included, even if they would be picked up by the files array. The ".npmignore" file works just like a ".gitignore".

https://docs.npmjs.com/files/package.json

[shinnn]

@jonathanong Thank you for letting me know.

[sonewman]

yeah, @jonathanong that is a good shout. - I guess this is because it has already been published minus the license file

[shinnn]

And, +1 for adding files. That's especially meaningful for this module as @kevva said.

[isaacs]

And yes, size is a problem here, especially with this module.

The npm website (https://github.com/npm/newww) has 33 instances of readable-stream installed, pre-deduping.

With this change, that means it'd save a whopping 1351680 bytes on disk in this rather extreme use case. (After deduping, it's a mere 40960 bytes on disk, but let's assume we're not deduping for now.)

Note that almost none of that ever goes over the wire. Browserify already restricts the bundle to only pieces that are actually loaded, so it's minimal by default. It's almost always loaded from cache on install, so it's only a single download and multiple unpacks.

npm 3.0 will dedupe by default, so this becomes even less of an issue, and will result in only 40kb saved, about half the size of our 404 image.

If we want to remove the example files, then fine, let's make them separate modules, and remove them from this repo entirely. It'd be good to have proper tests and documentation for them at any rate. But having unnecessary divergence from the published artifact and the repo on the grounds of disk space is kind of misguided, imo.

[chrisdickinson]

I'm -1 on whitelisting files, for the reasons @isaacs lists above. Additionally, it seems like a potential footgun for publishers disguised as an optimization – I can easily see someone publishing a copy of readable-stream that works locally, but breaks for others due to forgetting to put a file in the whitelist.

[sindresorhus]

1351680 * all the sub-dependencies which also are wasteful with space. For larger projects that's hundreds of sub dependencies. It adds up quickly. I see no valid downside with doing this (even though the savings are minuscule in your eyes).

Just as an anecdote. A year ago yo had >200MB in dependencies, because they were wasteful. Today I've managed to get it down to 10MB. One dependency even included a video file...

I can easily see someone publishing a copy of readable-stream that works locally, but breaks for others due to forgetting to put a file in the whitelist.

Forgetting to add installed dependencies to package.json is also a foot-gun, but that's still totally possible.

[Raynos]

no. you should include the readme tests and examples.

Being able to debug and understand node_modules locally is more important then saving bytes.

[sindresorhus]

That's what git clone is for. And readme is included by default.

[Raynos]

we fundamentally disagree. node_modules is part of my application.

It's incredibly important for all the information to be there.

If your using npm and node_modules to publish "binary libraries" and complain about the size, maybe have less modules.

[sonewman]

I agree

[medikoo]

Looking at it closer I think the problem lies in fact that npm with npm install -g and npm install addresses two very different use cases.

First one is for production install of utilities (like yeoman or bower), and second one is for development, and while in development we usually prefer to have tests and documentation to be included with an installed package, then for production install, it's not needed and may attribute to some crazy package bloat as @sindresorhus pointed.

Probably real problem is that npm uses same strategy for both cases, which in my opinion would be better if addressed with different approach and possibly with two different binaries (just adding -g suggest small change, while difference is huge).

Speaking from my experience I would never ever use npm (as it works now) for production installs, too many things can go wrong. So @sindresorhus probably best solution for you is to install projects like yo not through npm, but via pre-prepared bundles which would contain complete project with all dependencies (stripped out of not needed files), and very short install script. I think using package managers like brew for that would work way better than npm . You would have full control over utility size, and exact state of JS dependencies wherever it is installed.

[feross]

Related discussion in browserify issue: browserify/browserify#1093
Related discussion in sha.js issue: browserify/sha.js#5

FWIW, I'm also -1 on this.

[vkurchatkin]

+1. size is important for node-webkit apps. it is ridiculous to package tests, examples, docs or whatever into client-side app

[sonewman]

This is a contentious point because i find it really helpful to use nd to read packages readme that I have installed. I think it is going to matter more in specific use cases such as you describe @vkurchatkin, but a tool to prune this stuff would be more advantageous IMO

[vkurchatkin]

@sonewman I can't imagine such a tool. We can use browserify-related packages to find used js files, but that way is not reliable at all. Binary addons make things even harder

[kevva]

@sonewman, as mentioned before, the readme and license are always included, no matter if you've defined them in the files prop or not.

[chrisdickinson]

@vkurchatkin If it's a post-install tool, one should be able to only look for .{js{on,},node} files that the requirement dep graph visits, and eliminate all other files.

---

Generally speaking, it strikes me that approaching the package size problem is best approached from the angle of pruning unnecessary files after having installed them, instead of attacking the problem at the individual package level. There is a use case for pruning the files (node-webkit!), but:

• the problem exists on the distribution side of downstream projects, not in this project.
• we would be doing a disservice to folks that expect to be able to access the tests and (eventual, non-README) docs by omitting these files.
• it adds an unnecessary potential footgun for future readable-stream developers.

Building a tool to prune unused files from node_modules seems like it would be more expedient than tracking down individual offending packages and convincing them to adopt a whitelisting scheme.

[sonewman]

+1 on @chrisdickinson suggestion

@kevva I know you explained before, but the other files included are also useful, since reading tests can be very useful in understanding how code is intended to work.

[vkurchatkin]

If it's a post-install tool, one should be able to only look for .{js{on,},node} files that the requirement dep graph visits, and eliminate all other files.

Tried that, and it generally doesn't work. There are all sorts of dynamic requires and even fs.readFile of packaged assets.

[kevva]

Any conclusion on this?

[stevemao]

Its interesting to see people having the exact opposite way to see the problem. What is the recommended way of npm registry? Is it supposed to host files that are only used by the consumers or should it include tests and related files that might be used by a few people (so they don't have to manually find the exact version of the code on github, which might be hard)? I can't find any on the npm website.

[rmg]

The actual effect of this PR on readable-stream-2.0.0.tgz:

--- before.txt  2015-06-12 19:57:31.000000000 -0700
+++ after.txt   2015-06-12 19:57:38.000000000 -0700
@@ -1,16 +1,13 @@
-package/.npmignore
-package/.travis.yml
 package/LICENSE
 package/README.md
-package/doc/wg-meetings/2015-01-30.md
 package/duplex.js
 package/lib/_stream_duplex.js
 package/lib/_stream_passthrough.js
 package/lib/_stream_readable.js
 package/lib/_stream_transform.js
 package/lib/_stream_writable.js
 package/package.json
 package/passthrough.js
 package/readable.js
 package/transform.js
 package/writable.js

[dcousens]

@rmg plus you have to maintain it.

[calvinmetcalf]

Yeah what if we just added doc/ and .* to the .npmignore?

Edit:correct folder

[feross]

It's useful to have the docs when you're offline. This will only save a few KB, and this doesn't even get shipped to the user in the browser or loaded into memory, so it's really not worth it.

cc @dominictarr

[calvinmetcalf]

Though these are just the meeting notes, not the actual docs probably
should bring over the nodejs docs as well

On Sat, Jun 13, 2015, 12:29 PM Feross Aboukhadijeh [email protected]
wrote:

It's useful to have the docs when you're offline. This will only save a
few KB, and this doesn't even get shipped to the user in the browser or
loaded into memory, so it's really not worth it.

cc @dominictarr https://github.com/dominictarr

—
Reply to this email directly or view it on GitHub
#100 (comment)
.

[stevemao]

I feel like in this case it might be better not to have the files entry. what about best practises in general?

[calvinmetcalf]

$ cat .npmignore .travis.yml doc/wg-meetings/2015-01-30.md | wc -c
2436 <-- bytes

closing this as the savings are incredibly small, if we ever have more working group meetings it may make sense to put doc/wg-meetings/ into the .npmignore

[rmg]

@dcousens agreed. My comment wasn't meant as an endorsement of any kind, I just thought it was missing from the discussion.

[calvinmetcalf]

@rmg and I would characterize it as a decisive contribution to the discussion 🎉