From 37f975ab994e8b39b254f017ae0b0d11e53d11df Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Thu, 2 Mar 2017 20:47:48 -0500 Subject: [PATCH 1/8] Properly extract package name --- packages/create-react-app/index.js | 68 ++++++++++++++++++++------ packages/create-react-app/package.json | 2 + 2 files changed, 54 insertions(+), 16 deletions(-) diff --git a/packages/create-react-app/index.js b/packages/create-react-app/index.js index 5c2205dc6fc..082c1b50b3a 100755 --- a/packages/create-react-app/index.js +++ b/packages/create-react-app/index.js @@ -60,6 +60,8 @@ var execSync = require('child_process').execSync; var spawn = require('cross-spawn'); var semver = require('semver'); var dns = require('dns'); +var tmp = require('tmp'); +var unpack = require('tar-pack').unpack; var projectName; @@ -201,20 +203,25 @@ function install(useYarn, dependencies, verbose, isOnline) { function run(root, appName, version, verbose, originalDirectory, template) { var packageToInstall = getInstallPackage(version); - var packageName = getPackageName(packageToInstall); + var packageName = packageToInstall; var allDependencies = ['react', 'react-dom', packageToInstall]; console.log('Installing packages. This might take a couple minutes.'); - console.log( - 'Installing ' + chalk.cyan('react') + ', ' + chalk.cyan('react-dom') + - ', and ' + chalk.cyan(packageName) + '...' - ); - console.log(); - + var useYarn = shouldUseYarn(); - checkIfOnline(useYarn) + getPackageName(packageToInstall) + .then(function(_packageName) { + packageName = _packageName; + return checkIfOnline(useYarn); + }) .then(function(isOnline) { + console.log( + 'Installing ' + chalk.cyan('react') + ', ' + chalk.cyan('react-dom') + + ', and ' + chalk.cyan(packageName) + '...' + ); + console.log(); + return install(useYarn, allDependencies, verbose, isOnline); }) .then(function() { @@ -288,19 +295,48 @@ function getInstallPackage(version) { // Extract package name from tarball url or path. function getPackageName(installPackage) { if (installPackage.indexOf('.tgz') > -1) { - // The package name could be with or without semver version, e.g. react-scripts-0.2.0-alpha.1.tgz - // However, this function returns package name only without semver version. - return installPackage.match(/^.+\/(.+?)(?:-\d+.+)?\.tgz$/)[1]; + return new Promise(function(resolve, reject) { + tmp.dir({ unsafeCleanup: true }, function(err, tmpdir, callback) { + if (err) { + reject(err); + } else { + resolve({ + tmpdir: tmpdir, + callback: callback + }); + } + }); + }).then(function(obj) { + return new Promise(function(resolve, reject) { + fs.createReadStream(installPackage).pipe(unpack(obj.tmpdir, function(err) { + if (err) { + reject(err); + } else { + var packageName = require(path.join(obj.tmpdir, 'package.json')).name; + try { + obj.callback(); + } catch (ignored) {} + resolve(packageName); + } + })); + }); + }).catch(function(err) { + // The package name could be with or without semver version, e.g. react-scripts-0.2.0-alpha.1.tgz + // However, this function returns package name only without semver version. + console.log('Failed to extract package: ' + err.message); + console.log('Falling back to naive behavior ...') + return Promise.resolve(installPackage.match(/^.+\/(.+?)(?:-\d+.+)?\.tgz$/)[1]); + }); } else if (installPackage.indexOf('git+') === 0) { // 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 - return installPackage.match(/([^\/]+)\.git(#.*)?$/)[1]; + return Promise.resolve(installPackage.match(/([^\/]+)\.git(#.*)?$/)[1]); } else if (installPackage.indexOf('@') > 0) { // Do not match @scope/ when stripping off @version or @tag - return installPackage.charAt(0) + installPackage.substr(1).split('@')[0]; + return Promise.resolve(installPackage.charAt(0) + installPackage.substr(1).split('@')[0]); } - return installPackage; + return Promise.resolve(installPackage); } function checkNpmVersion() { @@ -356,7 +392,7 @@ function checkAppName(appName) { printValidationResults(validationResult.warnings); process.exit(1); } - + // TODO: there should be a single place that holds the dependencies var dependencies = ['react', 'react-dom']; var devDependencies = ['react-scripts']; @@ -449,7 +485,7 @@ function checkIfOnline(useYarn) { // 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); diff --git a/packages/create-react-app/package.json b/packages/create-react-app/package.json index add92d005ca..e2aea1502a5 100644 --- a/packages/create-react-app/package.json +++ b/packages/create-react-app/package.json @@ -25,6 +25,8 @@ "cross-spawn": "^4.0.0", "fs-extra": "^1.0.0", "semver": "^5.0.3", + "tar-pack": "^3.4.0", + "tmp": "0.0.31", "validate-npm-package-name": "^3.0.0" } } From 290174aa547dc79cd2c2eca815026588cb47b009 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Fri, 3 Mar 2017 11:20:11 -0500 Subject: [PATCH 2/8] Download package if need be ... --- packages/create-react-app/index.js | 70 +++++++++++++++++--------- packages/create-react-app/package.json | 1 + 2 files changed, 48 insertions(+), 23 deletions(-) diff --git a/packages/create-react-app/index.js b/packages/create-react-app/index.js index 082c1b50b3a..bf12695a1cd 100755 --- a/packages/create-react-app/index.js +++ b/packages/create-react-app/index.js @@ -62,6 +62,7 @@ var semver = require('semver'); var dns = require('dns'); var tmp = require('tmp'); var unpack = require('tar-pack').unpack; +var hyperquest = require('hyperquest'); var projectName; @@ -292,34 +293,57 @@ function getInstallPackage(version) { return packageToInstall; } +function getTemporaryDirectory() { + return new Promise(function(resolve, reject) { + // Unsafe cleanup lets us recursively delete the directory if it contains + // contents; by default it only allows removal if it's empty + tmp.dir({ unsafeCleanup: true }, function(err, tmpdir, callback) { + if (err) { + reject(err); + } else { + resolve({ + tmpdir: tmpdir, + cleanup: function() { + try { + // Callback might throw and fail, since it's a temp directory the + // OS will clean it up eventually... + callback(); + } catch (ignored) {} + } + }); + } + }); + }); +} + +function extractStream(stream, dest) { + return new Promise(function(resolve, reject) { + stream.pipe(unpack(dest, function(err) { + if (err) { + reject(err); + } else { + resolve(dest); + } + })); + }); +} + // Extract package name from tarball url or path. function getPackageName(installPackage) { if (installPackage.indexOf('.tgz') > -1) { - return new Promise(function(resolve, reject) { - tmp.dir({ unsafeCleanup: true }, function(err, tmpdir, callback) { - if (err) { - reject(err); - } else { - resolve({ - tmpdir: tmpdir, - callback: callback - }); - } + return getTemporaryDirectory().then(function(obj) { + if (installPackage.test(/^http/)) { + var stream = hyperquest(installPackage); + } else { + var stream = fs.createReadStream(installPackage); + } + return extractStream(stream).then(function() { + return obj; }); }).then(function(obj) { - return new Promise(function(resolve, reject) { - fs.createReadStream(installPackage).pipe(unpack(obj.tmpdir, function(err) { - if (err) { - reject(err); - } else { - var packageName = require(path.join(obj.tmpdir, 'package.json')).name; - try { - obj.callback(); - } catch (ignored) {} - resolve(packageName); - } - })); - }); + var packageName = require(path.join(obj.tmpdir, 'package.json')).name; + obj.cleanup(); + return packageName; }).catch(function(err) { // The package name could be with or without semver version, e.g. react-scripts-0.2.0-alpha.1.tgz // However, this function returns package name only without semver version. diff --git a/packages/create-react-app/package.json b/packages/create-react-app/package.json index e2aea1502a5..8752f50273b 100644 --- a/packages/create-react-app/package.json +++ b/packages/create-react-app/package.json @@ -24,6 +24,7 @@ "commander": "^2.9.0", "cross-spawn": "^4.0.0", "fs-extra": "^1.0.0", + "hyperquest": "^2.1.2", "semver": "^5.0.3", "tar-pack": "^3.4.0", "tmp": "0.0.31", From dac32092494ef74dd517458c01b9d349213e5542 Mon Sep 17 00:00:00 2001 From: Joe Haddad Date: Fri, 3 Mar 2017 12:52:49 -0500 Subject: [PATCH 3/8] Oops --- packages/create-react-app/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/create-react-app/index.js b/packages/create-react-app/index.js index bf12695a1cd..a6da047780b 100755 --- a/packages/create-react-app/index.js +++ b/packages/create-react-app/index.js @@ -332,12 +332,12 @@ function extractStream(stream, dest) { function getPackageName(installPackage) { if (installPackage.indexOf('.tgz') > -1) { return getTemporaryDirectory().then(function(obj) { - if (installPackage.test(/^http/)) { + if (/^http/.test(installPackage)) { var stream = hyperquest(installPackage); } else { var stream = fs.createReadStream(installPackage); } - return extractStream(stream).then(function() { + return extractStream(stream, obj.tmpdir).then(function() { return obj; }); }).then(function(obj) { From 454f908296e1d02dd0ebcdc8489b993d05fe733d Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sun, 5 Mar 2017 15:46:59 +0000 Subject: [PATCH 4/8] Add e2e test based on #1537, but without specific filename --- tasks/e2e-installs.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tasks/e2e-installs.sh b/tasks/e2e-installs.sh index 35ec3521b4c..2ab91bdd8e7 100755 --- a/tasks/e2e-installs.sh +++ b/tasks/e2e-installs.sh @@ -145,6 +145,18 @@ if [ "$(ls -1 ./test-app-should-remain | wc -l | tr -d '[:space:]')" != "1" ]; t false fi +# ****************************************************************************** +# Test --scripts-version with a scoped fork tgz of react-scripts +# ****************************************************************************** + +cd $temp_app_path +curl "https://registry.npmjs.org/@enoah_netzach/react-scripts/-/react-scripts-0.9.0.tgz" -o enoah-scripts-0.9.0.tgz +create_react_app --scripts-version=$temp_app_path/enoah-scripts-0.9.0.tgz test-app-scoped-fork-tgz +cd test-app-scoped-fork + +# Check corresponding scripts version is installed. +exists node_modules/@enoah_netzach/react-scripts + # ****************************************************************************** # Test nested folder path as the project name # ****************************************************************************** From b78b49edc0438d0e8ee9b9d98d9a687149810e11 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sun, 5 Mar 2017 15:54:16 +0000 Subject: [PATCH 5/8] Pass packageName through promises A little bit more verbose but explicit and doesn't rely on shared mutable state. --- packages/create-react-app/index.js | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/create-react-app/index.js b/packages/create-react-app/index.js index a6da047780b..c4e54a90615 100755 --- a/packages/create-react-app/index.js +++ b/packages/create-react-app/index.js @@ -204,28 +204,34 @@ function install(useYarn, dependencies, verbose, isOnline) { function run(root, appName, version, verbose, originalDirectory, template) { var packageToInstall = getInstallPackage(version); - var packageName = packageToInstall; - var allDependencies = ['react', 'react-dom', packageToInstall]; console.log('Installing packages. This might take a couple minutes.'); var useYarn = shouldUseYarn(); getPackageName(packageToInstall) - .then(function(_packageName) { - packageName = _packageName; - return checkIfOnline(useYarn); + .then(function(packageName) { + return checkIfOnline(useYarn).then(function(isOnline) { + return { + isOnline: isOnline, + packageName: packageName, + }; + }); }) - .then(function(isOnline) { + .then(function(info) { + var isOnline = info.isOnline; + var packageName = info.packageName; console.log( 'Installing ' + chalk.cyan('react') + ', ' + chalk.cyan('react-dom') + ', and ' + chalk.cyan(packageName) + '...' ); console.log(); - return install(useYarn, allDependencies, verbose, isOnline); + return install(useYarn, allDependencies, verbose, isOnline).then(function() { + return packageName; + }); }) - .then(function() { + .then(function(packageName) { checkNodeVersion(packageName); // Since react-scripts has been installed with --save From 5d64ee4ad2fea36fad782133f0e7d193deeb2a15 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sun, 5 Mar 2017 17:07:29 +0000 Subject: [PATCH 6/8] Fix up directory name in test --- tasks/e2e-installs.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/e2e-installs.sh b/tasks/e2e-installs.sh index 2ab91bdd8e7..cacf4f96e1f 100755 --- a/tasks/e2e-installs.sh +++ b/tasks/e2e-installs.sh @@ -152,7 +152,7 @@ fi cd $temp_app_path curl "https://registry.npmjs.org/@enoah_netzach/react-scripts/-/react-scripts-0.9.0.tgz" -o enoah-scripts-0.9.0.tgz create_react_app --scripts-version=$temp_app_path/enoah-scripts-0.9.0.tgz test-app-scoped-fork-tgz -cd test-app-scoped-fork +cd test-app-scoped-fork-tgz # Check corresponding scripts version is installed. exists node_modules/@enoah_netzach/react-scripts From 22f69457e9343b2a2249b57bf02e49a670ec0eb8 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sun, 5 Mar 2017 17:11:09 +0000 Subject: [PATCH 7/8] Tweak failure message --- packages/create-react-app/index.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/create-react-app/index.js b/packages/create-react-app/index.js index c4e54a90615..6e14cf7abbd 100755 --- a/packages/create-react-app/index.js +++ b/packages/create-react-app/index.js @@ -353,9 +353,10 @@ function getPackageName(installPackage) { }).catch(function(err) { // The package name could be with or without semver version, e.g. react-scripts-0.2.0-alpha.1.tgz // However, this function returns package name only without semver version. - console.log('Failed to extract package: ' + err.message); - console.log('Falling back to naive behavior ...') - return Promise.resolve(installPackage.match(/^.+\/(.+?)(?:-\d+.+)?\.tgz$/)[1]); + console.log('Could not extract the package name from the archive: ' + err.message); + var assumedProjectName = installPackage.match(/^.+\/(.+?)(?:-\d+.+)?\.tgz$/)[1]; + console.log('Based on the filename, assuming it is "' + chalk.cyan(assumedProjectName) + '"'); + return Promise.resolve(assumedProjectName); }); } else if (installPackage.indexOf('git+') === 0) { // Pull package name out of git urls e.g: From 07c57de0c59bfe24810b503d6477d8891be601a0 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Sun, 5 Mar 2017 23:22:38 +0000 Subject: [PATCH 8/8] Fix lint --- packages/create-react-app/index.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/create-react-app/index.js b/packages/create-react-app/index.js index 6e14cf7abbd..ebeae5b41b0 100755 --- a/packages/create-react-app/index.js +++ b/packages/create-react-app/index.js @@ -311,10 +311,11 @@ function getTemporaryDirectory() { tmpdir: tmpdir, cleanup: function() { try { + callback(); + } catch (ignored) { // Callback might throw and fail, since it's a temp directory the // OS will clean it up eventually... - callback(); - } catch (ignored) {} + } } }); } @@ -338,10 +339,11 @@ function extractStream(stream, dest) { function getPackageName(installPackage) { if (installPackage.indexOf('.tgz') > -1) { return getTemporaryDirectory().then(function(obj) { + var stream; if (/^http/.test(installPackage)) { - var stream = hyperquest(installPackage); + stream = hyperquest(installPackage); } else { - var stream = fs.createReadStream(installPackage); + stream = fs.createReadStream(installPackage); } return extractStream(stream, obj.tmpdir).then(function() { return obj;