From ad9d0f8c7fbb080fa628f302bddb52e6c035c9d2 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Sun, 15 Dec 2013 13:30:22 -0800 Subject: [PATCH 1/3] Make options object prototype-inherited Fixes #2294 --- spec/suites/core/ClassSpec.js | 20 ++++++++++++----- spec/suites/core/UtilSpec.js | 42 ++++++++++++++++++++++++++++++++++- src/core/Class.js | 4 ++-- src/core/Util.js | 14 ++++++++++-- 4 files changed, 70 insertions(+), 10 deletions(-) diff --git a/spec/suites/core/ClassSpec.js b/spec/suites/core/ClassSpec.js index 81673e3f2bd..81efe789f18 100644 --- a/spec/suites/core/ClassSpec.js +++ b/spec/suites/core/ClassSpec.js @@ -99,12 +99,22 @@ describe("Class", function () { }); var a = new KlassWithOptions2(); + expect(a.options.foo1).to.eql(1); + expect(a.options.foo2).to.eql(3); + expect(a.options.foo3).to.eql(4); + }); - expect(a.options).to.eql({ - foo1: 1, - foo2: 3, - foo3: 4 - }); + it("gives new classes a distinct options object", function () { + var K1 = L.Class.extend({options: {}}); + var K2 = K1.extend({}); + expect(K2.prototype.options).not.to.equal(K1.prototype.options); + }); + + it("inherits options prototypally", function () { + var K1 = L.Class.extend({options: {}}); + var K2 = K1.extend({options: {}}); + K1.prototype.options.foo = 'bar'; + expect(K2.prototype.options.foo).to.eql('bar'); }); it("adds constructor hooks correctly", function () { diff --git a/spec/suites/core/UtilSpec.js b/spec/suites/core/UtilSpec.js index d66e6236b99..799016f024b 100644 --- a/spec/suites/core/UtilSpec.js +++ b/spec/suites/core/UtilSpec.js @@ -185,7 +185,47 @@ describe('Util', function () { }); }); - // TODO setOptions + describe('#setOptions', function () { + it('sets specified options on object', function () { + var o = {}; + L.Util.setOptions(o, {foo: 'bar'}); + expect(o.options.foo).to.eql('bar'); + }); + + it('returns options', function () { + var o = {}; + var r = L.Util.setOptions(o, {foo: 'bar'}); + expect(r).to.equal(o.options); + }); + + it('accepts undefined', function () { + var o = {}; + L.Util.setOptions(o, undefined); + expect(o.options).to.eql({}); + }); + + it('creates a distinct options object', function () { + var opts = {}, + o = L.Util.create({options: opts}); + L.Util.setOptions(o, {}); + expect(o.options).not.to.equal(opts); + }); + + it("doesn't create a distinct options object if object already has own options", function () { + var opts = {}, + o = {options: opts}; + L.Util.setOptions(o, {}); + expect(o.options).to.equal(opts); + }); + + it('inherits options prototypally', function () { + var opts = {}, + o = L.Util.create({options: opts}); + L.Util.setOptions(o, {}); + opts.foo = 'bar'; + expect(o.options.foo).to.eql('bar'); + }); + }); describe('#template', function () { it('evaluates templates with a given data object', function () { diff --git a/src/core/Class.js b/src/core/Class.js index e5fc37844ef..69f0f2741bf 100644 --- a/src/core/Class.js +++ b/src/core/Class.js @@ -50,8 +50,8 @@ L.Class.extend = function (props) { } // merge options - if (props.options && proto.options) { - props.options = L.extend({}, proto.options, props.options); + if (proto.options) { + props.options = L.Util.extend(L.Util.create(proto.options), props.options); } // mix given properties into the prototype diff --git a/src/core/Util.js b/src/core/Util.js index 2bf7ed93d25..3effa4ef903 100644 --- a/src/core/Util.js +++ b/src/core/Util.js @@ -18,6 +18,14 @@ L.Util = { return dest; }, + create: Object.create || (function () { + function F() {} + return function (proto) { + F.prototype = proto; + return new F(); + }; + })(), + bind: function (fn, obj) { var slice = Array.prototype.slice; @@ -107,8 +115,10 @@ L.Util = { }, setOptions: function (obj, options) { - obj.options = options ? L.extend({}, obj.options, options) : obj.options || {}; - return obj.options; + if (!obj.hasOwnProperty('options')) { + obj.options = obj.options ? L.Util.create(obj.options) : {}; + } + return L.extend(obj.options, options); }, getParamString: function (obj, existingUrl, uppercase) { From f2b34cd012ad0d5517a7c368efd7c1827a1a2669 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Sun, 15 Dec 2013 14:35:02 -0800 Subject: [PATCH 2/3] Use L.Util.create for class extension --- src/core/Class.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/core/Class.js b/src/core/Class.js index 69f0f2741bf..9c3635cfea1 100644 --- a/src/core/Class.js +++ b/src/core/Class.js @@ -21,11 +21,7 @@ L.Class.extend = function (props) { } }; - // instantiate class without calling constructor - var F = function () {}; - F.prototype = this.prototype; - - var proto = new F(); + var proto = L.Util.create(this.prototype); proto.constructor = NewClass; NewClass.prototype = proto; From e44b8d944e700b29757ed7fee3692ba196c9a750 Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Mon, 16 Dec 2013 18:37:06 -0500 Subject: [PATCH 3/3] use lighter extend in setOptions, simplify extend --- src/core/Util.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/core/Util.js b/src/core/Util.js index 3effa4ef903..69fc86c2da6 100644 --- a/src/core/Util.js +++ b/src/core/Util.js @@ -3,16 +3,14 @@ */ L.Util = { - extend: function (dest) { // (Object[, Object, ...]) -> + extend: function (dest) { var sources = Array.prototype.slice.call(arguments, 1), i, j, len, src; for (j = 0, len = sources.length; j < len; j++) { - src = sources[j] || {}; + src = sources[j]; for (i in src) { - if (src.hasOwnProperty(i)) { - dest[i] = src[i]; - } + dest[i] = src[i]; } } return dest; @@ -118,7 +116,10 @@ L.Util = { if (!obj.hasOwnProperty('options')) { obj.options = obj.options ? L.Util.create(obj.options) : {}; } - return L.extend(obj.options, options); + for (var i in options) { + obj.options[i] = options[i]; + } + return obj.options; }, getParamString: function (obj, existingUrl, uppercase) {