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 requires callable or constructable right-hand side #12728

Merged
merged 2 commits into from Jan 6, 2017

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Dec 7, 2016

Fixes #12709

The check for the right-hand side of instanceof previously required that the type be any or a subtype of Function. This is too strict, and breaks if a class has some otherwise-unrelated functions like toString, bind, etc that don't match Function.toString, Function.bind, etc.

This PR loosens the check to allow a right-hand side type with a call or construct signature, even if the type isn't assignable to Function.

Also includes a fast-path for null and undefined even though they are
technically Function subtypes.
@sandersn
Copy link
Member Author

sandersn commented Dec 7, 2016

@mhegazy do you want to take a look?

@mhegazy
Copy link
Contributor

mhegazy commented Dec 21, 2016

I would like to discuss this in the design meeting first. will put it on the agenda.

@DanielRosenwasser
Copy link
Member

To get some discussion going, one of two requirements must be satisfied for instanceof to work in a reasonable way:

  1. The object has an method whose key is Symbol.hasInstance.
  2. If the object has no member keyed by Symbol.hasInstance, the object must be callable.

Check out the relevant section of the spec itself.

@DanielRosenwasser
Copy link
Member

So in short, I think checking for some number of call/construct signatures is the right move - however, we could also check for a Symbol.hasInstance.

@mhegazy
Copy link
Contributor

mhegazy commented Dec 30, 2016

So in short, I think checking for some number of call/construct signatures is the right move - however, we could also check for a Symbol.hasInstance.

I would say we should add that check as well to be ES6 complaint.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 6, 2017

as per #13331, let's get this one in.

@sandersn sandersn merged commit 5b075ff into master Jan 6, 2017
@sandersn sandersn deleted the instanceof-requires-callable-rhs branch January 6, 2017 21:49
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants