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

Change error wording and list conflicting files when initializing app #2785

Merged
merged 3 commits into from Jul 14, 2017

Conversation

Projects
None yet
3 participants
@OwenFlood
Copy link
Contributor

commented Jul 13, 2017

PR for #2780:

Tested by running create-react-app with and without folder containing possible conflicting files:
screen shot 2017-07-13 at 2 36 08 pm

@@ -585,7 +581,30 @@ function isSafeToCreateProjectIn(root) {
'.hgignore',
'.hgcheck',
];
return fs.readdirSync(root).every(file => validFiles.indexOf(file) >= 0);
console.log();
let conflicts = fs.readdirSync(root).map((file, index) => {

This comment has been minimized.

Copy link
@Timer

Timer Jul 13, 2017

Collaborator

This could be accomplished more succinctly with a filter:

const conflicts = fs.readdirSync(root).filter(file => !validFiles.includes(file));
}
});

if (conflicts.length > 0) {

This comment has been minimized.

Copy link
@Timer

Timer Jul 13, 2017

Collaborator

How about we invert this for less nesting?

if (conflicts.length < 1) {
  return true;
}
)} already exists and contains files that could cause conflicts:`
);
console.log();
console.log(JSON.stringify(conflicts, null, ' '));

This comment has been minimized.

Copy link
@Timer

Timer Jul 13, 2017

Collaborator

I'm not a fan of JSON output; can we list them instead?

for (const file of conflicts) {
  console.log(`  ${file}`);
}
console.log(
`The directory ${chalk.green(
name
)} already exists and contains files that could cause conflicts:`

This comment has been minimized.

Copy link
@Timer

Timer Jul 13, 2017

Collaborator

I feel like "already exists" is redundant, the original message should be fine:

`The directory ${chalk.green(name)} contains files that could conflict.`
@OwenFlood

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2017

Cool, updated the requested changes 😃

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Jul 14, 2017

Fantastic! Could you please upload a new picture?

@Timer Timer added this to the 1.0.11 milestone Jul 14, 2017

@Timer

Timer approved these changes Jul 14, 2017

@OwenFlood

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2017

New screenshot:
screen shot 2017-07-13 at 6 29 13 pm

@Timer

This comment has been minimized.

Copy link
Collaborator

commented Jul 14, 2017

Beautiful, thank you so much! I switched the period to a colon after conflict.

@Timer Timer merged commit 3354ab9 into facebook:master Jul 14, 2017

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@Timer

This comment has been minimized.

Copy link
Collaborator

commented Aug 9, 2017

Hi there! This change is out in react-scripts@1.0.11; please give it a go! Thanks.

JohnNilsson referenced this pull request in JohnNilsson/create-react-app-typescript Aug 9, 2017

List conflicting files when initializing app (#2785)
* change error wording and list conflicting files when initializing app

* update code

* Update createReactApp.js

morgs32 added a commit to BrickworkSoftware/create-react-app that referenced this pull request Sep 1, 2017

List conflicting files when initializing app (facebook#2785)
* change error wording and list conflicting files when initializing app

* update code

* Update createReactApp.js

JohnNilsson referenced this pull request in JohnNilsson/create-react-app-typescript Sep 9, 2017

List conflicting files when initializing app (#2785)
* change error wording and list conflicting files when initializing app

* update code

* Update createReactApp.js

kasperpeulen added a commit to kasperpeulen/create-react-app that referenced this pull request Sep 24, 2017

List conflicting files when initializing app (facebook#2785)
* change error wording and list conflicting files when initializing app

* update code

* Update createReactApp.js

swengorschewski referenced this pull request in swengorschewski/cra-typescript-electron Oct 16, 2017

List conflicting files when initializing app (#2785)
* change error wording and list conflicting files when initializing app

* update code

* Update createReactApp.js

mrmckeb pushed a commit to mrmckeb/create-react-app that referenced this pull request Nov 6, 2018

List conflicting files when initializing app (facebook#2785)
* change error wording and list conflicting files when initializing app

* update code

* Update createReactApp.js

@lock lock bot locked and limited conversation to collaborators Jan 21, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.