8000 feat: Remove (hook, next) signature and SKIP support (#1269) · feathersjs/feathers@211c0f8 · GitHub
[go: up one dir, main page]
Skip to content

Commit 211c0f8

Browse files
daffldaffl
authored
feat: Remove (hook, next) signature and SKIP support (#1269)
1 parent b487bbd commit 211c0f8

File tree

8 files changed

+75
-220
lines changed

8 files changed

+75
-220
lines changed

‎packages/authentication/lib/service.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ module.exports = class AuthenticationService extends AuthenticationCore {
5656
if (authResult.accessToken) {
5757
return authResult;
5858
}
59-
59+
6060
debug('Creating JWT with', payload, jwtOptions);
6161

6262
return this.createJWT(payload, jwtOptions, params.secret)

‎packages/commons/lib/hooks.js‎

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
const { _: { each, pick }, createSymbol } = require('./utils');
22

3-
// To skip further hooks
4-
const SKIP = createSymbol('__feathersSkipHooks');
5-
6-
exports.SKIP = SKIP;
73
exports.ACTIVATE_HOOKS = createSymbol('__feathersActivateHooks');
84

95
exports.createHookObject = function createHookObject (method, data = {}) {
@@ -115,10 +111,6 @@ exports.processHooks = function processHooks (hooks, initialHookObject) {
115111
// Either use the returned hook object or the current
116112
// hook object from the chain if the hook returned undefined
117113
if (current) {
118-
if (current === SKIP) {
119-
return SKIP;
120-
}
121-
122114
if (!exports.isHookObject(current)) {
123115
throw new Error(`${hookObject.type} hook for '${hookObject.method}' method returned invalid hook object`);
124116
}
@@ -132,18 +124,8 @@ exports.processHooks = function processHooks (hooks, initialHookObject) {
132124
const promise = hooks.reduce((promise, fn) => {
133125
const hook = fn.bind(this);
134126

135-
if (hook.length === 2) { // function(hook, next)
136-
promise = promise.then(hookObject => hookObject === SKIP ? SKIP : new Promise((resolve, reject) => {
137-
hook(hookObject, (error, result) =>
138-
error ? reject(error) : resolve(result)
139-
);
140-
}));
141-
} else { // function(hook)
142-
promise = promise.then(hookObject => hookObject === SKIP ? SKIP : hook(hookObject));
143-
}
144-
145127
// Use the returned hook object or the old one
146-
return promise.then(updateCurrentHook);
128+
return promise.then(hookObject => hook(hookObject)).then(updateCurrentHook);
147129
}, Promise.resolve(hookObject));
148130

149131
return promise.then(() => hookObject).catch(error => {

‎packages/commons/test/hooks.test.js‎

Lines changed: 1 addition & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -274,67 +274,14 @@ describe('hook utilities', () => {
274274
return Promise.resolve(hook);
275275
},
276276

277-
function (hook, next) {
278-
hook.chain.push('second');
279-
280-
next();
281-
},
282-
283277
hook => {
284-
hook.chain.push('third');
285-
},
286-
287-
function (hook) {
288-
hook.chain.push('fourth');
289-
290-
return hook;
291-
}
292-
], dummyHook);
293-
294-
return promise.then(result => {
295-
expect(result).to.deep.equal({
296-
type: 'dummy',
297-
method: 'something',
298-
chain: [ 'first', 'second', 'third', 'fourth' ]
299-
});
300-
});
301-
});
302-
303-
it('skip further hooks', () => {
304-
const dummyHook = {
305-
type: 'dummy',
306-
method: 'something'
307-
};
308-
309-
const promise = hooks.processHooks([
310-
function (hook) {
311-
hook.chain = [ 'first' ];
312-
313-
return Promise.resolve(hook);
314-
},
315-
316-
function (hook, next) {
317278
hook.chain.push('second');
318-
319-
next();
320-
},
321-
322-
hook => {
323-
hook.chain.push('third');
324-
325-
return hooks.SKIP;
326279
},
327280

328281
function (hook) {
329-
hook.chain.push('fourth');
282+
hook.chain.push('third');
330283

331284
return hook;
332-
},
333-
334-
function (hook, next) {
335-
hook.chain.push('fourth');
336-
337-
next();
338285
}
339286
], dummyHook);
340287

@@ -347,30 +294,6 @@ describe('hook utilities', () => {
347294
});
348295
});
349296

350-
it('next and next with error', () => {
351-
const dummyHook = {
352-
type: 'dummy',
353-
method: 'something'
354-
};
355-
356-
const promise = hooks.processHooks([
357-
function (hook, next) {
358-
hook.test = 'first ran';
359-
360-
next();
361-
},
362-
363-
function (hook, next) {
364-
next(new Error('This did not work'));
365-
}
366-
], dummyHook);
367-
368-
return promise.catch(error => {
369-
expect(error.message).to.equal('This did not work');
370-
expect(error.hook.test).to.equal('first ran');
371-
});
372-
});
373-
374297
it('errors when invalid hook object is returned', () => {
375298
const dummyHook = {
376299
type: 'dummy',

‎packages/feathers/lib/index.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
const { hooks } = require('@feathersjs/commons');
21
const Proto = require('uberproto');
32
const Application = require('./application');
43
const version = require('./version');
@@ -19,7 +18,6 @@ function createApplication () {
1918
}
2019

2120
createApplication.version = version;
22-
createApplication.SKIP = hooks.SKIP;
2321
createApplication.ACTIVATE_HOOKS = ACTIVATE_HOOKS;
2422
createApplication.activateHooks = activateHooks;
2523

‎packages/feathers/test/application.test.js‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
const assert = require('assert');
22
const Proto = require('uberproto');
3-
const { hooks } = require('@feathersjs/commons');
43
const feathers = require('../lib');
54

65
describe('Feathers application', () => {
@@ -23,10 +22,6 @@ describe('Feathers application', () => {
2322
assert.strictEqual(app.version, '3.0.0-development');
2423
});
2524

26-
it('sets SKIP on main', () => {
27-
assert.strictEqual(feathers.SKIP, hooks.SKIP);
28-
});
29-
3025
it('is an event emitter', done => {
3126
const app = feathers();
3227
const original = { hello: 'world' };

‎packages/feathers/test/hooks/after.test.js‎

Lines changed: 33 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,12 @@ describe('`after` hooks', () => {
120120

121121
service.hooks({
122122
after: {
123-
create (hook, next) {
123+
create (hook) {
124124
assert.strictEqual(hook.type, 'after');
125125

126126
hook.result.some = 'thing';
127127

128-
next(null, hook);
128+
return hook;
129129
}
130130
}
131131
});
@@ -147,11 +147,11 @@ describe('`after` hooks', () => {
147147

148148
service.hooks({
149149
after: {
150-
create (hook, next) {
150+
create (hook) {
151151
hook.result.appPresent = typeof hook.app !== 'undefined';
152152
assert.strictEqual(hook.result.appPresent, true);
153153

154-
next(null, hook);
154+
return hook;
155155
}
156156
}
157157
});
@@ -173,8 +173,8 @@ describe('`after` hooks', () => {
173173

174174
service.hooks({
175175
after: {
176-
update (hook, next) {
177-
next(new Error('This did not work'));
176+
update () {
177+
throw new Error('This did not work');
178178
}
179179
}
180180
});
@@ -221,19 +221,18 @@ describe('`after` hooks', () => {
221221

222222
service.hooks({
223223
after: {
224-
create (hook, next) {
224+
create (hook) {
225225
hook.result.some = 'thing';
226-
next();
226+
227+
return hook;
227228
}
228229
}
229230
});
230231

231232
service.hooks({
232233
after: {
233-
create (hook, next) {
234+
create (hook) {
234235
hook.result.other = 'stuff';
235-
236-
next();
237236
}
238237
}
239238
});
@@ -260,14 +259,15 @@ describe('`after` hooks', () => {
260259
service.hooks({
261260
after: {
262261
create: [
263-
function (hook, next) {
262+
function (hook) {
264263
hook.result.some = 'thing';
265-
next();
264+
265+
return hook;
266266
},
267-
function (hook, next) {
267+
function (hook) {
268268
hook.result.other = 'stuff';
269269

270-
next();
270+
return hook;
271271
}
272272
]
273273
}
@@ -293,23 +293,26 @@ describe('`after` hooks', () => {
293293

294294
service.hooks({
295295
after: {
296-
get (hook, next) {
296+
get (hook) {
297297
hook.result.items = ['first'];
298-
next();
298+
299+
return hook;
299300
}
300301
}
301302
});
302303

303304
service.hooks({
304305
after: {
305306
get: [
306-
function (hook, next) {
307+
function (hook) {
307308
hook.result.items.push('second');
308-
next();
309+
310+
return hook;
309311
},
310-
function (hook, next) {
312+
function (hook) {
311313
hook.result.items.push('third');
312-
next();
314+
315+
return hook;
313316
}
314317
]
315318
}
@@ -338,18 +341,20 @@ describe('`after` hooks', () => {
338341

339342
service E593 .hooks({
340343
after: {
341-
all (hook, next) {
344+
all (hook) {
342345
hook.result.afterAllObject = true;
343-
next();
346+
347+
return hook;
344348
}
345349
}
346350
});
347351

348352
service.hooks({
349353
after: [
350-
function (hook, next) {
354+
function (hook) {
351355
hook.result.afterAllMethodArray = true;
352-
next();
356+
357+
return hook;
353358
}
354359
]
355360
});
@@ -380,9 +385,10 @@ describe('`after` hooks', () => {
380385

381386
service.hooks({
382387
after: {
383-
get (hook, next) {
388+
get (hook) {
384389
hook.result.test = this.number + 1;
385-
next();
390+
391+
return hook;
386392
}
387393
}
388394
});
@@ -395,32 +401,5 @@ describe('`after` hooks', () => {
395401
});
396402
});
397403
});
398-
399-
it('can not call next() multiple times', () => {
400-
const app = feathers().use('/dummy', {
401-
get (id) {
402-
return Promise.resolve({ id });
403-
}
404-
});
405-
406-
const service = app.service('dummy');
407-
408-
service.hooks({
409-
after: {
410-
get: [
411-
function (hook, next) {
412-
next();
413-
},
414-
415-
function (hook, next) {
416-
next();
417-
next();
418-
}
419-
]
420-
}
421-
});
422-
423-
return service.get(10);
424-
});
425404
});
426405
});

0 commit comments

Comments
 (0)
0