From fcfc0ca6423a8062959d41f34e673f81d3c006dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Berthommier?= Date: Fri, 11 Dec 2020 02:38:50 +0100 Subject: [PATCH] fix(hooks): fix some errors for feathers integration (#67) --- packages/hooks/src/base.ts | 42 +++++++++++++++++-------------------- packages/hooks/src/hooks.ts | 16 +------------- packages/hooks/src/index.ts | 2 +- packages/hooks/src/utils.ts | 26 +++++++++++++++++++++-- 4 files changed, 45 insertions(+), 41 deletions(-) diff --git a/packages/hooks/src/base.ts b/packages/hooks/src/base.ts index df0f5c8..e4ff3a7 100644 --- a/packages/hooks/src/base.ts +++ b/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,30 +46,22 @@ 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) { @@ -77,7 +69,7 @@ export class HookManager { this._props = {}; } - Object.assign(this._props, props); + copyProperties(this._props, props); return this; } @@ -85,11 +77,11 @@ export class HookManager { 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; diff --git a/packages/hooks/src/hooks.ts b/packages/hooks/src/hooks.ts index d3283fa..1edee45 100644 --- a/packages/hooks/src/hooks.ts +++ b/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 (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 (fn: F, managerOrMiddleware: HookOptions) { if (typeof fn !== 'function') { throw new Error('Can not apply hooks to non-function'); diff --git a/packages/hooks/src/index.ts b/packages/hooks/src/index.ts index 2a3b594..dd1170b 100644 --- a/packages/hooks/src/index.ts +++ b/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) { diff --git a/packages/hooks/src/utils.ts b/packages/hooks/src/utils.ts index 5fff989..936cac9 100644 --- a/packages/hooks/src/utils.ts +++ b/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 (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; +}