Fix updates by guarding length

list/docs/define-list.md

@@ -70,7 +70,7 @@ var todos = new TodoList([{complete: true}, {complete:false}]);
 todos.completed.length //-> 1
 ```
 
-Finally, DefineMap instances are observable, so you can use the [can-event]
+Finally, DefineList instances are observable, so you can use the [can-event]
 methods to listen to its [can-define/list/list/AddEvent],
 [can-define/list/list/LengthEvent], [can-define/list/list/RemoveEvent],
 and [can-define/list/list/PropertyNameEvent] events:

list/list-test.js

@@ -1203,3 +1203,73 @@ QUnit.test("registered symbols", function() {
 	a[canSymbol.for("can.offKeyValue")]("a", handler);
 	a.a = "d"; // doesn't trigger handler
 });
+
+QUnit.test("cannot remove length", function() {
+	var list = new DefineList(["a"]);
+
+	list.set("length", undefined);
+
+	QUnit.equal(list.length, 1, "list length is unchanged");
+
+});
+
+QUnit.test("cannot set length to a non-number", function() {
+	var list = new DefineList(["a"]);
+
+	list.set("length", null);
+	QUnit.equal(list.length, 1, "list length is unchanged");
+
+	list.set("length", "foo");
+	QUnit.equal(list.length, 1, "list length is unchanged");
+
+	list.set("length", {});
+	QUnit.equal(list.length, 1, "list length is unchanged");
+});
+
+QUnit.test("_length is not enumerable", function() {
+	QUnit.ok(!Object.getOwnPropertyDescriptor(new DefineList(), "_length").enumerable, "_length is not enumerable");
+});
+
+QUnit.test("update with no indexed items sets length to 0", function() {
+	var list = new DefineList(["a"]);
+	QUnit.equal(list.length, 1, "list length is correct before update");
+
+	list.update({ foo: "bar" });
+
+	QUnit.equal(list.length, 0, "list length is correct after update");
+});
+
+["length", "_length"].forEach(function(prop) {
+	QUnit.test("setting " + prop + " does not overwrite definition", function () {
+		var list = new DefineList();
+
+		list.get(prop);
+		var proto = list, listDef, listDef2;
+		while(!listDef && proto) {
+			listDef = Object.getOwnPropertyDescriptor(proto, prop);
+			proto = Object.getPrototypeOf(proto);
+		}
+
+		list.set(prop, 1);
+
+		proto = list;
+		while(!listDef2 && proto) {
+			listDef2 = Object.getOwnPropertyDescriptor(proto, prop);
+			proto = Object.getPrototypeOf(proto);
+		}
+		delete listDef2.value;
+		delete listDef.value;
+
+		QUnit.deepEqual(listDef2, listDef, "descriptor hasn't changed");
+	});
+});
+
+QUnit.test("iterator can recover from bad _length", function() {
+	var list = new DefineList(["a"]);
+	list.set("_length", null);
+	QUnit.equal(list._length, null, "Bad value for _length");
+
+	var iterator = list[canSymbol.iterator]();
+	var iteration = iterator.next();
+	QUnit.ok(iteration.done, "Didn't fail");
+});

list/list.js

@@ -66,7 +66,10 @@ var DefineList = Construct.extend("DefineList",
 				Object.defineProperty(this, "_define", {
 					enumerable: false,
 					value: {
-						definitions: {}
+						definitions: {
+							length: { type: "number" },
+							_length: { type: "number" }
+						}
 					}
 				});
 				Object.defineProperty(this, "_data", {
@@ -75,7 +78,12 @@ var DefineList = Construct.extend("DefineList",
 				});
 			}
 			define.setup.call(this, {}, false);
-			this._length = 0;
+			Object.defineProperty(this, "_length", {
+				enumerable: false,
+				configurable: true,
+				writable: true,
+				value: 0
+			});
 			if (items) {
 				this.splice.apply(this, [ 0, 0 ].concat(canReflect.toArray(items)));
 			}
@@ -1445,7 +1453,7 @@ Object.defineProperty(DefineList.prototype, "length", {
 			return;
 		}
 
-		if (newVal === this._length) {
+		if (newVal == null || isNaN(+newVal) || newVal === this._length) {
 			return;
 		}
 
@@ -1535,8 +1543,11 @@ canReflect.assignSymbols(DefineList.prototype,{
 	},
 
 	"can.deleteKeyValue": function(prop) {
+		prop = isNaN(+prop) || (prop % 1) ? prop : +prop;
 		if(typeof prop === "number") {
 			this.splice(prop, 1);
+		} else if(prop === "length" || prop === "_length") {
+			return; // length must not be deleted
 		} else {
 			this.set(prop, undefined);
 		}
@@ -1582,12 +1593,15 @@ canReflect.assignSymbols(DefineList.prototype,{
 
 canReflect.setKeyValue(DefineList.prototype, canSymbol.iterator, function() {
 	var index = -1;
+	if(this._length == null) {
+		this.length = 0;
+	}
 	return {
 		next: function() {
 			index++;
 			return {
 				value: this[index],
-				done: index >= this.length
+				done: index >= this._length
 			};
 		}.bind(this)
 	};