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

Use most recent React version #477

Merged
merged 5 commits into from
Aug 25, 2016
Merged

Conversation

wdhorton
Copy link
Contributor

Fixes #473. I thought the best approach would be calling npm view react[-dom] version and then using the output. If it fails, it falls back to the old method of getting the version. The other alternative I considered was just doing a separate install of react and react-dom, but then you'd end up with more extra npm installs.

Test plan: I ran init and the new app had version ^15.3.1 in the package.json, instead of ^15.3.0, which is how it is in the create-react-app repo and when creating a new app off of master.

@ghost ghost added the CLA Signed label Aug 23, 2016
@gaearon
Copy link
Contributor

gaearon commented Aug 23, 2016

Hmm. Can we change the script to run npm install --save react react-dom instead? It already adds them as a later step so seems like changing that would be simpler.

@vjeux
Copy link
Contributor

vjeux commented Aug 23, 2016

@gaearon that would be way better than trying to move dev dependencies around and re-run npm blindly on the host folder!

@ghost ghost added the CLA Signed label Aug 23, 2016
@wdhorton
Copy link
Contributor Author

Should I also run npm install --save-dev react-test-renderer so that there's no need for a blind npm install at all?

@gaearon
Copy link
Contributor

gaearon commented Aug 23, 2016

Yes please. (We might remove it until some issues are solved but I'll make the call before cutting the release.)

@wdhorton
Copy link
Contributor Author

@gaearon Updated it to run separate installs for each package.

@ghost ghost added the CLA Signed label Aug 24, 2016
pkg === 'react-test-renderer' ? '--save-dev' : '--save',
verbose && '--verbose'
].filter(function(e) { return e; });
var result = spawnSync('npm', args, {stdio: 'inherit'});
Copy link
Contributor

Choose a reason for hiding this comment

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

These should not be three separate spawns.
Let's just use --save for all of them, and do it as one spawn.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok!

@gaearon gaearon merged commit 250605f into facebook:master Aug 25, 2016
@gaearon gaearon added this to the 0.3.0 milestone Aug 25, 2016
@gaearon
Copy link
Contributor

gaearon commented Aug 25, 2016

Thanks!

@gaearon gaearon modified the milestones: 0.2.3, 0.3.0 Aug 25, 2016
gaearon pushed a commit that referenced this pull request Aug 25, 2016
* Get latest version numbers of react and react-dom from npm before install.

* Run separate npm installs for react, react-dom, and react-test-renderer.

* Consolidate into a single npm install.

* Fix misplaced parenthesis, add missing semicolon.

* Add missing semicolon.
@gaearon gaearon mentioned this pull request Aug 25, 2016
stayradiated pushed a commit to stayradiated/create-react-app that referenced this pull request Sep 7, 2016
* Get latest version numbers of react and react-dom from npm before install.

* Run separate npm installs for react, react-dom, and react-test-renderer.

* Consolidate into a single npm install.

* Fix misplaced parenthesis, add missing semicolon.

* Add missing semicolon.
feiqitian pushed a commit to feiqitian/create-react-app that referenced this pull request Oct 25, 2016
* Get latest version numbers of react and react-dom from npm before install.

* Run separate npm installs for react, react-dom, and react-test-renderer.

* Consolidate into a single npm install.

* Fix misplaced parenthesis, add missing semicolon.

* Add missing semicolon.
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants