Skip to content

Commit 7dfe592

Browse files
marshallswainmarshallswain
authored
Merge pull request #2483 from vonagam/feat-improve-legacy
feat(core): Improve legacy hooks integration
2 parents 12b2ae8 + 408aff4 commit 7dfe592

5 files changed

Lines changed: 174 additions & 110 deletions

File tree

‎packages/express/test/rest.test.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,7 @@ describe('@feathersjs/express/rest provider', () => {
244244
type: null,
245245
event: null,
246246
method: 'get',
247-
path: 'hook-error',
248-
original: data.hook.original
247+
path: 'hook-error'
249248
},
250249
error: { message: 'I blew up' }
251250
});

‎packages/feathers/src/hooks/index.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,18 @@ import {
77
import { defaultServiceArguments, getHookMethods } from '../service';
88
import {
99
collectLegacyHooks,
10-
enableLegacyHooks,
11-
fromAfterHook,
10+
enableLegacyHooks
11+
} from './legacy';
12+
13+
export {
1214
fromBeforeHook,
15+
fromBeforeHooks,
16+
fromAfterHook,
17+
fromAfterHooks,
18+
fromErrorHook,
1319
fromErrorHooks
1420
} from './legacy';
1521

16-
export { fromAfterHook, fromBeforeHook, fromErrorHooks };
17-
1822
export function createContext (service: Service, method: string, data: HookContextData = {}) {
1923
const createContext = (service as any)[method].createContext;
2024

‎packages/feathers/src/hooks/legacy.ts‎

Lines changed: 160 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -1,150 +1,207 @@
1-
import { _ } from '../dependencies';
2-
import { LegacyHookFunction } from '../declarations';
3-
4-
const { each } = _;
5-
const mergeContext = (context: any) => (res: any) => {
6-
if (res && res !== context) {
7-
Object.assign(context, res);
8-
}
9-
return res;
10-
}
11-
12-
export function fromBeforeHook (hook: LegacyHookFunction) {
13-
return (context: any, next: any) => {
14-
context.type = 'before';
1+
import { HookFunction, LegacyHookFunction, LegacyHookMap } from '../declarations';
2+
import { defaultServiceMethods } from '../service';
3+
4+
const runHook = <A, S> (hook: LegacyHookFunction<A, S>, context: any, type?: string) => {
5+
if (type) context.type = type;
6+
return Promise.resolve(hook.call(context.self, context))
7+
.then((res: any) => {
8+
if (type) context.type = null;
9+
if (res && res !== context) {
10+
Object.assign(context, res);
11+
}
12+
});
13+
};
1514

16-
return Promise.resolve(hook.call(context.self, context))
17-
.then(mergeContext(context))
18-
.then(() => {
19-
context.type = null;
20-
return next();
21-
});
15+
export function fromBeforeHook<A, S> (hook: LegacyHookFunction<A, S>): HookFunction<A, S> {
16+
return (context, next) => {
17+
return runHook(hook, context, 'before').then(next);
2218
};
2319
}
2420

25-
export function fromAfterHook (hook: LegacyHookFunction) {
26-
return (context: any, next: any) => {
27-
return next()
28-
.then(() => {
29-
context.type = 'after';
30-
return hook.call(context.self, context)
31-
})
32-
.then(mergeContext(context))
33-
.then(() => {
34-
context.type = null;
35-
});
21+
export function fromAfterHook<A, S> (hook: LegacyHookFunction<A, S>): HookFunction<A, S> {
22+
return (context, next) => {
23+
return next().then(() => runHook(hook, context, 'after'));
3624
}
3725
}
3826

39-
export function fromErrorHooks (hooks: LegacyHookFunction[]) {
40-
return (context: any, next: any) => {
27+
export function fromErrorHook<A, S> (hook: LegacyHookFunction<A, S>): HookFunction<A, S> {
28+
return (context, next) => {
4129
return next().catch((error: any) => {
42-
let promise: Promise<any> = Promise.resolve();
43-
44-
context.original = { ...context };
45-
context.error = error;
46-
context.type = 'error';
47-
48-
delete context.result;
49-
50-
for (const hook of hooks) {
51-
promise = promise.then(() => hook.call(context.self, context))
52-
.then(mergeContext(context))
30+
if (context.error !== error || context.result !== undefined) {
31+
(context as any).original = { ...context };
32+
context.error = error;
33+
delete context.result;
5334
}
5435

55-
return promise.then(() => {
56-
context.type = null;
57-
58-
if (context.result === undefined) {
36+
return runHook(hook, context, 'error').then(() => {
37+
if (context.result === undefined && context.error !== undefined) {
5938
throw context.error;
6039
}
6140
});
6241
});
6342
}
6443
}
6544

45+
const RunHooks = <A, S> (hooks: LegacyHookFunction<A, S>[]) => (context: any) => {
46+
return hooks.reduce((promise, hook) => {
47+
return promise.then(() => runHook(hook, context))
48+
}, Promise.resolve(undefined));
49+
};
50+
51+
export function fromBeforeHooks<A, S> (hooks: LegacyHookFunction<A, S>[]) {
52+
return fromBeforeHook(RunHooks(hooks));
53+
}
54+
55+
export function fromAfterHooks<A, S> (hooks: LegacyHookFunction<A, S>[]) {
56+
return fromAfterHook(RunHooks(hooks));
57+
}
58+
59+
export function fromErrorHooks<A, S> (hooks: LegacyHookFunction<A, S>[]) {
60+
return fromErrorHook(RunHooks(hooks));
61+
}
62+
6663
export function collectLegacyHooks (target: any, method: string) {
67-
const {
68-
before: { [method]: before = [] },
69-
after: { [method]: after = [] },
70-
error: { [method]: error = [] }
71-
} = target.__hooks;
72-
const beforeHooks = before;
73-
const afterHooks = [...after].reverse();
74-
const errorHook = fromErrorHooks(error);
75-
76-
return [errorHook, ...beforeHooks, ...afterHooks];
64+
return target.__hooks.hooks[method] || [];
7765
}
7866

7967
// Converts different hook registration formats into the
8068
// same internal format
81-
export function convertHookData (obj: any) {
82-
let hook: any = {};
69+
export function convertHookData (input: any) {
70+
const result: { [ method: string ]: LegacyHookFunction[] } = {};
8371

84-
if (Array.isArray(obj)) {
85-
hook = { all: obj };
86-
} else if (typeof obj !== 'object') {
87-
hook = { all: [ obj ] };
72+
if (Array.isArray(input)) {
73+
result.all = input;
74+
} else if (typeof input !== 'object') {
75+
result.all = [ input ];
8876
} else {
89-
each(obj, function (value, key) {
90-
hook[key] = !Array.isArray(value) ? [ value ] : value;
91-
});
77+
for (const key of Object.keys(input)) {
78+
const value = input[key];
79+
result[key] = Array.isArray(value) ? value : [ value ];
80+
}
9281
}
9382

94-
return hook;
83+
return result;
9584
}
9685

97-
// Add `.hooks` functionality to an object
98-
export function enableLegacyHooks (
99-
obj: any,
100-
methods: string[] = ['find', 'get', 'create', 'update', 'patch', 'remove'],
101-
types: string[] = ['before', 'after', 'error']
102-
) {
103-
const hookData: any = {};
86+
type LegacyType = 'before' | 'after' | 'error';
10487

105-
types.forEach(type => {
106-
// Initialize properties where hook functions are stored
107-
hookData[type] = {};
108-
});
88+
type LegacyMap = { [ type in LegacyType ]: ReturnType< typeof convertHookData > };
89+
90+
type LegacyAdapter = HookFunction & { hooks: LegacyHookFunction[] };
91+
92+
type LegacyStore = {
93+
before: { [ method: string ]: LegacyAdapter },
94+
after: { [ method: string ]: LegacyAdapter },
95+
error: { [ method: string ]: LegacyAdapter },
96+
hooks: { [ method: string ]: HookFunction[] }
97+
};
98+
99+
const types: LegacyType[] = ['before', 'after', 'error'];
100+
101+
const isType = (value: any): value is LegacyType => types.includes(value);
109102

110-
// Add non-enumerable `__hooks` property to the object
111-
Object.defineProperty(obj, '__hooks', {
103+
const wrappers = {
104+
before: fromBeforeHooks,
105+
after: fromAfterHooks,
106+
error: fromErrorHooks,
107+
};
108+
109+
const createStore = (methods: string[]) => {
110+
const store: LegacyStore = {
111+
before: {},
112+
after: {},
113+
error: {},
114+
hooks: {}
115+
};
116+
117+
for (const method of methods) {
118+
store.hooks[method] = [];
119+
}
120+
121+
return store;
122+
};
123+
124+
const setStore = (object: any, store: LegacyStore) => {
125+
Object.defineProperty(object, '__hooks', {
112126
configurable: true,
113-
value: hookData,
127+
value: store,
114128
writable: true
115129
});
130+
};
131+
132+
const getStore = (object: any): LegacyStore => object.__hooks;
116133

117-
return function legacyHooks (this: any, allHooks: any) {
118-
each(allHooks, (current: any, type) => {
119-
if (!this.__hooks[type]) {
120-
throw new Error(`'${type}' is not a valid hook type`);
134+
const createMap = (input: LegacyHookMap<any, any>, methods: string[]) => {
135+
const map = {} as LegacyMap;
136+
137+
Object.keys(input).forEach((type) => {
138+
if (!isType(type)) {
139+
throw new Error(`'${type}' is not a valid hook type`);
140+
}
141+
142+
const data = convertHookData(input[type]);
143+
144+
Object.keys(data).forEach((method) => {
145+
if (method !== 'all' && !methods.includes(method)) {
146+
throw new Error(`'${method}' is not a valid hook method`);
121147
}
148+
});
122149

123-
const hooks = convertHookData(current);
150+
map[type] = data;
151+
});
124152

125-
each(hooks, (_value, method) => {
126-
if (method !== 'all' && methods.indexOf(method) === -1) {
127-
throw new Error(`'${method}' is not a valid hook method`);
128-
}
129-
});
153+
return map;
154+
};
130155

131-
methods.forEach(method => {
132-
let currentHooks = [...(hooks.all || []), ...(hooks[method] || [])];
156+
const createAdapter = (type: LegacyType) => {
157+
const hooks: LegacyHookFunction[] = [];
158+
const hook = wrappers[type](hooks);
159+
const adapter = Object.assign(hook, { hooks });
133160

134-
this.__hooks[type][method] = this.__hooks[type][method] || [];
161+
return adapter;
162+
};
135163

136-
if (type === 'before') {
137-
currentHooks = currentHooks.map(fromBeforeHook);
138-
}
164+
const updateStore = (store: LegacyStore, map: LegacyMap) => {
165+
Object.keys(store.hooks).forEach((method) => {
166+
let adapted = false;
139167

140-
if (type === 'after') {
141-
currentHooks = currentHooks.map(fromAfterHook);
142-
}
168+
Object.keys(map).forEach((key) => {
169+
const type = key as LegacyType;
170+
const allHooks = map[type].all || [];
171+
const methodHooks = map[type][method] || [];
143172

144-
this.__hooks[type][method].push(...currentHooks);
145-
});
173+
if (allHooks.length || methodHooks.length) {
174+
const adapter = store[type][method] ||= (adapted = true, createAdapter(type));
175+
176+
adapter.hooks.push(...allHooks, ...methodHooks);
177+
}
146178
});
147179

180+
if (adapted) {
181+
store.hooks[method] = [
182+
store.error[method],
183+
store.before[method],
184+
store.after[method]
185+
].filter(hook => hook);
186+
}
187+
});
188+
};
189+
190+
// Add `.hooks` functionality to an object
191+
export function enableLegacyHooks (
192+
object: any,
193+
methods: string[] = defaultServiceMethods
194+
) {
195+
const store = createStore(methods);
196+
197+
setStore(object, store);
198+
199+
return function legacyHooks (this: any, input: LegacyHookMap<any, any>) {
200+
const store = getStore(this);
201+
const map = createMap(input, methods);
202+
203+
updateStore(store, map);
204+
148205
return this;
149206
}
150207
}

‎packages/feathers/src/service.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export const defaultEventMap = {
2424
export const protectedMethods = Object.keys(Object.prototype)
2525
.concat(Object.keys(EventEmitter.prototype))
2626
.concat([
27+
'all',
2728
'before',
2829
'after',
2930
'error',

‎packages/feathers/test/hooks/error.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ describe('`error` hooks', () => {
1111
});
1212
const service = app.service('dummy');
1313

14-
afterEach(() => (service as any).__hooks.error.get = []);
14+
afterEach(() => {
15+
(service as any).__hooks.error.get = undefined;
16+
(service as any).__hooks.hooks.get = [];
17+
});
1518

1619
it('basic error hook', async () => {
1720
service.hooks({

0 commit comments

Comments
 (0)