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.

Sign up for GitHub

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

Minor refactors in create-react-app #8178

Merged
merged 8 commits into from
Dec 16, 2019
Merged

Minor refactors in create-react-app #8178

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4f8d314
Remove templates version minimum stopgap
heyimalex Dec 13, 2019
0d9a3a0
Replace indexOf with more idiomatic alternatives
heyimalex Dec 13, 2019
8fb4d66
Refactor isSafeToCreateProjectIn
heyimalex Dec 13, 2019
b4c7b3f
Differentiate conflicting directories from files
heyimalex Dec 13, 2019
32abc1c
Document yarn version special case
heyimalex Dec 13, 2019
7eadc5c
Refactor checkAppName
heyimalex Dec 13, 2019
2a641a8
Add link for checkThatNpmCanReadCwd
heyimalex Dec 13, 2019
fcbc452
Fixup b4c7b3fe CI failure
heyimalex Dec 13, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 69 additions & 58 deletions packages/create-react-app/createReactApp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -192,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,
Expand Down Expand Up @@ -247,6 +231,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();
Expand Down Expand Up @@ -448,10 +433,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)) {
Expand Down Expand Up @@ -591,7 +573,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(
Expand Down Expand Up @@ -743,7 +725,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
Expand Down Expand Up @@ -785,17 +767,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
}
Expand Down Expand Up @@ -840,22 +830,29 @@ 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);
}

// 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(
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`
) +
Expand Down Expand Up @@ -917,41 +914,57 @@ 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)
.filter(file => !validFiles.includes(file))
// 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(
`The directory ${chalk.green(name)} contains files that could conflict:`
);
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 (e) {
console.log(` ${file}`);
}
}
console.log();
console.log(
Expand All @@ -961,15 +974,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;
}
Expand All @@ -989,6 +998,8 @@ function getProxy() {
}
}
}

// See https://github.com/facebook/create-react-app/pull/3355
function checkThatNpmCanReadCwd() {
const cwd = process.cwd();
let childOutput = null;
Expand All @@ -1013,7 +1024,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;
Expand Down