webidl icon indicating copy to clipboard operation
webidl copied to clipboard

Need pattern for feature detecting dictionary members

Open RByers opened this issue 8 years ago • 87 comments

Many new APIs (and some new arguments to existing APIs) are relying on dictionaries. But doing feature detection of such members requires ugly and complex code like:

var supportsCaptureOption = false;
try {
  addEventListener("test", null, Object.defineProperty({}, 'capture', {get: function () {
    supportsCaptureOption = true;
  }}));
} catch(e) {}

This increases the concern that new APIs will lead to sites being broken on older browsers because developers didn't understand or couldn't be bothered with the difficult feature detection.

In WICG/EventListenerOptions#31 @tabatkins proposed a mechanism whereby all dictionary types would automatically get a JS-exposed object with a property-per-member to enable consistent and easy feature detection.

Thoughts?

RByers avatar Apr 08 '16 21:04 RByers

I'm a little scared of this idea since dictionaries right now are not "reified" in any way. Their names are for spec purposes only, and can be changed at will. They just represent normal JavaScript objects that authors pass in. Having there be a property on the global for each dictionary, which is going to be some type of... object? array? of supported property keys (if object, what is their value?) is pretty weird.

I don't really have any other great solution though. Something like window.dictionarySupports("EventListenerOptions", "passive") isn't great either.

domenic avatar Apr 08 '16 22:04 domenic

(None of this would be necessary if JS hadn't punted on the named-arguments thing. If we had named arguments like Python, this would all be way easier - methods would just throw if you passed a new argument in an old browser, like they do for new positional arguments today. Ugh.)

To be specific, if you define a dictionary like:

dictionary InterfaceMethodOptions {
  DOMString foo = "";
  long long bar = 0;
}

This would define an InterfaceMethodOptions value on the global shaped like:

window.InterfaceMethodOptions = {foo: true, bar: true};

By making this happen automatically in IDL, we get reasonably dependable support, without needing to rely on impls (a) remembering to update their "is this supported?" code, and (b) not lying. This is similar to how @supports works "automatically" by just basing itself on whether the value parses or not.

tabatkins avatar Apr 08 '16 22:04 tabatkins

Can you say why that's better than one of

window.EventListenerOptions = new Set("foo", "bar");
window.EventListenerOptions = ["foo", "bar"];

?

domenic avatar Apr 08 '16 22:04 domenic

We would definitely need to audit dictionary names in the platform to make sure that none of them have names that are likely to collide with real-world stuff.... Past that this is probably OK, but I agree it should not be raw object. And the reason it shouldn't be is that we don't want to things to go sideways if a dictionary has a "toString" member or whatnot. So a Set is probably a better idea.

bzbarsky avatar Apr 08 '16 22:04 bzbarsky

Sure, I don't have strong opinions on the exact shape. A Set works for me.

tabatkins avatar Apr 08 '16 23:04 tabatkins

FWIW we had this exact problem with getUserMedia(constraints). We ended up defining navigator.mediaDevices.getSupportedConstraints(). The implementation was quite trivial. :)

Still, that seems like a lot of bloat on the window.

jan-ivar avatar Apr 08 '16 23:04 jan-ivar

I suppose we would only need this for dictionaries that are taken as inputs? Some specs have lots of dictionaries that are only ever returned to content.

jan-ivar avatar Apr 08 '16 23:04 jan-ivar

I kind of prefer the dictionarySupports() version over defining lots of additional properties on the global. With a method we could also potentially extend it in the future so you can check whether your particular value is supported for a member or not.

annevk avatar Apr 09 '16 09:04 annevk

Just so long as it's something that can reasonably be done automagically by our IDL handlers.

tabatkins avatar Apr 09 '16 15:04 tabatkins

Yeah, it would totally be IDL-driven. We will probably need to start annotating dictionaries with something akin to Exposed. Otherwise IDL code will need to find where the dictionary is used in order to know what global it's supported on (maybe that's okay?).

Another thing is that we should indeed probably not do this for dictionaries that are solely returned. It only makes sense for those accepted as an argument somewhere. That probably requires an annotation or again some finding "magic".

annevk avatar Apr 09 '16 16:04 annevk

Why don't we start by adding an extended-attribute that opts dictionaries into this behavior? We can try that out in a few cases, then revisit if/how to change the default.

RByers avatar Apr 09 '16 16:04 RByers

That probably requires an annotation or again some finding "magic".

You can't do this without annotations in general: there are some APIs that take object and examine something on it to decide which dictionary type to convert it to...

bzbarsky avatar Apr 09 '16 18:04 bzbarsky

At the risk of sounding spoiled, I think I would expect the webidl compiler to do this automatically whenever it can (which should be most of the time?), and instead require annotation when using dictionaries in unobvious ways. I worry most spec writers would forget otherwise.

jan-ivar avatar Apr 09 '16 19:04 jan-ivar

Yeah, I don't see why return-only dictionaries are a problem here. It's not useful to feature-detect them (probably, tho I could imagine some cases), but if leaving them out means we have to affirmatively annotate dictionaries we want in, it's not worth the trouble - we should just put them all in.

I'm fine with the dictionaries using the same [Global] defaulting that interfaces use.

tabatkins avatar Apr 09 '16 22:04 tabatkins

FWIW, this problem applies to enum values in addition to dictionary members.

sicking avatar Apr 18 '16 20:04 sicking

Good point. Do enums live in the same or different namespace from dictionaries?

tabatkins avatar Apr 18 '16 21:04 tabatkins

Same namespace, because when you use it as an arg type, you just use the name.

bzbarsky avatar Apr 18 '16 21:04 bzbarsky

Ah, right. I... should probably address that in Bikeshed. (It treats all the name-definers as separate namespaces right now and won't warn you if names collide.)

tabatkins avatar Apr 18 '16 22:04 tabatkins

Isn't enum arg detection straightforward? obj.foo = 'bar'; obj.foo == 'bar'

jan-ivar avatar Apr 19 '16 01:04 jan-ivar

Not if the only use of the enum is in a method argument (same as the dictionary issues we're discussing).

RByers avatar Apr 19 '16 01:04 RByers

Do we want to allow enumeration of supported dictionary/enumeration members? I can see arguments either way, without a compelling use case I'd probably prefer we keep this scoped to feature detection.

RByers avatar Apr 19 '16 01:04 RByers

The other thing I was wondering about is if we are going to expose dictionaries, should we attempt at normalizing their names somehow?

I think we should do enums separately by the way. They are either ignored (setters) or the method throws, which makes them reasonably detectable. Apart from that, they would require a different API.

annevk avatar Apr 19 '16 05:04 annevk

And yes, agreed that we should keep it simple initially.

annevk avatar Apr 19 '16 06:04 annevk

@RByers unknown enum args in methods throw.

jan-ivar avatar Apr 19 '16 18:04 jan-ivar

Do we want to allow enumeration of supported dictionary/enumeration members? I can see arguments either way, without a compelling use case I'd probably prefer we keep this scoped to feature detection.

I'm confused - enumerating the supported dictionary members is literally the request here. (Or at least, being able to ask if a given name is a supported dictionary member.) I'm 100% against anything attempting to be smarter such that it can no longer be trivially automated with no human intervention required.

I think we should do enums separately by the way. They are either ignored (setters) or the method throws, which makes them reasonably detectable. Apart from that, they would require a different API.

Why would they require a different API? Afaict they'd have the identical "set of supported names for this type" API.

tabatkins avatar Apr 19 '16 21:04 tabatkins

@tabatkins well, e.g., do enums and dictionary share a namespace? It's also not clear to me why they would be the same API, since you can do much more with dictionaries than simple member checking going forward as I hinted earlier (e.g., seeing whether a member accepts a particular value).

annevk avatar Apr 20 '16 06:04 annevk

I'm confused - enumerating the supported dictionary members is literally the request here. (Or at least, being able to ask if a given name is a supported dictionary member.)

Right. There have been two main classes of APIs discussed:

  1. dictionarySupports("EventListenerOptions", "passive") - provides feature detection without enumeration
  2. "passive" in EventListenerOptions - provides both feature detection and enumeration

My question was just whether we considered supporting enumeration in addition to feature detection a good or bad thing. I can certainly imagine cases where allowing enumeration causes more problems than benefits. If we don't have any good reason to want to support it, then we should probably prefer the 1) style over the 2) style as a result.

RByers avatar Apr 20 '16 16:04 RByers

well, e.g., do enums and dictionary share a namespace?

Yes, this was asked by me and answered by bz immediately prior to your comment: https://github.com/heycam/webidl/issues/107#issuecomment-211592249 Everything that can be used as an argument type shares a namespace: interfaces, dictionaries, enums, and typedefs. I opened a Bikeshed bug to enforce that more thoroughly.

tabatkins avatar Apr 20 '16 23:04 tabatkins

Does this issue address or cover the same use cases as https://www.w3.org/Bugs/Public/show_bug.cgi?id=19936? If so, should we close it with a reference to this issue?

ddorwin avatar Apr 26 '16 21:04 ddorwin

@ddorwin I think the motivation for that bug is different. That is about treating invalid values as another value. This is about testing whether a value is supported.

annevk avatar Apr 27 '16 09:04 annevk