Skip to content

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
scripts/init.js Outdated
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.

3 participants