From 0471aa6815439b44c3c01370c6f79f2df345d297 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Fri, 2 Oct 2015 10:24:48 +0200 Subject: [PATCH 01/16] Clarifies the second setup case --- test/test.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.html b/test/test.html index 5aab32a..6f8d11e 100644 --- a/test/test.html +++ b/test/test.html @@ -85,7 +85,7 @@ }); - it("Uses 'this' for the collection, like a regular Backbone member function", function(done) { + it("Can be used with .call instead of as mixin", function(done) { var element1 = document.querySelector('#collection1'); From e707740824139dbbf25d4c0125e47e304ea0fc93 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Tue, 6 Oct 2015 06:52:29 +0200 Subject: [PATCH 02/16] Adds test that can run with command line mocha --- test/backbone-emulated-events-spec.js | 60 +++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 test/backbone-emulated-events-spec.js diff --git a/test/backbone-emulated-events-spec.js b/test/backbone-emulated-events-spec.js new file mode 100644 index 0000000..145c0b2 --- /dev/null +++ b/test/backbone-emulated-events-spec.js @@ -0,0 +1,60 @@ + +var expect = require('chai').expect; +var BackbonePolymerAttach = require('../backbone-polymer-mixin'); +var Backbone = require('yobo').Backbone; + +var PolymerElementMock = function() { + + this.splice = function(path, index, removeCount, items) { + console.log('Polymer got splice', arguments); + }; + + this.notifyPath = function() { + console.log('Polymer got notifyPath', arguments); + }; + +}; + +describe("Array modification through Polymer splices, emulate backbone events", function() { + + describe("Backbone events before backbone-polymer", function() { + + it("add model", function() { + var c = new Backbone.Collection(); + var m = new Backbone.Model({id: 'a1', type: 'testmodel'}) + c.on('add', function(model, collection, options) { + console.log('add', m === model ? '(model)' : model, c === collection ? '(collection)' : collection, JSON.stringify(options)); + }); + c.add(m); + }); + + it("add model at index", function() { + var c = new Backbone.Collection(); + c.add({id: 'a2', type: 'testmodel2'}); + c.add({id: 'a3', type: 'testmodel3'}); + var m = new Backbone.Model({id: 'a1', type: 'testmodel'}) + c.on('add', function(model, collection, options) { + console.log('add', m === model ? '(model)' : model, c === collection ? '(collection)' : collection, JSON.stringify(options)); + }); + var added = c.add(m, {at: 1}); + console.log('add returned', m === added ? '(model)' : model); + }); + + }); + + describe("#add", function() { + + it("Is transparent to backbone add event listener", function() { + var e = new PolymerElementMock(); + var c = new Backbone.Collection(); + var adds = []; + c.on('add', function() { adds.push(arguments); }); + BackbonePolymerAttach.call(c, e, 'edit.units'); + var m = c.add(new Backbone.Model({id: 'add1', type: 'test'})); + expect(m.get('type')).to.equal('test'); + expect(adds).to.have.length(1); + }); + + }); + +}); From ef752783c4fb71786bfb560ef58b89333bd5da8e Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Tue, 6 Oct 2015 06:54:16 +0200 Subject: [PATCH 03/16] Reduces backbone-polymer to only run notifications on changes to existing attributes, no splices support --- backbone-polymer-mixin.js | 36 ------------------------------------ 1 file changed, 36 deletions(-) diff --git a/backbone-polymer-mixin.js b/backbone-polymer-mixin.js index 33e5997..26094b9 100644 --- a/backbone-polymer-mixin.js +++ b/backbone-polymer-mixin.js @@ -14,43 +14,7 @@ var BackbonePolymerAttach = function(element, pathPrefix) { }); }; - var splicesObject = this.models; - - var addNotify = function(model) { - var ix = indexOf(model); - - // https://www.polymer-project.org/1.0/docs/devguide/properties.html#array-observation - var change = {keySplices:[], indexSplices:[]}; - change.keySplices.push({ - index: ix, - removed: [], - removedItems: [], - added: [ix] - }); - change.indexSplices.push({ - index: ix, - addedCount: 1, - removed: [], - object: splicesObject, - type: 'splice', - addedKeys: [ix] - }); - - element.notifyPath(pathPrefix + '.models.splices', change); - - // remove this timeout and the rendered element goes blank - window.setTimeout(function() { - // TODO would it be possible to notify .* here? - for (var attribute in model.attributes) { - // we could reuse code with modelSetup here - var value = model.get(attribute); - element.notifyPath(pathPrefix + '.models.' + ix + '.attributes.' + attribute, value); - } - }, 1); - }; - this.each(modelSetup.bind(this)); - this.on('add', addNotify.bind(this)); this.on('add', modelSetup.bind(this)); }; From 8da661e5ecae5333dffccf42f4c3afc4d5eedecf Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Tue, 6 Oct 2015 07:07:55 +0200 Subject: [PATCH 04/16] Moves modelSetup to our overridden .add --- backbone-polymer-mixin.js | 14 +++++++++++++- test/backbone-emulated-events-spec.js | 16 ++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/backbone-polymer-mixin.js b/backbone-polymer-mixin.js index 26094b9..3a555f1 100644 --- a/backbone-polymer-mixin.js +++ b/backbone-polymer-mixin.js @@ -15,7 +15,19 @@ var BackbonePolymerAttach = function(element, pathPrefix) { }; this.each(modelSetup.bind(this)); - this.on('add', modelSetup.bind(this)); + + var _add = this.add; + this.add = function(models, options) { + if (typeof models.length !== 'undefined') { + throw new Error('backbone-polymer only accepts add of single model'); + } + // we should probably operate on the Collection.set level to be more allowing + if (!this._isModel(models)) { + throw new Error('backbone-polymer requires model instances, not just attributes'); + } + + modelSetup.bind(models); + }; }; if (typeof module !== 'undefined') { diff --git a/test/backbone-emulated-events-spec.js b/test/backbone-emulated-events-spec.js index 145c0b2..902fbd5 100644 --- a/test/backbone-emulated-events-spec.js +++ b/test/backbone-emulated-events-spec.js @@ -44,6 +44,22 @@ describe("Array modification through Polymer splices, emulate backbone events", describe("#add", function() { + it("ATM accepts only a single item", function() { + expect(function() { + var c = new Backbone.Collection(); + BackbonePolymerAttach.call(c, new PolymerElementMock(), 'edit.units'); + c.add([]); + }).to.throw(/single/); + }); + + it("ATM requires that item to be a real model", function() { + expect(function() { + var c = new Backbone.Collection(); + BackbonePolymerAttach.call(c, new PolymerElementMock(), 'edit.units'); + c.add({id: 'add1', type: 'test'}); + }).to.throw(/requires model instance/); + }); + it("Is transparent to backbone add event listener", function() { var e = new PolymerElementMock(); var c = new Backbone.Collection(); From 5ce4ad029e46df92052c2d72ffda5ba477d2bf13 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Tue, 6 Oct 2015 07:34:35 +0200 Subject: [PATCH 05/16] First attempt at running a regular Polymer splice and an emulated Backbone add --- backbone-polymer-mixin.js | 19 +++++++++++++++---- test/backbone-emulated-events-spec.js | 12 ++++++++++-- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/backbone-polymer-mixin.js b/backbone-polymer-mixin.js index 3a555f1..f60eef0 100644 --- a/backbone-polymer-mixin.js +++ b/backbone-polymer-mixin.js @@ -16,17 +16,28 @@ var BackbonePolymerAttach = function(element, pathPrefix) { this.each(modelSetup.bind(this)); + // override Backbone add var _add = this.add; - this.add = function(models, options) { - if (typeof models.length !== 'undefined') { + var addOptions = {add: true, remove: false}; // from backbone source + this.add = function(model, options) { + if (_.isArray(model)) { throw new Error('backbone-polymer only accepts add of single model'); } // we should probably operate on the Collection.set level to be more allowing - if (!this._isModel(models)) { + if (!this._isModel(model)) { throw new Error('backbone-polymer requires model instances, not just attributes'); } - modelSetup.bind(models); + var options = _.extend({merge: false}, options, addOptions); + var ix = options.at || 0; + + element.splice(pathPrefix + '.models', ix, 0, [model]); + if (!options.silent) { + this.trigger('add', model, this, options); + } + + modelSetup.bind(model); + return model; }; }; diff --git a/test/backbone-emulated-events-spec.js b/test/backbone-emulated-events-spec.js index 902fbd5..d7c3e3d 100644 --- a/test/backbone-emulated-events-spec.js +++ b/test/backbone-emulated-events-spec.js @@ -1,12 +1,19 @@ var expect = require('chai').expect; -var BackbonePolymerAttach = require('../backbone-polymer-mixin'); + +// still undecided on how the mixin gets dependencies +_ = require('yobo')._; + var Backbone = require('yobo').Backbone; +var BackbonePolymerAttach = require('../backbone-polymer-mixin'); + var PolymerElementMock = function() { + var spliced = this.spliced = []; + this.splice = function(path, index, removeCount, items) { - console.log('Polymer got splice', arguments); + spliced.push({path: path, index: index, removeCount: removeCount, items: items}); }; this.notifyPath = function() { @@ -69,6 +76,7 @@ describe("Array modification through Polymer splices, emulate backbone events", var m = c.add(new Backbone.Model({id: 'add1', type: 'test'})); expect(m.get('type')).to.equal('test'); expect(adds).to.have.length(1); + expect(e.spliced).to.have.length(1); }); }); From b363b42ee8c9e09b9299e6067b76aba7dca3558b Mon Sep 17 00:00:00 2001 From: Jonathan Andersson Date: Tue, 6 Oct 2015 09:37:19 +0200 Subject: [PATCH 06/16] tests for Options object. --- test/backbone-emulated-events-spec.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/backbone-emulated-events-spec.js b/test/backbone-emulated-events-spec.js index d7c3e3d..734db1f 100644 --- a/test/backbone-emulated-events-spec.js +++ b/test/backbone-emulated-events-spec.js @@ -71,12 +71,35 @@ describe("Array modification through Polymer splices, emulate backbone events", var e = new PolymerElementMock(); var c = new Backbone.Collection(); var adds = []; + c.on('add', function() { adds.push(arguments); }); BackbonePolymerAttach.call(c, e, 'edit.units'); var m = c.add(new Backbone.Model({id: 'add1', type: 'test'})); + expect(m.get('type')).to.equal('test'); expect(adds).to.have.length(1); expect(e.spliced).to.have.length(1); + + //Expects on the options obj. + expect(adds[0][2]).to.be.an('object'); + expect(adds[0][2]).to.have.property('add').and.equal(true); + expect(adds[0][2]).to.have.property('merge').and.equal(false); + expect(adds[0][2]).to.have.property('remove').and.equal(false); + + var m1 = new Backbone.Model({id: 'add2', type: 'test'}); + c.add(m1, {at: 1}); + + //Adding at an index, expects Options to have 'at' + expect(adds[1][2]).to.have.property('add').and.equal(true); + expect(adds[1][2]).to.have.property('at').and.equal(1); + expect(e.spliced).to.have.length(2); + + //Adding again at index 2 + var m2 = new Backbone.Model({id: 'add3', type: 'test'}); + c.add(m2, {at: 2}); + + expect(e.spliced[2]).to.have.property('index').and.equal(2); + }); }); From 99bfa393589a96e44c513c011ff1b624f15537fe Mon Sep 17 00:00:00 2001 From: Jonathan Andersson Date: Wed, 7 Oct 2015 07:24:10 +0200 Subject: [PATCH 07/16] The length property is now set in the add override, and tests for it. --- backbone-polymer-mixin.js | 4 +++- test/backbone-emulated-events-spec.js | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/backbone-polymer-mixin.js b/backbone-polymer-mixin.js index f60eef0..6a236e3 100644 --- a/backbone-polymer-mixin.js +++ b/backbone-polymer-mixin.js @@ -20,6 +20,8 @@ var BackbonePolymerAttach = function(element, pathPrefix) { var _add = this.add; var addOptions = {add: true, remove: false}; // from backbone source this.add = function(model, options) { + this.length = this.length + 1; + console.log('add (model)', model, '(options)', options); if (_.isArray(model)) { throw new Error('backbone-polymer only accepts add of single model'); } @@ -31,7 +33,7 @@ var BackbonePolymerAttach = function(element, pathPrefix) { var options = _.extend({merge: false}, options, addOptions); var ix = options.at || 0; - element.splice(pathPrefix + '.models', ix, 0, [model]); + element.splice(pathPrefix + '.models', ix, 0, model); if (!options.silent) { this.trigger('add', model, this, options); } diff --git a/test/backbone-emulated-events-spec.js b/test/backbone-emulated-events-spec.js index 734db1f..37db30d 100644 --- a/test/backbone-emulated-events-spec.js +++ b/test/backbone-emulated-events-spec.js @@ -85,6 +85,7 @@ describe("Array modification through Polymer splices, emulate backbone events", expect(adds[0][2]).to.have.property('add').and.equal(true); expect(adds[0][2]).to.have.property('merge').and.equal(false); expect(adds[0][2]).to.have.property('remove').and.equal(false); + expect(adds[0][1]).to.have.property('length').and.equal(1); var m1 = new Backbone.Model({id: 'add2', type: 'test'}); c.add(m1, {at: 1}); @@ -92,16 +93,15 @@ describe("Array modification through Polymer splices, emulate backbone events", //Adding at an index, expects Options to have 'at' expect(adds[1][2]).to.have.property('add').and.equal(true); expect(adds[1][2]).to.have.property('at').and.equal(1); + expect(adds[0][1]).to.have.property('length').and.equal(2); expect(e.spliced).to.have.length(2); //Adding again at index 2 var m2 = new Backbone.Model({id: 'add3', type: 'test'}); c.add(m2, {at: 2}); - - expect(e.spliced[2]).to.have.property('index').and.equal(2); + expect(e.spliced[2]).to.have.property('index').and.equal(2); + expect(adds[0][1]).to.have.property('length').and.equal(3); }); - }); - }); From be395a3cd50d090bbb5d9d94c5bf36af4a8381e0 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Wed, 7 Oct 2015 15:42:20 +0200 Subject: [PATCH 08/16] Removes debug logging --- backbone-polymer-mixin.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/backbone-polymer-mixin.js b/backbone-polymer-mixin.js index 6a236e3..faaa5bc 100644 --- a/backbone-polymer-mixin.js +++ b/backbone-polymer-mixin.js @@ -1,6 +1,5 @@ var BackbonePolymerAttach = function(element, pathPrefix) { - console.log('BackbonePolymerAttach', this, element, pathPrefix); var indexOf = this.indexOf.bind(this); @@ -21,7 +20,6 @@ var BackbonePolymerAttach = function(element, pathPrefix) { var addOptions = {add: true, remove: false}; // from backbone source this.add = function(model, options) { this.length = this.length + 1; - console.log('add (model)', model, '(options)', options); if (_.isArray(model)) { throw new Error('backbone-polymer only accepts add of single model'); } From 2dd756b3e2c5f9fcfbb2431bd2291e7e0ff084e7 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Wed, 7 Oct 2015 15:42:46 +0200 Subject: [PATCH 09/16] Clarifies some test and uncovers a serious flaw in backbone compatibility --- test/backbone-emulated-events-spec.js | 45 +++++++++++++++++---------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/test/backbone-emulated-events-spec.js b/test/backbone-emulated-events-spec.js index 37db30d..1d61f7d 100644 --- a/test/backbone-emulated-events-spec.js +++ b/test/backbone-emulated-events-spec.js @@ -17,7 +17,7 @@ var PolymerElementMock = function() { }; this.notifyPath = function() { - console.log('Polymer got notifyPath', arguments); + // our code still does this for attribute changes }; }; @@ -45,13 +45,16 @@ describe("Array modification through Polymer splices, emulate backbone events", }); var added = c.add(m, {at: 1}); console.log('add returned', m === added ? '(model)' : model); + // common Backbone operations + expect(c.get('a1')).to.exist.and.have.property('cid'); + expect(c.at(0)).to.exist.and.have.property('cid'); }); }); - describe("#add", function() { + describe("#add, reduced Backbone functionality", function() { - it("ATM accepts only a single item", function() { + it("Accepts only a single item", function() { expect(function() { var c = new Backbone.Collection(); BackbonePolymerAttach.call(c, new PolymerElementMock(), 'edit.units'); @@ -59,7 +62,7 @@ describe("Array modification through Polymer splices, emulate backbone events", }).to.throw(/single/); }); - it("ATM requires that item to be a real model", function() { + it("Requires item to be a real model", function() { expect(function() { var c = new Backbone.Collection(); BackbonePolymerAttach.call(c, new PolymerElementMock(), 'edit.units'); @@ -70,9 +73,12 @@ describe("Array modification through Polymer splices, emulate backbone events", it("Is transparent to backbone add event listener", function() { var e = new PolymerElementMock(); var c = new Backbone.Collection(); + var adds = []; + c.on('add', function(m, c, o) { + adds.push({model:m, collection:c, options:o}); + }); - c.on('add', function() { adds.push(arguments); }); BackbonePolymerAttach.call(c, e, 'edit.units'); var m = c.add(new Backbone.Model({id: 'add1', type: 'test'})); @@ -81,27 +87,32 @@ describe("Array modification through Polymer splices, emulate backbone events", expect(e.spliced).to.have.length(1); //Expects on the options obj. - expect(adds[0][2]).to.be.an('object'); - expect(adds[0][2]).to.have.property('add').and.equal(true); - expect(adds[0][2]).to.have.property('merge').and.equal(false); - expect(adds[0][2]).to.have.property('remove').and.equal(false); - expect(adds[0][1]).to.have.property('length').and.equal(1); + expect(adds[0].options).to.be.an('object'); + expect(adds[0].options).to.have.property('add').and.equal(true); + expect(adds[0].options).to.have.property('merge').and.equal(false); + expect(adds[0].options).to.have.property('remove').and.equal(false); + expect(adds[0].options).to.not.have.property('at'); + expect(adds[0].collection).to.have.property('length').and.equal(1); var m1 = new Backbone.Model({id: 'add2', type: 'test'}); c.add(m1, {at: 1}); //Adding at an index, expects Options to have 'at' - expect(adds[1][2]).to.have.property('add').and.equal(true); - expect(adds[1][2]).to.have.property('at').and.equal(1); - expect(adds[0][1]).to.have.property('length').and.equal(2); + expect(adds[1].options).to.have.property('add').and.equal(true); + expect(adds[1].options).to.have.property('at').and.equal(1); + expect(adds[0].collection).to.have.property('length').and.equal(2); expect(e.spliced).to.have.length(2); - //Adding again at index 2 + //Adding again at index 1, i.e. insert var m2 = new Backbone.Model({id: 'add3', type: 'test'}); - c.add(m2, {at: 2}); + c.add(m2, {at: 1}); + + expect(e.spliced[2]).to.have.property('index').and.equal(1); + expect(adds[0].collection).to.have.property('length').and.equal(3); - expect(e.spliced[2]).to.have.property('index').and.equal(2); - expect(adds[0][1]).to.have.property('length').and.equal(3); + // common Backbone operations + expect(c.get('add1')).to.exist.and.have.property('cid'); + expect(c.at(1)).to.exist.and.have.property('cid'); }); }); }); From afca1534b302cd51ddd19e3fb2bcac93d060ac93 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Wed, 7 Oct 2015 15:54:29 +0200 Subject: [PATCH 10/16] Uncovers another flaw: that we don't trigger the event on model. --- test/backbone-emulated-events-spec.js | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/test/backbone-emulated-events-spec.js b/test/backbone-emulated-events-spec.js index 1d61f7d..8c844b8 100644 --- a/test/backbone-emulated-events-spec.js +++ b/test/backbone-emulated-events-spec.js @@ -39,10 +39,13 @@ describe("Array modification through Polymer splices, emulate backbone events", var c = new Backbone.Collection(); c.add({id: 'a2', type: 'testmodel2'}); c.add({id: 'a3', type: 'testmodel3'}); - var m = new Backbone.Model({id: 'a1', type: 'testmodel'}) + var m = new Backbone.Model({id: 'a1', type: 'testmodel'}); c.on('add', function(model, collection, options) { console.log('add', m === model ? '(model)' : model, c === collection ? '(collection)' : collection, JSON.stringify(options)); }); + m.on('add', function(model, collection, options) { + console.log('model add', m === model ? '(model)' : model, c === collection ? '(collection)' : collection, JSON.stringify(options)); + }); var added = c.add(m, {at: 1}); console.log('add returned', m === added ? '(model)' : model); // common Backbone operations @@ -88,20 +91,26 @@ describe("Array modification through Polymer splices, emulate backbone events", //Expects on the options obj. expect(adds[0].options).to.be.an('object'); - expect(adds[0].options).to.have.property('add').and.equal(true); - expect(adds[0].options).to.have.property('merge').and.equal(false); - expect(adds[0].options).to.have.property('remove').and.equal(false); + expect(adds[0].options).to.have.property('add').that.equals(true); + expect(adds[0].options).to.have.property('merge').that.equals(false); + expect(adds[0].options).to.have.property('remove').that.equals(false); expect(adds[0].options).to.not.have.property('at'); expect(adds[0].collection).to.have.property('length').and.equal(1); var m1 = new Backbone.Model({id: 'add2', type: 'test'}); + modeladd = []; + m1.on('add', function(m, c, o) { + modeladd.push({model:m, collection:c, options:o}); + }); c.add(m1, {at: 1}); //Adding at an index, expects Options to have 'at' - expect(adds[1].options).to.have.property('add').and.equal(true); - expect(adds[1].options).to.have.property('at').and.equal(1); - expect(adds[0].collection).to.have.property('length').and.equal(2); + expect(adds[1].options).to.have.property('add').that.equals(true); + expect(adds[1].options).to.have.property('at').that.equals(1); + expect(adds[1].collection).to.have.length(2); expect(e.spliced).to.have.length(2); + expect(modeladd).to.have.length(1); + expect(modeladd[0]).to.deep.equal(adds[1]); //Adding again at index 1, i.e. insert var m2 = new Backbone.Model({id: 'add3', type: 'test'}); From 5469a386039a2ab0398e83206d7d227e65d4369b Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Wed, 7 Oct 2015 16:05:34 +0200 Subject: [PATCH 11/16] Further reduces scope of add, so we don't need to re-implement all of collection.set --- backbone-polymer-mixin.js | 4 +++- test/backbone-emulated-events-spec.js | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/backbone-polymer-mixin.js b/backbone-polymer-mixin.js index faaa5bc..76bdbec 100644 --- a/backbone-polymer-mixin.js +++ b/backbone-polymer-mixin.js @@ -23,10 +23,12 @@ var BackbonePolymerAttach = function(element, pathPrefix) { if (_.isArray(model)) { throw new Error('backbone-polymer only accepts add of single model'); } - // we should probably operate on the Collection.set level to be more allowing if (!this._isModel(model)) { throw new Error('backbone-polymer requires model instances, not just attributes'); } + if (this.get(model)) { + throw new Error('model already exists as' + this.get(model).cid); + } var options = _.extend({merge: false}, options, addOptions); var ix = options.at || 0; diff --git a/test/backbone-emulated-events-spec.js b/test/backbone-emulated-events-spec.js index 8c844b8..513f1e7 100644 --- a/test/backbone-emulated-events-spec.js +++ b/test/backbone-emulated-events-spec.js @@ -73,6 +73,16 @@ describe("Array modification through Polymer splices, emulate backbone events", }).to.throw(/requires model instance/); }); + it("Bails out if the model already exists", function() { + expect(function() { + var c = new Backbone.Collection(); + BackbonePolymerAttach.call(c, new PolymerElementMock(), 'edit.units'); + var m = new Backbone.Model({id: 'add1', type: 'test'}); + c.add(m); + c.add(m); + }).to.throw(/Model add blocked because it already exists as cid \d+/); + }); + it("Is transparent to backbone add event listener", function() { var e = new PolymerElementMock(); var c = new Backbone.Collection(); From 2c89fed653b25deeeb976261cb1ed2f89806bd5a Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Wed, 7 Oct 2015 16:10:22 +0200 Subject: [PATCH 12/16] Fixes .get but fails to fix .at --- backbone-polymer-mixin.js | 6 ++++-- test/backbone-emulated-events-spec.js | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/backbone-polymer-mixin.js b/backbone-polymer-mixin.js index 76bdbec..5e8482d 100644 --- a/backbone-polymer-mixin.js +++ b/backbone-polymer-mixin.js @@ -27,15 +27,17 @@ var BackbonePolymerAttach = function(element, pathPrefix) { throw new Error('backbone-polymer requires model instances, not just attributes'); } if (this.get(model)) { - throw new Error('model already exists as' + this.get(model).cid); + throw new Error('backbone-polymer model already exists as cid ' + this.get(model).cid); } var options = _.extend({merge: false}, options, addOptions); var ix = options.at || 0; element.splice(pathPrefix + '.models', ix, 0, model); + this._addReference(model, options); if (!options.silent) { - this.trigger('add', model, this, options); + model.trigger('add', model, this, options); + this.trigger('update', this, options); } modelSetup.bind(model); diff --git a/test/backbone-emulated-events-spec.js b/test/backbone-emulated-events-spec.js index 513f1e7..9d9724f 100644 --- a/test/backbone-emulated-events-spec.js +++ b/test/backbone-emulated-events-spec.js @@ -80,7 +80,7 @@ describe("Array modification through Polymer splices, emulate backbone events", var m = new Backbone.Model({id: 'add1', type: 'test'}); c.add(m); c.add(m); - }).to.throw(/Model add blocked because it already exists as cid \d+/); + }).to.throw(/backbone-polymer model already exists as cid \w+/); }); it("Is transparent to backbone add event listener", function() { From 528552be120aeb85f16b7f503c72727679e6e0bd Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Wed, 7 Oct 2015 16:11:43 +0200 Subject: [PATCH 13/16] Something is pretty badly messed up --- test/backbone-emulated-events-spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/backbone-emulated-events-spec.js b/test/backbone-emulated-events-spec.js index 9d9724f..fab4ef2 100644 --- a/test/backbone-emulated-events-spec.js +++ b/test/backbone-emulated-events-spec.js @@ -131,6 +131,7 @@ describe("Array modification through Polymer splices, emulate backbone events", // common Backbone operations expect(c.get('add1')).to.exist.and.have.property('cid'); + expect(c.models).to.have.length(3); expect(c.at(1)).to.exist.and.have.property('cid'); }); }); From 80440b7112a54c450753b24afecc673f59b4b30a Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Wed, 7 Oct 2015 20:32:33 +0200 Subject: [PATCH 14/16] Does the actual splice from the mock element, so that tests can assert on backbone behavior --- test/backbone-emulated-events-spec.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/backbone-emulated-events-spec.js b/test/backbone-emulated-events-spec.js index fab4ef2..ee08a0f 100644 --- a/test/backbone-emulated-events-spec.js +++ b/test/backbone-emulated-events-spec.js @@ -8,12 +8,15 @@ var Backbone = require('yobo').Backbone; var BackbonePolymerAttach = require('../backbone-polymer-mixin'); -var PolymerElementMock = function() { +var PolymerElementMock = function(testArray) { var spliced = this.spliced = []; this.splice = function(path, index, removeCount, items) { spliced.push({path: path, index: index, removeCount: removeCount, items: items}); + if (typeof testArray !== 'undefined') { + testArray.splice.apply(testArray, [index, removeCount].concat(items)); + } }; this.notifyPath = function() { @@ -84,8 +87,8 @@ describe("Array modification through Polymer splices, emulate backbone events", }); it("Is transparent to backbone add event listener", function() { - var e = new PolymerElementMock(); var c = new Backbone.Collection(); + var e = new PolymerElementMock(c.models); var adds = []; c.on('add', function(m, c, o) { From ee94e65e7ab5c0371e8de195f9d0c84cacab255a Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Wed, 7 Oct 2015 20:12:32 +0200 Subject: [PATCH 15/16] Copies a big chunk of Backbone source, so we can patch --- backbone-polymer-mixin.js | 128 ++++++++++++++++++++++---- test/backbone-emulated-events-spec.js | 12 +-- 2 files changed, 115 insertions(+), 25 deletions(-) diff --git a/backbone-polymer-mixin.js b/backbone-polymer-mixin.js index 5e8482d..d8ae73b 100644 --- a/backbone-polymer-mixin.js +++ b/backbone-polymer-mixin.js @@ -15,34 +15,124 @@ var BackbonePolymerAttach = function(element, pathPrefix) { this.each(modelSetup.bind(this)); - // override Backbone add - var _add = this.add; - var addOptions = {add: true, remove: false}; // from backbone source - this.add = function(model, options) { - this.length = this.length + 1; - if (_.isArray(model)) { - throw new Error('backbone-polymer only accepts add of single model'); + // Default options for `Collection#set`. + var setOptions = {add: true, remove: true, merge: true}; + // Splices `insert` into `array` at index `at`. + var splice = function(array, insert, at) { + at = Math.min(Math.max(at, 0), array.length); + var tail = Array(array.length - at); + var length = insert.length; + for (var i = 0; i < tail.length; i++) tail[i] = array[i + at]; + for (i = 0; i < length; i++) array[i + at] = insert[i]; + for (i = 0; i < tail.length; i++) array[i + length + at] = tail[i]; + }; + // Update a collection by `set`-ing a new list of models, adding new ones, + // removing models that are no longer present, and merging models that + // already exist in the collection, as necessary. Similar to **Model#set**, + // the core operation for updating the data contained by the collection. + this.set = function(models, options) { + if (models == null) return; + + options = _.defaults({}, options, setOptions); + if (options.parse && !this._isModel(models)) models = this.parse(models, options); + + var singular = !_.isArray(models); + models = singular ? [models] : models.slice(); + + var at = options.at; + if (at != null) at = +at; + if (at < 0) at += this.length + 1; + + var set = []; + var toAdd = []; + var toRemove = []; + var modelMap = {}; + + var add = options.add; + var merge = options.merge; + var remove = options.remove; + + var sort = false; + var sortable = this.comparator && (at == null) && options.sort !== false; + var sortAttr = _.isString(this.comparator) ? this.comparator : null; + + // Turn bare objects into model references, and prevent invalid models + // from being added. + var model; + for (var i = 0; i < models.length; i++) { + model = models[i]; + + // If a duplicate is found, prevent it from being added and + // optionally merge it into the existing model. + var existing = this.get(model); + if (existing) { + if (merge && model !== existing) { + var attrs = this._isModel(model) ? model.attributes : model; + if (options.parse) attrs = existing.parse(attrs, options); + existing.set(attrs, options); + if (sortable && !sort) sort = existing.hasChanged(sortAttr); + } + if (!modelMap[existing.cid]) { + modelMap[existing.cid] = true; + set.push(existing); + } + models[i] = existing; + + // If this is a new, valid model, push it to the `toAdd` list. + } else if (add) { + model = models[i] = this._prepareModel(model, options); + if (model) { + toAdd.push(model); + this._addReference(model, options); + modelMap[model.cid] = true; + set.push(model); + } + } } - if (!this._isModel(model)) { - throw new Error('backbone-polymer requires model instances, not just attributes'); + + // Remove stale models. + if (remove) { + for (i = 0; i < this.length; i++) { + model = this.models[i]; + if (!modelMap[model.cid]) toRemove.push(model); + } + if (toRemove.length) this._removeModels(toRemove, options); } - if (this.get(model)) { - throw new Error('backbone-polymer model already exists as cid ' + this.get(model).cid); + + // See if sorting is needed, update `length` and splice in new models. + var orderChanged = false; + var replace = !sortable && add && remove; + if (set.length && replace) { + orderChanged = this.length != set.length || _.some(this.models, function(model, index) { + return model !== set[index]; + }); + this.models.length = 0; + splice(this.models, set, 0); + this.length = this.models.length; + } else if (toAdd.length) { + if (sortable) sort = true; + splice(this.models, toAdd, at == null ? this.length : at); + this.length = this.models.length; } - var options = _.extend({merge: false}, options, addOptions); - var ix = options.at || 0; + // Silently sort the collection if appropriate. + if (sort) this.sort({silent: true}); - element.splice(pathPrefix + '.models', ix, 0, model); - this._addReference(model, options); + // Unless silenced, it's time to fire all appropriate add/sort events. if (!options.silent) { - model.trigger('add', model, this, options); - this.trigger('update', this, options); + for (i = 0; i < toAdd.length; i++) { + if (at != null) options.index = at + i; + model = toAdd[i]; + model.trigger('add', model, this, options); + } + if (sort || orderChanged) this.trigger('sort', this, options); + if (toAdd.length || toRemove.length) this.trigger('update', this, options); } - modelSetup.bind(model); - return model; + // Return the added (or merged) model (or models). + return singular ? models[0] : models; }; + }; if (typeof module !== 'undefined') { diff --git a/test/backbone-emulated-events-spec.js b/test/backbone-emulated-events-spec.js index ee08a0f..9e94303 100644 --- a/test/backbone-emulated-events-spec.js +++ b/test/backbone-emulated-events-spec.js @@ -60,7 +60,7 @@ describe("Array modification through Polymer splices, emulate backbone events", describe("#add, reduced Backbone functionality", function() { - it("Accepts only a single item", function() { + xit("Accepts only a single item", function() { expect(function() { var c = new Backbone.Collection(); BackbonePolymerAttach.call(c, new PolymerElementMock(), 'edit.units'); @@ -68,7 +68,7 @@ describe("Array modification through Polymer splices, emulate backbone events", }).to.throw(/single/); }); - it("Requires item to be a real model", function() { + xit("Requires item to be a real model", function() { expect(function() { var c = new Backbone.Collection(); BackbonePolymerAttach.call(c, new PolymerElementMock(), 'edit.units'); @@ -76,7 +76,7 @@ describe("Array modification through Polymer splices, emulate backbone events", }).to.throw(/requires model instance/); }); - it("Bails out if the model already exists", function() { + xit("Bails out if the model already exists", function() { expect(function() { var c = new Backbone.Collection(); BackbonePolymerAttach.call(c, new PolymerElementMock(), 'edit.units'); @@ -100,7 +100,7 @@ describe("Array modification through Polymer splices, emulate backbone events", expect(m.get('type')).to.equal('test'); expect(adds).to.have.length(1); - expect(e.spliced).to.have.length(1); + //expect(e.spliced).to.have.length(1); //Expects on the options obj. expect(adds[0].options).to.be.an('object'); @@ -121,7 +121,7 @@ describe("Array modification through Polymer splices, emulate backbone events", expect(adds[1].options).to.have.property('add').that.equals(true); expect(adds[1].options).to.have.property('at').that.equals(1); expect(adds[1].collection).to.have.length(2); - expect(e.spliced).to.have.length(2); + //expect(e.spliced).to.have.length(2); expect(modeladd).to.have.length(1); expect(modeladd[0]).to.deep.equal(adds[1]); @@ -129,7 +129,7 @@ describe("Array modification through Polymer splices, emulate backbone events", var m2 = new Backbone.Model({id: 'add3', type: 'test'}); c.add(m2, {at: 1}); - expect(e.spliced[2]).to.have.property('index').and.equal(1); + //expect(e.spliced[2]).to.have.property('index').and.equal(1); expect(adds[0].collection).to.have.property('length').and.equal(3); // common Backbone operations From 8cd03ad6940f9d0dabf08c75a6c781198743bb66 Mon Sep 17 00:00:00 2001 From: Staffan Olsson Date: Wed, 7 Oct 2015 20:33:54 +0200 Subject: [PATCH 16/16] Uses Polymer to do Backbone's splice. Sadly it's a private method that can't be overridden. --- backbone-polymer-mixin.js | 7 ++----- test/backbone-emulated-events-spec.js | 6 +++--- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/backbone-polymer-mixin.js b/backbone-polymer-mixin.js index d8ae73b..87f6f0c 100644 --- a/backbone-polymer-mixin.js +++ b/backbone-polymer-mixin.js @@ -20,11 +20,8 @@ var BackbonePolymerAttach = function(element, pathPrefix) { // Splices `insert` into `array` at index `at`. var splice = function(array, insert, at) { at = Math.min(Math.max(at, 0), array.length); - var tail = Array(array.length - at); - var length = insert.length; - for (var i = 0; i < tail.length; i++) tail[i] = array[i + at]; - for (i = 0; i < length; i++) array[i + at] = insert[i]; - for (i = 0; i < tail.length; i++) array[i + length + at] = tail[i]; + var args = [pathPrefix + '.models', at, 0].concat(insert); + element.splice.apply(this, args); }; // Update a collection by `set`-ing a new list of models, adding new ones, // removing models that are no longer present, and merging models that diff --git a/test/backbone-emulated-events-spec.js b/test/backbone-emulated-events-spec.js index 9e94303..9d964fe 100644 --- a/test/backbone-emulated-events-spec.js +++ b/test/backbone-emulated-events-spec.js @@ -100,7 +100,7 @@ describe("Array modification through Polymer splices, emulate backbone events", expect(m.get('type')).to.equal('test'); expect(adds).to.have.length(1); - //expect(e.spliced).to.have.length(1); + expect(e.spliced).to.have.length(1); //Expects on the options obj. expect(adds[0].options).to.be.an('object'); @@ -121,7 +121,7 @@ describe("Array modification through Polymer splices, emulate backbone events", expect(adds[1].options).to.have.property('add').that.equals(true); expect(adds[1].options).to.have.property('at').that.equals(1); expect(adds[1].collection).to.have.length(2); - //expect(e.spliced).to.have.length(2); + expect(e.spliced).to.have.length(2); expect(modeladd).to.have.length(1); expect(modeladd[0]).to.deep.equal(adds[1]); @@ -129,7 +129,7 @@ describe("Array modification through Polymer splices, emulate backbone events", var m2 = new Backbone.Model({id: 'add3', type: 'test'}); c.add(m2, {at: 1}); - //expect(e.spliced[2]).to.have.property('index').and.equal(1); + expect(e.spliced[2]).to.have.property('index').and.equal(1); expect(adds[0].collection).to.have.property('length').and.equal(3); // common Backbone operations