Merge branch 'master' into 202-warn-on-ignored-set

can-define.js

@@ -182,6 +182,17 @@ define.property = function(objPrototype, prop, definition, dataInitializers, com
 
 	var type = definition.type;
 
+	//!steal-remove-start
+	if (type && canReflect.isConstructorLike(type)) {
+		dev.warn(
+			"can-define: the definition for " +
+			prop +
+			(objPrototype.constructor.shortName ? " on " + objPrototype.constructor.shortName : "") +
+			" uses a constructor for \"type\". Did you mean \"Type\"?"
+		);
+	}
+	//!steal-remove-end
+
 	// Special case definitions that have only `type: "*"`.
 	if (type && onlyType(definition) && type === define.types["*"]) {
 		Object.defineProperty(objPrototype, prop, {
@@ -226,6 +237,13 @@ define.property = function(objPrototype, prop, definition, dataInitializers, com
 
 	// Determine a function that will provide the initial property value.
 	if ((definition.value !== undefined || definition.Value !== undefined)) {
+		//!steal-remove-start
+
+		// If value is an object or array, give a warning 
+		if (definition.value !== null && typeof definition.value === 'object') {
+			dev.warn("can-define: The value for options is set to an object. This will be shared by all instances of the DefineMap. Use a function that returns the object instead.");
+		}
+		//!steal-remove-end
 		getInitialValue = Observation.ignore(make.get.defaultValue(prop, definition, typeConvert, eventsSetter));
 	}
 
@@ -856,40 +874,40 @@ define.makeSimpleGetterSetter = function(prop){
 };
 
 define.Iterator = function(obj){
-  this.obj = obj;
-  this.definitions = Object.keys(obj._define.definitions);
-  this.instanceDefinitions = obj._instanceDefinitions ?
-    Object.keys(obj._instanceDefinitions) :
-    Object.keys(obj);
-  this.hasGet = typeof obj.get === "function";
+	this.obj = obj;
+	this.definitions = Object.keys(obj._define.definitions);
+	this.instanceDefinitions = obj._instanceDefinitions ?
+		Object.keys(obj._instanceDefinitions) :
+		Object.keys(obj);
+	this.hasGet = typeof obj.get === "function";
 };
 
 define.Iterator.prototype.next = function(){
-  var key;
-  if(this.definitions.length) {
-    key = this.definitions.shift();
-
-    // Getters should not be enumerable
-    var def = this.obj._define.definitions[key];
-    if(def.get) {
-      return this.next();
-    }
-  } else if(this.instanceDefinitions.length) {
-    key = this.instanceDefinitions.shift();
-  } else {
-    return {
-      value: undefined,
-      done: true
-    };
-  }
-
-  return {
-    value: [
-      key,
-      this.hasGet ? this.obj.get(key) : this.obj[key]
-    ],
-    done: false
-  };
+	var key;
+	if(this.definitions.length) {
+		key = this.definitions.shift();
+
+		// Getters should not be enumerable
+		var def = this.obj._define.definitions[key];
+		if(def.get) {
+			return this.next();
+		}
+	} else if(this.instanceDefinitions.length) {
+		key = this.instanceDefinitions.shift();
+	} else {
+		return {
+			value: undefined,
+			done: true
+		};
+	}
+
+	return {
+		value: [
+			key,
+			this.hasGet ? this.obj.get(key) : this.obj[key]
+		],
+		done: false
+	};
 };
 
 isDefineType = function(func){

define-test.js

@@ -1407,4 +1407,43 @@ if(System.env.indexOf("production") < 0) {
 		vm.derivedProp = 'prop is set';
 		canDev.warn = oldwarn;
 	});
+
+	QUnit.test("warn on using a Constructor for small-t type definintions", function() {
+		expect(2);
+		var oldWarn = canDev.warn;
+		canDev.warn = function(mesg) {
+			QUnit.equal(mesg, "can-define: the definition for currency uses a constructor for \"type\". Did you mean \"Type\"?");
+		};
+
+		function Currency() {
+			return this;
+		}
+		Currency.prototype = {
+			symbol: "USD"
+		};
+
+		function VM() {}
+		define(VM.prototype, {
+		    currency: {
+		        type: Currency, // should be `Type: Currency`
+		        value: new Currency({})
+		    }
+		});
+
+		canDev.warn = function(mesg) {
+			QUnit.equal(mesg, "can-define: the definition for currency on VM2 uses a constructor for \"type\". Did you mean \"Type\"?");
+		};
+
+		function VM2() {}
+		VM2.shortName = "VM2";
+		define(VM2.prototype, {
+		    currency: {
+		        type: Currency, // should be `Type: Currency`
+		        value: new Currency({})
+		    }
+		});
+
+		canDev.warn = oldWarn;
+	});
+
 }

map/map-test.js

@@ -8,6 +8,7 @@ var compute = require("can-compute");
 var assign = require("can-util/js/assign/assign");
 var canReflect = require("can-reflect");
 var isPlainObject = require("can-util/js/is-plain-object/is-plain-object");
+var canDev = require("can-util/js/dev/dev");
 
 var sealWorks = (function() {
 	try {
@@ -836,3 +837,40 @@ QUnit.test("non-Object constructor", function() {
 	QUnit.ok(!isPlainObject(new DefineMap()), "instance of DefineMap is not a plain object");
 	QUnit.ok(!isPlainObject(new Constructor()), "instance of extended DefineMap is not a plain object");
 });
+
+if(System.env.indexOf("production") < 0) {
+	QUnit.test('Setting a value with an object type generates a warning (#148)', function() {
+		QUnit.expect(2);
+		var oldwarn = canDev.warn;
+		canDev.warn = function(mesg) {
+			QUnit.equal(mesg, "can-define: The value for options is set to an object. This will be shared by all instances of the DefineMap. Use a function that returns the object instead.");
+		};
+		//should issue a warning
+		DefineMap.extend({
+			options: {
+				value: {}
+			}
+		});
+		//should issue a warning
+		DefineMap.extend({
+			options: {
+				value: []
+			}
+		});
+
+		//should not issue a warning
+		DefineMap.extend({
+			options: {
+				value: function(){}
+			}
+		});
+
+		//should not issue a warning
+		DefineMap.extend({
+			options: {
+				value: 2
+			}
+		});
+		canDev.warn = oldwarn;
+	});
+}

package.json

@@ -44,7 +44,7 @@
   },
   "devDependencies": {
     "jshint": "^2.9.1",
-    "serve": "^5.1.4",
+    "serve": "^6.0.1",
     "steal": "^1.0.7",
     "steal-qunit": "^1.0.0",
     "steal-tools": "^1.4.0",