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

navigator.share's argument needs to be optional because ShareData has no required members #47

Closed
foolip opened this issue Jun 22, 2017 · 3 comments

Comments

@foolip
Copy link
Member

foolip commented Jun 22, 2017

This is another thing that's a bit surprising in Web IDL, but if a dictionary has no required members, then when used as an argument type, that argument must be optional. It's this bit from https://heycam.github.io/webidl/#idl-operations:

If the type of an argument is a dictionary type or a union type that has a dictionary as one of its flattened member types, and that dictionary type and its ancestors have no required members, and the argument is either the final argument or is followed only by optional arguments, then the argument must be specified as optional. Such arguments are always considered to have a default value of an empty dictionary, unless otherwise specified.

The reason is that with no required arguments, navigator.share({}) still works, and then it looks "silly" to require the empty object.

@mgiuca
Copy link
Collaborator

mgiuca commented Jun 23, 2017

Hmm I wonder if it's worth debating this. The reason all the attributes of ShareData are optional is that none of them are required, but you are supposed to pass at least one of them. It doesn't actually make sense to call navigator.share({}) so it seems the non-normative rationale there ("This is to encourage API designs that do not require authors to pass an empty dictionary value when they wish only to use the dictionary’s default values.") does not apply.

Although I don't think there's a way to express it in WebIDL, perhaps we could add a check in the share method that "if all fields of the dictionary are not present, reject with a TypeError", to actually force the user to supply at least one argument. Then leave the argument as non-optional.

We would still be in violation of the letter of WebIDL but not the spirit. And in fact omitting the argument would have the same result as passing the empty dict (rejected with TypeError).

@foolip
Copy link
Member Author

foolip commented Jun 27, 2017

I agree it's weird when some member is required, but no individual one. This was also the case with getUserMedia, debated in whatwg/webidl#130. whatwg/dom@e4f7faa was also done as a result of that.

Making it optional and explicitly throwing when missing wouldn't actually be possible with the spec equivalent of "custom bindings", as when you get into the prose, Web IDL has already done its thing and created a dictionary for you. The existing error handling will do the right thing, but the message given will be a bit cryptic, unfortunately.

@mgiuca
Copy link
Collaborator

mgiuca commented Jun 28, 2017

MutationObserver.observe sounds like the same case here. It's an optional argument that will (according to its steps always throw a TypeError if omitted.

This makes no sense to me. If you removed the word "optional" from that IDL, there would be no user-visible changes other than observe.length being 2 instead of 1. So the word "optional" is just a lie...

Making it optional and explicitly throwing when missing wouldn't actually be possible with the spec equivalent of "custom bindings", as when you get into the prose, Web IDL has already done its thing and created a dictionary for you.

I'm not suggesting that my prose throw a TypeError if the dictionary is omitted. I'm suggesting that we throw a TypeError if all fields of the dictionary are omitted. (Implicitly, this would be true if the argument was optional and the dictionary was omitted.) This would be the same situation as MutationObserver.observe.

mgiuca added a commit to mgiuca/web-share that referenced this issue Jul 3, 2017
This has no behavioural change, as passing an empty dictionary is a
TypeError. But it is required by the WebIDL spec.

Closes w3c#47.
mgiuca added a commit to mgiuca/web-share that referenced this issue Jul 3, 2017
This has no behavioural change, as passing an empty dictionary is a
TypeError. But it is required by the WebIDL spec.

Closes w3c#47.
mgiuca added a commit to mgiuca/web-share that referenced this issue Jul 4, 2017
This has no behavioural change, as passing an empty dictionary is a
TypeError. But it is required by the WebIDL spec.

Closes w3c#47.
mgiuca added a commit to mgiuca/web-share that referenced this issue Jul 4, 2017
This has no behavioural change, as passing an empty dictionary is a
TypeError. But it is required by the WebIDL spec.

Closes w3c#47.
mgiuca added a commit to mgiuca/web-share that referenced this issue Jul 4, 2017
This has no behavioural change, as passing an empty dictionary is a
TypeError. But it is required by the WebIDL spec.

Closes w3c#47.
@mgiuca mgiuca closed this as completed in #51 Jul 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants