Add second test case for value assigned to a constructor

canjs · Aug 31, 2017 · cce5b90 · cce5b90
1 parent 3bf43ab
commit cce5b90
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 7 deletions.
diff --git a/can-define.js b/can-define.js
@@ -237,13 +237,18 @@ 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, array, or constructor give a warning
-		if (definition.value !== null && (typeof definition.value === 'object' || canReflect.isConstructorLike(definition.value || {}))) {
-			dev.warn("can-define: The value for " + prop + " is set to an object or constructor. Use a function or primitive instead.");
+		// 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 " + prop + " is set to an object. This will be shared by all instances of the DefineMap. Use a function that returns the object instead.");
+		}
+		// If value is a constructor, give a warning
+		if (definition.value && canReflect.isConstructorLike(definition.value)) {
+			dev.warn("can-define: The \"value\" for " + prop + " is set to a constructor. Did you mean \"Value\" instead?");
 		}
 		//!steal-remove-end
+
 		getInitialValue = Observation.ignore(make.get.defaultValue(prop, definition, typeConvert, eventsSetter));
 	}
 

diff --git a/map/map-test.js b/map/map-test.js
@@ -839,11 +839,11 @@ QUnit.test("non-Object constructor", function() {
 });
 
 if(System.env.indexOf("production") < 0) {
-	QUnit.test('Setting a value with an object or constructor type generates a warning (#148)', function() {
+	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 or constructor. Use a function or primitive instead.");
+			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({
@@ -854,7 +854,7 @@ if(System.env.indexOf("production") < 0) {
 		//should issue a warning
 		DefineMap.extend({
 			options: {
-				value: DefineMap
+				value: []
 			}
 		});
 
@@ -873,4 +873,21 @@ if(System.env.indexOf("production") < 0) {
 		});
 		canDev.warn = oldwarn;
 	});
+	QUnit.test('Setting a value to a constructor type generates a warning', function() {
+		QUnit.expect(1);
+		var oldwarn = canDev.warn;
+		canDev.warn = function(mesg) {
+			QUnit.equal(mesg, "can-define: The \"value\" for options is set to a constructor. Did you mean \"Value\" instead?");
+		};
+
+		//should issue a warning
+		DefineMap.extend({
+			options: {
+				value: DefineMap
+			}
+		});
+
+		canDev.warn = oldwarn;
+	});
+
 }