Re: Maybe we should think about Interface.isInterface functions again

On Aug 7, 2013, at 7:55 AM, Domenic Denicola wrote:

> Allen, I have a case that I would appreciate your input on. At first glance it seems like `Interface.isInterface` is exactly what we need, but I dislike the idea of adding such methods everywhere, so any alternative would be appreciated.
> 
> Consider a hypothetical "Elements" class, subclassing array, and meant to hold `Element`s. You can see a rough definition [in this gist][1].
> 
> For ease of use it allows non-`Element`s to be added, e.g. via `elementsInstance.push(5)`. There's a few reasons for this:
> 
> - Overriding all the array methods to add type-checking is not fun.
> - Even if you did that, `Array.prototype.push.call(elementsInstance, 5)` would still work, so you end up needing to fall back to a proxy, losing most of the benefits of subclassing `Array` in the first place.
> - All that aside, it's really convenient to be able to easily do things like `elementsInstance.map(el => el.tagName)` and treat that as an array, just by not using any of the special `Elements`-only subclass methods. You can even see an example of such usage in the gist.
> 
> I am assuming this is good design; if you think otherwise I guess that's a different conversation (feel free to fork the thread).
> 
> Anyway, with that in place: certain methods of `Elements`, e.g. `Elements.prototype.findAll`, want to ignore any non-`Element` members. The question is, how can we do that, without `Element.isElement`? A test like `x instanceof Element` does not work, because both `Element.prototype` and `HTMLElement.prototype` pass this test, but are not "really" elements; you can't do anything useful with them, e.g. calling `querySelectorAll` just blows up.
> 
> Would love your feedback on this case, which feels like a more general case of how the web platform uses branding.
> 
> [1]: https://gist.github.com/domenic/5864658
> 
> 

First some off-topic naming issues.

ES6 already has an Array.prototype.find method so you would probably want to name your 'find' method 'findSelector'  or something else. Perhaps, it shouldn't even include the word 'find' because it isn't clear to me that the result is necessarily an element of the original collection.  Why not just call these methods of Elements  'querySelector' and 'querySelectorAll'?

Also, if you want Elements itself to be meaningfully subclassable your definition of  createFinder() isn't correct.  You might redefine it as:

    function createFinder(selector, collector {
            return element => collector.from(element.querySelectorAll(":scope " + selector));

 and call it as:
   const finder = createFinder(selector, this.constructor);

so simply:
    const finder = element => this.constructor.from(element.querySelectorAll(":scope " + selector));    

On to conceptual design issues...

Any time you add different kinds of objects to a generic collection and subsequently process them in a manner that requires re-identifying each object's  "kind" you should probably ask why you are doing that. What is it about the objects that make it meaningful to aggregate them?

In this case, you seem to be saying that Elements should only contain elements that conform to the Element interface but you don't want to enforce that restriction. So, regardless of how many ways there may be insert a non-conforming value into an Elements collection, it is still a logic bug to do so.  What happens if that bug occurs?  Let's use you gist sample but assume that the instanceof Element tests were removed.  In this case, you are still duck typing the elements of the collection with the querySelectorAll call.  At worse, the element is an object that implements that method but in some other way does not conform to the Element interface, perhaps by returning non-Element values.  Even in that case it doesn't really matter, the code is still, by definition, buggy.  If such derived objects are actually used they may case other downstream duck typing failures.  At worse, you eventually reach some code that depends upon some checkable internal implementation requirement and that code presumably will reject bogus objects.

For your 'findAll' method, you could try to explicitly filter non-Element collection elements from the findAll query.  But unless there is a valid application level reason why such elements should exist, I don't see any point in bothering. It could happen, but its just a bug that will ultimately show up one way or another. 

Descriptive type systems such as JSIDL or TypeScript's are very useful for help developers reason about their code.  But there is no particular reason to turn all of their assertions or invariants into explicit runtime code.

Allen