instanceof

[icylace]

I notice that you use instanceof a few times throughout Mithril.

Unfortunately, instanceof will not work correctly when dealing with multiple frames in a window. The MDN docs take note of this. There are those who consider instanceof to be ultimately worthless for that reason.

From what I've read on the subject Object.prototype.toString() is the current best replacement for instanceof. For example, validator.js uses it.

I admit that I only skimmed the Mithril source and perhaps came to a hasty conclusion about the use of instanceof that I thought would be problematic.

What say you ?

P.S. Ever since I learned that semicolons in JavaScript are statement separators instead of statement terminators I've come to really appreciate it when they are used minimally as you have done with your code :)

[darsain]

Can confirm. Have been bitten by instanceof several times in the past when working between contexts, whether it has been different frames on a page or node-webkit nodejs vs window context.

+1 for dropping it everywhere.

[pygy]

Edit: sorry for the double post, I replied from gmail and a browser extension scrambled the result. Even from the web interface, the text in this window wouldn't behave. I suppose there were hidden characters somewhere.

[pygy]

Possible solutions:

function is (t, o) {
   return Object.prototype.toString.call(o) == ( is[t] || ( is[t]="[object "+t+"]" ) );
}
is("String", "foo") // --> true

or

var type = Object.prototype.toString
type.call("string") == "[object String]"

With 7 ​instanceof occurrences, the first solution is tighter at the source level, but I don't know how ​it compresses​.

[lhorie]

Oh. The Array ones are definitely an oversight on my part. The var type on the very top of the file is there precisely for proper type checking for these. I'll fix those.

I think the lines about Error and SyntaxError however are fine. Those are there specifically to deal with the prototype chain of thrown/caught exceptions, and I don't imagine anyone would ever import an Error instance from one frame to another.

[darsain]

It's not just frames, it's also different contexts. For example if you're creating a node-webkit application, you can have 1 js file but the code in it is being executed in different contexts:

// js file running in current window context
var fs = require('fs'); // fs module running in nodejs context

try {
  fs.readFileSync('doesntexist.lol');
} catch (err) {
  err instanceof Error; // false => different contexts
}

Just so you'd be aware of that :)

[lhorie]

@darsain interesting... I wasn't aware of that. I've changed the code for the Error classes, but AFAIK there's no good way to check if something is an instance of SyntaxError (and it doesn't really matter anyways, because that line is only there to throw a user-friendly error, and falls back to simply rethrowing it in the NodeJS different context case)