From f023cbce3d8d1dfc73c3e40aabd809d4ca2048c6 Mon Sep 17 00:00:00 2001 From: Bradley Momberger Date: Mon, 1 May 2017 13:01:33 -0600 Subject: [PATCH 1/5] Don't attempt to redefine _data or _computed on DefineMaps if they're already defined --- can-define.js | 40 ++++++++++++++++++++++++++-------------- map/map-test.js | 23 +++++++++++++++++++++++ 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/can-define.js b/can-define.js index 9971fad..60d381a 100644 --- a/can-define.js +++ b/can-define.js @@ -72,26 +72,38 @@ module.exports = define = ns.define = function(objPrototype, defines, baseDefine // Places a `_data` on the prototype that when first called replaces itself // with a `_data` object local to the instance. It also defines getters // for any value that has a default value. - replaceWith(objPrototype, "_data", function() { - var map = this; - var data = {}; + if(objPrototype.hasOwnProperty("_data")) { for (var prop in dataInitializers) { - replaceWith(data, prop, dataInitializers[prop].bind(map), true); - } - return data; - }); + replaceWith(objPrototype._data, prop, dataInitializers[prop].bind(objPrototype), true); + } + } else { + replaceWith(objPrototype, "_data", function() { + var map = this; + var data = {}; + for (var prop in dataInitializers) { + replaceWith(data, prop, dataInitializers[prop].bind(map), true); + } + return data; + }); + } // Places a `_computed` on the prototype that when first called replaces itself // with a `_computed` object local to the instance. It also defines getters // that will create the property's compute when read. - replaceWith(objPrototype, "_computed", function() { - var map = this; - var data = Object.create(null); - for (var prop in computedInitializers) { - replaceWith(data, prop, computedInitializers[prop].bind(map)); + if(objPrototype.hasOwnProperty("_computed")) { + for (prop in computedInitializers) { + replaceWith(objPrototype._computed, prop, computedInitializers[prop].bind(objPrototype)); } - return data; - }); + } else { + replaceWith(objPrototype, "_computed", function() { + var map = this; + var data = Object.create(null); + for (var prop in computedInitializers) { + replaceWith(data, prop, computedInitializers[prop].bind(map)); + } + return data; + }); + } // Add necessary event methods to this object. diff --git a/map/map-test.js b/map/map-test.js index 03afe0d..6589e84 100644 --- a/map/map-test.js +++ b/map/map-test.js @@ -1,6 +1,7 @@ "use strict"; var QUnit = require("steal-qunit"); var DefineMap = require("can-define/map/map"); +var define = require("can-define"); var Observation = require("can-observation"); var canTypes = require("can-types"); var each = require("can-util/js/each/each"); @@ -662,4 +663,26 @@ QUnit.test(".value values are overwritten by props in DefineMap construction", f }); equal(foo.bar, "quux", "Value set properly"); +}); + +QUnit.test("Does not attempt to redefine _data if already defined", function() { + var Bar = DefineMap.extend({seal: false}, { + baz: { value : "thud" } + }); + + var baz = new Bar(); + + define(baz, { + quux: { value: "jeek" }, + plonk: { + get: function() { + return "waldo"; + } + } + }, baz._define); + + QUnit.equal(baz.quux, "jeek", "New definitions successful"); + QUnit.equal(baz.plonk, "waldo", "New computed definitions successful"); + QUnit.equal(baz.baz, "thud", "Old definitions still available"); + }); \ No newline at end of file From 9715a881063e948db070f670ee0504a2a08cbaea Mon Sep 17 00:00:00 2001 From: Bradley Momberger Date: Mon, 1 May 2017 13:12:36 -0600 Subject: [PATCH 2/5] Add test to show that sealed objects are still not extensible --- map/map-test.js | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/map/map-test.js b/map/map-test.js index 6589e84..2216ab0 100644 --- a/map/map-test.js +++ b/map/map-test.js @@ -685,4 +685,37 @@ QUnit.test("Does not attempt to redefine _data if already defined", function() { QUnit.equal(baz.plonk, "waldo", "New computed definitions successful"); QUnit.equal(baz.baz, "thud", "Old definitions still available"); -}); \ No newline at end of file +}); + +QUnit.test("redefines still not allowed on sealed objects", function() { + QUnit.expect(6); + var Bar = DefineMap.extend({seal: true}, { + baz: { value : "thud" } + }); + + var baz = new Bar(); + + try { + define(baz, { + quux: { value: "jeek" } + }, baz._define); + } catch(e) { + QUnit.ok(/object is not extensible/.test(e.message), "Sealed object throws on data property defines"); + QUnit.ok(!Object.getOwnPropertyDescriptor(baz, "quux"), "nothing set on object"); + QUnit.ok(!Object.getOwnPropertyDescriptor(baz._data, "quux"), "nothing set on _data"); + } + + try { + define(baz, { + plonk: { + get: function() { + return "waldo"; + } + } + }, baz._define); + } catch(e) { + QUnit.ok(/object is not extensible/.test(e.message), "Sealed object throws on computed property defines"); + QUnit.ok(!Object.getOwnPropertyDescriptor(baz, "plonk"), "nothing set on object"); + QUnit.ok(!Object.getOwnPropertyDescriptor(baz._computed, "plonk"), "nothing set on _computed"); + } +}); From b2c10a858dee508a7b34d8da5738cf76f666917d Mon Sep 17 00:00:00 2001 From: Bradley Momberger Date: Mon, 1 May 2017 13:16:20 -0600 Subject: [PATCH 3/5] fix jshint error --- can-define.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/can-define.js b/can-define.js index 60d381a..d44b6ed 100644 --- a/can-define.js +++ b/can-define.js @@ -107,7 +107,7 @@ module.exports = define = ns.define = function(objPrototype, defines, baseDefine // Add necessary event methods to this object. - for (var prop in eventsProto) { + for (prop in eventsProto) { Object.defineProperty(objPrototype, prop, { enumerable: false, value: eventsProto[prop], From fbf76d171567bd58a29fd3dc9845996b4e22ea83 Mon Sep 17 00:00:00 2001 From: Bradley Momberger Date: Mon, 1 May 2017 13:23:40 -0600 Subject: [PATCH 4/5] fix jshint error --- can-define.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/can-define.js b/can-define.js index d44b6ed..470c120 100644 --- a/can-define.js +++ b/can-define.js @@ -54,7 +54,8 @@ var eachPropertyDescriptor = function(map, cb){ module.exports = define = ns.define = function(objPrototype, defines, baseDefine) { // default property definitions on _data - var dataInitializers = Object.create(baseDefine ? baseDefine.dataInitializers : null), + var prop, + dataInitializers = Object.create(baseDefine ? baseDefine.dataInitializers : null), // computed property definitions on _computed computedInitializers = Object.create(baseDefine ? baseDefine.computedInitializers : null); @@ -73,7 +74,7 @@ module.exports = define = ns.define = function(objPrototype, defines, baseDefine // with a `_data` object local to the instance. It also defines getters // for any value that has a default value. if(objPrototype.hasOwnProperty("_data")) { - for (var prop in dataInitializers) { + for (prop in dataInitializers) { replaceWith(objPrototype._data, prop, dataInitializers[prop].bind(objPrototype), true); } } else { From e3c385840ac8be72da2bed95b5bbc5b7451f3aa3 Mon Sep 17 00:00:00 2001 From: Bradley Momberger Date: Mon, 1 May 2017 13:50:16 -0600 Subject: [PATCH 5/5] Make tests work on all supported platforms, including unrelated IE issue --- list/list-test.js | 4 +++- map/map-test.js | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/list/list-test.js b/list/list-test.js index 643db64..0db8bf6 100644 --- a/list/list-test.js +++ b/list/list-test.js @@ -375,7 +375,9 @@ test('list.map', function() { newExtendedList.testMe(); }, { name: 'TypeError', - message: 'newExtendedList.testMe is not a function' + message: (navigator.userAgent.indexOf("Trident") > -1 ? + "Object doesn't support property or method 'testMe'" : + "newExtendedList.testMe is not a function") }, 'Does not return the same type of list.'); }); diff --git a/map/map-test.js b/map/map-test.js index 2216ab0..6fe42f5 100644 --- a/map/map-test.js +++ b/map/map-test.js @@ -700,7 +700,7 @@ QUnit.test("redefines still not allowed on sealed objects", function() { quux: { value: "jeek" } }, baz._define); } catch(e) { - QUnit.ok(/object is not extensible/.test(e.message), "Sealed object throws on data property defines"); + QUnit.ok(/object is not extensible/i.test(e.message), "Sealed object throws on data property defines"); QUnit.ok(!Object.getOwnPropertyDescriptor(baz, "quux"), "nothing set on object"); QUnit.ok(!Object.getOwnPropertyDescriptor(baz._data, "quux"), "nothing set on _data"); } @@ -714,7 +714,7 @@ QUnit.test("redefines still not allowed on sealed objects", function() { } }, baz._define); } catch(e) { - QUnit.ok(/object is not extensible/.test(e.message), "Sealed object throws on computed property defines"); + QUnit.ok(/object is not extensible/i.test(e.message), "Sealed object throws on computed property defines"); QUnit.ok(!Object.getOwnPropertyDescriptor(baz, "plonk"), "nothing set on object"); QUnit.ok(!Object.getOwnPropertyDescriptor(baz._computed, "plonk"), "nothing set on _computed"); }