Refactored how middleware is applied. Resolves #11

author · Mar 30, 2022 · 3e4a034 · 3e4a034
1 parent 2eec3f6
commit 3e4a034
Show file tree

Hide file tree

Showing 5 changed files with 108 additions and 22 deletions.
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@author.io/shell",
-  "version": "1.9.1",
+  "version": "1.9.2",
   "description": "A micro-framework for creating CLI-like experiences. This supports Node.js and browsers.",
   "main": "./src/index.js",
   "module": "./index.js",
@@ -14,7 +14,7 @@
     "test": "npm run test:node && npm run test:deno && npm run test:browser && npm run report:syntax && npm run report:size",
     "test:node": "dev test -rt node tests/*.js",
     "test:node:sanity": "dev test -rt node tests/01-sanity.js",
-    "test:node:base": "dev test -rt node tests/02-base.js",
+    "test:node:middleware": "dev test -rt node tests/02-middleware.js",
     "test:node:relationships": "dev test -rt node tests/06-relationships.js",
     "test:node:metadata": "dev test -rt node tests/04-metadata.js",
     "test:node:regression": "dev test -rt node tests/100-regression.js",

diff --git a/src/base.js b/src/base.js
@@ -18,7 +18,9 @@ export default class Base {
   #width = 80
   #name = 'Unknown'
   #middleware = new Middleware()
+  #middlewareChain = []
   #trailer = new Middleware()
+  #trailerwareChain = []
   #commonflags = {}
   #display = {
     // These are all null, representing they're NOT configured.
@@ -173,33 +175,46 @@ export default class Base {
           return this.#arguments
         }
       },
+      applyMiddleware: {
+        enumerable: false,
+        configurable: false,
+        writable: false,
+        value: () => this.#middlewareChain.forEach(mw => this.#middleware.use(mw))
+      },
+      applyTrailerware: {
+        enumerable: false,
+        configurable: false,
+        writable: false,
+        value: () => {
+          this.#trailer = this.#trailer || new Middleware()
+          this.#trailerwareChain.forEach(mw => this.#trailer.use(mw))
+        }
+      },
       initializeMiddleware: {
         enumerable: false,
         configurable: false,
         writable: false,
-        value: code => {
+        value: code => this.use(this.normalizeMiddleware(code))
+      },
+      normalizeMiddleware: {
+        enumerable: false,
+        configurable: false,
+        writable: false,
+        value: (code, type = 'middleware') => {
           if (typeof code === 'string') {
-            this.use(Function('return ' + code)()) // eslint-disable-line no-new-func
+            return Function('return ' + code)() // eslint-disable-line no-new-func
           } else if (typeof code === 'function') {
-            this.use(code)
+            return code
           } else {
-            throw new Error('Invalid middleware: ' + code.toString())
+            throw new Error(`Invalid ${type}: ` + code.toString())
           }
         }
       },
       initializeTrailer: {
         enumerable: false,
         configurable: false,
         writable: false,
-        value: code => {
-          if (typeof code === 'string') {
-            this.trailer(Function('return ' + code)()) // eslint-disable-line no-new-func
-          } else if (typeof code === 'function') {
-            this.trailer(code)
-          } else {
-            throw new Error('Invalid trailer: ' + code.toString())
-          }
-        }
+        value: code => this.trailer(this.normalizeMiddleware(code, 'trailer'))
       },
       initializeHelpAnnotations: {
         enumerable: false,
@@ -453,32 +468,56 @@ export default class Base {
     }
   }
 
+  // use () {
+  //   for (const arg of arguments) {
+  //     if (typeof arg !== 'function') {
+  //       throw new Error(`All "use()" arguments must be valid functions.\n${arg.toString().substring(0, 50)} ${arg.toString().length > 50 ? '...' : ''}`)
+  //     }
+
+  //     this.#middleware.use(arg)
+  //   }
+
+  //   this.#processors.forEach(subCmd => subCmd.use(...arguments))
+  // }
+
   use () {
     for (const arg of arguments) {
       if (typeof arg !== 'function') {
         throw new Error(`All "use()" arguments must be valid functions.\n${arg.toString().substring(0, 50)} ${arg.toString().length > 50 ? '...' : ''}`)
       }
 
-      this.#middleware.use(arg)
+      this.#middlewareChain.push(arg)
     }
 
     this.#processors.forEach(subCmd => subCmd.use(...arguments))
   }
 
   trailer () {
-    this.#trailer = this.#trailer || new Middleware()
-
     for (const arg of arguments) {
       if (typeof arg !== 'function') {
         throw new Error(`All "trailer()" arguments must be valid functions.\n${arg.toString().substring(0, 50)} ${arg.toString().length > 50 ? '...' : ''}`)
       }
 
-      this.#trailer.use(arg)
+      this.#trailerwareChain.push(arg)
     }
 
     this.#processors.forEach(subCmd => subCmd.trailer(...arguments))
   }
 
+  // trailer () {
+  //   this.#trailer = this.#trailer || new Middleware()
+
+  //   for (const arg of arguments) {
+  //     if (typeof arg !== 'function') {
+  //       throw new Error(`All "trailer()" arguments must be valid functions.\n${arg.toString().substring(0, 50)} ${arg.toString().length > 50 ? '...' : ''}`)
+  //     }
+
+  //     this.#trailer.use(arg)
+  //   }
+
+  //   this.#processors.forEach(subCmd => subCmd.trailer(...arguments))
+  // }
+
   add () {
     for (let command of arguments) {
       if (!(command instanceof Command)) {

diff --git a/src/command.js b/src/command.js
@@ -537,13 +537,20 @@ export default class Command extends Base {
     arguments[0].plugins = this.plugins
 
     if (this.shell !== null) {
-      const parentMiddleware = this.shell.getCommandMiddleware(this.commandroot.replace(new RegExp(`^${this.shell.name}`, 'i'), '').trim())
+      const { shellware } = this.shell
+      if (shellware.length > 0) {
+        this.middleware.use(...shellware)
+      }
 
+      const parentMiddleware = this.shell.getCommandMiddleware(this.commandroot.replace(new RegExp(`^${this.shell.name}`, 'i'), '').trim())
       if (parentMiddleware.length > 0) {
         this.middleware.use(...parentMiddleware)
       }
     }
 
+    this.applyMiddleware()
+    this.applyTrailerware()
+
     const trailers = this.trailers
 
     if (arguments[0].help && arguments[0].help.requested) {

diff --git a/src/shell.js b/src/shell.js
@@ -9,6 +9,7 @@ export default class Shell extends Base {
   #version
   #cursor = 0
   #tabWidth
+  #middleware = []
   #runtime = globalThis.hasOwnProperty('window') // eslint-disable-line no-prototype-builtins
     ? 'browser'
     : (
@@ -27,7 +28,7 @@ export default class Shell extends Base {
     this.__commonflags = cfg.commonflags || {}
 
     if (cfg.hasOwnProperty('use') && Array.isArray(cfg.use)) { // eslint-disable-line no-prototype-builtins
-      cfg.use.forEach(code => this.initializeMiddleware(code))
+      cfg.use.forEach(code => this.#middleware.push(this.normalizeMiddleware(code)))
     }
 
     if (cfg.hasOwnProperty('trailer') && Array.isArray(cfg.trailer)) { // eslint-disable-line no-prototype-builtins
@@ -193,6 +194,7 @@ export default class Shell extends Base {
       this.#history.pop()
     }
 
+    // The extra space is added to guarantee the pattern is recognized
     let parsed = COMMAND_PATTERN.exec(input + ' ')
 
     if (parsed === null) {
@@ -227,6 +229,8 @@ export default class Shell extends Base {
       return Command.stderr('Command not found.')
     }
 
+    // "terminal command" refers to the last command in the input
+    // string (i.e. last subcommand)
     const term = processor.getTerminalCommand(args)
 
     if (typeof callback === 'function') {
@@ -236,6 +240,10 @@ export default class Shell extends Base {
     return await Command.reply(await term.command.run(term.arguments, callback, reference))
   }
 
+  get shellware () {
+    return this.#middleware
+  }
+
   getCommandMiddleware (cmd) {
     const results = []
     cmd.split(/\s+/).forEach((c, i, a) => {

diff --git a/tests/100-regression.js b/tests/100-regression.js
@@ -1,5 +1,5 @@
 import test from 'tappedout'
-import { Shell } from '@author.io/shell'
+import { Shell, Command } from '@author.io/shell'
 
 // The range error was caused by the underlying table library.
 // When a default help message was generated, a negative column
@@ -155,3 +155,35 @@ test('Exec method should return callback value', async t => {
     t.end()
   })
 })
+
+test('Shell level middleware should execeute before command level middleware', t => {
+  let status = []
+  const cmd = new Command({
+    name: 'run',
+    async handler(meta) {
+      return meta.input
+    }
+  })
+
+  cmd.use(async function(meta, next) {
+    status.push('2')
+    next()
+  })
+
+  const sh = new Shell({
+    name: 'test',
+    use: [
+      (meta, next) => {
+        status.push('1')
+        next()
+      }
+    ],
+    commands: [cmd]
+  })
+
+  sh.exec(['run', 'other'], data => {
+    t.expect(2, status.length, 'corrent number of middleware functions executed')
+    t.expect('1,2', status.join(','), 'middleware runs in correct order')
+    t.end()
+  })
+})