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

Creating a new app in the current directory #334

Closed
jamiebuilds opened this issue Aug 2, 2016 · 20 comments · Fixed by #368 · May be fixed by Neverbland/create-react-app#5
Closed

Creating a new app in the current directory #334

jamiebuilds opened this issue Aug 2, 2016 · 20 comments · Fixed by #368 · May be fixed by Neverbland/create-react-app#5

Comments

@jamiebuilds
Copy link

$ mkdir app
$ cd app
$ git init .
$ create-react-app .
> The directory `.` already exists. Aborting.

It would be nice if this worked

@torifat
Copy link
Contributor

torifat commented Aug 2, 2016

@gaearon I can take this one.

@gaearon
Copy link
Contributor

gaearon commented Aug 2, 2016

@torifat 👍

@torifat
Copy link
Contributor

torifat commented Aug 2, 2016

@gaearon @thejameskyle in my opinion, allowing it in empty directory is a better choice. But, in case you guys prefer it in a non-empty directory. What would be the strategy for conflict?

@lacker
Copy link
Contributor

lacker commented Aug 2, 2016

Currently the behavior is that you can only run create-react-app in a nonexistent directory. It doesn't let you run in an already-existing directory because you don't want to accidentally splat these files into somewhere like ~.

But, if we let you run create-react-app in an empty directory, I think that would be ok. I think it would also be OK if there was a readme / readme.md and dotfiles like .gitignore in the directory. That makes things work with a workflow where you first create a new repo on github, you clone it to your local machine, and then you run create-react-app. It's hard for me to imagine a case where that causes you much pain.

@torifat
Copy link
Contributor

torifat commented Aug 2, 2016

@lacker yeah, I thought about ignoring hidden files/directories. But, since we have a .gitignore and it might conflict with an existing .gitignore file.

@lacker
Copy link
Contributor

lacker commented Aug 2, 2016

If it's an empty directory except for a .gitignore file and a README, then how useful can that old .gitignore file be? I bet in that case it is basically always the github autogenerated one for new projects & it is fine to just replace it.

@torifat
Copy link
Contributor

torifat commented Aug 2, 2016

Checking for .gitignore, README, README.md, LICENSE, LICENSE.md, CONTRIBUTING.md, etc didn't seemed like a good idea to me. That's why I was asking for opinion.

@vjeux
Copy link
Contributor

vjeux commented Aug 3, 2016

If we're in the game of guessing things that would not conflict, I would only check if there's a package.json file. If that's the case, then it's probably unsafe, otherwise should be good to go.

@stephenkingsley
Copy link

@torifat you do this? if not, I am willing to do it~

@torifat
Copy link
Contributor

torifat commented Aug 4, 2016

@stephenkingsley yes, I'm working on it. I will knock you if I couldn't manage any extra time in next 2 days.

@stephenkingsley
Copy link

@torifat 💪 👍

@torifat
Copy link
Contributor

torifat commented Aug 4, 2016

@gaearon we are using deprecated NodeJS APIs. Should I update them too?

screenshot 2016-08-05 04 46 46

@gaearon
Copy link
Contributor

gaearon commented Aug 4, 2016

Ideally yes. It doesn't make sense to rewrite this as callback-heavy code though (this is a console utility after all). So maybe worth looking if fs-extra already has a helper for this.

@jamiebuilds
Copy link
Author

Sindre is here to help

npm install path-exists

@torifat
Copy link
Contributor

torifat commented Aug 4, 2016

@gaearon BTW, is there any way to do it without modifying the globel-cli index file?

// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
//   /!\ DO NOT MODIFY THIS FILE /!\
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

@jamiebuilds
Copy link
Author

Replacing fs.existsSync() with pathExists.sync() isn't really a breaking change. I don't know if you need to make any other changes than that

@torifat
Copy link
Contributor

torifat commented Aug 5, 2016

@thejameskyle I have already done that.

@vjeux vjeux closed this as completed in #368 Aug 8, 2016
@KevinMind
Copy link

status on this @torifat? did it ever get merged?

@DSBalaban
Copy link

@KevinMind Gave this a go in an empty directory (that also happened to be a repo). Worked fine with create-react-app .

@mearleycf
Copy link

Works for me.

@facebook facebook locked as resolved and limited conversation to collaborators Jan 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
9 participants