From 4f8d314c7ab5596318c397e6856bb35497e68a0c Mon Sep 17 00:00:00 2001 From: Alex Guerra Date: Fri, 13 Dec 2019 15:52:52 -0600 Subject: [PATCH 1/8] Remove templates version minimum stopgap --- packages/create-react-app/createReactApp.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/create-react-app/createReactApp.js b/packages/create-react-app/createReactApp.js index f39b0f6af9e..b90b95da9d4 100755 --- a/packages/create-react-app/createReactApp.js +++ b/packages/create-react-app/createReactApp.js @@ -448,10 +448,7 @@ function run( .then(({ isOnline, packageInfo, templateInfo }) => { let packageVersion = semver.coerce(packageInfo.version); - // This environment variable can be removed post-release. - const templatesVersionMinimum = process.env.CRA_INTERNAL_TEST - ? '3.2.0' - : '3.3.0'; + const templatesVersionMinimum = '3.3.0'; // Assume compatibility if we can't test the version. if (!semver.valid(packageVersion)) { From 0d9a3a08c2f69fa67fe39e57e5aa4fcf8e1efaa0 Mon Sep 17 00:00:00 2001 From: Alex Guerra Date: Fri, 13 Dec 2019 15:54:00 -0600 Subject: [PATCH 2/8] Replace indexOf with more idiomatic alternatives --- packages/create-react-app/createReactApp.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/create-react-app/createReactApp.js b/packages/create-react-app/createReactApp.js index b90b95da9d4..1ca7babe06c 100755 --- a/packages/create-react-app/createReactApp.js +++ b/packages/create-react-app/createReactApp.js @@ -588,7 +588,7 @@ function getInstallPackage(version, originalDirectory) { if (validSemver) { packageToInstall += `@${validSemver}`; } else if (version) { - if (version[0] === '@' && version.indexOf('/') === -1) { + if (version[0] === '@' && !version.includes('/')) { packageToInstall += version; } else if (version.match(/^file:/)) { packageToInstall = `file:${path.resolve( @@ -740,7 +740,7 @@ function getPackageInfo(installPackage) { ); return Promise.resolve({ name: assumedProjectName }); }); - } else if (installPackage.indexOf('git+') === 0) { + } else if (installPackage.startsWith('git+')) { // Pull package name out of git urls e.g: // git+https://github.com/mycompany/react-scripts.git // git+ssh://github.com/mycompany/react-scripts.git#v1.2.3 @@ -848,7 +848,7 @@ function checkAppName(appName) { // TODO: there should be a single place that holds the dependencies const dependencies = ['react', 'react-dom', 'react-scripts'].sort(); - if (dependencies.indexOf(appName) >= 0) { + if (dependencies.includes(appName)) { console.error( chalk.red( `We cannot create a project called ${chalk.green( @@ -1010,7 +1010,7 @@ function checkThatNpmCanReadCwd() { // "; cwd = C:\path\to\current\dir" (unquoted) // I couldn't find an easier way to get it. const prefix = '; cwd = '; - const line = lines.find(line => line.indexOf(prefix) === 0); + const line = lines.find(line => line.startsWith(prefix)); if (typeof line !== 'string') { // Fail gracefully. They could remove it. return true; From 8fb4d66f7d2057f7f024d2e267b2887ddd477954 Mon Sep 17 00:00:00 2001 From: Alex Guerra Date: Fri, 13 Dec 2019 16:05:47 -0600 Subject: [PATCH 3/8] Refactor isSafeToCreateProjectIn - Inline errorLogFilePatterns; it's only ever used here. - Alphabetize validFiles. - Simplify log file removal code. - Move unconditional console.log() call outside of the function. This keeps output roughly the same in the common case. --- packages/create-react-app/createReactApp.js | 56 ++++++++++----------- 1 file changed, 26 insertions(+), 30 deletions(-) diff --git a/packages/create-react-app/createReactApp.js b/packages/create-react-app/createReactApp.js index 1ca7babe06c..25d1bd6973d 100755 --- a/packages/create-react-app/createReactApp.js +++ b/packages/create-react-app/createReactApp.js @@ -53,14 +53,6 @@ const validateProjectName = require('validate-npm-package-name'); const packageJson = require('./package.json'); -// These files should be allowed to remain on a failed install, -// but then silently removed during the next create. -const errorLogFilePatterns = [ - 'npm-debug.log', - 'yarn-error.log', - 'yarn-debug.log', -]; - let projectName; const program = new commander.Command(packageJson.name) @@ -247,6 +239,7 @@ function createApp( if (!isSafeToCreateProjectIn(root, name)) { process.exit(1); } + console.log(); console.log(`Creating a new React app in ${chalk.green(root)}.`); console.log(); @@ -914,23 +907,32 @@ function setCaretRangeForRuntimeDeps(packageName) { function isSafeToCreateProjectIn(root, name) { const validFiles = [ '.DS_Store', - 'Thumbs.db', '.git', + '.gitattributes', '.gitignore', - '.idea', - 'README.md', - 'LICENSE', + '.gitlab-ci.yml', '.hg', - '.hgignore', '.hgcheck', + '.hgignore', + '.idea', '.npmignore', - 'mkdocs.yml', - 'docs', '.travis.yml', - '.gitlab-ci.yml', - '.gitattributes', + 'docs', + 'LICENSE', + 'README.md', + 'mkdocs.yml', + 'Thumbs.db', ]; - console.log(); + // These files should be allowed to remain on a failed install, but then + // silently removed during the next create. + const errorLogFilePatterns = [ + 'npm-debug.log', + 'yarn-error.log', + 'yarn-debug.log', + ]; + const isErrorLog = file => { + return errorLogFilePatterns.some(pattern => file.startsWith(pattern)); + }; const conflicts = fs .readdirSync(root) @@ -938,9 +940,7 @@ function isSafeToCreateProjectIn(root, name) { // IntelliJ IDEA creates module files before CRA is launched .filter(file => !/\.iml$/.test(file)) // Don't treat log files from previous installation as conflicts - .filter( - file => !errorLogFilePatterns.some(pattern => file.indexOf(pattern) === 0) - ); + .filter(file => !isErrorLog(file)); if (conflicts.length > 0) { console.log( @@ -958,15 +958,11 @@ function isSafeToCreateProjectIn(root, name) { return false; } - // Remove any remnant files from a previous installation - const currentFiles = fs.readdirSync(path.join(root)); - currentFiles.forEach(file => { - errorLogFilePatterns.forEach(errorLogFilePattern => { - // This will catch `(npm-debug|yarn-error|yarn-debug).log*` files - if (file.indexOf(errorLogFilePattern) === 0) { - fs.removeSync(path.join(root, file)); - } - }); + // Remove any log files from a previous installation. + fs.readdirSync(root).forEach(file => { + if (isErrorLog(file)) { + fs.removeSync(path.join(root, file)); + } }); return true; } From b4c7b3fe18074f938f3226f505625e1d918577b5 Mon Sep 17 00:00:00 2001 From: Alex Guerra Date: Fri, 13 Dec 2019 16:18:44 -0600 Subject: [PATCH 4/8] Differentiate conflicting directories from files --- packages/create-react-app/createReactApp.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/create-react-app/createReactApp.js b/packages/create-react-app/createReactApp.js index 25d1bd6973d..60a259b8fae 100755 --- a/packages/create-react-app/createReactApp.js +++ b/packages/create-react-app/createReactApp.js @@ -948,7 +948,16 @@ function isSafeToCreateProjectIn(root, name) { ); console.log(); for (const file of conflicts) { - console.log(` ${file}`); + try { + const stats = fs.lstatSync(path.join(root, file)); + if (stats.isDirectory()) { + console.log(` ${chalk.blue(`${file}/`)}`); + } else { + console.log(` ${file}`); + } + } catch { + console.log(` ${file}`); + } } console.log(); console.log( From 32abc1c6f5d1f1d145d58fb286482639c9e11abe Mon Sep 17 00:00:00 2001 From: Alex Guerra Date: Fri, 13 Dec 2019 16:08:43 -0600 Subject: [PATCH 5/8] Document yarn version special case It's not obvious why this code exists, so I made the code a little more obvious and added a comment. --- packages/create-react-app/createReactApp.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/create-react-app/createReactApp.js b/packages/create-react-app/createReactApp.js index 60a259b8fae..608753fe023 100755 --- a/packages/create-react-app/createReactApp.js +++ b/packages/create-react-app/createReactApp.js @@ -775,17 +775,25 @@ function checkNpmVersion() { } function checkYarnVersion() { + const minYarnPnp = '1.12.0'; let hasMinYarnPnp = false; let yarnVersion = null; try { yarnVersion = execSync('yarnpkg --version') .toString() .trim(); - let trimmedYarnVersion = /^(.+?)[-+].+$/.exec(yarnVersion); - if (trimmedYarnVersion) { - trimmedYarnVersion = trimmedYarnVersion.pop(); + if (semver.valid(yarnVersion)) { + hasMinYarnPnp = semver.gte(yarnVersion, minYarnPnp); + } else { + // Handle non-semver compliant yarn version strings, which yarn currently + // uses for nightly builds. The regex truncates anything after the first + // dash. See #5362. + const trimmedYarnVersionMatch = /^(.+?)[-+].+$/.exec(yarnVersion); + if (trimmedYarnVersionMatch) { + const trimmedYarnVersion = trimmedYarnVersionMatch.pop(); + hasMinYarnPnp = semver.gte(trimmedYarnVersion, minYarnPnp); + } } - hasMinYarnPnp = semver.gte(trimmedYarnVersion || yarnVersion, '1.12.0'); } catch (err) { // ignore } From 7eadc5ce56a406144566c32bee5561dc4197ea3a Mon Sep 17 00:00:00 2001 From: Alex Guerra Date: Fri, 13 Dec 2019 16:11:07 -0600 Subject: [PATCH 6/8] Refactor checkAppName - Inline printValidationResults. - Standardize the output on failure. Manually verified the output looks nice! --- packages/create-react-app/createReactApp.js | 29 ++++++++++----------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/packages/create-react-app/createReactApp.js b/packages/create-react-app/createReactApp.js index 608753fe023..0fd00d89ad2 100755 --- a/packages/create-react-app/createReactApp.js +++ b/packages/create-react-app/createReactApp.js @@ -184,14 +184,6 @@ if (typeof projectName === 'undefined') { process.exit(1); } -function printValidationResults(results) { - if (typeof results !== 'undefined') { - results.forEach(error => { - console.error(chalk.red(` * ${error}`)); - }); - } -} - createApp( projectName, program.verbose, @@ -838,12 +830,19 @@ function checkAppName(appName) { const validationResult = validateProjectName(appName); if (!validationResult.validForNewPackages) { console.error( - `Could not create a project called ${chalk.red( - `"${appName}"` - )} because of npm naming restrictions:` + chalk.red( + `Cannot create a project named ${chalk.green( + `"${appName}"` + )} because of npm naming restrictions:\n` + ) ); - printValidationResults(validationResult.errors); - printValidationResults(validationResult.warnings); + [ + ...(validationResult.errors || []), + ...(validationResult.warnings || []), + ].forEach(error => { + console.error(chalk.red(` * ${error}`)); + }); + console.error(chalk.red('\nPlease choose a different project name.')); process.exit(1); } @@ -852,8 +851,8 @@ function checkAppName(appName) { if (dependencies.includes(appName)) { console.error( chalk.red( - `We cannot create a project called ${chalk.green( - appName + `Cannot create a project named ${chalk.green( + `"${appName}"` )} because a dependency with the same name exists.\n` + `Due to the way npm works, the following names are not allowed:\n\n` ) + From 2a641a8c881495ace508a726506c557f97181a3f Mon Sep 17 00:00:00 2001 From: Alex Guerra Date: Fri, 13 Dec 2019 16:11:44 -0600 Subject: [PATCH 7/8] Add link for checkThatNpmCanReadCwd --- packages/create-react-app/createReactApp.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/create-react-app/createReactApp.js b/packages/create-react-app/createReactApp.js index 0fd00d89ad2..b150c25596f 100755 --- a/packages/create-react-app/createReactApp.js +++ b/packages/create-react-app/createReactApp.js @@ -998,6 +998,8 @@ function getProxy() { } } } + +// See https://github.com/facebook/create-react-app/pull/3355 function checkThatNpmCanReadCwd() { const cwd = process.cwd(); let childOutput = null; From fcbc452517b828704fbe97e057d1a8745c978faf Mon Sep 17 00:00:00 2001 From: Alex Guerra Date: Fri, 13 Dec 2019 16:35:29 -0600 Subject: [PATCH 8/8] Fixup b4c7b3fe CI failure --- packages/create-react-app/createReactApp.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/create-react-app/createReactApp.js b/packages/create-react-app/createReactApp.js index b150c25596f..bd60bed0d76 100755 --- a/packages/create-react-app/createReactApp.js +++ b/packages/create-react-app/createReactApp.js @@ -962,7 +962,7 @@ function isSafeToCreateProjectIn(root, name) { } else { console.log(` ${file}`); } - } catch { + } catch (e) { console.log(` ${file}`); } }