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

Adds semver to the package dev dependencies #12442

Merged
merged 1 commit into from
Mar 23, 2018
Merged

Conversation

arcanis
Copy link
Contributor

@arcanis arcanis commented Mar 23, 2018

This diff simply adds semver to the top-level dev dependencies. Semver was used in various scripts (ex here) without being listed, which was invalid (it worked because one of React's dependencies was itself depending on semver, which was then hoisted to the top-level through package managers optimizations).

package.json Outdated
@@ -119,5 +119,8 @@
"prettier": "node ./scripts/prettier/index.js write-changed",
"prettier-all": "node ./scripts/prettier/index.js write",
"version-check": "node ./scripts/tasks/version-check.js"
},
"dependencies": {
"semver": "^5.5.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe move to devDependencies with the others?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

fwiw devDependencies on workspace top-level aren't really useful, since workspaces are typically always installed on development environment (we do however respect them, so if you run yarn --production from the root, we won't install them, which will make your scripts unusable).

@gaearon gaearon merged commit cc616b0 into facebook:master Mar 23, 2018
rhagigi pushed a commit to rhagigi/react that referenced this pull request Apr 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants