fix(hooks): fix some errors for feathers integration (#67)

packages/hooks/src/base.ts

@@ -1,5 +1,5 @@
 import { Middleware } from './compose';
-import { copyToSelf } from './utils';
+import { copyToSelf, copyProperties } from './utils';
 
 export const HOOKS: string = Symbol('@feathersjs/hooks') as any;
 
@@ -46,50 +46,42 @@ export class HookManager {
   getMiddleware (): Middleware[]|null {
     const previous = this._parent?.getMiddleware();
 
-    if (previous) {
-      if (this._middleware) {
-        return previous.concat(this._middleware);
-      }
-
-      return previous;
+    if (previous && this._middleware) {
+      return previous.concat(this._middleware);
     }
 
-    return this._middleware;
+    return previous || this._middleware;
   }
 
   collectMiddleware (self: any, _args: any[]): Middleware[] {
     const otherMiddleware = getMiddleware(self);
     const middleware = this.getMiddleware();
 
-    if (otherMiddleware) {
-      if (middleware) {
-        return otherMiddleware.concat(middleware);
-      }
-
-      return otherMiddleware;
+    if (otherMiddleware && middleware) {
+      return otherMiddleware.concat(middleware);
     }
 
-    return middleware;
+    return otherMiddleware || middleware;
   }
 
   props (props: HookContextData) {
     if (!this._props) {
       this._props = {};
     }
 
-    Object.assign(this._props, props);
+    copyProperties(this._props, props);
 
     return this;
   }
 
   getProps (): HookContextData {
     const previous = this._parent?.getProps();
 
-    if (previous) {
-      return Object.assign({}, previous, this._props);
+    if (previous && this._props) {
+      return copyProperties({}, previous, this._props);
     }
 
-    return this._props;
+    return previous || this._props;
   }
 
   params (...params: string[]) {
@@ -101,6 +93,10 @@ export class HookManager {
   getParams (): string[] {
     const previous = this._parent?.getParams();
 
+    if (previous && this._params) {
+      return previous.concat(this._params);
+    }
+
     return previous || this._params;
   }
 
@@ -114,11 +110,11 @@ export class HookManager {
     const defaults = typeof this._defaults === 'function' ? this._defaults(self, args, context) : null;
     const previous = this._parent?.getDefaults(self, args, context);
 
-    if (previous) {
-      return Object.assign({}, previous, this._props);
+    if (previous && defaults) {
+      return Object.assign({}, previous, defaults);
     }
 
-    return defaults;
+    return previous || defaults;
   }
 
   getContextClass (Base: HookContextConstructor = HookContext): HookContextConstructor {
@@ -151,7 +147,7 @@ export class HookManager {
     }
 
     if (props) {
-      Object.assign(ContextClass.prototype, props);
+      copyProperties(ContextClass.prototype, props);
     }
 
     return ContextClass;

packages/hooks/src/hooks.ts

@@ -2,26 +2,12 @@ import { compose, Middleware } from './compose';
 import {
   HookContext, setManager, HookContextData, HookOptions, convertOptions, setMiddleware
 } from './base';
+import { copyProperties } from './utils';
 
 export function getOriginal (fn: any): any {
   return typeof fn.original === 'function' ? getOriginal(fn.original) : fn;
 }
 
-function copyProperties <F> (target: F, original: any) {
-  const originalProps = (Object.keys(original) as any)
-    .concat(Object.getOwnPropertySymbols(original));
-
-  for (const prop of originalProps) {
-    const propDescriptor = Object.getOwnPropertyDescriptor(original, prop);
-
-    if (!target.hasOwnProperty(prop)) {
-      Object.defineProperty(target, prop, propDescriptor);
-    }
-  }
-
-  return target;
-}
-
 export function functionHooks <F> (fn: F, managerOrMiddleware: HookOptions) {
   if (typeof fn !== 'function') {
     throw new Error('Can not apply hooks to non-function');

packages/hooks/src/index.ts

@@ -31,7 +31,7 @@ export function middleware (mw?: Middleware[], options?: MiddlewareOptions) {
 
   if (options) {
     if (options.params) {
-      manager.params(options.params);
+      manager.params(...options.params);
     }
 
     if (options.defaults) {

packages/hooks/src/utils.ts

@@ -12,9 +12,14 @@ export function copyToSelf (target: any) {
       const getter = hasProtoDefinitions ? target.constructor.prototype.__lookupGetter__(key)
         : Object.getOwnPropertyDescriptor(target, key);
 
-      if (getter && hasProtoDefinitions) {
+      if (hasProtoDefinitions && getter) {
         target.__defineGetter__(key, getter);
-        target.__defineSetter__(key, target.constructor.prototype.__lookupSetter__(key));
+
+        const setter = target.constructor.prototype.__lookupSetter__(key);
+
+        if (setter) {
+          target.__defineSetter__(key, setter);
+        }
       } else if (getter) {
         Object.defineProperty(target, key, getter);
       } else {
@@ -23,3 +28,20 @@ export function copyToSelf (target: any) {
     }
   }
 }
+
+export function copyProperties <F> (target: F, ...originals: any[]) {
+  for (const original of originals) {
+    const originalProps = (Object.keys(original) as any)
+        .concat(Object.getOwnPropertySymbols(original));
+
+    for (const prop of originalProps) {
+      const propDescriptor = Object.getOwnPropertyDescriptor(original, prop);
+
+      if (!target.hasOwnProperty(prop)) {
+        Object.defineProperty(target, prop, propDescriptor);
+      }
+    }
+  }
+
+  return target;
+}