Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

fix(git): git repos with prepare scripts always install with dev flag #17060

Merged
merged 1 commit into from Jun 29, 2017

Conversation

intellix
Copy link
Contributor

@intellix intellix commented Jun 7, 2017

Force devDependencies to always install with --dev flag in case NODE_ENV is production

Related: #16533
Closes #17059

Copy link
Contributor

@zkat zkat left a comment

Choose a reason for hiding this comment

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

Thanks for finding this!

lib/pack.js Outdated
@@ -163,6 +163,7 @@ function packGitDep (manifest, dir) {
const child = cp.spawn(process.env.NODE || process.execPath, [
require.main.filename,
'install',
'--only=dev',
Copy link
Contributor

Choose a reason for hiding this comment

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

This will only install devDependencies and not dependencies. Did you want to use --dev?

Copy link
Contributor Author

@intellix intellix Jun 8, 2017

Choose a reason for hiding this comment

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

I did originally have --dev but when I used it locally it told me that it's deprecated and that i should be using --only=dev so I assumed the name was misleading :)
Guess not and will revert back to --dev

@intellix
Copy link
Contributor Author

intellix commented Jun 8, 2017

Addressed comments :) (not sure if you get a notification for that)

@@ -163,6 +163,7 @@ function packGitDep (manifest, dir) {
const child = cp.spawn(process.env.NODE || process.execPath, [
require.main.filename,
'install',
'--dev',
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there should be a --prod here as well to protect us against users who were doing an --only=dev install previously.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SGTM and solved :)

@zkat zkat changed the base branch from latest to release-next June 29, 2017 02:44
@zkat zkat merged commit 6627d89 into npm:release-next Jun 29, 2017
zkat pushed a commit that referenced this pull request Jul 5, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants