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

fix: only include necessary files into npm #239

Closed
wants to merge 1 commit into from
Closed

fix: only include necessary files into npm #239

wants to merge 1 commit into from

Conversation

JounQin
Copy link

@JounQin JounQin commented Mar 23, 2021

close #238

cc @ljharb

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.

This would be a breaking change.

This is quite intentional. Those files must be included.

Tests should be published with every package, so that npm explore foo && npm install && npm test always works.

Installed package size is irrelevant; if you never require the test files, they'll never affect the runtime of your application.

Duplicate of #235. Duplicate of #58. Duplicate of #44. See #105 (comment).

Comment on lines +10 to +14
"files": [
"/lib",
"/index.js",
"/index.mjs"
],
Copy link
Member

Choose a reason for hiding this comment

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

"files" is highly dangerous and should never be used; only .npmignore should be used to control what files are published.

Copy link
Author

@JounQin JounQin Mar 23, 2021

Choose a reason for hiding this comment

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

Highly disagree, but you're the boss here. 🧐

A lot of popular packages are using files, it makes package cleaner and much easier to maintain.

Copy link
Member

Choose a reason for hiding this comment

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

"a lot of packages use it" isn't an argument for it being safe.

The worst possible damage caused by accidental omission from "npmignore" is "the package is slightly larger". The worst possible damage caused by accidental omission from "files" is "every consumer breaks".

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.

All files are included into npm
2 participants