fix(plugins): refactor instantiatePlugin from preproprocessor (#3628)

johnjbarton · web-flow · commit e02858ae0d0d · 2021-01-27T10:59:51.000-08:00
add unit test for plugin.js
add comments on role of function and cache
diff --git a/lib/plugin.js b/lib/plugin.js
@@ -50,4 +50,40 @@ function resolve (plugins, emitter) {
   return modules
 }
 
-exports.resolve = resolve
+/**
+  Create a function to handle errors in plugin loading.
+  @param {Object} injector, the dict of dependency injection objects.
+  @return function closed over injector, which reports errors.
+*/
+function createInstantiatePlugin (injector) {
+  const emitter = injector.get('emitter')
+  // Cache to avoid report errors multiple times per plugin.
+  const pluginInstances = new Map()
+  return function instantiatePlugin (kind, name) {
+    if (pluginInstances.has(name)) {
+      return pluginInstances.get(name)
+    }
+
+    let p
+    try {
+      p = injector.get(`${kind}:${name}`)
+      if (!p) {
+        log.error(`Failed to instantiate ${kind} ${name}`)
+        emitter.emit('load_error', kind, name)
+      }
+    } catch (e) {
+      if (e.message.includes(`No provider for "${kind}:${name}"`)) {
+        log.error(`Cannot load "${name}", it is not registered!\n  Perhaps you are missing some plugin?`)
+      } else {
+        log.error(`Cannot load "${name}"!\n  ` + e.stack)
+      }
+      emitter.emit('load_error', kind, name)
+    }
+    pluginInstances.set(name, p, `${kind}:${name}`)
+    return p
+  }
+}
+
+createInstantiatePlugin.$inject = ['injector']
+
+module.exports = { resolve, createInstantiatePlugin }
diff --git a/lib/preprocessor.js b/lib/preprocessor.js
@@ -70,36 +70,8 @@ async function runProcessors (preprocessors, file, content) {
   file.sha = CryptoUtils.sha1(content)
 }
 
-function createPriorityPreprocessor (config = {}, preprocessorPriority, basePath, injector) {
-  const emitter = injector.get('emitter')
-  const instances = new Map()
-
-  function instantiatePreprocessor (name) {
-    if (instances.has(name)) {
-      return instances.get(name)
-    }
-
-    let p
-    try {
-      p = injector.get('preprocessor:' + name)
-      if (!p) {
-        log.error(`Failed to instantiate preprocessor ${name}`)
-        emitter.emit('load_error', 'preprocessor', name)
-      }
-    } catch (e) {
-      if (e.message.includes(`No provider for "preprocessor:${name}"`)) {
-        log.error(`Can not load "${name}", it is not registered!\n  Perhaps you are missing some plugin?`)
-      } else {
-        log.error(`Can not load "${name}"!\n  ` + e.stack)
-      }
-      emitter.emit('load_error', 'preprocessor', name)
-    }
-
-    instances.set(name, p)
-    return p
-  }
-  _.union.apply(_, Object.values(config)).forEach(instantiatePreprocessor)
-
+function createPriorityPreprocessor (config = {}, preprocessorPriority, basePath, instantiatePlugin) {
+  _.union.apply(_, Object.values(config)).forEach((name) => instantiatePlugin('preprocessor', name))
   return async function preprocess (file) {
     const buffer = await tryToRead(file.originalPath, log)
     let isBinary = file.isBinary
@@ -121,7 +93,7 @@ function createPriorityPreprocessor (config = {}, preprocessorPriority, basePath
       .sort((a, b) => b[1] - a[1])
       .map((duo) => duo[0])
       .reduce((preProcs, name) => {
-        const p = instantiatePreprocessor(name)
+        const p = instantiatePlugin('preprocessor', name)
 
         if (!isBinary || (p && p.handleBinaryFiles)) {
           preProcs.push(p)
@@ -135,5 +107,5 @@ function createPriorityPreprocessor (config = {}, preprocessorPriority, basePath
   }
 }
 
-createPriorityPreprocessor.$inject = ['config.preprocessors', 'config.preprocessor_priority', 'config.basePath', 'injector']
+createPriorityPreprocessor.$inject = ['config.preprocessors', 'config.preprocessor_priority', 'config.basePath', 'instantiatePlugin']
 exports.createPriorityPreprocessor = createPriorityPreprocessor
diff --git a/lib/server.js b/lib/server.js
@@ -76,6 +76,7 @@ class Server extends KarmaEventEmitter {
       watcher: ['value', watcher],
       launcher: ['factory', Launcher.factory],
       config: ['value', config],
+      instantiatePlugin: ['factory', plugin.createInstantiatePlugin],
       preprocess: ['factory', preprocessor.createPriorityPreprocessor],
       fileList: ['factory', FileList.factory],
       webServer: ['factory', createWebServer],
diff --git a/test/unit/plugin.spec.js b/test/unit/plugin.spec.js
@@ -0,0 +1,61 @@
+'use strict'
+
+const createInstantiatePlugin = require('../../lib/plugin').createInstantiatePlugin
+
+describe('plugin', () => {
+  describe('createInstantiatePlugin', () => {
+    it('creates the instantiatePlugin function', () => {
+      const fakeGet = sinon.stub()
+      const fakeInjector = { get: fakeGet }
+
+      expect(typeof createInstantiatePlugin(fakeInjector)).to.be.equal('function')
+      expect(fakeGet).to.have.been.calledWith('emitter')
+    })
+
+    it('creates the instantiatePlugin function', () => {
+      const fakes = {
+        emitter: { emit: sinon.stub() }
+      }
+      const fakeInjector = { get: (id) => fakes[id] }
+
+      const instantiatePlugin = createInstantiatePlugin(fakeInjector)
+      expect(typeof instantiatePlugin('kind', 'name')).to.be.equal('undefined')
+      expect(fakes.emitter.emit).to.have.been.calledWith('load_error', 'kind', 'name')
+    })
+
+    it('caches plugins', () => {
+      const fakes = {
+        emitter: { emit: sinon.stub() },
+        'kind:name': { my: 'plugin' }
+      }
+      const fakeInjector = {
+        get: (id) => {
+          return fakes[id]
+        }
+      }
+
+      const instantiatePlugin = createInstantiatePlugin(fakeInjector)
+      expect(instantiatePlugin('kind', 'name')).to.be.equal(fakes['kind:name'])
+      fakeInjector.get = (id) => { throw new Error('failed to cache') }
+      expect(instantiatePlugin('kind', 'name')).to.be.equal(fakes['kind:name'])
+    })
+
+    it('errors if the injector errors', () => {
+      const fakes = {
+        emitter: { emit: sinon.stub() }
+      }
+      const fakeInjector = {
+        get: (id) => {
+          if (id in fakes) {
+            return fakes[id]
+          }
+          throw new Error('fail')
+        }
+      }
+
+      const instantiatePlugin = createInstantiatePlugin(fakeInjector)
+      expect(typeof instantiatePlugin('unknown', 'name')).to.be.equal('undefined')
+      expect(fakes.emitter.emit).to.have.been.calledWith('load_error', 'unknown', 'name')
+    })
+  })
+})
diff --git a/test/unit/preprocessor.spec.js b/test/unit/preprocessor.spec.js
