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

Update .npmignore #72

Merged
merged 1 commit into from
Nov 18, 2015
Merged

Update .npmignore #72

merged 1 commit into from
Nov 18, 2015

Conversation

Hypercubed
Copy link
Contributor

When publishing a npm module the entire working directory is published unless excluded using .npmignore. In this case it means installing commonmark includes tests, benchmarks, bower_components, documentation (which itself includes bootstrap, jquery, and lodash), and many temporary files that I am certain you never intended to publish. The current commonmark tgz downloaded from npm is 1.3 Mb (https://registry.npmjs.org/commonmark/-/commonmark-0.22.0.tgz). With this change only package.json, LICENSE, README.md and the code files will be packaged. You can test what files get packaged by executing npm pack. Should be ~120k after this change.

jgm added a commit that referenced this pull request Nov 18, 2015
@jgm jgm merged commit fb82315 into commonmark:master Nov 18, 2015
@jgm
Copy link
Member

jgm commented Nov 18, 2015

Many thanks. Obviously, I'm only a tourist in the JavaScript world.

By the way, that's a pretty crazy way for npm publish to work! Surely it would make more sense to have an explicit manifest of files to include...

@jgm
Copy link
Member

jgm commented Nov 18, 2015

By the way, is there really any reason to include /dist in the npm package? lib and bin should be sufficient, right?

@Hypercubed
Copy link
Contributor Author

I was making the same mistake for a long time as well. An alternative is to include a files attribute in the package.json which which is an explicit manifest (see https://docs.npmjs.com/files/package.json#files). I did a PR this way because you already have an .npmignore.

You probably don't need the dist folder for npm installs, but wasn't sure about all your use cases. Trying to make the PR as light as possible.

@Hypercubed
Copy link
Contributor Author

By the way: 👍

image

@Hypercubed Hypercubed deleted the patch/npmignore branch November 18, 2015 07:07
@jgm
Copy link
Member

jgm commented Nov 18, 2015

Yeah, it made a huge difference. Thanks again for pointing
this out.

+++ Jayson Harshbarger [Nov 17 15 23:02 ]:

By the way: 👍

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

Successfully merging this pull request may close these issues.

2 participants