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

Pinned react (and co) to 16.0.0-alpha.4 #13121

Closed
wants to merge 2 commits into from

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Mar 23, 2017

Some time ago (24a7665) the version of React specified in package.json was relaxed (presumably unintentionally). As a result, a recent backwards-incompatible release of React 16 alpha 5 caused some breakage in OSS.

This PR pins the version of React to avoid breaking changes in subsequent alpha releases until the React team has created codemods and prepared the ReactNative codebase for this.

Since the change to package.json was made in February, this PR will need to go into the RC branch for the next ReactNative release in order to avoid breaking other apps in the OSS community.

cc @mkonicek, @gaearon, @javache

This will avoid breaking changes in subsequent alpha releases until the React team has created codemods and prepared the ReactNative codebase for this
@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Mar 23, 2017
@bvaughn
Copy link
Contributor Author

bvaughn commented Mar 23, 2017

Why is CI trying to install react-test-renderer@16.0.0-alpha.4 ? This version doesn't exist. That's why the package.json points at 16.0.0-alpha.3

@janicduplessis
Copy link
Contributor

@bvaughn Looks like the script for react-native init assumes that it all use the same version https://github.com/facebook/react-native/blob/master/local-cli/init/init.js#L91.

@bvaughn
Copy link
Contributor Author

bvaughn commented Mar 23, 2017

Gah! Thanks for pointing that out @janicduplessis

Updated that script as well.

if (!peerDependencies) {
console.error('Missing React peer dependency in React Native\'s package.json. Aborting.');
return;
} else if (!devDependencies) {
console.error('Missing react-test-renderer dev dependency in React Native\'s package.json. Aborting.');
Copy link
Contributor

Choose a reason for hiding this comment

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

Should return here

Copy link
Contributor

@janicduplessis janicduplessis Mar 23, 2017

Choose a reason for hiding this comment

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

Also maybe update the message to Missing dev dependencies ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just copied above wording/formatting.

@@ -72,6 +74,12 @@ function generateProject(destinationRoot, newProjectName, options) {
return;
}

var reactTestRendererVersion = devDependencies['react-test-renderer'];
if (!reactVersion) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be reactTestRendererVersion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Blah. Yeah, sorry. Multi-tasking fail. Will fix. :)

@janicduplessis
Copy link
Contributor

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

@janicduplessis has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@bvaughn
Copy link
Contributor Author

bvaughn commented Mar 24, 2017

Thanks for the review, @janicduplessis!

@hramos
Copy link
Contributor

hramos commented Mar 31, 2017

Closing as these dependencies were recently updated to alpha6 in 6f9447e

@hramos hramos closed this Mar 31, 2017
@janicduplessis
Copy link
Contributor

janicduplessis commented Mar 31, 2017

Shoud we still ship the change in local-cli/init/init.js in case versions of react and test-utils don't match or is that never supposed to happen?

@bvaughn bvaughn deleted the pin-react-version branch March 31, 2017 19:06
@hramos
Copy link
Contributor

hramos commented Mar 31, 2017

@bvaughn ?

@bvaughn
Copy link
Contributor Author

bvaughn commented Mar 31, 2017

Unsure. There was a transient problem with 16.0.0-alpha.4 because whoever published it did not also publish react-test-renderer@16.0.0-alpha.6 which caused problems with the CI. Maybe this won't happen again? Hopefully not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants