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

Install react packages as dev dependencies #67

Merged
merged 2 commits into from
May 17, 2018
Merged

Conversation

eatyourgreens
Copy link
Contributor

Move react, react-dom and prop-types to dev dependencies.

Move react, react-dom and prop-types to dev dependencies.
@eatyourgreens eatyourgreens requested a review from srallen May 16, 2018 13:29
@eatyourgreens
Copy link
Contributor Author

I think this will stop Markdownz from installing React 15 when the parent project is using React 16.

Copy link
Contributor

@srallen srallen left a comment

Choose a reason for hiding this comment

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

This is working with the react 16 branch.

Similar to my comment on zooniverse/Seven-TenClient#react-dependency, using peer dependency might be more appropriate.

@eatyourgreens
Copy link
Contributor Author

Should zooniverse/modal-form use peer dependencies too? We've not declared them before.

@srallen
Copy link
Contributor

srallen commented May 16, 2018

I think they might be a better way to go for long term maintenance. I had the same thought as you that they were deprecated, but I learned from the grommet team that they are not, just the auto-install part was deprecated.

@eatyourgreens
Copy link
Contributor Author

eatyourgreens commented May 16, 2018

Would something like

peerDependencies: {
 "react": ">= 15.6"
}

include all versions since 15.6, including 16?

@srallen
Copy link
Contributor

srallen commented May 16, 2018

Hm, I'm not sure if that would include 16. The grommet source uses an || to specify the React 15 and 16 ranges they support: https://github.com/grommet/grommet/blob/master/package.json#L27

@eatyourgreens
Copy link
Contributor Author

That's going to be painful when the major version changes again. ☹️

@eatyourgreens
Copy link
Contributor Author

Let's see if the latest commit throws a warning with React 16.

@eatyourgreens eatyourgreens merged commit 11fb053 into master May 17, 2018
@eatyourgreens eatyourgreens deleted the react-dependency branch May 17, 2018 10:05
@eatyourgreens
Copy link
Contributor Author

Published as 7.6.0.

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