Skip to content

Commit

Permalink
Merge pull request #250 from canjs/245-deprecate-set
Browse files Browse the repository at this point in the history 
Deprecate passing an object to .set
  • Loading branch information
@bmomberger-bitovi
bmomberger-bitovi committed Oct 3, 2017
2 parents dd48c52 + f38e699 commit a2ab97f
Show file tree
Hide file tree
Showing 6 changed files with 560 additions and 18 deletions.
2 changes: 1 addition & 1 deletion list/docs/define-list.md
View file
Expand Up @@ -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:
Expand Down
120 changes: 119 additions & 1 deletion list/list-test.js
View file
Expand Up @@ -504,7 +504,7 @@ QUnit.test("setting expandos on a DefineList", function() {
});

var dl = new DL();
dl.set({ count: 5, skip: 2 });
dl.assign({ count: 5, skip: 2 });

QUnit.equal(dl.get("count"), 5, "read with .get defined"); //-> 5
QUnit.equal(dl.count, 5, "read with . defined");
Expand Down Expand Up @@ -1137,6 +1137,54 @@ QUnit.test("can-reflect getKeyDependencies", function() {

});

QUnit.test("assign property", function() {
var list = new DefineList(["A","B"]);
list.assign({count: 0, skip: 2, arr: ['1', '2', '3']});
equal(list.get('count'), 0, 'Count set properly');

list.assign({count: 1000, arr: ['first']});

deepEqual(list.get('arr'), new DefineList(['first']), 'Array is set properly');
equal(list.get('count'), 1000, 'Count set properly');
equal(list.get('skip'), 2, 'Skip is unchanged');
});


QUnit.test("update property", function() {
var list = new DefineList(["A","B"]);
list.update({count: 0, skip: 2});
equal(list.get('count'), 0, 'Count set properly');

list.update({count: 1000});

equal(list.get('count'), 1000, 'Count set properly');
equal(list.get('skip'), undefined, 'Skip is changed');
});

QUnit.test("assignDeep property", function() {
var list = new DefineList(["A","B"]);
list.assignDeep({count: 0, skip: 2, foo: { bar: 'zed', tar: 'yap' }});

equal(list.get('count'), 0, 'Count set properly');

list.assignDeep({count: 1000, foo: {bar: 'updated'}});
equal(list.get('count'), 1000, 'Count set properly');
equal(list.get('skip'), 2, 'Skip is unchanged');
propEqual(list.get('foo'), { bar: 'updated', tar: 'yap' }, 'Foo was updated properly');
});

QUnit.test("updateDeep property", function() {
var list = new DefineList(["A","B"]);
list.updateDeep({count: 0, skip: 2, foo: { bar: 'zed', tar: 'yap' }});
equal(list.get('count'), 0, 'Count set properly');

list.updateDeep({count: 1000});

equal(list.get('count'), 1000, 'Count set properly');
equal(list.get('skip'), undefined, 'Skip is set to undefined');
propEqual(list.get('foo'), undefined, 'Foo is set to undefined');
});

QUnit.test("registered symbols", function() {
var a = new DefineMap({ "a": "a" });

Expand All @@ -1155,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");
});
156 changes: 150 additions & 6 deletions list/list.js
View file
Expand Up @@ -5,7 +5,7 @@ var canEvent = require("can-event");
var canBatch = require("can-event/batch/batch");
var Observation = require("can-observation");
var canLog = require("can-util/js/log/log");

var canDev = require("can-util/js/dev/dev");
var defineHelpers = require("../define-helpers/define-helpers");

var assign = require("can-util/js/assign/assign");
Expand Down Expand Up @@ -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", {
Expand All @@ -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)));
}
Expand Down Expand Up @@ -177,7 +185,9 @@ var DefineList = Construct.extend("DefineList",
* @function can-define/list/list.prototype.set set
* @parent can-define/list/list.prototype
*
* Sets an item or property or items or properties on a list.
* @deprecated {3.10.1} Using .set with {Object} `props` has been deprecated in favour of `assign` and `update`
*
* @description Sets an item or property or items or properties on a list.
*
* @signature `list.set(prop, value)`
*
Expand Down Expand Up @@ -261,6 +271,11 @@ var DefineList = Construct.extend("DefineList",
}
// otherwise we are setting multiple
else {
//!steal-remove-start
canDev.warn('can-define/list/list.prototype.set is deprecated; please use can-define/list/list.prototype.assign or can-define/list/list.prototype.update instead');
//!steal-remove-end

//we are deprecating this in #245
if (canReflect.isListLike(prop)) {
if (value) {
this.replace(prop);
Expand All @@ -273,6 +288,121 @@ var DefineList = Construct.extend("DefineList",
}
return this;
},
/**
* @function can-define/list/list.prototype.assign assign
* @parent can-define/list/list.prototype
*
* Sets items or properties on a list.
*
* @signature `list.assign(newProps)`
*
* Assigns the properties on the list with `newProps`. Properties not present in `newProps` will be left unchanged.
*
* ```js
* var list = new DefineList(["A","B"]);
* list.assign({count: 1000, skip: 2});
* list.get("count") //-> 1000
* ```
* @param {Array} newProps Properties that need to be assigned to the list instance
* @return {can-define/list/list} The list instance.
*/
assign: function(prop) {
if (canReflect.isListLike(prop)) {
canReflect.assignList(this, prop);
} else {
canReflect.assignMap(this, prop);
}
return this;
},
/**
* @function can-define/list/list.prototype.update update
* @parent can-define/list/list.prototype
*
* Sets an item or property or items or properties on a list.
*
* @signature `list.update(newProps)`
*
* Updates the properties on the list with `newProps`. Properties not in `newProps` will be set to `undefined`.
*
* ```js
* var list = new DefineList(["A","B"]);
* list.assign({count: 0, skip: 2});
* list.update({count: 1000});
* list.get("count") //-> 1000
* list.get("skip") //-> undefined
* ```
* @param {Array} newProps Properties that need to be updated to the list instance
* @return {can-define/list/list} The list instance.
*/
update: function(prop) {
if (canReflect.isListLike(prop)) {
canReflect.updateList(this, prop);
} else {
canReflect.updateMap(this, prop);
}
return this;
},
/**
* @function can-define/list/list.prototype.assignDeep assignDeep
* @parent can-define/list/list.prototype
*
* Sets an item or property or items or properties on a list.
*
* @signature `list.assignDeep(newProps)`
*
* Updates the properties on the list with `newProps`. Properties not in `newProps` will be left unchanged.
*
* ```js
* var list = new DefineList(["A","B"]);
* list.assign({count: 1, skip: 2});
* list.get("count") //-> 1
*
* list.assignDeep({count: 1000});
* list.get("count") //-> 1000
* list.get("skip") //-> 2
* ```
*
* @param {Array} newProps Properties that need to be assigned to the list instance
* @return {can-define/list/list} The list instance.
*/
assignDeep: function(prop) {
if (canReflect.isListLike(prop)) {
canReflect.assignDeepList(this, prop);
} else {
canReflect.assignDeepMap(this, prop);
}
return this;
},
/**
* @function can-define/list/list.prototype.updateDeep updateDeep
* @parent can-define/list/list.prototype
*
* Sets an item or property or items or properties on a list.
*
* @signature `list.updateDeep(newProps)`
*
* Recursively updates the properties on the list with `newProps`. Properties not in `newProps` will be set to `undefined`.
*
* ```js
* var list = new DefineList(["A","B"]);
* list.assign({count: 0, skip: 2, foo: {bar: 'zed', a: 'b'}});
* list.updateDeep({foo: {bar: 'yay'}});
*
* list.get("count") //-> undefined
* list.get("skip") //-> undefined
* list.get("foo") // -> {bar: 'yay', a: undefined}
* ```
* @param {Array} newProps Properties that need to be updated on the list instance
* @return {can-define/list/list} The list instance.
*/
updateDeep: function(prop) {
if (canReflect.isListLike(prop)) {
canReflect.updateDeepList(this, prop);
} else {
canReflect.updateDeepMap(this, prop);
}
return this;
},
_items: function() {
var arr = [];
this._each(function(item) {
Expand Down Expand Up @@ -1323,7 +1453,11 @@ Object.defineProperty(DefineList.prototype, "length", {
return;
}

if (newVal === this._length) {
// Don't set _length if:
// - null or undefined
// - a string that doesn't convert to number
// - already the length being set
if (newVal == null || isNaN(+newVal) || newVal === this._length) {
return;
}

Expand Down Expand Up @@ -1413,8 +1547,15 @@ canReflect.assignSymbols(DefineList.prototype,{
},

"can.deleteKeyValue": function(prop) {
// convert string key to number index if key can be an integer:
// isNaN if prop isn't a numeric representation
// (prop % 1) if numeric representation is a float
// In both of the above cases, leave as string.
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);
}
Expand Down Expand Up @@ -1460,12 +1601,15 @@ canReflect.assignSymbols(DefineList.prototype,{

canReflect.setKeyValue(DefineList.prototype, canSymbol.iterator, function() {
var index = -1;
if(typeof this._length !== "number") {
this._length = 0;
}
return {
next: function() {
index++;
return {
value: this[index],
done: index >= this.length
done: index >= this._length
};
}.bind(this)
};
Expand Down

0 comments on commit a2ab97f

Please sign in to comment.