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

Publish only required files #128

Closed
wants to merge 1 commit into from
Closed

Publish only required files #128

wants to merge 1 commit into from

Conversation

MarkHerhold
Copy link

This PR removes unnecessary files (test, examples) when publishing to npm. Please see the npm docs on the files key in package.json.

To see how this will be packaged without publishing, run:

npm pack
# produces resolve-1.3.3.tgz

Files that will be published with this PR:

./index.js
./lib
./lib/async.js
./lib/caller.js
./lib/core.js
./lib/core.json
./lib/node-modules-paths.js
./lib/sync.js
./LICENSE
./package.json
./readme.markdown

Thanks for the awesome library!

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.

a) the "files" property is very dangerous; only .npmignore is a safe way to exclude files from publishing.
b) all files are required; npm explore foo && npm install && npm test should always work, for every package.

@ljharb ljharb closed this Jul 2, 2017
@MarkHerhold
Copy link
Author

@ljharb Why do you say the "files" property is dangerous?

@MarkHerhold MarkHerhold deleted the packaging branch July 2, 2017 01:59
@MarkHerhold MarkHerhold restored the packaging branch July 2, 2017 01:59
@MarkHerhold MarkHerhold deleted the packaging branch July 2, 2017 01:59
@ljharb
Copy link
Member

ljharb commented Jul 2, 2017

If I add a new file that's needed in production, and forget to add it to "files", production breaks but all my tests pass.

If I add a new file that's not needed in production, and forget to add it to "npmignore", nothing breaks, and a package that's downloaded once and cached is insignificantly larger, until I notice the error.

The latter is maybe inconvenient; the former is likely breaking and thus user-hostile.

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