Revert "feat: allow filter functions that return promises or the data directly"

This reverts commit e6c52cf26c.

src/plugins/hooks.js

@@ -1,5 +1,6 @@
 'use strict';
 
+const async = require('async');
 const util = require('util');
 const winston = require('winston');
 const plugins = require('.');
@@ -115,38 +116,37 @@ async function fireFilterHook(hook, hookList, params) {
 	if (!Array.isArray(hookList) || !hookList.length) {
 		return params;
 	}
-
+	return await async.reduce(hookList, params, (params, hookObj, next) => {
-	async function fireMethod(hookObj, params) {
 		if (typeof hookObj.method !== 'function') {
 			if (global.env === 'development') {
 				winston.warn(`[plugins] Expected method for hook '${hook}' in plugin '${hookObj.id}' not found, skipping.`);
 			}
-			return params;
+			return next(null, params);
 		}
-
+		const returned = hookObj.method(params, next);
-		if (hookObj.method.constructor && hookObj.method.constructor.name === 'AsyncFunction') {
+		if (utils.isPromise(returned)) {
-			return await hookObj.method(params);
+			returned.then(
+				payload => setImmediate(next, null, payload),
+				err => setImmediate(next, err)
+			);
 		}
-		return new Promise((resolve, reject) => {
+	});
-			const returned = hookObj.method(params, (err, result) => {
+	// breaks plugins that use a non-async function ie emoji-one parse.raw
-				if (err) reject(err); else resolve(result);
+	// for (const hookObj of hookList) {
-			});
+	// 	if (typeof hookObj.method !== 'function') {
-
+	// 		if (global.env === 'development') {
-			if (utils.isPromise(returned)) {
+	// 			winston.warn(`[plugins] Expected method for hook '${hook}' in plugin '${hookObj.id}' not found, skipping.`);
-				returned.then(
+	// 		}
-					payload => resolve(payload),
+	// 	} else {
-					err => reject(err)
+	// 		let hookFn = hookObj.method;
-				);
+	// 		if (hookFn.constructor && hookFn.constructor.name !== 'AsyncFunction') {
-				return;
+	// 			hookFn = util.promisify(hookFn);
-			}
+	// 		}
-			resolve(returned);
+	// 		// eslint-disable-next-line
-		});
+	// 		params = await hookFn(params);
-	}
+	// 	}
-	for (const hookObj of hookList) {
+	// }
-		// eslint-disable-next-line
+	// return params;
-		params = await fireMethod(hookObj, params);
-	}
-	return params;
 }
 
 async function fireActionHook(hook, hookList, params) {

test/plugins.js

@@ -47,7 +47,7 @@ describe('Plugins', () => {
 		});
 	});
 
-	it('should register and fire a filter hook having 3 methods, one returning a promise, one calling the callback and one just returning', async () => {
+	it('should register and fire a filter hook having 2 methods, one returning a promise and the other calling the callback', (done) => {
 		function method1(data, callback) {
 			data.foo += 1;
 			callback(null, data);
@@ -58,17 +58,15 @@ describe('Plugins', () => {
 				resolve(data);
 			});
 		}
-		function method3(data) {
-			data.foo += 1;
-			return data;
-		}
 
 		plugins.hooks.register('test-plugin', { hook: 'filter:test.hook2', method: method1 });
 		plugins.hooks.register('test-plugin', { hook: 'filter:test.hook2', method: method2 });
-		plugins.hooks.register('test-plugin', { hook: 'filter:test.hook2', method: method3 });
 
-		const data = await plugins.hooks.fire('filter:test.hook2', { foo: 1 });
+		plugins.hooks.fire('filter:test.hook2', { foo: 1 }, (err, data) => {
-		assert.strictEqual(data.foo, 8);
+			assert.ifError(err);
+			assert.equal(data.foo, 7);
+			done();
+		});
 	});
 
 	it('should register and fire a filter hook that returns a promise that gets rejected', (done) => {