Symbol.hasInstance and [[Get]] vs [[GetOwnProperty]]

# /#!/JoePea (8 years ago)

It seems like using [[Get]] for looking up @@hasInstance can be confusing and requires devs to write extra code. For example, suppose we have the following code:

class A {
    static [Symbol.hasInstance] (obj) {
        return false
    }
}

class B extends A {}
class C extends B {}

let c = new C

console.log(c instanceof B) // false, but expected true!
console.log(c instanceof A) // false, as expected

The c instanceof B check (arguably unintuitively) fails. Defining a Symbol.hasInstance method on a base class causes instanceof checks on any subclasses to automatically fail unless the developer takes care to write an ugly and fragile workaround:

class A {
    static [Symbol.hasInstance](obj) {
        if (this === A) return false
        else return Function.prototype[Symbol.hasInstance].call(this, obj)
    }
}

class B extends A {}
class C extends B {}

let c = new C

console.log(c instanceof B) // true, as expected
console.log(c instanceof A) // false, as expected

That seems likely to introduce errors or headaches.

What if the lookup for Symbol.hasInstance on an object use [[GetOwnProperty]]? Then the first version without the conditional checks would not break subclasses.

# /#!/JoePea (8 years ago)

I want to add also that using super makes it more readable, but still requires boilerplate code, and is just as statically limited as using a direct reference to a constructor ([I wish it wasn't](esdiscuss.org topic/the-super-keyword-doesnt-work-as-it-should)):

class A {
    static [Symbol.hasInstance](obj) {
        if (this === A) return false
        else return super[Symbol.hasInstance](obj)
    }
}
# Claude Pache (8 years ago)

Your mileage may vary, but for me it is, on contrary, intuitive that a subclass inherits by default all the methods of its superclass, without arbitrary exceptions such as [Symbol.hasInstance]().

Whether it is desirable, I am sure it depends on what you put in the [Symbol.hasInstance]() method. But the situation for that particular method is the same as for any random method: a superclass must be careful if it wants to be subclass-friendly.

# Jason Orendorff (8 years ago)

On Mon, Aug 15, 2016 at 8:45 PM, /#!/JoePea <joe at trusktr.io> wrote:

It seems like using [[Get]] for looking up @@hasInstance can be confusing and requires devs to write extra code.

I think the emotionally loaded words in this post ("fail", "ugly", "fragile", "break") are unhelpful. You have to make your point some other way.

The c instanceof B check (arguably unintuitively) fails.

It doesn't "fail". B inherits behavior from its base class A. That's the whole point of subclassing.

# Andrea Giammarchi (8 years ago)

fwiw I agree with Jason thanks gosh inheritance worked as expected even for public statics.

If you want to overwrite the inherited behaviour just do that on class B and eventually C.

# Allen Wirfs-Brock (8 years ago)

On Aug 16, 2016, at 9:56 AM, Andrea Giammarchi <andrea.giammarchi at gmail.com> wrote:

fwiw I agree with Jason thanks gosh inheritance worked as expected even for public statics.

If you want to overwrite the inherited behaviour just do that on class B and eventually C.

And if you want to revert to the default behavior set the value of B or C’s [Symbol.hasInstance] own property to undefined.

# /#!/JoePea (8 years ago)

Well, I do think this is fragile, because a class constructor should be checking if an instance is an instance created by said constructor, and the logic should not have to worry about super and sub class Function behavior like that, requiring needless boilerplate conditional code in every hasInstance function.

Symbol.hasInstance could have been spec'd differently, for example it could just be a static property without inheritance (using [[GetOwnProperty]]?). I don't think there would be anything wrong with that, and it would work nicer in my opinion, and it would still be possible to call super if you really really wanted to by simply referencing the super constructor directly.

I am willing to bet that in most cases people will not need inheritance in the implementation of a hasInstance function, and will find the needed boilerplate code unwanted.

if you want to revert to the default behavior set the value of B or C’s [Symbol.hasInstance] own property to undefined.

That just causes errors. In my environment (Babel),

Object.defineProperty(SomeClass, Symbol.hasInstance, {
    value: undefined
})

 // ...

someClass instanceof SomeClass // error

results in

Uncaught TypeError: Cannot read property 'call' of undefined

At the top of all my hasInstance functions so far, I have boilerplate code like the following, and it is not likely that there will be a case where I don't want such boilerplate code:

class SomeClass extends SomeOtherClass {}

Object.defineProperty(SomeClass, Symbol.hasInstance, {
    value: function(obj) {
        if (this !== SomeClass) return
Object.getPrototypeOf(SomeClass)[Symbol.hasInstance].call(this, obj)  //
needed boilerplate

        // ... instance-checking logic ...
    }
})

// or, using `static`:

class SomeClass extends SomeOtherClass {
    static [Symbol.hasInstance](obj) {
        if (this !== SomeClass) return super[Symbol.hasInstance](obj) //
needed boilerplate

        // ... instance-checking logic ...
    }
}

It seems somehow wrong to require that boilerplate code.

I can live with it though, now that I know how it works.

# Claude Pache (8 years ago)

Le 24 août 2016 à 03:35, /#!/JoePea <joe at trusktr.io> a écrit :

if you want to revert to the default behavior set the value of B or C’s [Symbol.hasInstance] own property to undefined.

That just causes errors. In my environment (Babel),

Object.defineProperty(SomeClass, Symbol.hasInstance, {
    value: undefined
})

 // ...

someClass instanceof SomeClass // error

results in

Uncaught TypeError: Cannot read property 'call' of undefined

If that’s the case, it's a bug (or limitation?) of Babel, as the spec is clear, see: tc39.github.io/ecma262/#sec-instanceofoperator

Also, reading the message of your TypeError, I wonder if Babel may also fail unexpectedly in case SomeClass does not inherit the standard Function#call, as in: class SomeClass { static call() { throw "Pwnd!" } }.

# Claude Pache (8 years ago)
Uncaught TypeError: Cannot read property 'call' of undefined

(...)

Also, reading the message of your TypeError, I wonder if Babel may also fail unexpectedly in case SomeClass does not inherit the standard Function#call, as in: class SomeClass { static call() { throw "Pwnd!" } }.

Correction: The potential issue I was thinking of, is when SomeClass[Symbol.hasInstance] does not inherit the standard Function#call:

class SomeClass { 
    static [Symbol.hasInstance]() { /* foo */ }
}
SomeClass[Symbol.hasInstance].call = function () { throw "Pwnd!" }

which is quite a corner case.

# /#!/JoePea (8 years ago)

Well, after looking at "Runtime Semantics: InstanceofOperator(O, C)" in the spec that you linked to, the mere fact that @@hasInstance can be undefined in which case inheritance is ignored completely goes against... inheritance.

So, this proves that we can make the spec be anything we wish it to be (as currently it doesn't follow pure prototypal inheritance), so we could easily choose to use [[GetOwnProperty]] without inheritance if we wanted to, and in my opinion that is much better.

# Jordan Harband (8 years ago)

All inheritance in JS stops a prototype chain lookup if an own property exists, no matter the value (iow, including undefined) - so this is fully consistent with JS inheritance.