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

fix cdpath bug #166

Merged
merged 1 commit into from
Jul 25, 2016
Merged

fix cdpath bug #166

merged 1 commit into from
Jul 25, 2016

Conversation

lacker
Copy link
Contributor

@lacker lacker commented Jul 25, 2016

Fixes the instructions printed by the script so that if you create a new app with create-react-app foo, it will suggest getting there with cd foo rather than cd /some/big/long/path/foo.

This will require a new global-cli deploy to work, but it's backward compatible so that the new init script works with the old global-cli (and just is buggy in the same way the current one is).

@ghost ghost added the CLA Signed label Jul 25, 2016
@mxstbr
Copy link
Contributor

mxstbr commented Jul 25, 2016

One small question, doesn't the process.cwd() change the same way every time? Couldn't we just undo that change by joining it where the path.join(process.cwd(), appName) check is?

@lacker
Copy link
Contributor Author

lacker commented Jul 25, 2016

It changes the same way every time in the sense that process.chdir always turns the working directory into the directory where the app is created. But that means the original working directory is lost, and the original working directory is the one that the developer is going to be cd'ing from.

For example, if you start in /Users/bob/ and run create-react-app my-app, then process.cwd() will be /Users/bob/my-app in the init script. But if you start in /Users/bob/stuff and run create-react-app ~/my-app, then process.cwd() will also be /Users/bob/my-app in the init script. But in the former case you want to tell the user to cd my-app and in the latter case you want to tell the user to cd /Users/bob/my-app.

@mxstbr
Copy link
Contributor

mxstbr commented Jul 25, 2016

Ahh, fair enough! 👍 This LGTM then!

@lacker lacker merged commit 06df2ec into facebook:master Jul 25, 2016
@gaearon gaearon added this to the 0.2.0 milestone Jul 25, 2016
@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