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

Implicitly add an npm dev dependency on bower #157

Merged
merged 3 commits into from
Nov 12, 2017
Merged

Implicitly add an npm dev dependency on bower #157

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
477f2d1
Rather than installing bower when it is not found locally, look for
kategengler Oct 30, 2017
ea8afb7
Implicitly add an npm dev dependency on bower
hjdivad Nov 8, 2017
9f10230
Remove deprecated commands from all-commands.sh.
rwjblue Nov 12, 2017
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
46 changes: 18 additions & 28 deletions lib/dependency-manager-adapters/bower.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,18 +108,21 @@ module.exports = CoreObject.extend({
},
_install: function() {
var adapter = this;
var options = this.managerOptions || [];

return rimraf(path.join(adapter.cwd, 'bower_components'))
.then(function() {
debug('Remove bower_components');
return adapter._findBowerPath(adapter.cwd);
})
.then(function(bowerPath) {
debug('Run bower install using bower at %s', bowerPath);
return adapter.run('node', [].concat([bowerPath, 'install', '--config.interactive=false'], options), { cwd: adapter.cwd });
debug('Removed bower_components');
return adapter._runBowerInstall();
});
},
_runBowerInstall: function() {
var adapter = this;
var options = this.managerOptions || [];
var commandParts = ['install', '--config.interactive=false'];
return adapter._findBowerPath(adapter.cwd).then(function(bowerPath) {
debug('Run bower install using bower at %s', bowerPath);
return adapter.run('node', [].concat([bowerPath], commandParts, options), { cwd: adapter.cwd });
});
},
_bowerJSONForDependencySet: function(bowerJSON, depSet) {
if (!bowerJSON.resolutions) {
bowerJSON.resolutions = {};
Expand Down Expand Up @@ -172,26 +175,13 @@ module.exports = CoreObject.extend({
},
_findBowerPath: function() {
var adapter = this;
return findEmberPath(adapter.cwd)
.then(function(emberPath) {
/* Find bower's entry point module relative to
ember-cli's entry point script */
return adapter._resolveModule('bower', { basedir: path.dirname(emberPath) })
.catch(function() {
debug('Bower not found');
return adapter._installBower();
}).then(function() {
return adapter._resolveModule('bower', { basedir: path.dirname(emberPath) });
});
})
.then(function(bowerPath) {
return '"' + path.join(bowerPath, '..', '..', 'bin', 'bower') + '"';
});
},
_installBower: function() {
var adapter = this;
debug('Installing bower via npm');
return adapter.run('npm', [].concat(['install', 'bower@^1.3.12']), { cwd: adapter.cwd });
return findEmberPath(adapter.cwd).then(function(emberPath) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you didn't add this, but I wonder if finding bower relative to ember-cli is necessary anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kategengler hmm good point. So presumably the logic would be

  1. run npm before bower
  2. trust that bower exists in PROJECT_ROOT/node_modules/.bin/bower and assert otherwise

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, or we can just leave this since it works, but just thought it worth thinking about

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kategengler leaving it in this PR seems like the right thing to do; it'll be easier to track & revert in a separate PR if it causes issues (although I think you're right that finding it relative to ember-cli would no longer be useful).

/* Find bower's entry point module relative to ember-cli's entry point script */
return adapter._resolveModule('bower', { basedir: path.dirname(emberPath) })
.then(function(bowerPath) {
return '"' + path.join(bowerPath, '..', '..', 'bin', 'bower') + '"';
});
});
},
_resolveModule: function(moduleName, options) {
return resolve(moduleName, options);
Expand Down
64 changes: 59 additions & 5 deletions lib/utils/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ var RSVP = require('rsvp');
var findByName = require('./find-by-name');
var debug = require('debug')('ember-try:utils:config');

function config(options) {
var IMPLICIT_BOWER_VERSION = '^1.8.2';

function getBaseConfig(options) {
var relativePath = options.configPath || path.join('config', 'ember-try.js');
var configFile = path.join(options.project.root, relativePath);
var configData;
Expand Down Expand Up @@ -39,8 +41,39 @@ function config(options) {
}
}

function config(options) {
return getBaseConfig(options).then(function(configData) {
return addImplicitBowerToScenarios(configData);
});
}

module.exports = config;

function addImplicitBowerToScenarios(configData) {
configData.scenarios.forEach(function(scenario) {
if (!('bower' in scenario)) {
// Don't do anything for scenarios that don't include bower
return;
}

if ('npm' in scenario) {
var npm = scenario.npm;
if ((npm.dependencies && npm.dependencies.bower) ||
(npm.devDependencies && npm.devDependencies.bower)) {
// Dont' do anything for scenarios that already include bower in npm,
// either as a dependency or a dev dependency
return;
}
}

// add an implicit bower dev dependency to npm for this scenario
scenario.npm = scenario.npm || {};
scenario.npm.devDependencies = scenario.npm.devDependencies || {};
scenario.npm.devDependencies.bower = IMPLICIT_BOWER_VERSION;
});
return configData;
}

function mergeAutoConfigAndConfigFileData(autoConfig, configData) {
configData = configData || {};
configData.scenarios = configData.scenarios || [];
Expand Down Expand Up @@ -77,35 +110,56 @@ function defaultConfig() {
dependencies: { } /* No dependencies needed as the
default is already specified in
the consuming app's bower.json */
}
},
npm: {
devDependencies: {
bower: IMPLICIT_BOWER_VERSION,
}
},
},
{
name: 'ember-release',
bower: {
dependencies: {
ember: 'release'
}
}
},
npm: {
devDependencies: {
bower: IMPLICIT_BOWER_VERSION,
}
},
},
{
name: 'ember-beta',
bower: {
dependencies: {
ember: 'beta'
}
}
},
npm: {
devDependencies: {
bower: IMPLICIT_BOWER_VERSION,
}
},
},
{
name: 'ember-canary',
bower: {
dependencies: {
ember: 'canary'
}
}
},
npm: {
devDependencies: {
bower: IMPLICIT_BOWER_VERSION,
}
},
}
]
};
}

// Used for internal testing purposes.
module.exports._defaultConfig = defaultConfig;
module.exports._addImplicitBowerToScenarios = addImplicitBowerToScenarios;
3 changes: 2 additions & 1 deletion lib/utils/run.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ var debug = require('debug')('ember-try:utils:run');

function run(command, args, opts) {
opts = opts || {};
opts.stdio = 'inherit';

opts.stdio = opts.stdio || 'inherit';

if (process.env.SHUT_UP) {
opts.stdio = 'ignore';
Expand Down
75 changes: 20 additions & 55 deletions test/dependency-manager-adapters/bower-adapter-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,26 @@ describe('bowerAdapter', function() {
return new BowerAdapter({ cwd: tmpdir, run: stubbedRun })._install();
});

it('rejects if local bower is not found', function() {
var doNotFindLocalBower = function() {
return RSVP.reject('no local bower found');
};

var stubbedRun = function() {
return RSVP.reject();
};

return new BowerAdapter({
cwd: tmpdir,
_findBowerPath: doNotFindLocalBower,
run: stubbedRun
})._install().then(function() {
expect.fail(true, false, 'unreachable: _install promise rejects');
}, function(error) {
expect(error).to.equal('no local bower found');
});
});

it('runs bower install including managerOptions', function() {
writeJSONFile('bower.json', fixtureBower);
var stubbedRun = function(command, args) {
Expand Down Expand Up @@ -252,61 +272,6 @@ describe('bowerAdapter', function() {
expect(path).to.include('node_modules/bower/bin/bower');
});
});

it('does not attempt to install bower if bower is found', function() {
var installCount = 0;
var stubbedResolveModule = function() {
return RSVP.resolve('blip/bloop/foo/lib/index.js');
};
var stubbedInstallBower = function() {
installCount++;
};
return new BowerAdapter({ cwd: tmpdir, _installBower: stubbedInstallBower, _resolveModule: stubbedResolveModule })._findBowerPath().then(function(path) {
expect(path).to.include('blip/bloop/foo/bin/bower');
expect(installCount).to.equal(0);
});
});

it('installs bower if bower is not found', function() {
var installCount = 0;
var resolveModuleCount = 0;
var stubbedResolveModule = function() {
resolveModuleCount++;
if (resolveModuleCount === 1) {
return RSVP.reject();
}
if (resolveModuleCount === 2) {
return RSVP.resolve('flip/flop/gloop/lib/index.js');
}
};

var stubbedInstallBower = function() {
installCount++;
};

return new BowerAdapter({ cwd: tmpdir, _installBower: stubbedInstallBower, _resolveModule: stubbedResolveModule })._findBowerPath().then(function(path) {
expect(path).to.include('flip/flop/gloop/bin/bower');
expect(installCount).to.equal(1);
});
});
});

describe('#_installBower()', function() {
it('installs bower via npm', function() {
var command, args, opts;
var stubbedRun = function(c, a, o) {
command = c;
args = a;
opts = o;
return RSVP.resolve();
};
return new BowerAdapter({ cwd: tmpdir, run: stubbedRun })._installBower().then(function() {
expect(command).to.equal('npm');
expect(args[0]).to.equal('install');
expect(args[1]).to.equal('bower@^1.3.12');
expect(opts).to.have.property('cwd', tmpdir);
});
});
});
});

Expand Down
1 change: 0 additions & 1 deletion test/tasks/try-each-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@ describe('tryEach', function() {
});
});


it('fails scenarios when scenario\'s tests fail', function() {
this.timeout(300000);

Expand Down
Loading