Skip to content

Use "commander" for cli argv handling - #1195

Merged
gaearon merged 6 commits into
react:masterfrom
EnoahNetzach:use-commander
Dec 11, 2016
Merged

Use "commander" for cli argv handling#1195
gaearon merged 6 commits into
react:masterfrom
EnoahNetzach:use-commander

Conversation

@EnoahNetzach

@EnoahNetzachEnoahNetzach commented Dec 7, 2016

Copy link
Copy Markdown
Contributor

commander is more easily configurable than the previous tool, and gives some output out-of-the-box.

Also I've bumped the comment on the minimal node version as per #575.

@gaearon

gaearon commented Dec 7, 2016

Copy link
Copy Markdown
Contributor

Could you show the output difference before and after?

@EnoahNetzach

EnoahNetzach commented Dec 7, 2016•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Before:

  • create-react-app -h
Usage: create-react-app <project-directory> [--verbose]
  • create-react-app --version
create-react-app version: 1.0.0

After:

  • create-react-app -h
 Usage: index [options] <name>
Options:
-h, --help output usage information
-V, --version output the version number
-v, --verbose to print logs while init
-s, --scripts-version <alternative package> to select a react script variant [react-scripts]
Example of valid script version values:
- a specific npm version: "0.22.0-rc1"
- a .tgz archive from any npm repo: "https://registry.npmjs.org/react-scripts/-/react-scripts-0.20.0.tgz"
- a package prepared with `tasks/clean_pack.sh`: "/Users/home/vjeux/create-react-app/react-scripts-0.22.0.tgz"
  • create-react-app --version
1.0.0

@EnoahNetzachEnoahNetzach mentioned this pull request Dec 9, 2016
Merged
29 tasks
@gaearon

gaearon commented Dec 10, 2016

Copy link
Copy Markdown
Contributor

Does the CLI still run on old Node? (It should print an explanation before exiting the process.)

@EnoahNetzach

EnoahNetzach commented Dec 10, 2016

Copy link
Copy Markdown
ContributorAuthor

I tried it on a docker running node 0.10.84 and npm 2.15.1.

The output was the same.
After trying to install every and each module.
Wouldn't be better if we just console.error the explanation asap?

@gaearon

gaearon commented Dec 10, 2016

Copy link
Copy Markdown
Contributor

Wouldn't be better if we just console.error the explanation asap?

Yep. The thinking is that with time, minimal version of react-scripts may increase so we still need to keep check afterwards. It doesn't hurt to check sooner though!

@EnoahNetzach

EnoahNetzach commented Dec 10, 2016

Copy link
Copy Markdown
ContributorAuthor

Should we also add a quick check in travis?

@gaearon

gaearon commented Dec 10, 2016

Copy link
Copy Markdown
Contributor

It would be nice.

gaearon
gaearon reviewed Dec 10, 2016
View reviewed changes
Comment threadtasks/e2e.sh Outdated
if [ `node --version | sed -e 's/^v//' -e 's/\..\+//g'` -lt 4 ]
then
cd $temp_app_path
node "$root_path"/packages/create-react-app/index.js test-node-version && exit 1 || exit 0

@gaearongaearonDec 10, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should verify that it prints a nice message instead of failing with e.g. a parse error.

@gaearon

gaearon commented Dec 11, 2016

Copy link
Copy Markdown
Contributor

I'm a bit concerned the project is abandoned: tj/commander.js#568.

@gaearon

gaearon commented Dec 11, 2016

Copy link
Copy Markdown
Contributor

Yarn uses it so should be fine. Thanks!

@gaearon
gaearon merged commit 7f9fb29 into react:masterDec 11, 2016
@gaearongaearon added this to the 0.8.4 milestone Dec 11, 2016
This was referenced Dec 11, 2016
Merged
Merged
@EnoahNetzachEnoahNetzach mentioned this pull request Dec 11, 2016
Closed
alexdriaguine pushed a commit to alexdriaguine/create-react-app that referenced this pull request Jan 23, 2017
@EnoahNetzach@alexdriaguine
Use "commander" for cli argv handling (react#1195)
5d6a085
* Use "commander" for cli argv handling
* Handle different scripts version forms and exits without a name given
* Revert comment about min supported node version
* Check sooner for the minimal node version
* Add travis test for node <4
* Parse stderr in node versions <4
randycoulman pushed a commit to CodingZeal/create-react-app that referenced this pull request May 8, 2017
@EnoahNetzach@randycoulman
Use "commander" for cli argv handling (react#1195)
ae6664d
* Use "commander" for cli argv handling
* Handle different scripts version forms and exits without a name given
* Revert comment about min supported node version
* Check sooner for the minimal node version
* Add travis test for node <4
* Parse stderr in node versions <4
@locklockBot locked and limited conversation to collaborators Jan 21, 2019
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@gaearongaearongaearon left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CLA Signedtag: enhancement

Projects

None yet

Milestone

0.8.4

Development

Successfully merging this pull request may close these issues.

3 participants

@EnoahNetzach@gaearon@facebook-github-bot