[
https://issues.apache.org/jira/browse/CB-12242?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16527504#comment-16527504
]
ASF GitHub Bot commented on CB-12242:
-------------------------------------
raphinesse closed pull request #14: CB-12242 : Use yarn instead of npm
URL: https://github.com/apache/cordova-fetch/pull/14
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/index.js b/index.js
index 8bb6326..37790c6 100644
--- a/index.js
+++ b/index.js
@@ -29,7 +29,7 @@ var isGitUrl = require('is-git-url');
var hostedGitInfo = require('hosted-git-info');
/*
- * A function that npm installs a module from npm or a git url
+ * A function that installs a module from package manager or a git url
*
* @param {String} target the packageID or git url
* @param {String} dest destination of where to install the module
@@ -39,13 +39,13 @@ var hostedGitInfo = require('hosted-git-info');
*
*/
module.exports = function (target, dest, opts) {
- var fetchArgs = opts.link ? ['link'] : ['install'];
opts = opts || {};
+ var manager = opts.manager || 'npm';
+ var fetchArgs = opts.link ? ['link'] : manager === 'yarn' ? ['add'] :
['install'];
var tree1;
var nodeModulesDir = dest;
- // check if npm is installed
- return module.exports.isNpmInstalled()
+ return isPackageManagerInstalled(manager)
.then(function () {
if (dest && target) {
// add target to fetchArgs Array
@@ -61,16 +61,16 @@ module.exports = function (target, dest, opts) {
}
} else return Q.reject(new CordovaError('Need to supply a target
and destination'));
- // set the directory where npm install will be run
+ // set the directory where install command will be run
opts.cwd = dest;
- // npm should use production by default when install is npm run
+ // package manager should use production by default when install
command is run
if ((opts.production) || (opts.production === undefined)) {
fetchArgs.push('--production');
opts.production = true;
}
- // if user added --save flag, pass it to npm install command
+ // if user added --save flag, pass it to install command
if (opts.save_exact) {
events.emit('verbose', 'saving exact');
fetchArgs.push('--save-exact');
@@ -80,16 +80,16 @@ module.exports = function (target, dest, opts) {
} else {
fetchArgs.push('--no-save');
}
- // Grab json object of installed modules before npm install
+ // Grab json object of installed modules before install command
return depls(nodeModulesDir);
})
.then(function (depTree) {
tree1 = depTree;
// install new module
- return superspawn.spawn('npm', fetchArgs, opts);
+ return superspawn.spawn(manager, fetchArgs, opts);
})
.then(function (output) {
- // Grab object of installed modules after npm install
+ // Grab object of installed modules after install command
return depls(nodeModulesDir);
})
.then(function (depTree2) {
@@ -111,8 +111,8 @@ module.exports = function (target, dest, opts) {
* If a module already exists in node_modules, the diff will be blank.
* cordova-fetch will use trimID in that case.
*
- * @param {Object} obj1 json object representing installed modules before
latest npm install
- * @param {Object} obj2 json object representing installed modules after
latest npm install
+ * @param {Object} obj1 json object representing installed modules before
latest install command
+ * @param {Object} obj2 json object representing installed modules after
latest install command
*
* @return {String} String containing the key value of the difference
between the two objects
*
@@ -137,6 +137,7 @@ function getJsonDiff (obj1, obj2) {
// only return the first element
return result[0];
}
+
/*
* Takes the specified target and returns the moduleID
* If the git repoName is different than moduleID, then the
@@ -206,7 +207,6 @@ function trimID (target) {
* @return {String|Error} Returns the absolute url for the module or throws a
error
*
*/
-
function getPath (id, dest, target) {
var destination = path.resolve(path.join(dest, id));
var finalDest = fs.existsSync(destination) ? destination :
searchDirForTarget(dest, target);
@@ -219,6 +219,7 @@ function getPath (id, dest, target) {
}
module.exports.getPath = getPath;
+
/*
* Make an additional search in destination folder using repository.url
property from package.json
*
@@ -228,7 +229,6 @@ module.exports.getPath = getPath;
* @return {String} Returns the absolute url for the module or null
*
*/
-
function searchDirForTarget (dest, target) {
if (!isUrl(target)) {
return;
@@ -252,50 +252,76 @@ function searchDirForTarget (dest, target) {
* Checks to see if npm is installed on the users system
* @return {Promise|Error} Returns true or a cordova error.
*/
-
function isNpmInstalled () {
- if (!shell.which('npm')) {
- return Q.reject(new CordovaError('"npm" command line tool is not
installed: make sure it is accessible on your PATH.'));
+ return isPackageManagerInstalled('npm');
+}
+
+module.exports.isNpmInstalled = isNpmInstalled;
+
+/*
+ * Checks to see if yarn is installed on the users system
+ * @return {Promise|Error} Returns true or a cordova error.
+ */
+function isYarnInstalled () {
+ return isPackageManagerInstalled('yarn');
+}
+
+module.exports.isYarnInstalled = isYarnInstalled;
+
+/*
+ * Checks to see if selected package manager is installed on the users system
+ * @return {Promise|Error} Returns true or a cordova error.
+ */
+function isPackageManagerInstalled (manager) {
+ if (!shell.which(manager)) {
+ return Q.reject(new CordovaError('"' + manager + '" command line tool
is not installed: make sure it is accessible on your PATH.'));
}
return Q();
}
-module.exports.isNpmInstalled = isNpmInstalled;
+module.exports.isPackageManagerInstalled = isPackageManagerInstalled;
+
/*
- * A function that deletes the target from node_modules and runs npm uninstall
+ * A function that deletes the target from node_modules and runs uninstall
command
*
* @param {String} target the packageID
* @param {String} dest destination of where to uninstall the module from
- * @param {Object} opts [opts={save:true}] options to pass to npm uninstall
+ * @param {Object} opts [opts={save:true}] options to pass to uninstall
command
*
- * @return {Promise|Error} Returns a promise with the npm uninstall output
or an error.
+ * @return {Promise|Error} Returns a promise with the uninstall command
output or an error.
*
*/
module.exports.uninstall = function (target, dest, opts) {
- var fetchArgs = ['uninstall'];
opts = opts || {};
+ var manager = opts.manager || 'npm';
+ var fetchArgs = [];
+
+ if (manager === 'npm') {
+ fetchArgs.push('uninstall');
+ } else if (manager === 'yarn') {
+ fetchArgs.push('remove');
+ }
- // check if npm is installed on the system
- return isNpmInstalled()
+ return isPackageManagerInstalled(manager)
.then(function () {
if (dest && target) {
// add target to fetchArgs Array
fetchArgs.push(target);
} else return Q.reject(new CordovaError('Need to supply a target
and destination'));
- // set the directory where npm uninstall will be run
+ // set the directory where uninstall will be run
opts.cwd = dest;
- // if user added --save flag, pass it to npm uninstall command
+ // if user added --save flag, pass it to uninstall command
if (opts.save) {
fetchArgs.push('--save');
} else {
fetchArgs.push('--no-save');
}
- // run npm uninstall, this will remove dependency
+ // run uninstall command, this will remove dependency
// from package.json if --save was used.
- return superspawn.spawn('npm', fetchArgs, opts);
+ return superspawn.spawn(manager, fetchArgs, opts);
})
.then(function (res) {
var pluginDest = path.join(dest, 'node_modules', target);
diff --git a/spec/fetch-unit.spec.js b/spec/fetch-unit.spec.js
index c13f75a..ce84379 100644
--- a/spec/fetch-unit.spec.js
+++ b/spec/fetch-unit.spec.js
@@ -27,16 +27,17 @@ describe('unit tests for index.js', function () {
spyOn(superspawn, 'spawn').and.returnValue(true);
spyOn(shell, 'mkdir').and.returnValue(true);
spyOn(shell, 'which').and.returnValue(Q());
+ spyOn(fetch, 'isPackageManagerInstalled').and.returnValue(Q());
spyOn(fetch, 'isNpmInstalled').and.returnValue(Q());
+ spyOn(fetch, 'isYarnInstalled').and.returnValue(Q());
spyOn(fetch, 'getPath').and.returnValue('some/path');
spyOn(fs, 'existsSync').and.returnValue(false);
});
- it('npm install should be called with production flag (default)', function
(done) {
- var opts = { cwd: 'some/path', production: true, save: true};
- fetch('platform', 'tmpDir', opts)
+ it('npm should be the default package manager', function (done) {
+ fetch('platform', 'tmpDir')
.then(function (result) {
- expect(superspawn.spawn).toHaveBeenCalledWith('npm',
jasmine.stringMatching(/production/), jasmine.any(Object));
+ expect(superspawn.spawn).toHaveBeenCalledWith('npm',
jasmine.any(Array), jasmine.any(Object));
})
.fail(function (err) {
console.error(err);
@@ -45,42 +46,57 @@ describe('unit tests for index.js', function () {
.fin(done);
});
- it('save-exact should be true if passed in', function (done) {
- var opts = { cwd: 'some/path', save_exact: true };
- fetch('platform', 'tmpDir', opts)
- .then(function (result) {
- expect(superspawn.spawn).toHaveBeenCalledWith('npm',
jasmine.stringMatching(/save-exact/), jasmine.any(Object));
- })
- .fail(function (err) {
- console.error(err);
- expect(err).toBeUndefined();
- })
- .fin(done);
- });
+ ['npm', 'yarn'].forEach(function(manager) {
+ it(manager + ' install should be called with production flag
(default)', function (done) {
+ var opts = { cwd: 'some/path', production: true, save: true,
manager: manager };
+ fetch('platform', 'tmpDir', opts)
+ .then(function (result) {
+ expect(superspawn.spawn).toHaveBeenCalledWith(manager,
jasmine.stringMatching(/production/), jasmine.any(Object));
+ })
+ .fail(function (err) {
+ console.error(err);
+ expect(err).toBeUndefined();
+ })
+ .fin(done);
+ });
- it('noprod should turn production off', function (done) {
- var opts = { cwd: 'some/path', production: false};
- fetch('platform', 'tmpDir', opts)
- .then(function (result) {
- expect(superspawn.spawn).not.toHaveBeenCalledWith('npm',
jasmine.stringMatching(/production/), jasmine.any(Object));
- })
- .fail(function (err) {
- console.error(err);
- expect(err).toBeUndefined();
- })
- .fin(done);
- });
+ it('save-exact should be true if passed in', function (done) {
+ var opts = { cwd: 'some/path', save_exact: true, manager: manager
};
+ fetch('platform', 'tmpDir', opts)
+ .then(function (result) {
+ expect(superspawn.spawn).toHaveBeenCalledWith(manager,
jasmine.stringMatching(/save-exact/), jasmine.any(Object));
+ })
+ .fail(function (err) {
+ console.error(err);
+ expect(err).toBeUndefined();
+ })
+ .fin(done);
+ });
- it('when save is false, no-save flag should be passed through', function
(done) {
- var opts = { cwd: 'some/path', production: true, save: false};
- fetch('platform', 'tmpDir', opts)
- .then(function (result) {
- expect(superspawn.spawn).toHaveBeenCalledWith('npm',
jasmine.stringMatching(/--no-save/), jasmine.any(Object));
- })
- .fail(function (err) {
- console.error(err);
- expect(err).toBeUndefined();
- })
- .fin(done);
+ it('noprod should turn production off', function (done) {
+ var opts = { cwd: 'some/path', production: false, manager: manager
};
+ fetch('platform', 'tmpDir', opts)
+ .then(function (result) {
+ expect(superspawn.spawn).not.toHaveBeenCalledWith(manager,
jasmine.stringMatching(/production/), jasmine.any(Object));
+ })
+ .fail(function (err) {
+ console.error(err);
+ expect(err).toBeUndefined();
+ })
+ .fin(done);
+ });
+
+ it('when save is false, no-save flag should be passed through',
function (done) {
+ var opts = { cwd: 'some/path', production: true, save: false,
manager: manager };
+ fetch('platform', 'tmpDir', opts)
+ .then(function (result) {
+ expect(superspawn.spawn).toHaveBeenCalledWith(manager,
jasmine.stringMatching(/--no-save/), jasmine.any(Object));
+ })
+ .fail(function (err) {
+ console.error(err);
+ expect(err).toBeUndefined();
+ })
+ .fin(done);
+ });
});
});
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Use yarn js instead of npm when adding plugins
> ----------------------------------------------
>
> Key: CB-12242
> URL: https://issues.apache.org/jira/browse/CB-12242
> Project: Apache Cordova
> Issue Type: Improvement
> Components: cordova-fetch
> Reporter: Jacques de Villiers
> Priority: Major
>
> Currently it can take quite long to add certain plugins to my project (using
> cordova plugin add), especially if I need to re-add the plugins. My
> suggestion is to update the cordova cli to start using yarn js instead of npm
> directly.
> When I looked at this page, I realised yarn was just a wrapper for npm, and
> much better at caching packages locally.
> https://shift.infinite.red/npm-vs-yarn-cheat-sheet-8755b092e5cc#.jiz27n1hc
> I would imagine this change would be relatively straightforward, and would be
> a massive win for the cli.
> I was thinking of creating a fork to do a PR but realised I am not totally
> sure how to proceed on this.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org
For additional commands, e-mail: issues-h...@cordova.apache.org
