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

npm package includes non-production files #23

Closed
shadowspawn opened this issue Feb 5, 2023 · 6 comments
Closed

npm package includes non-production files #23

shadowspawn opened this issue Feb 5, 2023 · 6 comments

Comments

@shadowspawn
Copy link
Collaborator

I noticed today that the recent releases include example and test files. The package.json does not include a files property. Not sure how the old releases were done to skip those!

https://packagephobia.com/result?p=minimist

temp % npm init -y
...

temp % npm install minimist

added 1 package, and audited 2 packages in 589ms

1 package is looking for funding
  run `npm fund` for details

found 0 vulnerabilities

temp % ls -R node_modules 
minimist

node_modules/minimist:
CHANGELOG.md	LICENSE		README.md	example		index.js	package.json	test

node_modules/minimist/example:
parse.js

node_modules/minimist/test:
all_bool.js		default_bool.js		long.js			parse_modified.js	stop_early.js
bool.js			dotted.js		num.js			proto.js		unknown.js
dash.js			kv_short.js		parse.js		short.js		whitespace.js
@ljharb
Copy link
Member

ljharb commented Feb 5, 2023

In general, every package i maintain ships these on purpose - npm explore foo && npm install && npm test should always work.

However, if previous versions in the same major line didn’t include them, it’d be fine to add them to the npmignore config.

@silverwind
Copy link

silverwind commented Feb 13, 2023

In general, every package i maintain ships these on purpose - npm explore foo && npm install && npm test should always work.

I'd ask you to reconsider. Packages that ship tests are part of the problem why node_modules size explodes for the average project, which reflects badly on the ecosystem, not to mention it slows down install performance.

@ljharb
Copy link
Member

ljharb commented Feb 13, 2023

@silverwind that's a problem npm could solve holistically by providing a way for a package to ship multiple variants, eg "default" and "with tests", rather than trying to convince individual maintainers to do it.

@silverwind
Copy link

I'd suggest to at least use the files property in package.json to allowlist the files you intend to ship.

@ljharb
Copy link
Member

ljharb commented Feb 13, 2023

We're already set up to use npmignore, and the files property is very dangerous and should never be used - the failure mode of "consumers are broken" is far, far worse than the failure mode of "extra files are included".

@shadowspawn
Copy link
Collaborator Author

Not sure how the old releases were done to skip those!

One mystery solved. When I looked more carefully at difference between 1.2.6 and 1.2.7, the old package did have example/ and test/!

The size difference is largely due to the inclusion of a CHANGELOG. There are also small potentially surplus files with .eslintrc and .nycrc.

I am going to close this, as my initial concern was we had started including example and test files.

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

No branches or pull requests

3 participants