From 3ee556d4b9371b1a8d058a3c5328716acdeac1fb Mon Sep 17 00:00:00 2001 From: Alex Nau Date: Fri, 7 Feb 2014 17:19:33 -0800 Subject: [PATCH 1/2] Changed the callback listeners to take the abstract model interface as the "this" value instead of just passing in the model. This will allow users to more safely edit the abstract model in callback listeners. --- src/helper/helper.js | 4 +-- test/integration_test.js | 12 ++++----- test/processStates_test.js | 53 +++++++++++++++++++------------------- 3 files changed, 34 insertions(+), 35 deletions(-) diff --git a/src/helper/helper.js b/src/helper/helper.js index a84fd60d..5770006e 100755 --- a/src/helper/helper.js +++ b/src/helper/helper.js @@ -70,7 +70,7 @@ helper.DataLayerHelper = function(dataLayer, opt_listener, opt_listenToPast) { /** * The listener to notify of changes to the dataLayer. - * @type {function(!Object, !Object)} + * @type {function(!Object)} * @private */ this.listener_ = opt_listener || function() {}; @@ -199,7 +199,7 @@ helper.DataLayerHelper.prototype.processStates_ = } if (!opt_skipListener) { this.executingListener_ = true; - this.listener_(this.model_, update); + this.listener_.call(this.model_, update); this.executingListener_ = false; } } diff --git a/test/integration_test.js b/test/integration_test.js index 37ea8c97..f990786c 100755 --- a/test/integration_test.js +++ b/test/integration_test.js @@ -57,27 +57,27 @@ test('Basic Operations', function() { ok(helper.get('two') === undefined); dataLayer.push({one: 1, two: 2}); - assertCallback([{one: 1, two: 2}, {one: 1, two: 2}]); + assertCallback([{one: 1, two: 2}]); ok(helper.get('one') === 1); ok(helper.get('two') === 2); dataLayer.push({two: 3}); - assertCallback([{one: 1, two: 3}, {two: 3}]); + assertCallback([{two: 3}]); ok(helper.get('one') === 1); ok(helper.get('two') === 3); dataLayer.push({two: 2}); - assertCallback([{one: 1, two: 2}, {two: 2}]); + assertCallback([{two: 2}]); ok(helper.get('one') === 1); ok(helper.get('two') === 2); dataLayer.push({one: {three: 3}}); - assertCallback([{one: {three: 3}, two: 2}, {one: {three: 3}}]); + assertCallback([{one: {three: 3}}]); deepEqual(helper.get('one'), {three: 3}); ok(helper.get('two') === 2); dataLayer.push({one: {four: 4}}); - assertCallback([{one: {three: 3, four: 4}, two: 2}, {one: {four: 4}}]); + assertCallback([{one: {four: 4}}]); deepEqual(helper.get('one'), {three: 3, four: 4}); ok(helper.get('one.four') === 4); ok(helper.get('two') === 2); @@ -92,7 +92,7 @@ test('Basic Operations', function() { ok(helper.get('two') === 2); dataLayer.push({five: 5}); - assertCallback([{one: {three: 3, four: 4}, two: 2, five: 5}, {five: 5}]); + assertCallback([{five: 5}]); equal(dataLayer.length, 2); ok(helper.get('one.four') === 4); ok(helper.get('five') === 5); diff --git a/test/processStates_test.js b/test/processStates_test.js index abeabbcd..bcdd111e 100755 --- a/test/processStates_test.js +++ b/test/processStates_test.js @@ -36,8 +36,9 @@ function assertProcessStates(states, expectedModel, expectedListenerCalls) { this.unprocessed_ = []; this.executingListener_ = false; this.listenerCalls_ = []; + var that = this; this.listener_ = function() { - this.listenerCalls_.push( + that.listenerCalls_.push( jQuery.extend(true, [], [].slice.call(arguments, 0))); }; this.abstractModelInterface_ = helper.buildAbstractModelInterface_(this); @@ -57,11 +58,9 @@ function assertProcessStates(states, expectedModel, expectedListenerCalls) { test('processStates', function() { assertProcessStates([], {}, []); - assertProcessStates([{a: 1}], {a: 1}, [[{a: 1}, {a: 1}]]); - assertProcessStates([{a: 1}, {a: 2}], {a: 2}, - [[{a: 1}, {a: 1}], [{a: 2}, {a: 2}]]); - assertProcessStates([{'a.b': 1}], {a: {b: 1}}, - [[{a: {b: 1}}, {'a.b': 1}]]); + assertProcessStates([{a: 1}], {a: 1}, [[{a: 1}]]); + assertProcessStates([{a: 1}, {a: 2}], {a: 2}, [[{a: 1}], [{a: 2}]]); + assertProcessStates([{'a.b': 1}], {a: {b: 1}}, [[{'a.b': 1}]]); }); test('processStates_customMethods', function() { @@ -70,19 +69,19 @@ test('processStates_customMethods', function() { assertProcessStates( [{a: 0}, customMethod], {a: 1}, - [[{a: 0}, {a: 0}], [{a: 1}, customMethod]]); + [[{a: 0}], [customMethod]]); customMethod = function() { this.set('b', 'one'); }; assertProcessStates( [customMethod], {b: 'one'}, - [[{b: 'one'}, customMethod]]); + [[customMethod]]); customMethod = function() { this.set('c.d', [3]); }; assertProcessStates( [customMethod], {c: {d: [3]}}, - [[{c: {d: [3]}}, customMethod]]); + [[customMethod]]); var customMethod = function() { var a = this.get('a'); @@ -92,8 +91,8 @@ test('processStates_customMethods', function() { [{a: 1, b: 2}, customMethod], {a: 1, b: 2}, [ - [{a: 1, b: 2}, {a: 1, b: 2}], - [{a: 1, b: 2}, customMethod] + [{a: 1, b: 2}], + [customMethod] ]); customMethod = function() { @@ -104,8 +103,8 @@ test('processStates_customMethods', function() { [{a: {b: 2}}, customMethod], {a: {b: 2}}, [ - [{a: {b: 2}}, {a: {b: 2}}], - [{a: {b: 2}}, customMethod] + [{a: {b: 2}}], + [customMethod] ]); customMethod = function() { @@ -116,8 +115,8 @@ test('processStates_customMethods', function() { [{a: {b: {c: [3]}}}, customMethod], {a: {b: {c: [3]}}}, [ - [{a: {b: {c: [3]}}}, {a: {b: {c: [3]}}}], - [{a: {b: {c: [3]}}}, customMethod] + [{a: {b: {c: [3]}}}], + [customMethod] ]); var products = [ @@ -133,8 +132,8 @@ test('processStates_customMethods', function() { [{'products': products}, customMethod], {'products': products, numProducts: 3}, [ - [{'products': products}, {'products': products}], - [{'products': products, numProducts: 3}, customMethod] + [{'products': products}], + [customMethod] ]); var expectedProducts = [ @@ -152,8 +151,8 @@ test('processStates_customMethods', function() { [{'products': products}, customMethod], {'products': expectedProducts}, [ - [{'products': products}, {'products': products}], - [{'products': expectedProducts}, customMethod] + [{'products': products}], + [customMethod] ]); customMethod = function() { @@ -168,8 +167,8 @@ test('processStates_customMethods', function() { [{'products': products}, customMethod], {'products': products, orderTotal: 60}, [ - [{'products': products}, {'products': products}], - [{'products': products, orderTotal: 60}, customMethod] + [{'products': products}], + [customMethod] ]); // Test the behavior of processing custom methods where the methods throw @@ -177,7 +176,7 @@ test('processStates_customMethods', function() { var errorFunction = function() { throw 'Scary Error'; }; - assertProcessStates([errorFunction], {} , [[{}, errorFunction]]); + assertProcessStates([errorFunction], {} , [[errorFunction]]); errorFunction = function() { this.set('a', 1); @@ -187,7 +186,7 @@ test('processStates_customMethods', function() { assertProcessStates( [errorFunction], {a: 1}, - [[{a: 1}, errorFunction]]); + [[errorFunction]]); errorFunction = function() { this.set('a', 3); @@ -198,8 +197,8 @@ test('processStates_customMethods', function() { [{a: 1, b: 2}, errorFunction], {a: 3, b: 2}, [ - [{a: 1, b: 2}, {a: 1, b: 2}], - [{a: 3, b: 2}, errorFunction] + [{a: 1, b: 2}], + [errorFunction] ]); // Custom methods throwing errors shouldn't affect any messages further down @@ -212,7 +211,7 @@ test('processStates_customMethods', function() { [errorFunction, {a: 2}], {a: 2}, [ - [{a: 1}, errorFunction], - [{a: 2}, {a: 2}] + [errorFunction], + [{a: 2}] ]); }); From cff32458e13348de08ecdcf684ef7a0faea6a579 Mon Sep 17 00:00:00 2001 From: Alex Nau Date: Fri, 7 Feb 2014 17:22:25 -0800 Subject: [PATCH 2/2] Fixed a bug with the listner_, I was passing it the model instead of the abstractModelInterface_! --- src/helper/helper.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/helper/helper.js b/src/helper/helper.js index 5770006e..097c3742 100755 --- a/src/helper/helper.js +++ b/src/helper/helper.js @@ -199,7 +199,7 @@ helper.DataLayerHelper.prototype.processStates_ = } if (!opt_skipListener) { this.executingListener_ = true; - this.listener_.call(this.model_, update); + this.listener_.call(this.abstractModelInterface_, update); this.executingListener_ = false; } }