You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by ra...@apache.org on 2018/09/28 07:44:36 UTC

[cordova-fetch] branch master updated: Look for node_modules in any recursive parent directory (#44)

This is an automated email from the ASF dual-hosted git repository.

raphinesse pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cordova-fetch.git


The following commit(s) were added to refs/heads/master by this push:
     new f37c7ba  Look for node_modules in any recursive parent directory (#44)
f37c7ba is described below

commit f37c7baddc6728f91612f228f34a696b61bc1e89
Author: Andres Riofrio <ri...@gmail.com>
AuthorDate: Fri Sep 28 00:44:29 2018 -0700

    Look for node_modules in any recursive parent directory (#44)
    
    This change causes `cordova-fetch` to look for already installed packages using Node.js' module resolution algorithm. It considers directories from NODE_PATH if it is set, but it will not take the fixed set of Node.js GLOBAL_FOLDERS into account.
---
 index.js                | 46 +++++++++++++++++++++++++++++++---------
 package.json            |  2 +-
 spec/fetch-unit.spec.js | 56 +++++++++++++++++++++++++++++++++++++++++++++++++
 spec/fetch.spec.js      | 26 +++++++++++++++++++++++
 4 files changed, 119 insertions(+), 11 deletions(-)

diff --git a/index.js b/index.js
index da9f658..769ccc8 100644
--- a/index.js
+++ b/index.js
@@ -22,7 +22,7 @@ var events = require('cordova-common').events;
 var path = require('path');
 var fs = require('fs-extra');
 var CordovaError = require('cordova-common').CordovaError;
-const { getInstalledPath } = require('get-installed-path');
+const resolve = Q.denodeify(require('resolve'));
 const npa = require('npm-package-arg');
 const semver = require('semver');
 
@@ -57,11 +57,17 @@ module.exports = function (target, dest, opts = {}) {
 // Installs the package specified by target and returns the installation path
 function installPackage (target, dest, opts) {
     return isNpmInstalled()
+        // Ensure that `npm` installs to `dest` and not any of its ancestors
+        .then(_ => fs.ensureDir(path.join(dest, 'node_modules')))
+
+        // Run `npm` to install requested package
         .then(_ => npmArgs(target, opts))
         .then(args => {
             events.emit('verbose', `fetch: Installing ${target} to ${dest}`);
             return superspawn.spawn('npm', args, { cwd: dest });
         })
+
+        // Resolve path to installed package
         .then(getTargetPackageSpecFromNpmInstallOutput)
         .then(spec => pathToInstalledPackage(spec, dest));
 }
@@ -100,15 +106,35 @@ function getTargetPackageSpecFromNpmInstallOutput (npmInstallOutput) {
 // Resolves to installation path of package defined by spec if the right version
 // is installed, rejects otherwise.
 function pathToInstalledPackage (spec, dest) {
-    const { name, rawSpec } = npa(spec, dest);
-    return getInstalledPath(name, { local: true, cwd: dest })
-        .then(installPath => {
-            const { version } = fs.readJsonSync(path.join(installPath, 'package.json'));
-            if (!semver.satisfies(version, rawSpec)) {
-                throw new CordovaError(`Installed package ${name}@${version} does not satisfy ${name}@${rawSpec}`);
-            }
-            return installPath;
-        });
+    return Promise.resolve().then(_ => {
+        const { name, rawSpec } = npa(spec, dest);
+        if (!name) {
+            throw new CordovaError(`Cannot determine package name from spec ${spec}`);
+        }
+        return resolvePathToPackage(name, dest)
+            .then(([pkgPath, { version }]) => {
+                if (!semver.satisfies(version, rawSpec)) {
+                    throw new CordovaError(`Installed package ${name}@${version} does not satisfy ${name}@${rawSpec}`);
+                }
+                return pkgPath;
+            });
+    });
+}
+
+// Resolves to installation path and package.json of package `name` starting
+// from `basedir`
+function resolvePathToPackage (name, basedir) {
+    return Promise.resolve().then(_ => {
+        const { NODE_PATH } = process.env;
+        const paths = NODE_PATH ? NODE_PATH.split(path.delimiter) : [];
+
+        // We resolve the path to the module's package.json to avoid getting the
+        // path to `main` which could be located anywhere in the package
+        return resolve(path.join(name, 'package.json'), { paths, basedir })
+            .then(([pkgJsonPath, pkgJson]) => [
+                path.dirname(pkgJsonPath), pkgJson
+            ]);
+    });
 }
 
 /**
diff --git a/package.json b/package.json
index f96c454..ba465e1 100644
--- a/package.json
+++ b/package.json
@@ -23,9 +23,9 @@
   "dependencies": {
     "cordova-common": "^2.2.0",
     "fs-extra": "^6.0.1",
-    "get-installed-path": "^4.0.8",
     "npm-package-arg": "^6.1.0",
     "q": "^1.4.1",
+    "resolve": "^1.8.1",
     "semver": "^5.5.0",
     "which": "^1.3.1"
   },
diff --git a/spec/fetch-unit.spec.js b/spec/fetch-unit.spec.js
index 7bb9a04..889ef23 100644
--- a/spec/fetch-unit.spec.js
+++ b/spec/fetch-unit.spec.js
@@ -15,7 +15,10 @@
     under the License.
 */
 
+const fs = require('fs-extra');
+const path = require('path');
 const rewire = require('rewire');
+const { tmpDir: getTmpDir } = require('./helpers.js');
 
 describe('fetch', function () {
     let fetch, installPackage;
@@ -74,3 +77,56 @@ describe('npmArgs', function () {
         expect(npmArgs('platform', opts)).toContain('--no-save');
     });
 });
+
+describe('resolvePathToPackage', () => {
+    let tmpDir, resolvePathToPackage, expectedPath;
+
+    beforeEach(() => {
+        tmpDir = getTmpDir();
+        resolvePathToPackage = rewire('..').__get__('resolvePathToPackage');
+        expectedPath = path.join(tmpDir, 'app/node_modules/dummy-local-plugin');
+        fs.copySync(path.join(__dirname, 'support/dummy-local-plugin'), expectedPath);
+    });
+
+    afterEach(() => {
+        fs.removeSync(tmpDir);
+    });
+
+    function resolveToExpectedPathFrom (basedir) {
+        return Promise.resolve()
+            .then(_ => fs.mkdirp(basedir))
+            .then(_ => resolvePathToPackage('dummy-local-plugin', basedir))
+            .then(([p]) => expect(p).toEqual(expectedPath));
+    }
+
+    it('should return path and package.json of an installed package', () => {
+        return Promise.resolve(path.join(tmpDir, 'app'))
+            .then(basedir => resolvePathToPackage('dummy-local-plugin', basedir))
+            .then(([pkgPath, pkgJson]) => {
+                expect(pkgPath).toEqual(expectedPath);
+                expect(pkgJson).toEqual(jasmine.objectContaining({
+                    name: 'test-plugin', version: '1.0.0'
+                }));
+            });
+    });
+
+    it('should find a package installed in the parent of the given directory', () => {
+        return resolveToExpectedPathFrom(path.join(tmpDir, 'app/nested-directory'));
+    });
+
+    it('should find a package installed in an ancestor of the given directory', () => {
+        return resolveToExpectedPathFrom(path.join(tmpDir, 'app/nested-directory/nested-subdirectory'));
+    });
+
+    it('should not find a package installed elsewhere', () => {
+        return resolvePathToPackage('dummy-local-plugin').then(
+            _ => fail('expect promise to be rejected'),
+            err => expect(err).toBeDefined()
+        );
+    });
+
+    it('should find a package installed at $NODE_PATH', () => {
+        process.env.NODE_PATH = path.join(tmpDir, 'app/node_modules');
+        return resolveToExpectedPathFrom(path.join(tmpDir, 'another-app'));
+    });
+});
diff --git a/spec/fetch.spec.js b/spec/fetch.spec.js
index 15b45a0..0bb551d 100644
--- a/spec/fetch.spec.js
+++ b/spec/fetch.spec.js
@@ -187,3 +187,29 @@ describe('negative tests', function () {
             });
     }, 30000);
 });
+
+describe('fetching with node_modules in ancestor dirs', () => {
+    let fetchTarget, fetchDestination;
+
+    beforeEach(() => {
+        const testRoot = path.join(tmpDir, 'test-root');
+        fetchDestination = path.join(testRoot, 'fetch-dest');
+        fs.ensureDirSync(fetchDestination);
+
+        // Make test root look like a package root directory
+        fs.ensureDirSync(path.join(testRoot, 'node_modules'));
+        fs.outputJsonSync(path.join(testRoot, 'package.json'), { private: true });
+
+        // Copy test fixtures to avoid linking out of temp directory
+        fs.copySync(path.join(__dirname, 'support'), 'support');
+        fetchTarget = fileUrl(path.resolve('support/dummy-local-plugin'));
+    });
+
+    it('should still install to given destination', function () {
+        const expectedInstallPath = path.join(fetchDestination, 'node_modules/test-plugin');
+
+        return fetch(fetchTarget, fetchDestination).then(pkgInstallPath => {
+            expect(pkgInstallPath).toBe(expectedInstallPath);
+        });
+    }, 10 * 1000);
+});


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org