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

Getting rid of optional #905

Open
annevk opened this issue Aug 4, 2020 · 9 comments
Open

Getting rid of optional #905

annevk opened this issue Aug 4, 2020 · 9 comments

Comments

@annevk
Copy link
Member

annevk commented Aug 4, 2020

If we do #60, should we get rid of optional and instead use defaulting as sole mechanism?

  [NewObject] Document createHTMLDocument(optional DOMString title);

would become

  [NewObject] Document createHTMLDocument(DOMString title = undefined);

and

  void observe(Node target, optional MutationObserverInit options = {});

would become

  void observe(Node target, MutationObserverInit options = {});

(And if you need to distinguish between not given and undefined you use overloading, as XMLHttpRequest does for open().)

cc @saschanaz @tabatkins

@saschanaz
Copy link
Member

Sounds just right, no objection here.

@domenic
Copy link
Member

domenic commented Aug 4, 2020

This is kind of nice. And argues more strongly for undefined instead of void.

@tabatkins
Copy link
Contributor

I don't have a strong opinion here; I like the optional notation, but I'm fine with getting rid of it in favor of just being more JS-y.

@farukrana
Copy link

All good

@saschanaz
Copy link
Member

saschanaz commented Aug 7, 2020

A late question:

void foo(boolean abc = false); // okay, `false` is a boolean
void foo(float abc = 1.0); // okay, 1.0 is a float, sure.

void foo(DOMString abc = undefined);
// well, is `undefined` a DOMString? 🤔

I'm also a bit worried that the implementation would be different while the syntax is similar: DOMString bar = "str" is always a string while DOMString abc = undefined is something like Optional<DOMString>. Okay, same as the current syntax.

@domenic
Copy link
Member

domenic commented Aug 7, 2020

I agree that this is not like other defaults. But I think that's OK.

Another way to think about it is to realize how weird the current optional keyword is. In foo(boolean abc = false), the abc argument is "optional": the web developer doesn't have to pass it in. But we don't use the optional keyword for that---because it's redundant with the default value. Similarly, if we can use undefined as a default value, then adding the optional keyword becomes redundant for the foo(DOMString abc = undefined) case.

In other words, it is good to align the case of "allows no argument and gets a default value of false" with "allows no argument and gets a default value of undefined".

@saschanaz
Copy link
Member

But we don't use the optional keyword for that---because it's redundant with the default value.

Currently we do use optional keyword for every optional argument, and specifying a default value without optional is a syntax error.

ArgumentRest ::
    optional TypeWithExtendedAttributes ArgumentName Default
    Type Ellipsis ArgumentName

@domenic
Copy link
Member

domenic commented Aug 7, 2020

Ah, sorry, I was confused by your example and thought it was currently Web IDL.

Well, the larger point still stands. I think it will be nice to align both types of optional argument to use argName = default value, instead of one using optional argName = defaultValue and the other using optional argName.

tabatkins added a commit to tabatkins/webidl that referenced this issue Aug 7, 2020
…as a const value and forbid it from being an argument type.
domenic pushed a commit that referenced this issue Aug 17, 2020
Fixes #60. See potential follow-up in #905.
@annevk
Copy link
Member Author

annevk commented Mar 6, 2021

I guess one aspect that is a little weird here is that = undefined would also end up changing the type (similar to optional). As in, if T is a type that is not undefined, T x = undefined means that x can be undefined or T, which you would normally have to write as (T or undefined) x = undefined. That might be reasonable, but it's something to keep in mind.

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

No branches or pull requests

5 participants