Merge pull request #189 from canjs/dont-redefine-data

Don't attempt to redefine _data or _computed on DefineMaps if they're already defined

can-define.js

@@ -55,7 +55,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,30 +74,42 @@ 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 = {};
-		for (var prop in dataInitializers) {
-			replaceWith(data, prop, dataInitializers[prop].bind(map), true);
-		}
-		return data;
-	});
+	if(objPrototype.hasOwnProperty("_data")) {
+		for (prop in dataInitializers) {
+			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.
-	for (var prop in eventsProto) {
+	for (prop in eventsProto) {
 		Object.defineProperty(objPrototype, prop, {
 			enumerable: false,
 			value: eventsProto[prop],

list/list-test.js

@@ -375,7 +375,7 @@ test('list.map', function() {
 	try {
 		newExtendedList.testMe();
 	} catch(err) {
-		QUnit.ok(err.message.match(/testMe/));
+		QUnit.ok(err.message.match(/testMe/), 'Does not return the same type of list.');
 	}
 });
 

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,59 @@ 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");
+
+});
+
+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/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");
+	}
+
+	try {
+		define(baz, {
+			plonk: {
+				get: function() {
+					return "waldo";
+				}
+			}
+		}, baz._define);
+	} catch(e) {
+		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");
+	}
+});