-
Notifications
You must be signed in to change notification settings - Fork 29
document that strings-as-enums are kebab-case #165
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Chris de Almeida <ctcpip@users.noreply.github.com>
sffc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This directly contradicts the ECMA-402 style guide.
https://github.com/tc39/ecma402/blob/main/docs/style-guide.md#identifiers-defined-by-ecma-402
|
Doing some archaeology, this ECMA-402 style was approved in the October 2019 TC39 meeting. PR: tc39/ecma402#377 |
|
Ah, thanks for the pointer. That's... unfortunate. I don't understand the decision to go against W3C guidelines within 402 and I don't think it makes sense to continue to violate those for new APIs. |
|
The reasons behind the ECMA-402 style are well documented in the style guide doc. There were few if any string enumerations in ECMA-262 at the time, but since then, this style was adopted by Temporal (the value of the roundingMode enum). |
|
There was already But also, the weight of precedent on the web platform is very heavily for kebab-case, following W3C guidelines. Off the top of my head, at least It's not like the web platform is entirely consistent either, but I'll argue it's best not to intentionally deviate except in cases where there is specific reason to do so. |
|
The To be clear, I'm not super strongly attached to the ECMA-402 convention, despite being the one who proposed it. There were a number of reasons camel case had technical merit, specifically the ability for the enumeration values to be used as identifiers, and we didn't put a lot of weight on matching W3C style. Perhaps we should have put more weight on matching W3C style at the time. But, now that we do have that style, I think it's just as important for ECMA-262 to be self-consistent with ECMA-402 as it is to be consistent with other web platform APIs. |
|
That's a reasonable position which I disagree with. Many more JS programmers are going to run into fetch's I've increased the timebox for the agenda item so we can talk about it in plenary. |
|
One place where this seems quite relevant is the If this normative convention is adopted, should the Amount option values match Intl.NumberFormat, or use kebab-case ( |
|
These rules are all intended to be general guidelines, not hard-and-fast rules. In cases where there's strong reason to deviate that's fine. In that case I'd say the fact that these specific enum values are also used in different APIs is strong reason to match the existing APIs instead of following the guideline. |
Clarify the kebab-case convention for enum values.
|
Per discussion in plenary: this applies to 262, but not 402 except in cases where TG2 thinks it should. Also, in the case of Temporal and Amount and any other APIs which are adopting existing enums which currently violate this convention, they should match existing spelling rather than trying to keep to this convention. |
should this be documented here then? |
|
Added a sentence to that effect:
|
Co-authored-by: Chris de Almeida <ctcpip@users.noreply.github.com>
normative-conventions.md
Outdated
|
|
||
| ## Multi-word strings which are serving as enum values should be kebab-case | ||
|
|
||
| I.e., lowercase and dash-delimited. For example, `Atomics.wait` returns one of the three strings `"not-equal"`, `"ok"`, or `"timed-out"`, and `Uint8Array.prototype.setFromBase64` takes an argument with possible values `"loose"`, `"strict"`, or `"stop-before-partial"`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should actually restate the convention in the paragraph here. I don't want the convention to only be described in the section heading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done:
When a string is serving as an enum value, in any position (including arguments and return values), that string should be lowercase and words in it should be separated by dashes rather than spaces.
|
|
||
| When a string is serving as an enum value, in any position (including arguments and return values), that string should be lowercase and words in it should be separated by dashes rather than spaces. For example, `Atomics.wait` returns one of the three strings `"not-equal"`, `"ok"`, or `"timed-out"`, and `Uint8Array.prototype.setFromBase64` takes an argument with possible values `"loose"`, `"strict"`, or `"stop-before-partial"`. | ||
|
|
||
| The content between the dashes should generally be alphanumeric. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this add? What's the alternative? Say "as opposed to ...".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a request from Mark. As opposed to, I guess, random other punctuators? I would prefer to leave this as-is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just can't imagine what someone would want to put there that's not alphanumeric. If this isn't a reasonable expectation, I don't see why we would want to say anything about it. We also don't want to put ZWNJs between the hyphens, but nobody wants to do that, so I don't think it needs mentioning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, this was a specific request in plenary. I'm not going to remove it without going through plenary again, and I'm not going to do that. It's not hurting anything.
| When a string is serving as an enum value, in any position (including arguments and return values), that string should be lowercase and words in it should be separated by dashes rather than spaces. For example, `Atomics.wait` returns one of the three strings `"not-equal"`, `"ok"`, or `"timed-out"`, and `Uint8Array.prototype.setFromBase64` takes an argument with possible values `"loose"`, `"strict"`, or `"stop-before-partial"`. | ||
|
|
||
| The content between the dashes should generally be alphanumeric. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| When a string is serving as an enum value, in any position (including arguments and return values), that string should be lowercase and words in it should be separated by dashes rather than spaces. For example, `Atomics.wait` returns one of the three strings `"not-equal"`, `"ok"`, or `"timed-out"`, and `Uint8Array.prototype.setFromBase64` takes an argument with possible values `"loose"`, `"strict"`, or `"stop-before-partial"`. | |
| The content between the dashes should generally be alphanumeric. | |
| When a string is serving as an enum value, in any position (including arguments and return values), that string should be lowercase and words in it should be separated by hyphens. For example, `Atomics.wait` returns one of the three strings `"not-equal"`, `"ok"`, or `"timed-out"`, and `Uint8Array.prototype.setFromBase64` takes an argument with possible values `"loose"`, `"strict"`, or `"stop-before-partial"`. | |
| Content between hyphens should be alphanumeric. |
hyphens !== dashes and 'generally' was not doing any work here 😊
edit: also removed "rather than spaces" because spaces is not the only alternative. (but this is more of a nit than anything else)
edit 2: removed definite articles which were also not doing any work
No description provided.