Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

chore(npm): use require.resolve when possible to avoid hard coded mod… #15071

Closed
wants to merge 2 commits into from
Closed

chore(npm): use require.resolve when possible to avoid hard coded mod… #15071

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b60cd0c
chore(npm): use require.resolve when possible to avoid hard coded mod…
jdalton Nov 17, 2015
6d3865d
chore(*) use binaries from node_modules/.bin
Narretz Sep 1, 2016
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
2 changes: 1 addition & 1 deletion docs/app/e2e/app.scenario.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

var webdriver = require('protractor/node_modules/selenium-webdriver');
var webdriver = require('selenium-webdriver');

describe('docs.angularjs.org', function() {

Expand Down
4 changes: 2 additions & 2 deletions karma-shared.conf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,8 @@ module.exports = function(config, specificOptions) {
'/someSanitizedUrl',
'/{{testUrl}}'
];
var log4js = require('./node_modules/karma/node_modules/log4js');
var layouts = require('./node_modules/karma/node_modules/log4js/lib/layouts');
var log4js = require('log4js');
var layouts = require('log4js/lib/layouts');
var originalConfigure = log4js.configure;
log4js.configure = function(log4jsConfig) {
var consoleAppender = log4jsConfig.appenders.shift();
Expand Down
2 changes: 1 addition & 1 deletion lib/grunt/plugins.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ module.exports = function(grunt) {


grunt.registerTask('docs', 'create angular docs', function() {
var gruntProc = shelljs.exec('"node_modules/.bin/gulp" --gulpfile docs/gulpfile.js');
var gruntProc = shelljs.exec('node "' + require.resolve('gulp/bin/gulp') + '" --gulpfile docs/gulpfile.js');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to add node here, otherwise Windows wouldn't execute the command

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather not depend on the internal gulp package structure. How about creating the npm script gulp in package.json:

{
  "scripts": {
    "gulp": "gulp
  }
}

and then npm run gulp -- --gulpfile docs/gulpfile.js should work everywhere.

if (gruntProc.code !== 0) {
throw new Error('doc generation failed');
}
Expand Down
6 changes: 3 additions & 3 deletions lib/grunt/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = {
var reporters = grunt.option('reporters');
var noColor = grunt.option('no-colors');
var port = grunt.option('port');
var p = spawn('node', ['node_modules/karma/bin/karma', 'start', config,
var p = spawn('node', [require.resolve('karma/bin/karma'), 'start', config,
singleRun ? '--single-run=true' : '',
reporters ? '--reporters=' + reporters : '',
browsers ? '--browsers=' + browsers : '',
Expand All @@ -38,7 +38,7 @@ module.exports = {
done();
return;
}
var p = spawn('node', ['node_modules/protractor/bin/webdriver-manager', 'update']);
var p = spawn('node', [require.resolve('protractor/bin/webdriver-manager'), 'update']);
Copy link
Member

Choose a reason for hiding this comment

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

webdriver-manager is a separate package (and Protractor just uses it) so we could switch as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. So simply require.resolve('webdriver-manager')?

Copy link
Member

Choose a reason for hiding this comment

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

That would only resolve what's exported, not the binary.

Have you tried specifying the binary as './node_modules/.bin/webdriver-manager' instead of node and using cross-spawn for Windows compatibility?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to complicate the PR actually, but if this is future-proof then let's do it

p.stdout.pipe(process.stdout);
p.stderr.pipe(process.stderr);
p.on('exit', function(code) {
Expand All @@ -54,7 +54,7 @@ module.exports = {
var sauceBuild = grunt.option('capabilities.build');
var browser = grunt.option('browser');
var specs = grunt.option('specs');
var args = ['node_modules/protractor/bin/protractor', config];
var args = [require.resolve('protractor/bin/protractor'), config];
if (sauceUser) args.push('--sauceUser=' + sauceUser);
if (sauceKey) args.push('--sauceKey=' + sauceKey);
if (tunnelIdentifier) args.push('--capabilities.tunnel-identifier=' + tunnelIdentifier);
Expand Down
Loading