Skip to content
This repository has been archived by the owner on Mar 19, 2018. It is now read-only.

Add simpler feature detection mechanism? #31

Open
RByers opened this issue Apr 8, 2016 · 21 comments
Open

Add simpler feature detection mechanism? #31

RByers opened this issue Apr 8, 2016 · 21 comments

Comments

@RByers
Copy link
Member

RByers commented Apr 8, 2016

There's lots of concern in #12 about the complexity of dictionary-member based feature detection. Perhaps we should be designing something simpler, ideally as a general pattern since dictionary-based APIs are being used / extended everywhere on the web now.

I originally had a getSupportedListenerOptions() API that was easier to use, which we "fixed" in #16. But maybe we should really be thinking of a better pattern? Some have suggested EventTarget.supportsPassive. This is kind of similar to DOMTokenList.supports and CSS @supports.

Suggestions?

@jacobrossi
Copy link

Just thinking out loud here... could we expose the actual dictionaries somewhere as interface objects?

@dtapuska
Copy link
Collaborator

dtapuska commented Apr 8, 2016

Perhaps EventTarget.supportsEventListenerOptions should actually be used no? There are some cases where you'd like to test whether something supports passive or not. But I think the bulk of the concern is does this API possibly take a dictionary?

@jacobrossi
Copy link

[Expanding last comment] maybe not exposed on the global, to avoid pollution. But I like a more general:
if(EventListenerOptions.passive) kind of test.

@RByers
Copy link
Member Author

RByers commented Apr 8, 2016

Perhaps EventTarget.supportsEventListenerOptions should actually be used no? There are some cases where you'd like to test whether something supports passive or not. But I think the bulk of the concern is does this API possibly take a dictionary?

Yes, in this case. But we've got some other options we may want to add where you really need to know if that option is supported (eg, once). So it's probably worth thinking here of what a good general pattern for specific dictionary members is.

@RByers
Copy link
Member Author

RByers commented Apr 8, 2016

Just thinking out loud here... could we expose the actual dictionaries somewhere as interface objects?
Maybe not exposed on the global, to avoid pollution. But I like a more general:
if(EventListenerOptions.passive) kind of test.

Yeah that's really what getSupportedOptions was about but it was clunky. How about this?

if (EventTarget.EventListenerOptions && EventTarget.EventListenerOptions.passive)

@tabatkins
Copy link
Collaborator

Why not on the global, tho? I mean, we pollute the global with the interface name already; putting the dict on there (which is usually just InterfaceNameDict or InterfaceNameOptions) doesn't seem much worse.

@jacobrossi
Copy link

I'm actually ok with the global, but I've heard concerns from others in the past.

I think hanging off EventTarget could also work, but want to make sure this is a pattern that's going to work for the other dictionary uses.

@RByers
Copy link
Member Author

RByers commented Apr 8, 2016

Yeah. I'm happy with global in this case. As of Feb 1, the token EventListenerOptions occurred exactly never in the Alex top 500k.

@phistuck
Copy link

phistuck commented Apr 8, 2016

we pollute the global with the interface name already

Do we? undefined in Chrome 49.

I think EventTarget.EventListenerOptions is good, regardless of any existing (or nonexistent) global EventListenerOptions definition in the wild. It does not belong there, it belongs to event targets.
Regarding passive support on a certain EventTarget subclass, WheelEvent.EventListenerOptions can have a different dictionary, or maybe WheelEvent.supportedEventListenerOptions.passive will be true.

@tabatkins
Copy link
Collaborator

@phistuck Not all dictionaries have an associated global interface, or rather, exactly one associated global interface. I don't think the pattern of sticking it on an interface works in general.

If certain events support a different set of options, they should be using a different dict. I suggested very intentionally that we avoid anything that requires or encourages browsers to write custom logic to implement the support-detection, because long experience shows that always leads to incomplete or lying support tests. Just having IDL dictionaries show up on the global with their associated keys set to something truthy does this well, similar to how @supports works.

@tabatkins
Copy link
Collaborator

Do we? undefined in Chrome 49.

The interface name (unless it's [NoInterfaceObject]. Dictionaries are currently not exposed, so they obviously don't show up.

@RByers
Copy link
Member Author

RByers commented Apr 8, 2016

I think EventTarget.EventListenerOptions is good, regardless of any existing (or nonexistent) global EventListenerOptions definition in the wild. It does not belong there, it belongs to event targets.

Unfortunately it doesn't really belong to event targets, it belongs to event listeners and they don't have any representation in JS today. EventTarget is probably the closest thing.

@tabatkins I like your argument that this needs to be automatic to be reliable. So I guess the idea would be that we'd add some sort of WebIDL custom attribute to the dictionary definition to opt in to this? Do you think we'd want to block on that, or should we consider defining a one-off for EventListenerOptions and then trying to get consensus on the general pattern in parallel?

@tabatkins
Copy link
Collaborator

I think all dictionaries should do this by default, and we should allow [NoInterfaceObject] on dictionaries for the rare cases when we want to opt out, same as interfaces.

We should also do this as a one-off regardless, probably. We don't add arguments to APIs that often, but this one in particular already has several possibilities for addition lined up.

@RByers
Copy link
Member Author

RByers commented Apr 8, 2016

Ok so to be concrete, the one-off proposal (using today's WebIDL) is something like:

Option A:

interface EventTarget {
  ...
  static readonly attribute EventListenerOptions EventListenerOptions;
}

With the attribute defined to return a dictionary with all members set to true (at least until we define new non-boolean options).

@tabatkins
Copy link
Collaborator

Unless we start trying to use the detection for something else, like giving out the default values, we should stick with true as the value. This is intended for feature detection, and true makes that easier.

@RByers
Copy link
Member Author

RByers commented Apr 8, 2016

Sure, but what if we add a string option like class (as Anne proposed). We want to re-use the dictionary type, right? I'd be fine just defining that case to be "true" or " " or something similarly truthy.

@tabatkins
Copy link
Collaborator

What I mean is that the object that IDL dictionaries define on the global is purely for feature-testing purposes. It is not an instance of the dictionary in question (any more than the object that IDL interfaces define on the global is an instance of the interface). It just contains all the keys the engine supports enough to include in their internal IDL, set to a truthy value so you can do trivial property-existence checks.

@jacobrossi
Copy link

I think all dictionaries should do this by default, and we should allow [NoInterfaceObject] on dictionaries for the rare cases when we want to opt out, same as interfaces.

+1

@RByers
Copy link
Member Author

RByers commented Apr 8, 2016

What I mean is that the object that IDL dictionaries define on the global is purely for feature-testing purposes. It is not an instance of the dictionary in question (any more than the object that IDL interfaces define on the global is an instance of the interface). It just contains all the keys the engine supports enough to include in their internal IDL, set to a truthy value so you can do trivial property-existence checks.

Ah, so that would be kinda awkward to define in WebIDL, right? But in JavaScript that would be:

Option B)

  window.EventListenerOptions = {capture:true, passive:true}

@tabatkins
Copy link
Collaborator

I mean, the idea is that WebIDL just says that's what happens for dictionaries. Nothing needs to be defined manually; any time you define a dictionary type, it defines that sort of object on the global.

@RByers
Copy link
Member Author

RByers commented Apr 8, 2016

Yeah I know, I'm just trying to come up with a path for shipping this for EventListenerOptions ASAP (since we're actively getting people to adopt it) without necessarily blocking on the WebIDL question. But perhaps it's time to start a discussion with the WebIDL experts - I'll do that.

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

No branches or pull requests

5 participants