From 6f7970840a384cc728759cb0bab806bc7f600a5b Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Mon, 7 Aug 2017 22:54:36 -0500 Subject: [PATCH 01/13] Add useYarn to npm adapter initialisation --- lib/utils/dependency-manager-adapter-factory.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/utils/dependency-manager-adapter-factory.js b/lib/utils/dependency-manager-adapter-factory.js index 75e74c78..6ece4bb4 100644 --- a/lib/utils/dependency-manager-adapter-factory.js +++ b/lib/utils/dependency-manager-adapter-factory.js @@ -21,7 +21,7 @@ module.exports = { } }); if (hasNpm) { - adapters.push(new NpmAdapter({ cwd: root, managerOptions: config.npmOptions })); + adapters.push(new NpmAdapter({ cwd: root, managerOptions: config.npmOptions, useYarnCommand: config.useYarn })); } if (hasBower) { adapters.push(new BowerAdapter({ cwd: root, managerOptions: config.bowerOptions })); From 76106f68e2cbf44b94cd427e50302b900adc6fce Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Mon, 7 Aug 2017 23:22:48 -0500 Subject: [PATCH 02/13] Add warning when yarn.lock without useYarn: true --- lib/dependency-manager-adapters/npm.js | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/lib/dependency-manager-adapters/npm.js b/lib/dependency-manager-adapters/npm.js index 39c261d9..54dc95d2 100644 --- a/lib/dependency-manager-adapters/npm.js +++ b/lib/dependency-manager-adapters/npm.js @@ -7,13 +7,12 @@ var path = require('path'); var extend = require('extend'); var debug = require('debug')('ember-try:dependency-manager-adapter:npm'); var rimraf = RSVP.denodeify(require('rimraf')); -var execFileSync = require('child_process').execFileSync; module.exports = CoreObject.extend({ init: function() { this._super.apply(this, arguments); this.run = this.run || require('../utils/run'); - this._setYarnAvailability(); + this._runYarnCheck(); }, useYarnCommand: false, yarnLock: 'yarn.lock', @@ -70,18 +69,16 @@ module.exports = CoreObject.extend({ console.log('Error cleaning up npm scenario:', e); }); }, - _setYarnAvailability: function() { - try { - this._runYarnCheck(); - this.useYarnCommand = true; - debug('Using yarn as dependency manager'); - } catch (e) { - this.useYarnCommand = false; - debug('Using npm as dependency manager'); - } - }, _runYarnCheck: function() { - execFileSync('yarn', [].concat(['--version'])); + if (!this.useYarnCommand) { + try { + if (fs.statSync(path.join(this.cwd, this.yarnLock)).isFile()) { + console.log('Detected a yarn.lock file, add useYarn: true to your configuration if you want to use Yarn to install npm dependencies.'); + } + } catch (e) { + // If no yarn.lock is found, no need to warn. + } + } }, _findCurrentVersionOf: function(packageName) { var filename = path.join(this.cwd, this.nodeModules, packageName, this.packageJSON); From 7fd94ae8a3b82ffc8c074401d861f8fc5b700a85 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Mon, 7 Aug 2017 23:23:09 -0500 Subject: [PATCH 03/13] Add useYarn: true to smoke test --- test/fixtures/dummy-ember-try-config-with-npm-scenarios.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/fixtures/dummy-ember-try-config-with-npm-scenarios.js b/test/fixtures/dummy-ember-try-config-with-npm-scenarios.js index 4a6cf410..b0f93ed8 100644 --- a/test/fixtures/dummy-ember-try-config-with-npm-scenarios.js +++ b/test/fixtures/dummy-ember-try-config-with-npm-scenarios.js @@ -1,4 +1,5 @@ module.exports = { + useYarn: true, scenarios: [ { name: 'test1', From 18097192e16e10f72662e44d6c4d778cb6896092 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Mon, 7 Aug 2017 23:33:14 -0500 Subject: [PATCH 04/13] Remove unnecessary test This is no longer needed because useYarn is what matters, not the presence of the yarn command. --- .../npm-adapter-test.js | 26 ------------------- 1 file changed, 26 deletions(-) diff --git a/test/dependency-manager-adapters/npm-adapter-test.js b/test/dependency-manager-adapters/npm-adapter-test.js index 887299c8..6e6b2731 100644 --- a/test/dependency-manager-adapters/npm-adapter-test.js +++ b/test/dependency-manager-adapters/npm-adapter-test.js @@ -215,32 +215,6 @@ describe('npmAdapter', function() { expect(resultJSON.devDependencies).to.not.have.property('ember-feature-flags'); }); }); - - describe('#_setYarnAvailability', function() { - it('sets useYarnCommand to true if yarn check does not raise', function() { - var npmAdapter = new NpmAdapter({ - cwd: tmpdir, - _runYarnCheck: function() {} - }); - - npmAdapter._setYarnAvailability(); - - expect(npmAdapter.useYarnCommand).to.equal(true); - }); - - it('sets useYarnCommand to false if yarn check raises', function() { - var npmAdapter = new NpmAdapter({ - cwd: tmpdir, - _runYarnCheck: function() { - throw new Error(); - } - }); - - npmAdapter._setYarnAvailability(); - - expect(npmAdapter.useYarnCommand).to.equal(false); - }); - }); }); function assertFileContainsJSON(filename, expectedObj) { From d3f3fdb996ee6e99c9c4b1704de335d118d38519 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Mon, 7 Aug 2017 23:35:33 -0500 Subject: [PATCH 05/13] Update npm adapter tests with useYarn flag This needs to be directly passed into the adapter constructor in the tests, as it is in the code. --- .../npm-adapter-test.js | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/test/dependency-manager-adapters/npm-adapter-test.js b/test/dependency-manager-adapters/npm-adapter-test.js index 6e6b2731..c086cd45 100644 --- a/test/dependency-manager-adapters/npm-adapter-test.js +++ b/test/dependency-manager-adapters/npm-adapter-test.js @@ -15,12 +15,6 @@ var root = process.cwd(); var tmproot = path.join(root, 'tmp'); var tmpdir; -var failedYarnCheck = function() { - throw new Error('Yarn not available'); -}; - -var passedYarnCheck = function() {}; - describe('npmAdapter', function() { beforeEach(function() { tmpdir = tmp.in(tmproot); @@ -38,8 +32,7 @@ describe('npmAdapter', function() { writeJSONFile('node_modules/prove-it.json', { originalNodeModules: true }); writeJSONFile('package.json', { originalPackageJSON: true }); return new NpmAdapter({ - cwd: tmpdir, - _runYarnCheck: failedYarnCheck + cwd: tmpdir }).setup().then(function() { assertFileContainsJSON('package.json.ember-try', { originalPackageJSON: true }); assertFileContainsJSON('.node_modules.ember-try/prove-it.json', { originalNodeModules: true }); @@ -73,8 +66,7 @@ describe('npmAdapter', function() { return new NpmAdapter({ cwd: tmpdir, - run: stubbedRun, - _runYarnCheck: failedYarnCheck + run: stubbedRun })._install().then(function() { expect(runCount).to.equal(2, 'Both commands should run'); }).catch(function(err) { @@ -103,7 +95,6 @@ describe('npmAdapter', function() { return new NpmAdapter({ cwd: tmpdir, run: stubbedRun, - _runYarnCheck: failedYarnCheck, managerOptions: ['--no-shrinkwrap=true'] })._install().then(function() { expect(runCount).to.equal(2, 'Both commands should run'); @@ -129,7 +120,7 @@ describe('npmAdapter', function() { return new NpmAdapter({ cwd: tmpdir, run: stubbedRun, - _runYarnCheck: passedYarnCheck + useYarnCommand: true })._install().then(function() { expect(runCount).to.equal(1, 'Only yarn install should run'); }); @@ -149,7 +140,7 @@ describe('npmAdapter', function() { return new NpmAdapter({ cwd: tmpdir, run: stubbedRun, - _runYarnCheck: passedYarnCheck, + useYarnCommand: true, managerOptions: ['--flat'] })._install().then(function() { expect(runCount).to.equal(1, 'Only yarn install should run with manager options'); From 80679849a0421b3d06569255ee0326df629df883 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Tue, 8 Aug 2017 17:50:47 -0500 Subject: [PATCH 06/13] Change Yarn warning to use ui.writeLine --- lib/dependency-manager-adapters/npm.js | 8 ++++---- lib/utils/scenario-manager.js | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/dependency-manager-adapters/npm.js b/lib/dependency-manager-adapters/npm.js index 54dc95d2..ee0b1ee9 100644 --- a/lib/dependency-manager-adapters/npm.js +++ b/lib/dependency-manager-adapters/npm.js @@ -12,7 +12,6 @@ module.exports = CoreObject.extend({ init: function() { this._super.apply(this, arguments); this.run = this.run || require('../utils/run'); - this._runYarnCheck(); }, useYarnCommand: false, yarnLock: 'yarn.lock', @@ -21,7 +20,8 @@ module.exports = CoreObject.extend({ packageJSONBackupFileName: 'package.json.ember-try', nodeModules: 'node_modules', nodeModulesBackupLocation: '.node_modules.ember-try', - setup: function() { + setup: function({ ui }) { + this._runYarnCheck(ui); return this._backupOriginalDependencies(); }, changeToDependencySet: function(depSet) { @@ -69,11 +69,11 @@ module.exports = CoreObject.extend({ console.log('Error cleaning up npm scenario:', e); }); }, - _runYarnCheck: function() { + _runYarnCheck: function(ui) { if (!this.useYarnCommand) { try { if (fs.statSync(path.join(this.cwd, this.yarnLock)).isFile()) { - console.log('Detected a yarn.lock file, add useYarn: true to your configuration if you want to use Yarn to install npm dependencies.'); + ui.writeLine('Detected a yarn.lock file, add useYarn: true to your configuration if you want to use Yarn to install npm dependencies.'); } } catch (e) { // If no yarn.lock is found, no need to warn. diff --git a/lib/utils/scenario-manager.js b/lib/utils/scenario-manager.js index 40db315d..0e2bd3a9 100644 --- a/lib/utils/scenario-manager.js +++ b/lib/utils/scenario-manager.js @@ -17,8 +17,10 @@ module.exports = CoreObject.extend({ }, setup: function() { + var ui = this.ui; + return mapSeries(this.dependencyManagerAdapters, function(depManager) { - return depManager.setup(); + return depManager.setup({ ui }); }); }, From ef03c4048fda3a03daec664cb5d50d8e50815928 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Tue, 8 Aug 2017 17:51:13 -0500 Subject: [PATCH 07/13] Change Yarn warning to be in yellow --- lib/dependency-manager-adapters/npm.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/dependency-manager-adapters/npm.js b/lib/dependency-manager-adapters/npm.js index ee0b1ee9..a51055c0 100644 --- a/lib/dependency-manager-adapters/npm.js +++ b/lib/dependency-manager-adapters/npm.js @@ -7,6 +7,7 @@ var path = require('path'); var extend = require('extend'); var debug = require('debug')('ember-try:dependency-manager-adapter:npm'); var rimraf = RSVP.denodeify(require('rimraf')); +var chalk = require('chalk'); module.exports = CoreObject.extend({ init: function() { @@ -73,7 +74,7 @@ module.exports = CoreObject.extend({ if (!this.useYarnCommand) { try { if (fs.statSync(path.join(this.cwd, this.yarnLock)).isFile()) { - ui.writeLine('Detected a yarn.lock file, add useYarn: true to your configuration if you want to use Yarn to install npm dependencies.'); + ui.writeLine(chalk.yellow('Detected a yarn.lock file, add useYarn: true to your configuration if you want to use Yarn to install npm dependencies.')); } } catch (e) { // If no yarn.lock is found, no need to warn. From 2e190dc4d582e9f602ed9709985d62c3b9a357ea Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Tue, 8 Aug 2017 18:11:20 -0500 Subject: [PATCH 08/13] Add test expectation for Yarn warning --- test/tasks/try-each-test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/tasks/try-each-test.js b/test/tasks/try-each-test.js index 58631451..a99ff38b 100644 --- a/test/tasks/try-each-test.js +++ b/test/tasks/try-each-test.js @@ -264,10 +264,12 @@ describe('tryEach', function() { }); writeJSONFile('package.json', fixturePackage); + fs.closeSync(fs.openSync('yarn.lock', 'w')); fs.mkdirSync('node_modules'); writeJSONFile('bower.json', fixtureBower); return tryEachTask.run(config.scenarios, {}).then(function(exitCode) { expect(exitCode).to.equal(0, 'exits 0 when all scenarios succeed'); + expect(output).to.include('Detected a yarn.lock file, add useYarn: true to your configuration if you want to use Yarn to install npm dependencies.'); expect(output).to.include('Scenario first: SUCCESS'); expect(output).to.include('Scenario second: SUCCESS'); expect(output).to.include('Scenario with-bower-resolutions: SUCCESS'); From c0cddc25b77aa6b0b2d6de2cf91c69f96a00246f Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Tue, 8 Aug 2017 18:14:53 -0500 Subject: [PATCH 09/13] Update README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 78803519..935d9e2a 100644 --- a/README.md +++ b/README.md @@ -235,7 +235,7 @@ If no `config/ember-try.js` file is present, the default config will be used. Th ##### A note on npm scenarios / "What about yarn?" -If `yarn` is available globally on the system the `ember-try` command is run on, all npm scenarios will use `yarn` for install with the `--no-lockfile` option. +If you include `useYarn: true` in your `ember-try` config, all npm scenarios will use `yarn` for install with the `--no-lockfile` option. If the project has a `yarn.lock`, it will be used *only* on cleanup to restore the project to a clean state. ###### But I use yarn, why `--no-lockfile`? From fb6571c9f5b272dc9dd3f1453653df2e2d71bbf4 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Tue, 8 Aug 2017 18:25:12 -0500 Subject: [PATCH 10/13] Change to not use ES2015 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I should have realised why these weren’t in use elsewhere 😝 --- lib/dependency-manager-adapters/npm.js | 4 ++-- lib/utils/scenario-manager.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/dependency-manager-adapters/npm.js b/lib/dependency-manager-adapters/npm.js index a51055c0..cadac11a 100644 --- a/lib/dependency-manager-adapters/npm.js +++ b/lib/dependency-manager-adapters/npm.js @@ -21,8 +21,8 @@ module.exports = CoreObject.extend({ packageJSONBackupFileName: 'package.json.ember-try', nodeModules: 'node_modules', nodeModulesBackupLocation: '.node_modules.ember-try', - setup: function({ ui }) { - this._runYarnCheck(ui); + setup: function(options) { + this._runYarnCheck(options.ui); return this._backupOriginalDependencies(); }, changeToDependencySet: function(depSet) { diff --git a/lib/utils/scenario-manager.js b/lib/utils/scenario-manager.js index 0e2bd3a9..4b7df0c3 100644 --- a/lib/utils/scenario-manager.js +++ b/lib/utils/scenario-manager.js @@ -20,7 +20,7 @@ module.exports = CoreObject.extend({ var ui = this.ui; return mapSeries(this.dependencyManagerAdapters, function(depManager) { - return depManager.setup({ ui }); + return depManager.setup({ ui: ui }); }); }, From 93f9a9a9d9f3643ad18dd62bf300dbacdb6873e1 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Tue, 8 Aug 2017 18:35:03 -0500 Subject: [PATCH 11/13] Add guard against missing options --- lib/dependency-manager-adapters/npm.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/dependency-manager-adapters/npm.js b/lib/dependency-manager-adapters/npm.js index cadac11a..31727ee7 100644 --- a/lib/dependency-manager-adapters/npm.js +++ b/lib/dependency-manager-adapters/npm.js @@ -22,6 +22,9 @@ module.exports = CoreObject.extend({ nodeModules: 'node_modules', nodeModulesBackupLocation: '.node_modules.ember-try', setup: function(options) { + if (!options) { + options = {}; + } this._runYarnCheck(options.ui); return this._backupOriginalDependencies(); }, From 48753f38cfebc4e333759d267c9afe49391bf676 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Tue, 8 Aug 2017 19:35:46 -0500 Subject: [PATCH 12/13] Replace lockfile-writing implementation Thanks to @kategengler for letting me know about this! --- test/tasks/try-each-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tasks/try-each-test.js b/test/tasks/try-each-test.js index a99ff38b..f451ed73 100644 --- a/test/tasks/try-each-test.js +++ b/test/tasks/try-each-test.js @@ -264,7 +264,7 @@ describe('tryEach', function() { }); writeJSONFile('package.json', fixturePackage); - fs.closeSync(fs.openSync('yarn.lock', 'w')); + fs.writeFileSync('yarn.lock', 'w'); fs.mkdirSync('node_modules'); writeJSONFile('bower.json', fixtureBower); return tryEachTask.run(config.scenarios, {}).then(function(exitCode) { From d019bde0ed084e13e485b9d63eadac475faa9412 Mon Sep 17 00:00:00 2001 From: Buck Doyle Date: Tue, 8 Aug 2017 19:39:20 -0500 Subject: [PATCH 13/13] Remove unnecessary file contents --- test/tasks/try-each-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tasks/try-each-test.js b/test/tasks/try-each-test.js index f451ed73..57050d7e 100644 --- a/test/tasks/try-each-test.js +++ b/test/tasks/try-each-test.js @@ -264,7 +264,7 @@ describe('tryEach', function() { }); writeJSONFile('package.json', fixturePackage); - fs.writeFileSync('yarn.lock', 'w'); + fs.writeFileSync('yarn.lock', ''); fs.mkdirSync('node_modules'); writeJSONFile('bower.json', fixtureBower); return tryEachTask.run(config.scenarios, {}).then(function(exitCode) {