feat: allow removing registered hooks (#16)

README.md

@@ -59,6 +59,44 @@ lib.addHooks({
 })
 ```
 
+**Unregistering hooks:**
+
+```js
+const lib = newFooLib()
+
+const hook0 = async () => { /* ... */ }
+const hook1 = async () => { /* ... */ }
+const hook2 = async () => { /* ... */ }
+
+// The hook() method returns an "unregister" function
+const unregisterHook0 = lib.hook('hook0', hook0)
+const unregisterHooks1and2 lib.addHooks({ hook1, hook2 })
+
+/* ... */
+
+unregisterHook0()
+unregisterHooks1and2()
+
+// or
+
+lib.removeHooks({ hook0, hook1 })
+lib.removeHook('hook2', hook2)
+```
+
+**Triggering a hook handler once:**
+
+```js
+const lib = newFooLib()
+
+const unregister = lib.hook('hook0', async () => {
+  // Unregister as soon as the hook is executed
+  unregister()
+
+  /* ... */
+})
+```
+
+
 ## Hookable class
 
 ### `constructor(logger)`
@@ -74,6 +112,8 @@ It should be an object implementing following functions:
 
 Register a handler for a specific hook. `fn` must be a function.
 
+Returns an `unregister` function that, when called, will remove the registered handler.
+
 ### `addHooks(configHooks)`
 
 Flatten and register hooks object.
@@ -92,6 +132,8 @@ hookable.addHooks({
 
 This registers `test:before` and `test:after` hooks at bulk.
 
+Returns an `unregister` function that, when called, will remove all the registered handlers.
+
 ### `async callHook (name, ...args)`
 
 Used by class itself to **sequentially** call handlers of a specific hook.
@@ -104,6 +146,32 @@ Deprecate hook called `old` in favor of `name` hook.
 
 Deprecate all hooks from an object (keys are old and values or newer ones).
 
+### `removeHook (name, fn)`
+
+Remove a particular hook handler, if the `fn` handler is present.
+
+### `removeHooks (configHooks)`
+
+Remove multiple hook handlers.
+
+Example:
+
+```js
+const handler = async () => { /* ... */ }
+
+hookable.hook('test:before', handler)
+hookable.addHooks({ test: { after: handler } })
+
+// ...
+
+hookable.removeHooks({
+  test: {
+    before: handler,
+    after: handler
+  }
+})
+```
+
 ### `clearHook (name)`
 
 Clear all hooks for a specific hook.

src/hookable.js

@@ -38,6 +38,27 @@ export default class Hookable {
 
     this._hooks[name] = this._hooks[name] || []
     this._hooks[name].push(fn)
+
+    return () => {
+      if (fn) {
+        this.removeHook(name, fn)
+        fn = null // Free memory
+      }
+    }
+  }
+
+  removeHook (name, fn) {
+    if (this._hooks[name]) {
+      const idx = this._hooks[name].indexOf(fn)
+
+      if (idx !== -1) {
+        this._hooks[name].splice(idx, 1)
+      }
+
+      if (this._hooks[name].length === 0) {
+        delete this._hooks[name]
+      }
+    }
   }
 
   deprecateHook (old, name) {
@@ -49,9 +70,20 @@ export default class Hookable {
   }
 
   addHooks (configHooks) {
+    const hooks = flatHooks(configHooks)
+    const removeFns = Object.keys(hooks).map(key => this.hook(key, hooks[key]))
+
+    return () => {
+      // Splice will ensure that all fns are called once, and free all
+      // unreg functions from memory.
+      removeFns.splice(0, removeFns.length).forEach(unreg => unreg())
+    }
+  }
+
+  removeHooks (configHooks) {
     const hooks = flatHooks(configHooks)
     for (const key in hooks) {
-      this.hook(key, hooks[key])
+      this.removeHook(key, hooks[key])
     }
   }
 

test/hookable.test.js

@@ -131,6 +131,70 @@ describe('core: hookable', () => {
     expect(hook._hooks['test:before']).toBeUndefined()
   })
 
+  test('should return a self-removal function', async () => {
+    const hook = new Hookable()
+    const remove = hook.hook('test:hook', () => {
+      console.log('test:hook called')
+    })
+
+    await hook.callHook('test:hook')
+    remove()
+    await hook.callHook('test:hook')
+
+    expect(console.log).toBeCalledTimes(1)
+  })
+
+  test('should clear only its own hooks', () => {
+    const hook = new Hookable()
+    const callback = () => { }
+
+    hook.hook('test:hook', callback)
+    const remove = hook.hook('test:hook', callback)
+    hook.hook('test:hook', callback)
+
+    expect(hook._hooks['test:hook']).toEqual([callback, callback, callback])
+
+    remove()
+    remove()
+    remove()
+
+    expect(hook._hooks['test:hook']).toEqual([callback, callback])
+  })
+
+  test('should clear removed hooks', () => {
+    const hook = new Hookable()
+    const callback = () => { }
+    hook.hook('test:hook', callback)
+    hook.hook('test:hook', callback)
+
+    expect(hook._hooks['test:hook']).toHaveLength(2)
+
+    hook.removeHook('test:hook', callback)
+
+    expect(hook._hooks['test:hook']).toHaveLength(1)
+
+    hook.removeHook('test:hook', callback)
+
+    expect(hook._hooks['test:hook']).toBeUndefined()
+  })
+
+  test('should call self-removing hooks once', async () => {
+    const hook = new Hookable()
+    const remove = hook.hook('test:hook', () => {
+      console.log('test:hook called')
+      remove()
+    })
+
+    expect(hook._hooks['test:hook']).toHaveLength(1)
+
+    await hook.callHook('test:hook')
+    await hook.callHook('test:hook')
+
+    expect(console.log).toBeCalledWith('test:hook called')
+    expect(console.log).toBeCalledTimes(1)
+    expect(hook._hooks['test:hook']).toBeUndefined()
+  })
+
   test('should clear all registered hooks', () => {
     const hook = new Hookable()
     hook.hook('test:hook', () => { })
@@ -177,4 +241,52 @@ describe('core: hookable', () => {
     expect(hook._hooks['test:hook']).toHaveLength(1)
     expect(hook._hooks['test:before']).toHaveLength(1)
   })
+
+  test('should clear multiple hooks', () => {
+    const hook = new Hookable()
+    const callback = () => {}
+
+    const hooks = {
+      test: {
+        hook: () => { },
+        before: () => { }
+      }
+    }
+
+    hook.addHooks(hooks)
+
+    hook.hook('test:hook', callback)
+
+    expect(hook._hooks['test:hook']).toHaveLength(2)
+    expect(hook._hooks['test:before']).toHaveLength(1)
+
+    hook.removeHooks(hooks)
+
+    expect(hook._hooks['test:hook']).toEqual([callback])
+    expect(hook._hooks['test:before']).toBeUndefined()
+  })
+
+  test('should clear only the hooks added by addHooks', () => {
+    const hook = new Hookable()
+    const callback1 = () => {}
+    const callback2 = () => {}
+    hook.hook('test:hook', callback1)
+
+    const remove = hook.addHooks({
+      test: {
+        hook: () => { },
+        before: () => { }
+      }
+    })
+
+    hook.hook('test:hook', callback2)
+
+    expect(hook._hooks['test:hook']).toHaveLength(3)
+    expect(hook._hooks['test:before']).toHaveLength(1)
+
+    remove()
+
+    expect(hook._hooks['test:hook']).toEqual([callback1, callback2])
+    expect(hook._hooks['test:before']).toBeUndefined()
+  })
 })