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

"Make method arguments optional if possible" section has bad advice #437

Open
bakkot opened this issue Apr 22, 2023 · 3 comments
Open

"Make method arguments optional if possible" section has bad advice #437

bakkot opened this issue Apr 22, 2023 · 3 comments

Comments

@bakkot
Copy link
Contributor

bakkot commented Apr 22, 2023

This section says

The API must be designed so that if an argument is left out, the default value used is the same as converting undefined to the type of the argument. For example, if a boolean argument isn’t set, it must default to false.

and I think that's basically wrong? A string-taking API should not be required to use the default value "undefined", which is what you get when converting undefined to a string (unless this document is using "converting" in an unusual way, i.e. not just '' + undefined). Similarly, NaN is almost never the right default for a Number-taking API, and 0 is only sometimes a reasonable default for an integer-taking API, which are what you get when converting undefined to a Number or integer in JS.

Rather, the way I'd put it is that an explicit undefined should be treated exactly the same as an option being absent1. And, in addition, false should be the default for boolean parameters. This doesn't tell you anything about the appropriate default for other type, though.

(That rule appears to have been introducing in #228.)

Footnotes

  1. except in the (very rare) cases where undefined is a coherent value to use - for example, in JS the cause parameter in error constructors can take any JS value, including undefined, so that's one of the very rare cases which breaks this rule.

@bathos
Copy link

bathos commented Apr 22, 2023

[...] which are what you get when converting undefined to a Number or integer in JS.

I’d have imagined “converting” here referred to ES → Web IDL conversion, likewise type of the argument and boolean in their Web IDL senses. If that’s accurate, then it doesn’t follow that converting undefined to the type of the argument always aligns with outcomes like "undefined" and NaN associated with the basic ES language type conversions.

(I agree with the conclusion about this passage regardless, but figured it was worth pointing out that this reads pretty differently if type means Web IDL type and not ES language type. As for whether that was the intention, I’m not certain.)

@bakkot
Copy link
Contributor Author

bakkot commented Apr 22, 2023

Ah, that's probably correct. Looks like it works out the same in practice - converting undefined to a DOMString does still give you "undefined", e.g..

@bathos
Copy link

bathos commented Apr 22, 2023

Yep, but if the type were DOMString?, the “correct” default (for “undefined treated exactly the same as an option being absent”) would be = null. The original text makes a bit more sense to me with that reading, but it’s confusing and your suggested replacement gets the key point across better.

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