A non-optional dictionary argument makes sense at times.

[alvestrand]

See w3c/mediacapture-main#366

There are times where omitting a dictionary SHOULD result in a TypeError, even when the dictionary has no mandatory members. In mediacapture's case, the (prose) spec says that TypeError will be thrown from the procedure when you omit all the members.

In this particular case, it doesn't make sense to specify the dictionary as an optional argument, but the current spec forces us to. This is a WebIDL spec "developer gotcha" - not a Good Thing.

[bzbarsky]

This was a conscious design decision. The idea was that in general when using a dictionary in trailing position (i.e. as an options object), omitting it and passing empty object should have equivalent behavior. Since the default argument behavior is to not be optional, if we didn't have this spec requirement people would be commonly writing specs which had required option object arguments. And most option objects don't have the complicated "must provide at least one of these properties" behavior mediacapture does.

So the decision was made to nudge people in the right direction in the simple cases. The complicated cases get the behavior they want for free anyway, because they have to consider the case of {} getting passed and the behavior of not passing the argument is exactly the same as passing {}. So they don't even have to write extra prose to get the effect they want.

[alvestrand]

My complaint is the implicit statement made:

apiFunction(dictionaryType? argument)

states "You can omit the argument". Then prose will later say "you have to include the argument, and it has to be non-empty".

In this case, the behavior on missing argument and the behavior on empty argument is intended to be the same: TypeError - "you can't use this here".

If the WebIDL rule had been "CONVENTION: if an empty argument won't cause a TypeError, the argument MUST be marked Optional, to inform the user that an empty argument and an omitted argument has the same result", I would have no problem with it at all.

But that extra, misleading question mark bothers me.

[bzbarsky]

"CONVENTION: if an empty argument won't cause a TypeError, the argument MUST be marked Optional, to inform the user that an empty argument and an omitted argument has the same result"

The IDL layer has no way to tell whether an empty argument will cause a TypeError. How would this convention actually be enforced in practice?

But more to the point, for dictionaries the idea is that an empty argument and an omitted argument always has the same result. This doesn't depend on whether the empty argument causes a TypeError or not.

I assume you meant optional whenever you wrote ?.

[alvestrand]

@bzbarsky yes, I meant "optional", not "nullable". Sorry about that.
My opinion: The difference between a convention and a rule is that enforcement of the convention can be done by humans, while rules should be enforced by machinery (and therefore need to be written in ways that allow machinery to enforce them).

[bzbarsky]

Right, the whole point is that we did not expect humans to be any good at enforcing the optionality of trailing dictionaries (and in fact had seen them fail to do so repeatedly), which is why we made it a rule and not a convention.

[alvestrand]

OK, the point has been raised, and I think we've shown that there's no consensus for change. Closing.

[ayg]

So DOM has

  void observe(Node target, MutationObserverInit options);

which is invalid per this. It looks like Edge and Chrome implemented it per spec, and Firefox ignores the spec and makes it optional. This does not suggest to me that this way of encouraging good spec-writing practices is particularly effective, insofar as one of our largest and best-maintained specs just ignores it, and the one implementation that had the opportunity to notice (Gecko's WebIDL parser enforces validity) silently changed the IDL to make the warning go away. DOM has had this IDL line since at least 2012 and presumably forever, and Gecko made it optional also since 2012.

So while I don't argue with the goals of this restriction, I don't know if it's actually doing enough to warrant the confusingness of having a spec's IDL call something "optional" when really it's not. Where are these validity requirements expected to be enforced such that spec writers will notice or care?

[foolip]

@annevk

The spec has "If none of options’ childList, attributes, and characterData is true, throw a TypeError" which is hit for observe(target, {}) and would as well if the argument were allowed to be omitted. Loosening the IDL to allow things that always throw a TypeError doesn't seem very useful.

How to weigh this against spec authors that make dictionaries required when passing {} actually does something useful I don't know.

[bzbarsky]

silently changed the IDL to make the warning go away

That should not have happened; it's totally my fault as the reviewer of that patch that it did.

The good news is that this happened 4 years ago, as you note (when we initially converted MutationObserver to use WebIDL at all); I believe our IDL reviews have gotten a lot better since then.

Note that you've found one case where something slipped through the cracks, but if I recall correctly we've had multiple cases where we caught specs messing this up and got them fixed...

Loosening the IDL to allow things that always throw a TypeError doesn't seem very useful.

Just so we're clear: this is a really weird dictionary. It has all sorts of interdependency requirements on its members that can't be expressed declaratively in the IDL and hence have to be expressed in prose.

All IDL can enforce for a dictionary like this, and tries to, is that behavior is identical for passing undefined and passing {} (as long as there are no getters on Object.prototype, of course).

[bzbarsky]

Oh, and given that it's not clear to me what "Firefox ignores the spec and makes it optional" means in practice, if passing undefined just ends up throwing a TypeError anyway. That is, in this case the observable behavior is the same whether the dictionary is "optional" or not, if non-optional trailing dictionaries are supported...

[annevk]

Happy to change DOM.

I thought I filed a bug against IDL for this at some point, but all I could find that was a close match was https://www.w3.org/Bugs/Public/show_bug.cgi?id=23604 filed by bz.

[ayg]

@bzbarsky Well, it means that MutationObserver.prototype.observe.length is 2 instead of 1, which fails a wpt test and is how I noticed in the first place. I do not think this has big implications for web compat, though. :)

If Anne is happy to change DOM, then I don't have any objection to closing this and leaving WebIDL as it is, although I think it's slightly sad that the IDL will be slightly more confusing. If a confusing "optional" in an IDL were the worst problem with the web platform, we'd be in great shape. ;)

[annevk]

I updated DOM and filed a bug against the IDL parser used by bikeshed. Is there a WPT bug?

[ayg]

web-platform-tests/wpt#3605

[bzbarsky]

It means that MutationObserver.prototype.observe.length is 2 instead of 1

Ah, indeed.

For what it's worth, I can see a world in which having a non-optional dictionary affects .length but does not affect the argc check, effectively treating it as optional for all purposes except .length calculation. That is in fact what I proposed for the "any" type in https://lists.w3.org/Archives/Public/public-script-coord/2014JanMar/0176.html and is a sensible thing to do.

[mgiuca]

Sorry to necro this issue, but I just want to put in my opinion that this requirement should change in Web IDL. The intention of the requirement is good, but having it as a strict rule is silly. You should give spec authors good advice but ultimately the leeway to make sensible decisions.

This came up recently with regards to navigator.share (see w3c/web-share#47). As with MutationObserver, we take a dictionary with all fields optional, but at least one must be supplied. The prose text says to throw a TypeError if all fields are missing. Therefore, making it optional has no observable effect (other than changing the .length). But it's confusing for anyone casually reading the spec to see "optional" there.

Fixing this is a simple matter of changing "must" to "should" in the following paragraph (bold text indicates my change):

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 should be specified as optional.

Another variant, if you want to be stricter but achieve the same effect, is to allow a non-optional dictionary only if {} always results in a TypeError (as is the case with MutationObserver and navigator.share):

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 operation is not guaranteed to throw a TypeError when the argument is the empty dictionary value, and the argument is either the final argument or is followed only by optional arguments, then the argument must be specified as optional.

[annevk]

It's very much a special case though and in case of MutationObserver I don't think requiring at least one member would even help that much since we'd still have to check it's one of the members that can be supplied standalone (e.g., only specifying subtree still throws).

So given that I don't think there's enough to overturn this. You might want to point it out in a note though.

[mgiuca]

I don't think requiring at least one member would even help that much since we'd still have to check it's one of the members that can be supplied standalone

I'm not exactly sure what you mean (but I take it you mean "the logic of when it's a TypeError is non-trivial"). That shouldn't matter... all that matters is: "is it a TypeError in the case where you pass {}?". If the answer is "yes", then I think this rule shouldn't apply. Given that the spec itself states that "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," I feel it shouldn't apply in a case where authors are explicitly forbidden from using the dictionary's default values.

It's very much a special case

It is (though we now have at least 2 examples). But it's not like we are writing a special case in code that's going to need extra edge cases, tests, etc, in an implementation. This isn't a spec, it's a meta-spec. It should give us (as spec authors) the tools to do our work, and guidelines on how to do it well, not constrain us when we hit a special case.

You might want to point it out in a note though.

I've got a note about this in Web Share. I was hoping to remove it though :)

[annevk]

I'm saying that we don't have two cases, since even if we made {} throw, it wouldn't simplify the logic of MutationObserver at all.

[foolip]

There's also the getUserMedia case. In none of the cases do I think it would be a simplification in spec prose, since the steps that cause TypeError to be thrown still have to be there. However, in all three cases the specs initially didn't use optional, so at the very least what Web IDL requires isn't what spec authors are born believing.

I think downgrading to a "should" would be fine, but cases where the .length property is already the same everywhere are probably best left alone still.

[annevk]

I think downgrading that to a "should" is a rather bad idea. If you want a different processing model, okay, but then we'd have to make a bunch of changes. If you just want to make "optional" optional, but keep the processing model identical, I don't think that's okay.

[foolip]

What other changes would be needed? https://heycam.github.io/webidl/#es-overloads already throws TypeError at step 4 if required arguments are missing. Perhaps it's more involved than that, but it does appear before any dictionary-specific handling at least.

[mgiuca]

I'm saying that we don't have two cases, since even if we made {} throw, it wouldn't simplify the logic of MutationObserver at all.

Correct. The proposal isn't for simplifying prose. It's for removing confusion from the spec (explicitly stating "optional" on a parameter that actually isn't), requiring non-normative explanations such as the one I added in Web Share.

We should think of specs as documentation for the web platform. They aren't just some computer code that's technically correct if you consider all the logic in aggregate. They should make sense to people who glance at them to get clarification on what's allowed and what isn't. Having "optional" on an argument that always throws a TypeError if not supplied is deliberately misleading. Having a rule that forces me (as a spec author) to do this is deliberately forcing spec authors to be misleading.

If you want a different processing model, okay, but then we'd have to make a bunch of changes.

I don't understand why we have to change the processing model. Everything should just work if you have a method with a non-optional dictionary with no required fields (just as everything works if you have a non-optional dictionary with 1 required field).

[annevk]

I don't think it's acceptable that we change the processing model (and thereby complicate it even if there's no observable effect) just for documentation purposes (which wouldn't even clarify that much since you still have to look at the prose anyway).

[foolip]

@annevk, no change to the processing model is being proposed, because none seems to be required. If we merely allowed arguments to be non-optional when omitting them throws TypeError, are any other changes needed?

[annevk]

That seems like a change, because thus far that's not been possible. It also creates the problem where folks forget to mark it as optional which is why we started to require this to begin with. I'm not at all convinced this is a good idea.

[mgiuca]

That seems like a change, because thus far that's not been possible.

It isn't a change to the processing model, just the rules about when you can and can't use optional. (e.g., it doesn't change the built-in logic of how WebIDL converts JavaScript objects into dictionaries).

It also creates the problem where folks forget to mark it as optional which is why we started to require this to begin with.

We already have that problem today: see the three previous examples where the authors (yourself and myself included) forgot to mark it as optional. Having a guideline around this is helpful for educating spec authors. Making that guideline a hard-and-fast rule doesn't really solve anything.

The one reason I can think of for keeping it as a "MUST" is that it opens the possibility of writing an automated tool (perhaps idlharness or some other existing tool that processes IDLs) to actually mark it as an IDL error (which it could do just by parsing the IDL and not having to know anything about the prose). If this was a "SHOULD" requirement, that automated checking wouldn't be possible. Is that something we really need though?

[foolip]

A check like that was done in plinss/widlparser#16, and I think catching the cases where the dictionary really could be optional but isn't is valuable. What I don't know is how often that happens, compared to people being annoyed/confused in the cases where it actually isn't optional but still has to be marked as such to satisfy the requirement.

[annevk]

Having a guideline around this is helpful for educating spec authors.

No it's not. The only reason it was caught is because it's required to be optional. (At least Firefox's IDL code checks for that.)

[domenic]

I am also against changing this.

I think the issue is that we don't have any good tools in the JavaScript toolbox for expressing the requirement "please fill out at least one of these values". Speaking very generally, you're supposed to use parameters for required values, and dictionaries for optional values. But the use case presented here is neither of those.

In that case, when people choose to use dictionaries, they are going against how we have designed them in Web IDL. It's a weird fit, and will run into some semantic problems. I think it's OK for it then to be a bit awkward and exceptional and require workarounds. We should not weaken the concept of dictionaries as they are originally designed to fit this rare case where people are using them unusually.

Note that I don't think we have a good concept in Web IDL, or even in JavaScript, for expressing the "please fill out at least one of these values" idiom. (You could imagine a language with a type system that can express such constraints, but JavaScript is not it.) So abusing dictionaries for it is probably the best solution; I'm not saying you should do something else. I'm just saying we should not cater to it.

[foolip]

@domenic, with "parameters for required values, and dictionaries for optional values," do you mean that required dictionary members are also an oddity?

[domenic]

Well, that was one of the nuances I was leaving out while "speaking very generally". Sometimes indeed we have required dictionary members or optional parameters, for various reasons.

[mgiuca]

I think the issue is that we don't have any good tools in the JavaScript toolbox for expressing the requirement "please fill out at least one of these values". Speaking very generally, you're supposed to use parameters for required values, and dictionaries for optional values. But the use case presented here is neither of those.

I really don't understand this. Nowhere in WebIDL can I see any indication of this philosophy. It just says "A dictionary is a definition ... used to define an ordered map data type with a fixed, ordered set of entries, termed dictionary members." It goes on to say "One use of dictionary types is to allow a number of optional arguments to an operation without being constrained as to the order they are specified at the call site." I don't see it being a "weird fit" to have a dictionary with optional fields but at least one of them is required. That's really just an additional constraint; as programmers / API designers we can always add extra constraints to function inputs that can't be expressed declaratively by the programming language's data type syntax.

I feel like this rule makes about as much sense as saying that all integer arguments must be optional, because omitting them should be the same as passing 0. Now I write a method with an int argument that must be non-zero, and in the method I check it and throw a TypeError if it's <= 0. You tell me I must declare that argument as "optional" even though it will always be a TypeError to omit it. I say "but it must be explicitly supplied a non-zero value" and you tell me that it's a "weird fit for the int type" and I "shouldn't be using int, because that's not what int was originally designed for (even though we don't have a data type for non-zero int)".

I do buy the argument that we want to enforce this rule at some level (e.g., in Firefox's IDL parser, idlharness tests, etc) and I think it's okay to acknowledge that we're going to mandate "optional" even where it doesn't make sense in order that the common case be enforced. (I disagree that this is so important, but if that's the reason then I'll shut up and go along with it, but then I think Web IDL should acknowledge this friction and explain that it's enforced for pragmatic reasons.) It's just not satisfactory to say "you're holding it wrong" to people who are writing specs with this type of requirement.

[domenic]

I tried to be extremely clear that you are not holding it wrong.

[ByteEater-pl]

How about introducing a synonym for optional that wouldn't be confusing in such cases and add a guideline in WebIDL's prose on intended usage?

[annevk]

#602 goes somewhat in that direction. Renaming optional seems like too big a change

[domenic]

The particular pattern of "one member is required" has come up enough times that I think we might want to make it first-class. Follow the discussion at #903