From efb8eeae77db1aa140fa00a2216f974d603a0e8b Mon Sep 17 00:00:00 2001 From: Simon Vocella Date: Sat, 21 Jan 2017 02:24:54 +0100 Subject: [PATCH 1/8] add --offline flag when we are using yarn and we are offline --- packages/create-react-app/index.js | 116 +++++++++++++++---------- packages/react-scripts/scripts/init.js | 7 +- 2 files changed, 74 insertions(+), 49 deletions(-) diff --git a/packages/create-react-app/index.js b/packages/create-react-app/index.js index d5afee43b8f..6d50320bbdd 100755 --- a/packages/create-react-app/index.js +++ b/packages/create-react-app/index.js @@ -58,6 +58,7 @@ var path = require('path'); var execSync = require('child_process').execSync; var spawn = require('cross-spawn'); var semver = require('semver'); +var dns = require('dns'); var projectName; @@ -145,25 +146,37 @@ function shouldUseYarn() { } } -function install(dependencies, verbose, callback) { - var command; - var args; - if (shouldUseYarn()) { - command = 'yarnpkg'; - args = [ 'add', '--exact'].concat(dependencies); - } else { - checkNpmVersion(); - command = 'npm'; - args = ['install', '--save', '--save-exact'].concat(dependencies); - } +function install(dependencies, verbose, isOnline) { + return new Promise(function(resolve, reject) { + var command; + var args; + if (shouldUseYarn()) { + command = 'yarnpkg'; + args = [ + 'add', + '--exact', + isOnline === false && '--offline' + ].concat(dependencies); + } else { + checkNpmVersion(); + command = 'npm'; + args = ['install', '--save', '--save-exact'].concat(dependencies); + } - if (verbose) { - args.push('--verbose'); - } + if (verbose) { + args.push('--verbose'); + } + + var child = spawn(command, args, {stdio: 'inherit'}); + child.on('close', function(code) { + if (code !== 0) { + console.log(); + console.error('Aborting installation.', chalk.cyan(command + ' ' + args.join(' ')), 'has failed.'); + reject(); + } - var child = spawn(command, args, {stdio: 'inherit'}); - child.on('close', function(code) { - callback(code, command, args); + resolve(isOnline); + }); }); } @@ -180,35 +193,11 @@ function run(root, appName, version, verbose, originalDirectory, template) { ); console.log(); - install(allDependencies, verbose, function(code, command, args) { - if (code !== 0) { - console.log(); - console.error('Aborting installation.', chalk.cyan(command + ' ' + args.join(' ')), 'has failed.'); - // On 'exit' we will delete these files from target directory. - var knownGeneratedFiles = [ - 'package.json', 'npm-debug.log', 'yarn-error.log', 'yarn-debug.log', 'node_modules' - ]; - var currentFiles = fs.readdirSync(path.join(root)); - currentFiles.forEach(function (file) { - knownGeneratedFiles.forEach(function (fileToMatch) { - // This will catch `(npm-debug|yarn-error|yarn-debug).log*` files - // and the rest of knownGeneratedFiles. - if ((fileToMatch.match(/.log/g) && file.indexOf(fileToMatch) === 0) || file === fileToMatch) { - console.log('Deleting generated file...', chalk.cyan(file)); - fs.removeSync(path.join(root, file)); - } - }); - }); - var remainingFiles = fs.readdirSync(path.join(root)); - if (!remainingFiles.length) { - // Delete target folder if empty - console.log('Deleting', chalk.cyan(appName + '/'), 'from', chalk.cyan(path.resolve(root, '..'))); - fs.removeSync(path.join(root)); - } - console.log('Done.'); - process.exit(1); - } - + checkIfOnline() + .then(function(isOnline) { + return install(allDependencies, verbose, isOnline); + }) + .then(function(isOnline) { checkNodeVersion(packageName); // Since react-scripts has been installed with --save @@ -223,7 +212,32 @@ function run(root, appName, version, verbose, originalDirectory, template) { 'init.js' ); var init = require(scriptsPath); - init(root, appName, verbose, originalDirectory, template); + init(root, appName, verbose, originalDirectory, template, isOnline); + }) + .catch(function(command, args) { + // On 'exit' we will delete these files from target directory. + var knownGeneratedFiles = [ + 'package.json', 'npm-debug.log', 'yarn-error.log', 'yarn-debug.log', 'node_modules' + ]; + var currentFiles = fs.readdirSync(path.join(root)); + currentFiles.forEach(function (file) { + knownGeneratedFiles.forEach(function (fileToMatch) { + // This will catch `(npm-debug|yarn-error|yarn-debug).log*` files + // and the rest of knownGeneratedFiles. + if ((fileToMatch.match(/.log/g) && file.indexOf(fileToMatch) === 0) || file === fileToMatch) { + console.log('Deleting generated file...', chalk.cyan(file)); + fs.removeSync(path.join(root, file)); + } + }); + }); + var remainingFiles = fs.readdirSync(path.join(root)); + if (!remainingFiles.length) { + // Delete target folder if empty + console.log('Deleting', chalk.cyan(appName + '/'), 'from', chalk.cyan(path.resolve(root, '..'))); + fs.removeSync(path.join(root)); + } + console.log('Done.'); + process.exit(1); }); } @@ -364,3 +378,11 @@ function isSafeToCreateProjectIn(root) { return validFiles.indexOf(file) >= 0; }); } + +function checkIfOnline() { + return new Promise(function(resolve) { + dns.resolve('registry.yarnpkg.com', function(err) { + resolve(err === null); + }); + }); +} diff --git a/packages/react-scripts/scripts/init.js b/packages/react-scripts/scripts/init.js index aa62265ccc5..2a603156421 100644 --- a/packages/react-scripts/scripts/init.js +++ b/packages/react-scripts/scripts/init.js @@ -12,7 +12,7 @@ var path = require('path'); var spawn = require('cross-spawn'); var chalk = require('chalk'); -module.exports = function(appPath, appName, verbose, originalDirectory, template) { +module.exports = function(appPath, appName, verbose, originalDirectory, template, isOnline) { var ownPackageName = require(path.join(__dirname, '..', 'package.json')).name; var ownPath = path.join(appPath, 'node_modules', ownPackageName); var appPackage = require(path.join(appPath, 'package.json')); @@ -69,7 +69,10 @@ module.exports = function(appPath, appName, verbose, originalDirectory, template if (useYarn) { command = 'yarnpkg'; - args = ['add']; + args = [ + 'add', + isOnline === false && '--offline' + ]; } else { command = 'npm'; args = [ From 0a2c71a24cddeb18a3cded7a72f704b55847611e Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 28 Feb 2017 14:17:27 +0000 Subject: [PATCH 2/8] Revert changes to init script We only run these commands for backward compat mode, in which we wouldn't receive the offline flag anyway --- packages/react-scripts/scripts/init.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/react-scripts/scripts/init.js b/packages/react-scripts/scripts/init.js index 2a603156421..aa62265ccc5 100644 --- a/packages/react-scripts/scripts/init.js +++ b/packages/react-scripts/scripts/init.js @@ -12,7 +12,7 @@ var path = require('path'); var spawn = require('cross-spawn'); var chalk = require('chalk'); -module.exports = function(appPath, appName, verbose, originalDirectory, template, isOnline) { +module.exports = function(appPath, appName, verbose, originalDirectory, template) { var ownPackageName = require(path.join(__dirname, '..', 'package.json')).name; var ownPath = path.join(appPath, 'node_modules', ownPackageName); var appPackage = require(path.join(appPath, 'package.json')); @@ -69,10 +69,7 @@ module.exports = function(appPath, appName, verbose, originalDirectory, template if (useYarn) { command = 'yarnpkg'; - args = [ - 'add', - isOnline === false && '--offline' - ]; + args = ['add']; } else { command = 'npm'; args = [ From 9b60cc1f4db65e626961caee4724b952cf2c0e05 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 28 Feb 2017 14:19:15 +0000 Subject: [PATCH 3/8] Don't pass isOnline to init script because it doesn't need it --- packages/create-react-app/index.js | 94 +++++++++++++++--------------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/packages/create-react-app/index.js b/packages/create-react-app/index.js index 74f0ee866ed..a88dd77fcd0 100755 --- a/packages/create-react-app/index.js +++ b/packages/create-react-app/index.js @@ -182,9 +182,9 @@ function install(dependencies, verbose, isOnline) { console.log(); console.error('Aborting installation.', chalk.cyan(command + ' ' + args.join(' ')), 'has failed.'); reject(); + return; } - - resolve(isOnline); + resolve(); }); }); } @@ -203,53 +203,53 @@ function run(root, appName, version, verbose, originalDirectory, template) { console.log(); checkIfOnline() - .then(function(isOnline) { - return install(allDependencies, verbose, isOnline); - }) - .then(function(isOnline) { - checkNodeVersion(packageName); - - // Since react-scripts has been installed with --save - // we need to move it into devDependencies and rewrite package.json - // also ensure react dependencies have caret version range - fixDependencies(packageName); - - var scriptsPath = path.resolve( - process.cwd(), - 'node_modules', - packageName, - 'scripts', - 'init.js' - ); - var init = require(scriptsPath); - init(root, appName, verbose, originalDirectory, template, isOnline); - }) - .catch(function(command, args) { - // On 'exit' we will delete these files from target directory. - var knownGeneratedFiles = [ - 'package.json', 'npm-debug.log', 'yarn-error.log', 'yarn-debug.log', 'node_modules' - ]; - var currentFiles = fs.readdirSync(path.join(root)); - currentFiles.forEach(function (file) { - knownGeneratedFiles.forEach(function (fileToMatch) { - // This will catch `(npm-debug|yarn-error|yarn-debug).log*` files - // and the rest of knownGeneratedFiles. - if ((fileToMatch.match(/.log/g) && file.indexOf(fileToMatch) === 0) || file === fileToMatch) { - console.log('Deleting generated file...', chalk.cyan(file)); - fs.removeSync(path.join(root, file)); - } + .then(function(isOnline) { + return install(allDependencies, verbose, isOnline); + }) + .then(function() { + checkNodeVersion(packageName); + + // Since react-scripts has been installed with --save + // we need to move it into devDependencies and rewrite package.json + // also ensure react dependencies have caret version range + fixDependencies(packageName); + + var scriptsPath = path.resolve( + process.cwd(), + 'node_modules', + packageName, + 'scripts', + 'init.js' + ); + var init = require(scriptsPath); + init(root, appName, verbose, originalDirectory, template); + }) + .catch(function(command, args) { + // On 'exit' we will delete these files from target directory. + var knownGeneratedFiles = [ + 'package.json', 'npm-debug.log', 'yarn-error.log', 'yarn-debug.log', 'node_modules' + ]; + var currentFiles = fs.readdirSync(path.join(root)); + currentFiles.forEach(function (file) { + knownGeneratedFiles.forEach(function (fileToMatch) { + // This will catch `(npm-debug|yarn-error|yarn-debug).log*` files + // and the rest of knownGeneratedFiles. + if ((fileToMatch.match(/.log/g) && file.indexOf(fileToMatch) === 0) || file === fileToMatch) { + console.log('Deleting generated file...', chalk.cyan(file)); + fs.removeSync(path.join(root, file)); + } + }); }); + var remainingFiles = fs.readdirSync(path.join(root)); + if (!remainingFiles.length) { + // Delete target folder if empty + console.log('Deleting', chalk.cyan(appName + '/'), 'from', chalk.cyan(path.resolve(root, '..'))); + process.chdir(path.resolve(root, '..')); + fs.removeSync(path.join(root)); + } + console.log('Done.'); + process.exit(1); }); - var remainingFiles = fs.readdirSync(path.join(root)); - if (!remainingFiles.length) { - // Delete target folder if empty - console.log('Deleting', chalk.cyan(appName + '/'), 'from', chalk.cyan(path.resolve(root, '..'))); - process.chdir(path.resolve(root, '..')); - fs.removeSync(path.join(root)); - } - console.log('Done.'); - process.exit(1); - }); } function getInstallPackage(version) { From 90af8c20d0e304675985805487743af080446ec1 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 28 Feb 2017 14:25:02 +0000 Subject: [PATCH 4/8] Don't ping the Yarn registry if user doesn't have Yarn --- packages/create-react-app/index.js | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/create-react-app/index.js b/packages/create-react-app/index.js index a88dd77fcd0..840083e35d6 100755 --- a/packages/create-react-app/index.js +++ b/packages/create-react-app/index.js @@ -155,11 +155,11 @@ function shouldUseYarn() { } } -function install(dependencies, verbose, isOnline) { +function install(useYarn, dependencies, verbose, isOnline) { return new Promise(function(resolve, reject) { var command; var args; - if (shouldUseYarn()) { + if (useYarn) { command = 'yarnpkg'; args = [ 'add', @@ -201,10 +201,11 @@ function run(root, appName, version, verbose, originalDirectory, template) { ', and ' + chalk.cyan(packageName) + '...' ); console.log(); - - checkIfOnline() + + var useYarn = shouldUseYarn(); + checkIfOnline(useYarn) .then(function(isOnline) { - return install(allDependencies, verbose, isOnline); + return install(useYarn, allDependencies, verbose, isOnline); }) .then(function() { checkNodeVersion(packageName); @@ -422,7 +423,13 @@ function isSafeToCreateProjectIn(root) { }); } -function checkIfOnline() { +function checkIfOnline(useYarn) { + if (!useYarn) { + // Don't ping the Yarn registry. + // We'll just assume the best case. + return Promise.resolve(true); + } + return new Promise(function(resolve) { dns.resolve('registry.yarnpkg.com', function(err) { resolve(err === null); From 0f8e08ee494a8a87b4b808e9755085c49a4ac04f Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 28 Feb 2017 14:27:56 +0000 Subject: [PATCH 5/8] Remove unused/wrong arguments --- packages/create-react-app/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/create-react-app/index.js b/packages/create-react-app/index.js index 840083e35d6..d8a8edd2630 100755 --- a/packages/create-react-app/index.js +++ b/packages/create-react-app/index.js @@ -225,7 +225,7 @@ function run(root, appName, version, verbose, originalDirectory, template) { var init = require(scriptsPath); init(root, appName, verbose, originalDirectory, template); }) - .catch(function(command, args) { + .catch(function() { // On 'exit' we will delete these files from target directory. var knownGeneratedFiles = [ 'package.json', 'npm-debug.log', 'yarn-error.log', 'yarn-debug.log', 'node_modules' From c9cece5e67cf8af1b4f208e67766c573aab8df7f Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 28 Feb 2017 14:30:01 +0000 Subject: [PATCH 6/8] Move logs to error handler --- packages/create-react-app/index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/create-react-app/index.js b/packages/create-react-app/index.js index d8a8edd2630..86a4bdf21bf 100755 --- a/packages/create-react-app/index.js +++ b/packages/create-react-app/index.js @@ -179,8 +179,6 @@ function install(useYarn, dependencies, verbose, isOnline) { var child = spawn(command, args, {stdio: 'inherit'}); child.on('close', function(code) { if (code !== 0) { - console.log(); - console.error('Aborting installation.', chalk.cyan(command + ' ' + args.join(' ')), 'has failed.'); reject(); return; } @@ -226,6 +224,9 @@ function run(root, appName, version, verbose, originalDirectory, template) { init(root, appName, verbose, originalDirectory, template); }) .catch(function() { + console.log(); + console.error('Aborting installation.', chalk.cyan(command + ' ' + args.join(' ')), 'has failed.'); + // On 'exit' we will delete these files from target directory. var knownGeneratedFiles = [ 'package.json', 'npm-debug.log', 'yarn-error.log', 'yarn-debug.log', 'node_modules' From ea856e3bb28c3c98344a3fa28c5ad74e5cd279c5 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 28 Feb 2017 14:48:02 +0000 Subject: [PATCH 7/8] Fix error handling --- packages/create-react-app/index.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/create-react-app/index.js b/packages/create-react-app/index.js index 86a4bdf21bf..11a6e885f67 100755 --- a/packages/create-react-app/index.js +++ b/packages/create-react-app/index.js @@ -179,7 +179,9 @@ function install(useYarn, dependencies, verbose, isOnline) { var child = spawn(command, args, {stdio: 'inherit'}); child.on('close', function(code) { if (code !== 0) { - reject(); + reject({ + command: command + ' ' + args.join(' ') + }); return; } resolve(); @@ -223,9 +225,13 @@ function run(root, appName, version, verbose, originalDirectory, template) { var init = require(scriptsPath); init(root, appName, verbose, originalDirectory, template); }) - .catch(function() { + .catch(function(reason) { + console.log(); + console.log('Aborting installation.'); + if (reason.command) { + console.log(' ' + chalk.cyan(reason.command), 'has failed.') + } console.log(); - console.error('Aborting installation.', chalk.cyan(command + ' ' + args.join(' ')), 'has failed.'); // On 'exit' we will delete these files from target directory. var knownGeneratedFiles = [ From 14456628606fa4d2ecacfb4106e5cb032884bff9 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 28 Feb 2017 14:58:58 +0000 Subject: [PATCH 8/8] Report to the user that they're offline --- packages/create-react-app/index.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/create-react-app/index.js b/packages/create-react-app/index.js index 11a6e885f67..fda95f1c041 100755 --- a/packages/create-react-app/index.js +++ b/packages/create-react-app/index.js @@ -166,6 +166,13 @@ function install(useYarn, dependencies, verbose, isOnline) { '--exact', isOnline === false && '--offline' ].concat(dependencies); + + if (!isOnline) { + console.log(chalk.yellow('You appear to be offline.')); + console.log(chalk.yellow('Falling back to the local Yarn cache.')); + console.log(); + } + } else { checkNpmVersion(); command = 'npm';