From 77ac2328711c1b92a97a8b260efc4c0f58c788fa Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Fri, 6 Oct 2017 09:11:09 -0600 Subject: [PATCH 1/8] added hook module to prebid core that allows extension of arbitrary functions --- modules/currency.js | 67 ++++++++---------- package.json | 3 +- src/bidmanager.js | 5 +- src/hook.js | 78 +++++++++++++++++++++ test/spec/hook_spec.js | 105 +++++++++++++++++++++++++++++ test/spec/modules/currency_spec.js | 60 +++++++---------- 6 files changed, 239 insertions(+), 79 deletions(-) create mode 100644 src/hook.js create mode 100644 test/spec/hook_spec.js diff --git a/modules/currency.js b/modules/currency.js index 1d0286ed569..19e7d2903f4 100644 --- a/modules/currency.js +++ b/modules/currency.js @@ -13,9 +13,6 @@ var conversionCache = {}; var currencyRatesLoaded = false; var adServerCurrency = 'USD'; -// Used as reference to the original bidmanager.addBidResponse -var originalBidResponse; - export var currencySupportEnabled = false; export var currencyRates = {}; var bidderCurrencyDefault = {}; @@ -81,12 +78,9 @@ function initCurrency(url) { conversionCache = {}; currencySupportEnabled = true; - if (!originalBidResponse) { - utils.logInfo('Installing addBidResponse decorator for currency module', arguments); + utils.logInfo('Installing addBidResponse decorator for currency module', arguments); - originalBidResponse = bidmanager.addBidResponse; - bidmanager.addBidResponse = addBidResponseDecorator(bidmanager.addBidResponse); - } + bidmanager.addBidResponse.addHook(addBidResponseHook, 100); if (!currencyRates.conversions) { ajax(url, function (response) { @@ -103,12 +97,9 @@ function initCurrency(url) { } function resetCurrency() { - if (originalBidResponse) { - utils.logInfo('Uninstalling addBidResponse decorator for currency module', arguments); + utils.logInfo('Uninstalling addBidResponse decorator for currency module', arguments); - bidmanager.addBidResponse = originalBidResponse; - originalBidResponse = undefined; - } + bidmanager.addBidResponse.removeHook(addBidResponseHook); adServerCurrency = 'USD'; conversionCache = {}; @@ -118,37 +109,35 @@ function resetCurrency() { bidderCurrencyDefault = {}; } -export function addBidResponseDecorator(fn) { - return function(adUnitCode, bid) { - if (!bid) { - return fn.apply(this, arguments); // if no bid, call original and let it display warnings - } +export function addBidResponseHook(adUnitCode, bid, fn) { + if (!bid) { + return fn.apply(this, arguments); // if no bid, call original and let it display warnings + } - let bidder = bid.bidderCode || bid.bidder; - if (bidderCurrencyDefault[bidder]) { - let currencyDefault = bidderCurrencyDefault[bidder]; - if (bid.currency && currencyDefault !== bid.currency) { - utils.logWarn(`Currency default '${bidder}: ${currencyDefault}' ignored. adapter specified '${bid.currency}'`); - } else { - bid.currency = currencyDefault; - } + let bidder = bid.bidderCode || bid.bidder; + if (bidderCurrencyDefault[bidder]) { + let currencyDefault = bidderCurrencyDefault[bidder]; + if (bid.currency && currencyDefault !== bid.currency) { + utils.logWarn(`Currency default '${bidder}: ${currencyDefault}' ignored. adapter specified '${bid.currency}'`); + } else { + bid.currency = currencyDefault; } + } - // default to USD if currency not set - if (!bid.currency) { - utils.logWarn('Currency not specified on bid. Defaulted to "USD"'); - bid.currency = 'USD'; - } + // default to USD if currency not set + if (!bid.currency) { + utils.logWarn('Currency not specified on bid. Defaulted to "USD"'); + bid.currency = 'USD'; + } - // execute immediately if the bid is already in the desired currency - if (bid.currency === adServerCurrency) { - return fn.apply(this, arguments); - } + // execute immediately if the bid is already in the desired currency + if (bid.currency === adServerCurrency) { + return fn.apply(this, arguments); + } - bidResponseQueue.push(wrapFunction(fn, this, arguments)); - if (!currencySupportEnabled || currencyRatesLoaded) { - processBidResponseQueue(); - } + bidResponseQueue.push(wrapFunction(fn, this, arguments)); + if (!currencySupportEnabled || currencyRatesLoaded) { + processBidResponseQueue(); } } diff --git a/package.json b/package.json index c5017e3f149..d03c5408973 100644 --- a/package.json +++ b/package.json @@ -105,6 +105,7 @@ "dependencies": { "babel-plugin-transform-object-assign": "^6.22.0", "core-js": "^2.4.1", - "gulp-sourcemaps": "^2.6.0" + "gulp-sourcemaps": "^2.6.0", + "tinyqueue": "^1.2.2" } } diff --git a/src/bidmanager.js b/src/bidmanager.js index 800a0fe9579..189146e3e81 100644 --- a/src/bidmanager.js +++ b/src/bidmanager.js @@ -5,6 +5,7 @@ import { isValidVideoBid } from './video'; import { getCacheUrl, store } from './videoCache'; import { Renderer } from 'src/Renderer'; import { config } from 'src/config'; +import { createHook } from 'src/hook'; var CONSTANTS = require('./constants.json'); var AUCTION_END = CONSTANTS.EVENTS.AUCTION_END; @@ -82,7 +83,7 @@ exports.bidsBackAll = function () { /* * This function should be called to by the bidder adapter to register a bid response */ -exports.addBidResponse = function (adUnitCode, bid) { +exports.addBidResponse = createHook('asyncSeries', function (adUnitCode, bid) { if (isValid()) { prepareBidForAuction(); @@ -250,7 +251,7 @@ exports.addBidResponse = function (adUnitCode, bid) { doCallbacksIfNeeded(); } } -}; +}); function getKeyValueTargetingPairs(bidderCode, custBidObj) { var keyValues = {}; diff --git a/src/hook.js b/src/hook.js new file mode 100644 index 00000000000..38c3348f24a --- /dev/null +++ b/src/hook.js @@ -0,0 +1,78 @@ + +/** + * @typedef {function} PluginFunction + * @property {function(function(), [number])} addHook A method that takes a new function to attach as a hook + * to the PluginFunction + * @property {function(function())} removeHook A method to remove attached hooks + */ + +/** + * A map of global hook methods to allow easy extension of hooked functions that are intended to be extended globally + * @type {{}} + */ +export const hooks = {}; + +/** + * A utility function for allowing a regular function to be extensible with additional hook functions + * @param {string} type The method for applying all attached hooks when this hooked function is called + * @param {function()} fn The function to make hookable + * @param {string} hookName If provided this allows you to register a name for a global hook to have easy access to + * the addPlugin and removePlugin methods for that hook (which are usually accessed as methods on the function itself) + * @returns {PluginFunction} A new function that implements the PluginFunction interface + */ +export function createHook(type, fn, hookName) { + let _hooks = [{fn, priority: 0}]; + + let types = { + sync: function(...args) { + _hooks.forEach(hook => { + hook.fn.apply(null, args); + }); + }, + asyncSeries: function(...args) { + let curr = 0; + + return _hooks[curr].fn.apply(null, args.concat(asyncSeriesNext)); + + function asyncSeriesNext(...args) { + let hook = _hooks[++curr]; + if (typeof hook === 'object' && typeof hook.fn === 'function') { + return hook.fn.apply(null, args.concat(asyncSeriesNext)) + } + } + } + }; + + if (!types[type]) { + throw 'invalid hook type'; + } + + let methods = { + addHook: function(fn, priority = 10) { + if (typeof fn === 'function') { + _hooks.push({ + fn, + priority: priority + }); + + _hooks.sort((a, b) => b.priority - a.priority); + } + }, + removeHook: function(removeFn) { + _hooks = _hooks.filter(hook => hook.fn === fn || hook.fn !== removeFn); + } + }; + + if (typeof hookName === 'string') { + hooks[hookName] = methods; + } + + function hookedFn(...args) { + if (_hooks.length === 0) { + return fn.apply(null, args); + } + return types[type].apply(null, args); + } + + return Object.assign(hookedFn, methods); +} diff --git a/test/spec/hook_spec.js b/test/spec/hook_spec.js new file mode 100644 index 00000000000..aae2a93868b --- /dev/null +++ b/test/spec/hook_spec.js @@ -0,0 +1,105 @@ + +import { expect } from 'chai'; +import { createHook, hooks } from 'src/hook'; + +describe('the hook module', () => { + let sandbox; + + beforeEach(() => { + sandbox = sinon.sandbox.create(); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it('should call all sync hooks attached to a function', () => { + let called = []; + let calledWith; + + let testFn = () => { + called.push(testFn); + }; + let testHook = (...args) => { + called.push(testHook); + calledWith = args; + }; + let testHook2 = () => { + called.push(testHook2); + }; + let testHook3 = () => { + called.push(testHook3); + }; + + let hookedTestFn = createHook('sync', testFn, 'testHook'); + + hookedTestFn.addHook(testHook, 50); + hookedTestFn.addHook(testHook2, 100); + + // make sure global test hooks work as well (with default priority) + hooks['testHook'].addHook(testHook3); + + hookedTestFn(1, 2, 3); + + expect(called).to.deep.equal([ + testHook2, + testHook, + testHook3, + testFn + ]); + + expect(calledWith).to.deep.equal([1, 2, 3]); + + called = []; + + hookedTestFn.removeHook(testHook); + hooks['testHook'].removeHook(testHook3); + + hookedTestFn(1, 2, 3); + + expect(called).to.deep.equal([ + testHook2, + testFn + ]); + }); + + describe('asyncSeries', () => { + it('should call function as normal if no hooks attached', () => { + let fn = sandbox.spy(); + let hookFn = createHook('asyncSeries', fn); + + hookFn(1); + + expect(fn.calledOnce).to.equal(true); + expect(fn.firstCall.args[0]).to.equal(1); + }); + + it('should call hooks correctly applied in asyncSeries', () => { + let called = []; + + let testFn = (called) => { + called.push(testFn); + }; + let testHook = (called, next) => { + called.push(testHook); + next(called); + }; + let testHook2 = (called, next) => { + called.push(testHook2); + next(called); + }; + + let hookedTestFn = createHook('asyncSeries', testFn); + hookedTestFn.addHook(testHook); + hookedTestFn.addHook(testHook2); + + hookedTestFn(called); + + expect(called).to.deep.equal([ + testHook, + testHook2, + testFn + ]); + }); + }); +}); diff --git a/test/spec/modules/currency_spec.js b/test/spec/modules/currency_spec.js index 937e6a084e4..06faa5665c9 100644 --- a/test/spec/modules/currency_spec.js +++ b/test/spec/modules/currency_spec.js @@ -5,7 +5,7 @@ import { import { setConfig, - addBidResponseDecorator, + addBidResponseHook, currencySupportEnabled, currencyRates @@ -46,10 +46,6 @@ describe('currency', function () { var bid = { cpm: 1, bidder: 'rubicon' }; var innerBid; - var wrappedAddBidResponseFn = addBidResponseDecorator(function(adCodeId, bid) { - innerBid = bid; - }); - setConfig({ adServerCurrency: 'GBP', bidderCurrencyDefault: { @@ -57,7 +53,9 @@ describe('currency', function () { } }); - wrappedAddBidResponseFn('elementId', bid); + addBidResponseHook('elementId', bid, function(adCodeId, bid) { + innerBid = bid; + }); expect(innerBid.currency).to.equal('GBP') }); @@ -68,10 +66,6 @@ describe('currency', function () { var bid = { cpm: 1, currency: 'JPY', bidder: 'rubicon' }; var innerBid; - var wrappedAddBidResponseFn = addBidResponseDecorator(function(adCodeId, bid) { - innerBid = bid; - }); - setConfig({ adServerCurrency: 'JPY', bidderCurrencyDefault: { @@ -79,7 +73,9 @@ describe('currency', function () { } }); - wrappedAddBidResponseFn('elementId', bid); + addBidResponseHook('elementId', bid, function(adCodeId, bid) { + innerBid = bid; + }); expect(innerBid.currency).to.equal('JPY') }); @@ -97,12 +93,10 @@ describe('currency', function () { var bid = { cpm: 100, currency: 'JPY', bidder: 'rubicon' }; var innerBid; - var wrappedAddBidResponseFn = addBidResponseDecorator(function(adCodeId, bid) { + addBidResponseHook('elementId', bid, function(adCodeId, bid) { innerBid = bid; }); - wrappedAddBidResponseFn('elementId', bid); - expect(innerBid.cpm).to.equal('1.0000'); }); }); @@ -113,14 +107,15 @@ describe('currency', function () { fakeCurrencyFileServer.respondWith(JSON.stringify(getCurrencyRates())); - var marker = false; - var wrappedAddBidResponseFn = addBidResponseDecorator(function(adCodeId, bid) { - marker = true; - }); var bid = { 'cpm': 1, 'currency': 'USD' }; setConfig({ 'adServerCurrency': 'JPY' }); - wrappedAddBidResponseFn('elementId', bid); + + var marker = false; + addBidResponseHook('elementId', bid, function() { + marker = true; + }); + expect(marker).to.equal(false); fakeCurrencyFileServer.respond(); @@ -133,10 +128,9 @@ describe('currency', function () { setConfig({}); var bid = { 'cpm': 1, 'currency': 'USD' }; var innerBid; - var wrappedAddBidResponseFn = addBidResponseDecorator(function(adCodeId, bid) { + addBidResponseHook('elementId', bid, function(adCodeId, bid) { innerBid = bid; }); - wrappedAddBidResponseFn('elementId', bid); expect(innerBid.cpm).to.equal(1); }); @@ -144,10 +138,9 @@ describe('currency', function () { setConfig({}); var bid = { 'cpm': 1, 'currency': 'GBP' }; var innerBid; - var wrappedAddBidResponseFn = addBidResponseDecorator(function(adCodeId, bid) { + addBidResponseHook('elementId', bid, function(adCodeId, bid) { innerBid = bid; }); - wrappedAddBidResponseFn('elementId', bid); expect(innerBid.statusMessage).to.equal('Bid returned empty or error response'); }); @@ -157,10 +150,9 @@ describe('currency', function () { }); var bid = { 'cpm': 1, 'currency': 'USD' }; var innerBid; - var wrappedAddBidResponseFn = addBidResponseDecorator(function(adCodeId, bid) { + addBidResponseHook('elementId', bid, function(adCodeId, bid) { innerBid = bid; }); - wrappedAddBidResponseFn('elementId', bid); expect(bid).to.equal(innerBid); }); @@ -170,10 +162,9 @@ describe('currency', function () { fakeCurrencyFileServer.respond(); var bid = { 'cpm': 1, 'currency': 'ABC' }; var innerBid; - var wrappedAddBidResponseFn = addBidResponseDecorator(function(adCodeId, bid) { + addBidResponseHook('elementId', bid, function(adCodeId, bid) { innerBid = bid; }); - wrappedAddBidResponseFn('elementId', bid); expect(innerBid.statusMessage).to.equal('Bid returned empty or error response'); }); @@ -183,10 +174,9 @@ describe('currency', function () { fakeCurrencyFileServer.respond(); var bid = { 'cpm': 1, 'currency': 'GBP' }; var innerBid; - var wrappedAddBidResponseFn = addBidResponseDecorator(function(adCodeId, bid) { + addBidResponseHook('elementId', bid, function(adCodeId, bid) { innerBid = bid; }); - wrappedAddBidResponseFn('elementId', bid); expect(innerBid.statusMessage).to.equal('Bid returned empty or error response'); }); @@ -196,10 +186,9 @@ describe('currency', function () { fakeCurrencyFileServer.respond(); var bid = { 'cpm': 1, 'currency': 'JPY' }; var innerBid; - var wrappedAddBidResponseFn = addBidResponseDecorator(function(adCodeId, bid) { + addBidResponseHook('elementId', bid, function(adCodeId, bid) { innerBid = bid; }); - wrappedAddBidResponseFn('elementId', bid); expect(innerBid.cpm).to.equal(1); expect(innerBid.currency).to.equal('JPY'); }); @@ -210,10 +199,9 @@ describe('currency', function () { fakeCurrencyFileServer.respond(); var bid = { 'cpm': 1, 'currency': 'USD' }; var innerBid; - var wrappedAddBidResponseFn = addBidResponseDecorator(function(adCodeId, bid) { + addBidResponseHook('elementId', bid, function(adCodeId, bid) { innerBid = bid; }); - wrappedAddBidResponseFn('elementId', bid); expect(innerBid.cpm).to.equal('0.7798'); expect(innerBid.currency).to.equal('GBP'); }); @@ -224,10 +212,9 @@ describe('currency', function () { fakeCurrencyFileServer.respond(); var bid = { 'cpm': 1, 'currency': 'CNY' }; var innerBid; - var wrappedAddBidResponseFn = addBidResponseDecorator(function(adCodeId, bid) { + addBidResponseHook('elementId', bid, function(adCodeId, bid) { innerBid = bid; }); - wrappedAddBidResponseFn('elementId', bid); expect(innerBid.cpm).to.equal('0.1133'); expect(innerBid.currency).to.equal('GBP'); }); @@ -238,10 +225,9 @@ describe('currency', function () { fakeCurrencyFileServer.respond(); var bid = { 'cpm': 1, 'currency': 'JPY' }; var innerBid; - var wrappedAddBidResponseFn = addBidResponseDecorator(function(adCodeId, bid) { + addBidResponseHook('elementId', bid, function(adCodeId, bid) { innerBid = bid; }); - wrappedAddBidResponseFn('elementId', bid); expect(innerBid.cpm).to.equal('0.0623'); expect(innerBid.currency).to.equal('CNY'); }); From c380a1994bd111cb7f3825618e5ffb45436f062f Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Wed, 11 Oct 2017 15:23:28 -0600 Subject: [PATCH 2/8] remove unused dependency tiny-queue --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index d03c5408973..c5017e3f149 100644 --- a/package.json +++ b/package.json @@ -105,7 +105,6 @@ "dependencies": { "babel-plugin-transform-object-assign": "^6.22.0", "core-js": "^2.4.1", - "gulp-sourcemaps": "^2.6.0", - "tinyqueue": "^1.2.2" + "gulp-sourcemaps": "^2.6.0" } } From 0e09dd5d48548e867de6a9a40e4aa66d94a39d9e Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Wed, 11 Oct 2017 15:52:02 -0600 Subject: [PATCH 3/8] change PluginFunction to HookedFunction --- src/hook.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hook.js b/src/hook.js index 38c3348f24a..6d1760c4abd 100644 --- a/src/hook.js +++ b/src/hook.js @@ -1,8 +1,8 @@ /** - * @typedef {function} PluginFunction + * @typedef {function} HookedFunction * @property {function(function(), [number])} addHook A method that takes a new function to attach as a hook - * to the PluginFunction + * to the HookedFunction * @property {function(function())} removeHook A method to remove attached hooks */ @@ -18,7 +18,7 @@ export const hooks = {}; * @param {function()} fn The function to make hookable * @param {string} hookName If provided this allows you to register a name for a global hook to have easy access to * the addPlugin and removePlugin methods for that hook (which are usually accessed as methods on the function itself) - * @returns {PluginFunction} A new function that implements the PluginFunction interface + * @returns {HookedFunction} A new function that implements the HookedFunction interface */ export function createHook(type, fn, hookName) { let _hooks = [{fn, priority: 0}]; From 7b97b2ab12464c3108aa828cd8aa01ef911abbb4 Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Wed, 11 Oct 2017 15:52:36 -0600 Subject: [PATCH 4/8] more hook documentation fixes --- src/hook.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hook.js b/src/hook.js index 6d1760c4abd..ad2aefb2025 100644 --- a/src/hook.js +++ b/src/hook.js @@ -17,7 +17,7 @@ export const hooks = {}; * @param {string} type The method for applying all attached hooks when this hooked function is called * @param {function()} fn The function to make hookable * @param {string} hookName If provided this allows you to register a name for a global hook to have easy access to - * the addPlugin and removePlugin methods for that hook (which are usually accessed as methods on the function itself) + * the addHook and removeHook methods for that hook (which are usually accessed as methods on the function itself) * @returns {HookedFunction} A new function that implements the HookedFunction interface */ export function createHook(type, fn, hookName) { From 25502d371f19d8ede47b76f32a9110bdfbfd1cfa Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Wed, 18 Oct 2017 11:42:44 -0600 Subject: [PATCH 5/8] allow context for hooked functions --- src/hook.js | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/hook.js b/src/hook.js index ad2aefb2025..168b30499de 100644 --- a/src/hook.js +++ b/src/hook.js @@ -26,18 +26,18 @@ export function createHook(type, fn, hookName) { let types = { sync: function(...args) { _hooks.forEach(hook => { - hook.fn.apply(null, args); + hook.fn.apply(this, args); }); }, asyncSeries: function(...args) { let curr = 0; - return _hooks[curr].fn.apply(null, args.concat(asyncSeriesNext)); + return _hooks[curr].fn.apply(this, args.concat(asyncSeriesNext)); function asyncSeriesNext(...args) { let hook = _hooks[++curr]; if (typeof hook === 'object' && typeof hook.fn === 'function') { - return hook.fn.apply(null, args.concat(asyncSeriesNext)) + return hook.fn.apply(this, args.concat(asyncSeriesNext)) } } } @@ -69,10 +69,16 @@ export function createHook(type, fn, hookName) { function hookedFn(...args) { if (_hooks.length === 0) { - return fn.apply(null, args); + return fn.apply(this, args); } - return types[type].apply(null, args); + return types[type].apply(this, args); } + hookedFn.withContext = function(context) { + return function hookedFnWithContext(...args) { + hookedFn.apply(context, args); + } + }; + return Object.assign(hookedFn, methods); } From 38d17a4472f21653c90478d02e962f67946caf9c Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Wed, 18 Oct 2017 11:55:06 -0600 Subject: [PATCH 6/8] added tests for context --- test/spec/hook_spec.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/spec/hook_spec.js b/test/spec/hook_spec.js index aae2a93868b..7fa72a8c8e3 100644 --- a/test/spec/hook_spec.js +++ b/test/spec/hook_spec.js @@ -63,6 +63,28 @@ describe('the hook module', () => { ]); }); + it('should allow context to be passed to hooks, but keep bound contexts', () => { + let context; + let fn = function() { + context = this; + }; + + let boundContext = {}; + let calledBoundContext; + let hook = function() { + calledBoundContext = this; + }.bind(boundContext); + + let hookFn = createHook('sync', fn); + hookFn.addHook(hook); + + let newContext = {}; + hookFn.withContext(newContext)(); + + expect(context).to.equal(newContext); + expect(calledBoundContext).to.equal(boundContext); + }); + describe('asyncSeries', () => { it('should call function as normal if no hooks attached', () => { let fn = sandbox.spy(); From 9565c91709cf963c250593761065730f2c11983a Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Wed, 18 Oct 2017 14:06:54 -0600 Subject: [PATCH 7/8] remove withContext, just use bind --- src/hook.js | 6 ------ test/spec/hook_spec.js | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/hook.js b/src/hook.js index 168b30499de..8aeb06357ee 100644 --- a/src/hook.js +++ b/src/hook.js @@ -74,11 +74,5 @@ export function createHook(type, fn, hookName) { return types[type].apply(this, args); } - hookedFn.withContext = function(context) { - return function hookedFnWithContext(...args) { - hookedFn.apply(context, args); - } - }; - return Object.assign(hookedFn, methods); } diff --git a/test/spec/hook_spec.js b/test/spec/hook_spec.js index 7fa72a8c8e3..b9d0774016f 100644 --- a/test/spec/hook_spec.js +++ b/test/spec/hook_spec.js @@ -79,7 +79,7 @@ describe('the hook module', () => { hookFn.addHook(hook); let newContext = {}; - hookFn.withContext(newContext)(); + hookFn.bind(newContext)(); expect(context).to.equal(newContext); expect(calledBoundContext).to.equal(boundContext); From 37b291ebda050bcba5a69ba9762b4aafd7860ee4 Mon Sep 17 00:00:00 2001 From: Rich Snapp Date: Tue, 24 Oct 2017 10:18:34 -0600 Subject: [PATCH 8/8] fix in hooks so asyncSeries keeps proper bound context --- src/hook.js | 8 ++++---- test/spec/hook_spec.js | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/hook.js b/src/hook.js index 8aeb06357ee..5ba1d4b9bbf 100644 --- a/src/hook.js +++ b/src/hook.js @@ -32,14 +32,14 @@ export function createHook(type, fn, hookName) { asyncSeries: function(...args) { let curr = 0; - return _hooks[curr].fn.apply(this, args.concat(asyncSeriesNext)); - - function asyncSeriesNext(...args) { + const asyncSeriesNext = (...args) => { let hook = _hooks[++curr]; if (typeof hook === 'object' && typeof hook.fn === 'function') { return hook.fn.apply(this, args.concat(asyncSeriesNext)) } - } + }; + + return _hooks[curr].fn.apply(this, args.concat(asyncSeriesNext)); } }; diff --git a/test/spec/hook_spec.js b/test/spec/hook_spec.js index b9d0774016f..1fab4ecd1b7 100644 --- a/test/spec/hook_spec.js +++ b/test/spec/hook_spec.js @@ -123,5 +123,29 @@ describe('the hook module', () => { testFn ]); }); + + it('should allow context to be passed to hooks, but keep bound contexts', () => { + let context; + let fn = function() { + context = this; + }; + + let boundContext1 = {}; + let calledBoundContext1; + let hook1 = function(next) { + calledBoundContext1 = this; + next() + }.bind(boundContext1); + + let hookFn = createHook('asyncSeries', fn); + hookFn.addHook(hook1); + + let newContext = {}; + hookFn = hookFn.bind(newContext); + hookFn(); + + expect(context).to.equal(newContext); + expect(calledBoundContext1).to.equal(boundContext1); + }); }); });