diff --git a/can-define.js b/can-define.js index 9098a53..eeb3f00 100644 --- a/can-define.js +++ b/can-define.js @@ -237,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)); } @@ -855,40 +862,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){ diff --git a/map/map-test.js b/map/map-test.js index b106333..a01d2cf 100644 --- a/map/map-test.js +++ b/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; + }); +}