groupon / assertive

Assertive is a terse yet expressive assertion library
BSD 3-Clause "New" or "Revised" License
21 stars 11 forks source link

Support constructor function in hasType #10

Open jkrems opened 10 years ago

jkrems commented 10 years ago

Maybe I was missing something:

var assert = require('assertive');

function MyClass() {
};

var inst = new MyClass();

assert.truthy(inst instanceof MyClass);
assert.hasType(MyClass, inst);

Running it gives me:

/foo/bar/node_modules/assertive/lib/assertive.js:280
        throw new TypeError('' + name + ': unknown expectedType ' + badArg + '
              ^
TypeError: hasType: unknown expectedType "[object Undefined]"; you used:
hasType "[object Undefined]", MyClass
Did you mean null, Date, Array, String, RegExp, Boolean, Function, Object, NaN, Number or undefined?
johan commented 10 years ago

Assertive has thus far dodged problems like multiple global objects by just supporting the language's basic types.

The multiple globals situation shows up in browsers with multiple window objects, each being their own world that has different constructor functions for things like Array and where one window's FooConstructor won't have any relation to another's FooConstructor, and the like. This apparently also happens in Node under some circumstances, though I haven't personally dug into the details.

I am not opposed to getting this feature, but but I think it needs design work, because instanceof doesn't define type equality – as the current function name decrees – but a less-than-or-equals comparison on the directed acyclic type graph. I e: (new TypeError) instanceof Error === true.

An assertive instanceOf assertion might be useful, and a hasType strict class checker could be useful too, but for either I'd prefer a more thorough look at how to behave in the edge cases, and for Node, figuring out what they are, as in the above linked one David hit with his first #2 PR I backed out and part reimplemented differently.

jkrems commented 10 years ago

Personally I'd actually expect hasType to honor is-a relationships, e.g. hasType NetworkError, err to pass if the error happens to be a ConnectionRefusedError. That's also in line with the current behavior when testing for Object - it would be a breaking change if hasType Object, (new Error) would fail. So, maybe it might be worth adding a second hasConstructor (or similar) function to test for type without honoring is-a.

Node itself implements Buffer.isBuffer via instanceof, so I'd be surprised if there's any expected problem inside of node. And if you'd want to check type across frames in a browser, there's really not much assertive could help you with. Unless you'd expect fn.name to exist and "name equality" to be sufficient. Which I wouldn't.

johan commented 10 years ago

There apparently are cases in node demonstrated by above link where one String constructor wasn't identical to another String constructor and I'd be happy to know more but didn't go to town investigating.

hasType may have too sparse docs, but it's a fairly robust tool for asserting basic types. It was most likely initially meant for testing for anything that was expressable as an object literal, but as you note it doesn't seem to be quite that rigorous. A design goal was that for any two types x and y, at most one of hasType x and hasType y should be true, so hasType clearly divides every value into one type class alone, so hasType Object, new String('foo') is a failure and hasType Object, /regex/ is a failure.

Taking your reasoning further than I think you want, hasType Object, [] should also be true, which it intentionally isn't, for above reasons.

In retrospect, it might have been wise to make hasType Object only pass anything expressable as an object literal, leaving backwards-compatible room for extending the current design principle to any kind of object, but nobody thought of that at the time. Not too late for a major bump change, of course – it could be a pretty cool and useful thing to have.

The isA or instanceOf use case so far isn't covered by Assertive, but PR:s adding it are welcome.

johan commented 10 years ago

It's not terribly hard writing truthy 'is Error', foo instanceof Error though, so I think people wanting that test can handle the extra typing. Implementing the current clear divider between value types, though, is Pretty Hard Work in javascript, that is very prone to getting subtly and debilitatingly wrong.