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.

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

Change Yarn detection to look for lockfile #138

Merged
merged 13 commits into from
Aug 9, 2017
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`?
Expand Down
29 changes: 15 additions & 14 deletions lib/dependency-manager-adapters/npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
var chalk = require('chalk');

module.exports = CoreObject.extend({
init: function() {
this._super.apply(this, arguments);
this.run = this.run || require('../utils/run');
this._setYarnAvailability();
},
useYarnCommand: false,
yarnLock: 'yarn.lock',
Expand All @@ -22,7 +21,11 @@ module.exports = CoreObject.extend({
packageJSONBackupFileName: 'package.json.ember-try',
nodeModules: 'node_modules',
nodeModulesBackupLocation: '.node_modules.ember-try',
setup: function() {
setup: function(options) {
if (!options) {
options = {};
}
this._runYarnCheck(options.ui);
return this._backupOriginalDependencies();
},
changeToDependencySet: function(depSet) {
Expand Down Expand Up @@ -70,19 +73,17 @@ 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(ui) {
if (!this.useYarnCommand) {
try {
if (fs.statSync(path.join(this.cwd, this.yarnLock)).isFile()) {
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.
}
}
},
_runYarnCheck: function() {
execFileSync('yarn', [].concat(['--version']));
},
_findCurrentVersionOf: function(packageName) {
var filename = path.join(this.cwd, this.nodeModules, packageName, this.packageJSON);
if (fs.existsSync(filename)) {
Expand Down
2 changes: 1 addition & 1 deletion lib/utils/dependency-manager-adapter-factory.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }));
Expand Down
4 changes: 3 additions & 1 deletion lib/utils/scenario-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: ui });
});
},

Expand Down
43 changes: 4 additions & 39 deletions test/dependency-manager-adapters/npm-adapter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 });
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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');
Expand All @@ -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');
});
Expand All @@ -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');
Expand Down Expand Up @@ -215,32 +206,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) {
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/dummy-ember-try-config-with-npm-scenarios.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
module.exports = {
useYarn: true,
scenarios: [
{
name: 'test1',
Expand Down
2 changes: 2 additions & 0 deletions test/tasks/try-each-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,10 +264,12 @@ describe('tryEach', function() {
});

writeJSONFile('package.json', fixturePackage);
fs.writeFileSync('yarn.lock', '');
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');
Expand Down