Skip to content

Conversation

@thathenderson
Copy link

I used 15.x.x to try to future-proof the React dependencies; should be good for a little while. (15.0.1 was released while I was doing this, which fixed this cursor issue: facebook/react#6445)

No warnings in the console. I tried npm test, but there aren't any tests to run.

package.json Outdated
},
"peerDependencies": {
"react": "0.14.x"
"react": "15.x.x"

Choose a reason for hiding this comment

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

i would recommend "0.14.x || 15.0.x"

Copy link
Collaborator

Choose a reason for hiding this comment

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

agreed. Either that, or *, or remove peerDeps entirely.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, @szyablitsky and @iamdustan! Good points. I like the idea of 0.14.x || 15.0.x; it doesn't allow major or minor updates, which could introduce breaking changes, and communicates at a glance which versions are supported. Updated!

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh actually I would prefer 15.x.x

Copy link
Collaborator

Choose a reason for hiding this comment

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

in the || column. Minor versions are unlikely to introduce breaking changes and we by default support all future versions. The difference, IMO, will be when we drop support for older versions if newer versions make it difficult.

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense. Updated.

@iamdustan
Copy link
Collaborator

last nit: could you squash this to a single commit? Then I’ll merge and publish.

@thathenderson
Copy link
Author

@iamdustan Done!

@iamdustan iamdustan merged commit fc712ea into insin:master Apr 11, 2016
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.

3 participants