diff --git a/packages/express/test/rest.test.ts b/packages/express/test/rest.test.ts index 93f22c1dd..235dddc7b 100644 --- a/packages/express/test/rest.test.ts +++ b/packages/express/test/rest.test.ts @@ -244,8 +244,7 @@ describe('@feathersjs/express/rest provider', () => { type: null, event: null, method: 'get', - path: 'hook-error', - original: data.hook.original + path: 'hook-error' }, error: { message: 'I blew up' } }); diff --git a/packages/feathers/src/hooks/index.ts b/packages/feathers/src/hooks/index.ts index faac89bcb..ac0ba588e 100644 --- a/packages/feathers/src/hooks/index.ts +++ b/packages/feathers/src/hooks/index.ts @@ -7,13 +7,17 @@ import { import { defaultServiceArguments, getHookMethods } from '../service'; import { collectLegacyHooks, - enableLegacyHooks, - fromAfterHook, - fromBeforeHook, - fromErrorHooks + enableLegacyHooks } from './legacy'; -export { fromAfterHook, fromBeforeHook, fromErrorHooks }; +export { + fromBeforeHook, + fromBeforeHooks, + fromAfterHook, + fromAfterHooks, + fromErrorHook, + fromErrorHooks +} from './legacy'; export function createContext (service: Service, method: string, data: HookContextData = {}) { const createContext = (service as any)[method].createContext; diff --git a/packages/feathers/src/hooks/legacy.ts b/packages/feathers/src/hooks/legacy.ts index 205248943..51d426dac 100644 --- a/packages/feathers/src/hooks/legacy.ts +++ b/packages/feathers/src/hooks/legacy.ts @@ -1,61 +1,40 @@ -import { _ } from '../dependencies'; -import { LegacyHookFunction } from '../declarations'; +import { HookFunction, LegacyHookFunction, LegacyHookMap } from '../declarations'; +import { defaultServiceMethods } from '../service'; -const { each } = _; -const mergeContext = (context: any) => (res: any) => { - if (res && res !== context) { - Object.assign(context, res); - } - return res; -} +const runHook = (hook: LegacyHookFunction, context: any, type?: string) => { + if (type) context.type = type; + return Promise.resolve(hook.call(context.self, context)) + .then((res: any) => { + if (type) context.type = null; + if (res && res !== context) { + Object.assign(context, res); + } + }); +}; -export function fromBeforeHook (hook: LegacyHookFunction) { - return (context: any, next: any) => { - context.type = 'before'; - - return Promise.resolve(hook.call(context.self, context)) - .then(mergeContext(context)) - .then(() => { - context.type = null; - return next(); - }); +export function fromBeforeHook (hook: LegacyHookFunction): HookFunction { + return (context, next) => { + return runHook(hook, context, 'before').then(next); }; } -export function fromAfterHook (hook: LegacyHookFunction) { - return (context: any, next: any) => { - return next() - .then(() => { - context.type = 'after'; - return hook.call(context.self, context) - }) - .then(mergeContext(context)) - .then(() => { - context.type = null; - }); +export function fromAfterHook (hook: LegacyHookFunction): HookFunction { + return (context, next) => { + return next().then(() => runHook(hook, context, 'after')); } } -export function fromErrorHooks (hooks: LegacyHookFunction[]) { - return (context: any, next: any) => { +export function fromErrorHook (hook: LegacyHookFunction): HookFunction { + return (context, next) => { return next().catch((error: any) => { - let promise: Promise = Promise.resolve(); - - context.original = { ...context }; - context.error = error; - context.type = 'error'; - - delete context.result; - - for (const hook of hooks) { - promise = promise.then(() => hook.call(context.self, context)) - .then(mergeContext(context)) + if (context.error !== error || context.result !== undefined) { + (context as any).original = { ...context }; + context.error = error; + delete context.result; } - return promise.then(() => { - context.type = null; - - if (context.result === undefined) { + return runHook(hook, context, 'error').then(() => { + if (context.result === undefined && context.error !== undefined) { throw context.error; } }); @@ -63,17 +42,26 @@ export function fromErrorHooks (hooks: LegacyHookFunction[]) { } } -export function collectLegacyHooks (target: any, method: string) { - const { - before: { [method]: before = [] }, - after: { [method]: after = [] }, - error: { [method]: error = [] } - } = target.__hooks; - const beforeHooks = before; - const afterHooks = [...after].reverse(); - const errorHook = fromErrorHooks(error); +const RunHooks = (hooks: LegacyHookFunction[]) => (context: any) => { + return hooks.reduce((promise, hook) => { + return promise.then(() => runHook(hook, context)) + }, Promise.resolve(undefined)); +}; - return [errorHook, ...beforeHooks, ...afterHooks]; +export function fromBeforeHooks (hooks: LegacyHookFunction[]) { + return fromBeforeHook(RunHooks(hooks)); +} + +export function fromAfterHooks (hooks: LegacyHookFunction[]) { + return fromAfterHook(RunHooks(hooks)); +} + +export function fromErrorHooks (hooks: LegacyHookFunction[]) { + return fromErrorHook(RunHooks(hooks)); +} + +export function collectLegacyHooks (target: any, method: string) { + return target.__hooks.hooks[method] || []; } // Converts different hook registration formats into the @@ -86,26 +74,36 @@ export function convertHookData (obj: any) { } else if (typeof obj !== 'object') { hook = { all: [ obj ] }; } else { - each(obj, function (value, key) { + for (const [key, value] of Object.entries(obj)) { hook[key] = !Array.isArray(value) ? [ value ] : value; - }); + } } return hook; } +const types = ['before', 'after', 'error']; + +const wrappers: any = { + before: fromBeforeHooks, + after: fromAfterHooks, + error: fromErrorHooks, +}; + // Add `.hooks` functionality to an object export function enableLegacyHooks ( obj: any, - methods: string[] = ['find', 'get', 'create', 'update', 'patch', 'remove'], - types: string[] = ['before', 'after', 'error'] + methods: string[] = defaultServiceMethods ) { - const hookData: any = {}; + const hookData: any = {hooks: {}}; - types.forEach(type => { - // Initialize properties where hook functions are stored + for (const type of types) { hookData[type] = {}; - }); + } + + for (const method of methods) { + hookData.hooks[method] = []; + } // Add non-enumerable `__hooks` property to the object Object.defineProperty(obj, '__hooks', { @@ -114,36 +112,49 @@ export function enableLegacyHooks ( writable: true }); - return function legacyHooks (this: any, allHooks: any) { - each(allHooks, (current: any, type) => { - if (!this.__hooks[type]) { + return function legacyHooks (this: any, allHooks: LegacyHookMap) { + const touched = new Set(); + + for (const [type, current] of Object.entries(allHooks)) { + if (!types.includes(type)) { throw new Error(`'${type}' is not a valid hook type`); } const hooks = convertHookData(current); - each(hooks, (_value, method) => { - if (method !== 'all' && methods.indexOf(method) === -1) { + for (const method of Object.keys(hooks)) { + if (method !== 'all' && !methods.includes(method)) { throw new Error(`'${method}' is not a valid hook method`); } - }); + } - methods.forEach(method => { - let currentHooks = [...(hooks.all || []), ...(hooks[method] || [])]; + for (const method of methods) { + if (!hooks.all?.length && !hooks[method]?.length) continue; - this.__hooks[type][method] = this.__hooks[type][method] || []; + const hook = this.__hooks[type][method] ||= (() => { + const hooks: LegacyHookFunction[] = []; + const hook = wrappers[type](hooks); + hook.hooks = hooks; + touched.add(method); + return hook; + })(); - if (type === 'before') { - currentHooks = currentHooks.map(fromBeforeHook); - } + hook.hooks.push(...(hooks.all || []), ...(hooks[method] || [])); + } + } - if (type === 'after') { - currentHooks = currentHooks.map(fromAfterHook); - } + for (const method of touched) { + const before = this.__hooks.before[method]; + const after = this.__hooks.after[method]; + const error = this.__hooks.error[method]; - this.__hooks[type][method].push(...currentHooks); - }); - }); + const hooks: HookFunction[] = []; + if (error) hooks.push(error); + if (before) hooks.push(before); + if (after) hooks.push(after); + + this.__hooks.hooks[method] = hooks; + } return this; } diff --git a/packages/feathers/src/service.ts b/packages/feathers/src/service.ts index c9bfd021d..bf0bb7a25 100644 --- a/packages/feathers/src/service.ts +++ b/packages/feathers/src/service.ts @@ -24,6 +24,7 @@ export const defaultEventMap = { export const protectedMethods = Object.keys(Object.prototype) .concat(Object.keys(EventEmitter.prototype)) .concat([ + 'all', 'before', 'after', 'error', diff --git a/packages/feathers/test/hooks/error.test.ts b/packages/feathers/test/hooks/error.test.ts index 332738f67..824f2b418 100644 --- a/packages/feathers/test/hooks/error.test.ts +++ b/packages/feathers/test/hooks/error.test.ts @@ -11,7 +11,10 @@ describe('`error` hooks', () => { }); const service = app.service('dummy'); - afterEach(() => (service as any).__hooks.error.get = []); + afterEach(() => { + (service as any).__hooks.error.get = undefined; + (service as any).__hooks.hooks.get = []; + }); it('basic error hook', async () => { service.hooks({