Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

instanceof #34

Closed
icylace opened this issue Aug 2, 2014 · 6 comments
Closed

instanceof #34

icylace opened this issue Aug 2, 2014 · 6 comments

Comments

@icylace
Copy link

icylace commented Aug 2, 2014

I've been on a quest to find an ideal runtime type-checking library and yours is the most agreeable I've come across thus far.

I notice that you use instanceof quite a few times throughout typify.

You noted a weakness of instanceof which I was unaware of.

Unfortunately, there is another weakness instanceof has. When used in a browser environment it will not work correctly when dealing with multiple frames in a window. The MDN docs take note of this. This has been discussed elsewhere as well.

Of the solutions to this I've seen, the one used by Mithril is pretty nice. What it does is similar to the following:

var type = {}.toString
type.call(foo) == "[object Object]"

Note that {}.toString is the same as Object.prototype.toString which I notice you've used a few times.

@phadej
Copy link
Owner

phadej commented Aug 3, 2014

Nice to hear you like typify!

The instanceof is used in

  • instance functionality, which is triggered by user
  • data definition parsing (Thunk)
  • type checks of Date and RegExpthose probably won't work if Date and RegExp aren't shared

I'll fix Date and RegExp check shortly (looks like underscore.js is using Object.prototype.toString trick for them as well. Yet you could workaround it by the same trick as using bacon.js: https://github.com/baconjs/bacon.js/wiki/FAQ#what-if-i-have-multiple-frames-in-my-application

@phadej phadej closed this as completed in e533b15 Aug 3, 2014
@icylace
Copy link
Author

icylace commented Aug 3, 2014

That Bacon workaround is pretty interesting though I don't find it to be ideal.

Thanks for fixing the check for Date and RegExp but the fixes are seemingly not yet reflected in typify.standalone.js.

Since, like you said, instance is triggered by the user shouldn't that be reason enough to use the {}.toString technique for that too? I don't understand the reasoning for not doing so.

I was recently made aware that the instanceof gotcha affects a wider range of scenarios as well. So, wouldn't the interaction between _throws() and expectedException() be affected?

@phadej
Copy link
Owner

phadej commented Aug 4, 2014

The two below are equivalent:

typify.instance(name, cls);
typify.type(name, function (val) {
  return val instanceof cls;
});

So the responsibility of using instanceof is thus on library user.

You might exploit constructor.name ({}.toString doesn't work for user created "classes"):
It might be what you want, or maybe not, as it is less strict check:

var typify = require("./lib/typify.js");

function make(n) {
    return function foo() {};
}

var foo1 = make(1);
var foo2 = make(2); 

typify.instance("foo1", foo1);
typify.instance("foo2", foo2);

typify.type("foo1x", function (val) {
    return val.constructor.name === foo1.name;
});

typify.type("foo2x", function (val) {
    return val.constructor.name === foo2.name;
});

var x = new foo1();
var y = new foo2();

console.log(typify.check("foo1", x)); // true
console.log(typify.check("foo2", x)); // false
console.log(typify.check("foo1", y)); // false
console.log(typify.check("foo2", y)); // true

console.log(typify.check("foo1x", x)); // true
console.log(typify.check("foo2x", x)); // true
console.log(typify.check("foo1x", y)); // true
console.log(typify.check("foo2x", y)); // true

_throws() and expectedException are browserify probivided. I guess they work well (and that functionality isn't used or exposed).

Though I could make a standalone build without assert.

@icylace
Copy link
Author

icylace commented Aug 4, 2014

First, thanks for the explanations and updating typify.standalone.js.

I prefer strictness but the case of constructor.name is pretty interesting. Let's consider your example some more and say we're not able to see any of the code that preceded typify.instance("foo1", foo1);. Now if we look at those type checks then I think we could come to the conclusion that "foo1" and "foo2" are the result of an abstract factory. I don't believe inheritance would be a possible explanation here. Perhaps aliasing would be? Not completely sure.

In any case I don't know if such an observation would be useful but I find it interesting nonetheless.

@phadej
Copy link
Owner

phadej commented Aug 4, 2014

I'm not sure what is the right term for this behaviour.

I wanted to show with this example that: If you have multiple frames communicating data to each other, than there aren't fail-proof method to ensure in the first frame that foo created in second frame is instance of Foo, if you don't have access to Foo of the second frame. The best you can get is to make duck-typing checks; constructor name check is one, you could (should?) check more of course.

Yet I don't see this as a problem. You should communicate only raw data (plain objects, arrays, maybe Date's and RegExps which should work now), not "intelligent" objects (as they have references to constructing frame). And validating well-formedness of raw data is what typify is good at.

@icylace
Copy link
Author

icylace commented Aug 4, 2014

Yes, fair point. If someone is trying to be too clever with their data their effort could backfire on them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants