Fix plugins not being found when no node_modules exists (#171)

* Implement test for plugin load

This test should pass when it has been updated to use resolve

* Update findRoot to use resolve function

* Fix findRoot not working due to incorrect assumptions

Assume resolve returns an entry file not a directory

package.json

@@ -10,6 +10,7 @@
     "debug": "^4.1.1",
     "globby": "^11.0.1",
     "is-wsl": "^2.1.1",
+    "resolve": "^1.17.0",
     "tslib": "^2.0.0"
   },
   "devDependencies": {
@@ -19,6 +20,7 @@
     "@types/mocha": "^8.0.0",
     "@types/node": "^14.0.14",
     "@types/proxyquire": "^1.3.28",
+    "@types/resolve": "^1.17.1",
     "@types/wrap-ansi": "^3.0.0",
     "chai": "^4.2.0",
     "conventional-changelog-cli": "^2.0.21",

src/plugin.ts

@@ -9,7 +9,7 @@ import {Manifest} from './manifest'
 import {PJSON} from './pjson'
 import {Topic} from './topic'
 import {tsPath} from './ts-node'
-import {compact, exists, flatMap, loadJSON, mapValues} from './util'
+import {compact, exists, resolvePackage, flatMap, loadJSON, mapValues} from './util'
 
 const ROOT_INDEX_CMD_ID = ''
 
@@ -98,40 +98,32 @@ function topicsToArray(input: any, base?: string): Topic[] {
   })
 }
 
-// eslint-disable-next-line valid-jsdoc
-/**
- * find package root
- * for packages installed into node_modules this will go up directories until
- * it finds a node_modules directory with the plugin installed into it
- *
- * This is needed because of the deduping npm does
- */
-async function findRoot(name: string | undefined, root: string) {
-  // essentially just "cd .."
-  function * up(from: string) {
-    while (path.dirname(from) !== from) {
-      yield from
-      from = path.dirname(from)
-    }
+// essentially just "cd .."
+function * up(from: string) {
+  while (path.dirname(from) !== from) {
     yield from
+    from = path.dirname(from)
   }
+  yield from
+}
+
+async function findSourcesRoot(root: string) {
   for (const next of up(root)) {
-    let cur
-    if (name) {
-      cur = path.join(next, 'node_modules', name, 'package.json')
-      // eslint-disable-next-line no-await-in-loop
-      if (await exists(cur)) return path.dirname(cur)
-      try {
-        // eslint-disable-next-line no-await-in-loop
-        const pkg = await loadJSON(path.join(next, 'package.json'))
-        if (pkg.name === name) return next
-      } catch { }
-    } else {
-      cur = path.join(next, 'package.json')
-      // eslint-disable-next-line no-await-in-loop
-      if (await exists(cur)) return path.dirname(cur)
-    }
+    const cur = path.join(next, 'package.json')
+    // eslint-disable-next-line no-await-in-loop
+    if (await exists(cur)) return path.dirname(cur)
+  }
+}
+
+async function findRoot(name: string | undefined, root: string) {
+  if (name) {
+    let pkgPath
+    try {
+      pkgPath = await resolvePackage(name)
+    } catch (error) {}
+    return pkgPath ? findSourcesRoot(path.dirname(pkgPath)) : pkgPath
   }
+  return findSourcesRoot(root)
 }
 
 export class Plugin implements IPlugin {

src/util.ts

@@ -1,4 +1,5 @@
 import * as fs from 'fs'
+import * as _resolve from 'resolve'
 
 const debug = require('debug')('@oclif/config')
 
@@ -18,6 +19,17 @@ export function exists(path: string): Promise<boolean> {
   return new Promise(resolve => resolve(fs.existsSync(path)))
 }
 
+export function resolvePackage(id: string): Promise<string> {
+  return new Promise(
+    (resolve, reject) =>
+      _resolve(
+        id,
+        (err, pkgPath) =>
+          err ? reject(err) : resolve(pkgPath),
+      ),
+  )
+}
+
 export function loadJSON(path: string): Promise<any> {
   debug('loadJSON %s', path)
   // let loadJSON

test/plugin.test.ts

@@ -0,0 +1,53 @@
+import * as path from 'path'
+
+import {expect, fancy} from './test'
+import * as Plugin from '../src/plugin'
+import * as util from '../src/util'
+
+const root = path.resolve(__dirname, 'fixtures/typescript')
+const pluginName = '@oclif/plugin-help'
+const pluginLocation = 'some/external/directory'
+const pluginPjsonLocation = path.join(pluginLocation, 'package.json')
+
+const withPluginInstance = () => {
+  return fancy
+  .add('plugin', () => new Plugin.Plugin({
+    type: 'core',
+    root,
+    name: pluginName,
+    ignoreManifest: true,
+  }))
+  .stub(util, 'exists', (checkPath: string) => checkPath === pluginPjsonLocation)
+  .stub(util, 'resolvePackage', (id: string): Promise<string> => {
+    if (id !== pluginName) {
+      return Promise.reject()
+    }
+    return Promise.resolve(path.join(pluginLocation, 'lib', 'index.js'))
+  })
+  .stub(util, 'loadJSON', (jsonPath: string) => {
+    if (jsonPath !== pluginPjsonLocation) {
+      return {}
+    }
+    return {
+      name: pluginName,
+      version: '1.0.0',
+      files: [],
+      oclif: {},
+    }
+  })
+}
+
+describe('Plugin', () => {
+  withPluginInstance()
+  .it('Should correctly instantiate a Plugin instance', ctx => {
+    expect(ctx.plugin).to.be.instanceOf(
+      Plugin.Plugin,
+      'Expected instance to be an instance of Plugin!',
+    )
+  })
+
+  withPluginInstance()
+  .it('Should correctly load a Plugin', async ctx => {
+    await ctx.plugin.load()
+  })
+})

yarn.lock

@@ -149,6 +149,13 @@
   resolved "https://registry.yarnpkg.com/@types/proxyquire/-/proxyquire-1.3.28.tgz#05a647bb0d8fe48fc8edcc193e43cc79310faa7d"
   integrity sha512-SQaNzWQ2YZSr7FqAyPPiA3FYpux2Lqh3HWMZQk47x3xbMCqgC/w0dY3dw9rGqlweDDkrySQBcaScXWeR+Yb11Q==
 
+"@types/resolve@^1.17.1":
+  version "1.17.1"
+  resolved "https://registry.yarnpkg.com/@types/resolve/-/resolve-1.17.1.tgz#3afd6ad8967c77e4376c598a82ddd58f46ec45d6"
+  integrity sha512-yy7HuzQhj0dhGpD8RLXSZWEkLsV9ibvxvi6EiJ3bkqLAO1RGo0WbkWQiwpRlSFymTJRz0d3k5LM3kkx8ArDbLw==
+  dependencies:
+    "@types/node" "*"
+
 "@types/sinon@*":
   version "7.0.11"
   resolved "https://registry.yarnpkg.com/@types/sinon/-/sinon-7.0.11.tgz#6f28f005a36e779b7db0f1359b9fb9eef72aae88"
@@ -2387,7 +2394,7 @@ resolve-from@^4.0.0:
   resolved "https://registry.yarnpkg.com/resolve-from/-/resolve-from-4.0.0.tgz#4abcd852ad32dd7baabfe9b40e00a36db5f392e6"
   integrity sha512-pb/MYmXstAkysRFx8piNI1tGFNQIFA3vkE3Gq4EuA1dF6gHp/+vgZqsCGJapvy8N3Q+4o7FwvquPJcnZ7RYy4g==
 
-resolve@^1.1.6, resolve@^1.10.0, resolve@^1.11.1, resolve@^1.8.1:
+resolve@^1.1.6, resolve@^1.10.0, resolve@^1.11.1, resolve@^1.17.0, resolve@^1.8.1:
   version "1.17.0"
   resolved "https://registry.yarnpkg.com/resolve/-/resolve-1.17.0.tgz#b25941b54968231cc2d1bb76a79cb7f2c0bf8444"
   integrity sha512-ic+7JYiV8Vi2yzQGFWOkiZD5Z9z7O2Zhm9XMaTxdJExKasieFCr+yXZ/WmXsckHiKl12ar0y6XiXDx3m4RHn1w==