Skip to content
This repository was archived by the owner on May 29, 2018. It is now read-only.

#17 remove devdependencies#18

Open
xizhao wants to merge 130 commits intosourcegraph:masterfrom
xizhao:patch-6
Open

#17 remove devdependencies#18
xizhao wants to merge 130 commits intosourcegraph:masterfrom
xizhao:patch-6

Conversation

@xizhao
Copy link
Copy Markdown

@xizhao xizhao commented Sep 9, 2015

No description provided.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It now occurs to me that devDependencies include things like unit test libraries that it is useful to include as an "actual dependency." I think this was the original motivation behind including them in the list of deps. Is their presence causing major issues?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think that's rare especially for code you depend on -- you might have a point for code in your own environment. But w/ deps at least you're not going to be distributing tests or using them in actual code.

I think the npm convention is deps are for distribution and devDeps aren't, and you should only receive production-ready releases. Note the NPM install --production flag: https://docs.npmjs.com/cli/install, it seems like if people are using devDependencies to include "actual dependencies", they're not using devDependencies correctly.

So if you're depending on a package, you don't care about their dev-deps, only their production-ready release. Maybe there's a way we can provide a flag to srclib-javascript, so that if it's a local root project, we care about devDeps, but otherwise we ignore?

Also: what are your thoughts on peerdeps?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Default npm install behavior:

By default, npm install will install all modules listed as dependencies. With the --production flag (or when the NODE_ENV environment variable is set to production), npm will not install modules listed in devDependencies.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants