Skip to content
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

Restrict which files are packaged #180

Closed
wants to merge 1 commit into from

Conversation

paulvarache
Copy link

@paulvarache paulvarache commented Dec 31, 2018

Keeping npm modules as light as possible will speed up download times and disk space usage of users.

Adding the files key in package.json will let npm know which files to package. More info here: https://docs.npmjs.com/files/package.json#files

If I missed files required to use this module, please let me know and I will update this PR.

Npm pack results:
Before

npm notice === Tarball Details ===
npm notice name:          resolve
npm notice version:       1.9.0
npm notice filename:      resolve-1.9.0.tgz
npm notice package size:  18.3 kB
npm notice unpacked size: 92.3 kB
npm notice integrity:     sha512-K0N68lfSB1jWh[...]vyd7JamKd1vug==
npm notice total files:   85

After

npm notice === Tarball Details ===
npm notice name:          resolve
npm notice version:       1.9.0
npm notice filename:      resolve-1.9.0.tgz
npm notice package size:  7.4 kB
npm notice unpacked size: 28.6 kB
npm notice shasum:        53bde3e8bcf146287f3003b006190895f0cd10dc
npm notice integrity:     sha512-hnhiX8nZHj87g[...]uYZ2V+XlrYJnA==
npm notice total files:   11

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No thank you. The files property is incredibly dangerous and should never be used; and the size of the installed package is irrelevant. Additionally, npm explore resolve && npm install && npm test should always work (for all modules).

@ljharb ljharb closed this Dec 31, 2018
@paulvarache paulvarache deleted the files branch December 31, 2018 15:42
@ljharb
Copy link
Member

ljharb commented Dec 31, 2018

@paulvarache
Copy link
Author

It seems indeed that I'm not the only one questioning this, but that is up to the package maintainer. Thank you for the explanation.

@ljharb
Copy link
Member

ljharb commented Dec 31, 2018

Unfortunately the npm docs recommending “files” means that many people are unaware of how dangerous it is to use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants