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

Need pattern for feature detecting dictionary members #107

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

Need pattern for feature detecting dictionary members #107

RByers opened this issue Apr 8, 2016 · 87 comments
Labels
☕☕ difficulty:medium Hard to fix ⌛⌛⌛ duration:long There goes your week-end

Comments

@RByers
Copy link

RByers commented Apr 8, 2016

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?

@domenic
Copy link
Member

domenic commented Apr 8, 2016

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.

@tabatkins
Copy link
Contributor

(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.

@domenic
Copy link
Member

domenic commented Apr 8, 2016

Can you say why that's better than one of

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

?

@bzbarsky
Copy link
Collaborator

bzbarsky commented Apr 8, 2016

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.

@tabatkins
Copy link
Contributor

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

@jan-ivar
Copy link
Contributor

jan-ivar commented Apr 8, 2016

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
Copy link
Contributor

jan-ivar commented Apr 8, 2016

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.

@annevk
Copy link
Member

annevk commented Apr 9, 2016

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.

@tabatkins
Copy link
Contributor

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

@annevk
Copy link
Member

annevk commented Apr 9, 2016

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".

@RByers
Copy link
Author

RByers commented Apr 9, 2016

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.

@bzbarsky
Copy link
Collaborator

bzbarsky commented Apr 9, 2016

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...

@jan-ivar
Copy link
Contributor

jan-ivar commented Apr 9, 2016

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.

@tabatkins
Copy link
Contributor

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.

@sicking
Copy link

sicking commented Apr 18, 2016

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

@tabatkins
Copy link
Contributor

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

@bzbarsky
Copy link
Collaborator

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

@tabatkins
Copy link
Contributor

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.)

@jan-ivar
Copy link
Contributor

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

@RByers
Copy link
Author

RByers commented Apr 19, 2016

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

@RByers
Copy link
Author

RByers commented Apr 19, 2016

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.

@annevk
Copy link
Member

annevk commented Apr 19, 2016

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
Copy link
Member

annevk commented Apr 19, 2016

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

@jan-ivar
Copy link
Contributor

jan-ivar commented Apr 19, 2016

@RByers unknown enum args in methods throw.

@tabatkins
Copy link
Contributor

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.

@annevk
Copy link
Member

annevk commented Apr 20, 2016

@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).

@RByers
Copy link
Author

RByers commented Apr 20, 2016

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.

@tabatkins
Copy link
Contributor

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: #107 (comment) 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.

@ddorwin
Copy link
Contributor

ddorwin commented Apr 26, 2016

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?

@jan-ivar
Copy link
Contributor

The reason I hesitate, is I don't consider a dictionary to be a first class API.

Someone once told me, "you don't have to use dictionaries. Use object. Problem solved." Their point being (I think) was that WebIDL defines dictionaries to encourage certain API patterns and discourage others. Therefore, it's not always sufficient to have a problem. The question is: is it a good pattern? For example, is share() really several methods in one? Might a discrete shareFile() method have been cleaner? I'm not saying it is, just using it as an example.

@mgiuca
Copy link
Contributor

mgiuca commented Oct 17, 2018

For example, is share() really several methods in one? Might a discrete shareFile() method have been cleaner? I'm not saying it is, just using it as an example.

That decision isn't fixed in stone yet. It's part of this proposal:
http://wicg.github.io/web-share/level-2/

Maybe, but we also want to be able to share a file along with the other metadata at the same time (e.g., share a URL along with a screenshot).

What I'm asking here, at a high level, is: should there be a standardized pattern for how to do this kind of sub-feature detection, or should each API build its own? The suggestion to use a separate shareFile method suggests that each API should be finding its own way of supplying the new features.

@cben
Copy link

cben commented Feb 10, 2019

Not sure if this was proposed yet, but how about stuffing the supported options onto the functions/methods that take them?
If I want to know how to call someElem.addEventListener(...), I'll probe someElem.addEventListener.supportedOptions or someElem.addEventListener.AddEventListenerOptions or something like that.
Minimal namespace pollution, and a pretty obvious place to look?

(For dictionaries taken by multiple functions, or methods that exist on multiple objects, such as .addEventListener, the specific standard should probably promise that the supported options are same for all places. Or not.)

@yoichio
Copy link

yoichio commented May 11, 2020

I'm working with a customer who wants this kind of detection (with an origin trial, FYI).
I think this problem is not only for dictionary but function signature detection like "if this function supports boolean as the 3rd parameter?".
This reminds me System.Reflection on .Net C#.
I know they manages this kind of detection because of C# is strongly typed, but can WebIDL support this?

@npm1
Copy link

npm1 commented Sep 28, 2020

Has there been any progress on this? We recently discussed w3c/performance-timeline#176 in a call and someone pointed out this issue, which would fix the problem for us.

@yoavweiss
Copy link

In an unrelated thread, @mkruisselbrink points out that it's possible to feature detect dictionary members by providing the method an object with a getter and seeing if it was read.
That's cumbersome, but could work...

@RByers
Copy link
Author

RByers commented Sep 29, 2020

Yep, that's the pattern I used when opening this issue. We continue to hear developers complain that they really don't consider that acceptable (complexity, obscurity, desire to reliably avoid exceptions, etc).

@yoavweiss
Copy link

Apologies for not actually reading the OP...
Agree that a simpler method would be better.

@Kaiido
Copy link
Member

Kaiido commented Sep 11, 2021

I had an "exotic idea" come into my mind about this issue, I hope that after 5 years it's ok to propose such ideas, at least to make things move a bit.
Note that I am really not proficient in WebIDL and thus not entirely sure of what can and can't be done, so this proposal may very well be completely unfeasible, if so, sorry in advance.
Also, this assumes ES binding, which might once again be a completely wrong assumption.

Anyway, the idea would be to expose a new global method (name can totally change, I'm really bad at naming)

interface mixin WindowOrWorkerGlobalScope { // or any global context really
+  boolean methodSupports(Function method, any... arguments);
}

Where web authors would pass directly the ECMAScript function object they have access to as the first argument and the arguments they'd like to test as the following arguments.
Each argument would be tested against the UA's IDL implementation to see if it is recognized as potentially valid.

For instance to test for Worker's type = "module" support, one would write

globalThis.methodSupports(Worker, "" /* the URL param */, { type: "module" } /* the option param */) 

To avoid creating void objects only to pass the required arguments, maybe undefined or null values should be ignored from the tests, e.g

globalThis.methodSupports(EventTarget.prototype.addEventListener, null, null, { signal } );

would ignore both the event type and callback params but check if signal is an AbortSignal.

If the arguments length is bigger than the one expected, or if a dictionary member is not recognized, or if the value of an enum is not valid, the method should return false, otherwise it should return true (so it doesn't look if required options are passed or not, it only checks that the passed ones are recognized).

I believe such a model would solve the issues outlined with the two proposed solutions so far:

  • No need to expose every dictionary, nor to fix their names nor even to reify them in any other way than what is currently done.
  • Can be automatically added to any method, no need to subscribe at definition (though some definitions may want their own validation steps?)
  • Obviously works for many methods defined on the same interface
  • Works for both enums and dictionary options

However it also comes with its own questions (and many others I probably didn't see myself)

  • Can WebIDL link ECMAScript function objects to the actual IDL method? Even if there isn't any mechanism today, could one be built?
  • What about all the methods that are language specific and not linked to IDL? e.g should this method also work for ES Intl.numberFormats and Array.from and everything else? I doubt many web authors are interested in where a method has been specced from, they will probably not understand why some methods are excluded.
  • How should DOMStrings be tested? Many methods will actually fail on invalid values, for instance the HTMLCanvasElement.getContext() method uses a DOMString instead of an enum and returns null when the value provided isn't recognized, should methodSupports(canvas.getContext, "webgpu") return true even in UAs that don't support this context type?
  • Is the difference constructor vs method calls gonna be a problem?

@saschanaz
Copy link
Member

saschanaz commented Sep 11, 2021

Random idea on my bed: constructors for dictionaries

dictionary Foo {
  (DOMString or boolean) bar;
} ;
// implicitly creates an interface-like constructor
"bar" in Foo.prototype // typical existence check
new Foo({ bar: "bar" }).bar === "bar" // type support check; string in this case

Problem: massive namespace pollution 🤔
Workaround 1: Add createDictionary(name, dict) like document.createEvent() and hide the constructors as [LegacyNoInterfaceObject] does.

@alvestrand
Copy link

alvestrand commented Sep 11, 2021 via email

@Kaiido
Copy link
Member

Kaiido commented Sep 13, 2021

This idea of a createDictionary(name, dict) method sounds like a variation of the solution proposed in the OP and faces the same issues expressed in #107 (comment).

The most convincing argument against this model for me is that specs authors should be able to change these dictionaries in ways that would break this model.
As an example, the very EventListenerOptions dictionary this issue originally came from has been changed just seven days after this issue got open to be split in two dictionaries, and a few days later it went from holding two properties to just one. An author today would have to test for AddEventListenerOptions, but a website written in the intermediary time-span would get false negatives by still looking for EventListenerOptions.

@annevk
Copy link
Member

annevk commented Sep 13, 2021

@Kaiido I quite like that idea, but it does seem like a lot of work for something that can be solved with throwing getters. And as you note it doesn't work for all the things you might want to find out support for. Based on that I'd still lean toward throwing getters and purpose-built support() methods where needed.

@Kaiido
Copy link
Member

Kaiido commented Sep 13, 2021

@annevk But as has been said previously, "throwing getters" actually do not work or at least they only work in environments that do support an option that is alphabetically after the one you want to test. If you want to test them all, or the last one alphabetically, it will execute the method in environments that don't support this option.

As to .support(), it works only if there is a single method per interface...

...(💡) Unless we plan to add it on the methods directly? But this still sounds very exotic and I fear we'll still face the "why isn't there an Intl.numberFormat.support()" problem, though maybe less prominently, and I guess it would be easier to implement than the global methodSupports() 🤔

@gsnedders
Copy link
Member

As linked above, foolip/mdn-bcd-collector#1485 is about adding detection for parameters to mdn-bcd-collector, as I'm aware at least some data in BCD that is outdated because it's non-trivial to do this in a general way. Incorrect data in BCD does, inevitably, further hurt web developers (above and beyond their specific problems with feature detection).

Clearly as a general purpose solution we can't rely on the alphabetically last dictionary member being supported (nor does that allow the last one to be tested).

@saschanaz
Copy link
Member

saschanaz commented Sep 14, 2021

#107 (comment) makes sense to me, and still I'd prefer a general solution for automation purpose e.g. BCD mentioned above.

My second random thought, which also covers signature and argument support detection:

// Given this IDL:
interface Bar {
  foo(Bar bar);
  foo((boolean or DOMString) union);
  foo(float f, unsigned short s);
  foo(Baz baz);
};

dictionary Bar {
  DOMString bar;
};

enum Baz { "baz", "" };
Foo.prototype.foo[Symbol.inspect];
inspectSignature(Foo.prototype.foo);
// returns:
// [
//   { bar: String },
//   [[Boolean, String]],
//   [Number, Number]
//   [["baz", ""]]
// ]

(Not sure how to differentiate optional arguments though.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☕☕ difficulty:medium Hard to fix ⌛⌛⌛ duration:long There goes your week-end
Development

No branches or pull requests