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

Add radium, lodash to devDeps; react to peerDeps #179

Merged
merged 4 commits into from
Jan 27, 2016
Merged

Conversation

coopy
Copy link
Contributor

@coopy coopy commented Jan 22, 2016

Fixes #176

  • Update all subcomponent versions when they've been published

@boygirl
Copy link
Contributor

boygirl commented Jan 23, 2016

Approved!

@austinpray
Copy link
Contributor

Maybe lax up the react requirements to: >=0.14.0 ?

@coopy
Copy link
Contributor Author

coopy commented Jan 25, 2016

@austinpray We can't guarantee that Victory works with future React releases, though, can we?

@knowbody
Copy link
Contributor

this looks good to me! thanks @coopy

@austinpray I don't think we can do that until future version of React is released. we support all 0.14.x which is good

@austinpray
Copy link
Contributor

You can't guarantee compatibility with 14+, but by doing this you are guaranteeing incompatibility.

I would lax requirements and perhaps drop a warning in the console for versions greater than 14.

@ryan-roemer
Copy link
Member

@alexlande -- Can you comment on Radium's choice to not have a peerDependencies and just stick react in devDependencies instead? Thanks!

@alexlande
Copy link

@ryan-roemer: Afraid I can't speak to it much. The change was made in this PR: FormidableLabs/radium#242

I expect the reasoning was that at the time there was a lot of confusion going around re: peerDependencies (whether they were deprecated [they weren't], how they worked in npm3 vs npm2, etc), and that the benefits weren't clear. Radium opted to let users bring whatever React version they wanted so that you could work with beta versions and the like.

It's in the devDependencies because examples and tests use it, not as a hack to avoid using peerDependencies.

Per Nilsson added 3 commits January 25, 2016 15:49
See README for explanation.
This adds application dependencies (`radium`, `lodash`) to the projects.
@coopy
Copy link
Contributor Author

coopy commented Jan 27, 2016

@austinpray Thanks for the comment! I removed the peerDependencies requirement as it doesn't give much in terms of safety, and would likely just be a hindrance for folks trying out react-beta releases, etc.

I added some documentation instead: https://github.com/FormidableLabs/builder-victory-component/#peerdependencies

coopy pushed a commit that referenced this pull request Jan 27, 2016
Add radium, lodash to devDeps; react to peerDeps
@coopy coopy merged commit 71c2b3f into master Jan 27, 2016
@coopy coopy deleted the task-commonDeps branch January 27, 2016 05:49
boygirl added a commit that referenced this pull request Jul 17, 2018
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.

6 participants