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

Detect [[Class]] instead of using instanceof #54

Closed
bnjmnt4n opened this issue May 7, 2014 · 5 comments
Closed

Detect [[Class]] instead of using instanceof #54

bnjmnt4n opened this issue May 7, 2014 · 5 comments
Assignees
Labels
bug
Milestone

Comments

@bnjmnt4n
Copy link

@bnjmnt4n bnjmnt4n commented May 7, 2014

A quick glance through Hoek’s source shows that most of the type detection is done through instanceof, eg. instanceof RegExp, etc.

This means that values from different contexts, eg. RegExps generated from a different realm through Node.js’s vm module would not be detected correctly, as the RegExp constructor is different and thus the instanceof check would fail.

Instead, you should use Object.prototype.toString, by doing something like:

var getClass = Object.prototype.toString;
var className = getClass.call(object);
console.log(className); // "[object RegExp]"

This method instead detects the [[Class]] attribute of objects, which are constant across contexts.

@bnjmnt4n

This comment has been minimized.

Copy link
Author

@bnjmnt4n bnjmnt4n commented May 10, 2014

Ping?

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented May 11, 2014

I am worried about performance. The way we use this method, it should not be a problem, but I'll look into it. We might not need the special handling of those types anymore.

@hueniverse hueniverse added the bug label May 20, 2014
@hueniverse hueniverse added this to the 2.2.0 milestone May 20, 2014
@hueniverse hueniverse self-assigned this May 20, 2014
hueniverse added a commit that referenced this issue May 20, 2014
@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented May 20, 2014

So toString is just too slow. Since this is designed for node and we don't use multiple contexts it is not an issue. I reduce the number of instanceof where possible.

@hueniverse hueniverse closed this May 20, 2014
@bnjmnt4n

This comment has been minimized.

Copy link
Author

@bnjmnt4n bnjmnt4n commented May 20, 2014

Buffer.isBuffer is actually just a wrapper around b instanceof Buffer, and using it might actually slow down your code due to the extra function call, although that is a really really minor difference. However, using Array.isArray(a) is pretty much the same as {}.toString.call(a) as the ES5 specification defines it to check the [[Class]] value of the argument provided.

@hueniverse

This comment has been minimized.

Copy link
Member

@hueniverse hueniverse commented May 20, 2014

Interesting...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.