With the store.js.es6, one can store records on the server. Providing an id property for records upon creation makes it easier managing them in the long run (e.g. knowing when two records are the same on the client side, etc.).
I wonder what the correct data type of the id property is. Generally, it looks like a number and is used like that in the documentation I’ve seen (e.g. the guides on plugin development).
If it is a number, is an ID with the value 0 valid? I feel like it should, but writing a test for that quickly reveals a few culprits:
createRecord(type, attrs) {
attrs = attrs || {};
return !!attrs.id ? this._hydrate(type, attrs) : this._build(type, attrs);
}
Here, !!attrs.id will return false for attrs.id === 0; thus, attempting to build a new record although one might already exist. Should this code rather check for a) attrs.hasOwnProperty('id') and b) whether attrs.id is of type number?
In general, I think the store code should document what data type it expects for the record IDs. I’d like to write documentation for this class (e.g. method doc strings with JSDoc annotations for the method signature). Are pull requests for that welcome?
Additional note: The following assertions will fail:
QUnit.test("create record with id 0", assert => {
const store = createStore();
const widget = store.createRecord("widget", { id: 0 });
// Records with an `id` property are not treated as new. Usually.
assert.ok(!widget.get("isNew"), "it is not a new record");
assert.ok(widget.get("isCreated"), "it is created");
assert.ok(widget.get("id"), "it has an id");
});
In particular, assert.ok will treat widget.get("id") === 0 as falsey. To make tests like these bullet-proof, they should look like this:
assert.ok(widget.get("id") !== undefined, "it has an id");
This will only produce a false-positive when a property was explicitly set with a value of undefined.