TritonDataCenter / node-assert-plus

Extra assertions on top of node's assert module
MIT License
122 stars 25 forks source link

thoughts on strict .object check #15

Closed bahamas10 closed 9 years ago

bahamas10 commented 9 years ago

Right now, .object() only checks typeof (...) === 'object'. While this makes sense, it allows for a lot of things to pass that check that, while are objects, we might not consider to be objects as such.

For example:

assert.object(new Date());
assert.object(/regex/);
assert.object(null);
assert.object([]);
assert.object(new Error());

All pass the test.

null and [] are the most egregious, though everything listed above are technically javascript objects. Would a modification to .object to be stricter make sense? or maybe the addition of a .strictObject?

ex:

function isObject(o) {
  return o !== null && typeof (o) === 'object' && o.constructor && o.constructor === Object;
}

isObject(new Date());
// => false
isObject(/regex/);
// => false
isObject(null);
// => false
isObject([]);
// => false
isObject(new Error());
// => false

isObject({});
// => true
isObject(new Object());
// => true
bahamas10 commented 9 years ago

This implementation has been deemed too strict... based on a convo with me + @pfmooney

@pfmooney
3:02 > Array.prototype.specialFunc = function () { console.log('omg'); };
[Function]
> bar = new Array()
[]
> bar.specialFunc();
omg
undefined
@pfmooney
2:57 I think o.constructor == Object might be dangerous
2:58 we can't be that strict
2:59 > function Foo() { }
undefined
> var bar = new Foo();
undefined
> bar
{}
> bar.constructor
[Function: Foo]
2:59 assert.object(bar) should absolutely succeed

The solution right now is to just bail if o === null

pfmooney commented 9 years ago

Fixed in v0.2.0