-
Notifications
You must be signed in to change notification settings - Fork 25
use consistent semicolon elision rules #26
Comments
Please do not require us to use semicolons just for class fields and static properties. It might be true that including semicolons can speed up the parsing of the source, but that is true in a lot of other places as well. Yet, we don't require them. There are a huge number of people writing their javascript without semicolons, and requiring them in just a few places will increase the cognitive load of reading and writing the code. Parsing the code without semicolons isn't a problem, babel for example did this up until very recently. The decision to require semicolons is a pure performance question. I don't think it's fair at all to force this on to the developers. If it's that important to speed up parsing, we should require semicolons everywhere. I really don't think the benefit outweighs the negative here, and I think it's very unfair to force this upon people who normally doesn't use semicolons in their javascript. |
please be consistent with the core language specification |
Same feeling. All my recent codebase has not a single semilcolon (no, I don't have a single for loop), and now I have to have some. This feel like going backward. This should be discussed in ESDiscuss probably... |
yeah, it’s a matter of consistency/predictability. if you’re accustomed to ASI, a place where it doesn’t work is very surprising and therefore bad language design. |
I opened #25 because I don't generally use semi-colons and I wanted clarification. I would also prefer them not to be required. |
Where did you people come from? Please stop. The committee will not accept a proposal that requires a parser to either rewind the token stream or build multiple potential parse results simultaneously. In fact, a requirement for this was recently removed: I pray I never have to read any of the code that commenters in this thread have written. |
Yes, it seems like the most likely outcome of these complaints will be to sink the class fields proposal, if the evidence is that the community doesn't want it in a form the committee will accept. Be careful. |
First off: Thanks for the feedback everyone (no like really, I think it's important that we talk through this). It's a tough call to make here for sure. I want to reassure everyone that this isn't a decision we took lightly (and if we can find a reasonable way out of this, I'm definitely open to considering options). The choice to require semicolons was made based on technical considerations exclusively and was not an effort to enforce any particular style, so I hope that's clear to begin with. As mentioned in #25, there is a real ambiguity here that affects parser performance. Parser performance, in turn, affects the willingness of people within TC39 to accept the proposal and the willingness of people who implement JS runtimes (like browser vendors) to implement this feature. The reason the people within TC39 have an opinion on this is because this kind of parser performance won't just affect usage sites of this particular feature, but the parser as a whole. In other words: A misstep here could affect JS web performance as a whole. I'm empathetic to the concerns here, but I hope everyone understands the rock/hard-place situation we're in with this detail of the proposal. Please, if you feel strongly about this issue, try to help constructively by offering alternatives that address the core issue. I can't promise that just any alternative will succeed, but all will be considered if they make sense and the trade-offs seem reasonable. |
OK, so dumb question: using the same ASI rules as elsewhere would fail… why? |
Consider this JS of today: var obj = {}
var a = obj
[42]
console.log(a) // prints "undefined" Consider this example with this proposal: var obj = {}
class Foo {
a = obj
[someComputedPropertyName]() { ... }
} In the former, we can interpret the "var a = obj\n[42]" as a member expression, in the latter we could not. |
ok. hmm. two ways out:
both surprising, but better than requiring semicolons unlike everywhere else except in for loops. |
I don't know if it's true, but I have the feeling that classes in JS are especially helpful for newbies. Classes might be familiar as they know them from other languages. The explanation "In the former, we can interpret the 'var a = obj\n[42]' as a member expression, in the latter we could not." makes sense, but is really really hard to understand for anyone new or not deeply involved in JS development. Requiring semicolons here is a hard thing to remember. It's that kind of thing you will always accidentally do wrong, even when you know how to do it right. It could also heat up the "Do I need semicolons in JS" discussion, again. That said, I don't know if "Class Fields and Static Properties" in their current proposal are good for the language. I would really like to use them, but the semicolon thing could have a negative impact. At least the latest change in Babel shows that there's little understanding on what's going on. |
@flying-sheep Your first option is already the case thanks to ASI. You should only need to include a literal semicolon in places where it would cause a syntax error if you did not. This is okay: class a {
a = b
c(){}
} This is not okay: class a {
a = b
[c](){}
} This is okay: class a {
a = b
;[c](){}
} But please don't write it like that. |
there’s no reason apart from the grammar complexity to disallow the second one. it can only be interpreted in one way. |
@flying-sheep I am aware, and that's what I was saying would never be accepted by the committee. ASI rules insert a semicolon when the following token would cause a parse error. So in that example, the semicolon would be inserted before |
The "For the parser" argument really makes sense here. Omitting semicolons using ASI is nice and all but they are part of ECMAScript and if omitting them in this case would slow down the parser significantly, is it really worth it? My preference would be to drop ASI completely and require semicolons everywhere, but that's never going to happen so I guess I agree with the others above and have consistent language design at the cost of parser performance, altho if I understand @michaelficarra correctly that's also not going to fly, so I guess we're stuck... |
class a {
[c](){}
} already a thing? It seems like that's the real issue here. |
@mjackson Just a normal ES6 computed property method. |
Yes, it is already a thing. |
@loganfsmyth I know this ship has sailed, but there's nothing normal about An object property definition will always be preceded by either Besides, method definitions are not properties. In fact, there are a few important differences:
Besides this, consider that the list of places where you currently must use semi-colons in JavaScript is very short. It includes:
If requiring a |
Method definitions are properties.
Or
It can be automatically inserted for static property names. How often are you using computed property names? |
yeah, i think requiring it before computed class property names is just as surprising as requiring it after class property declarations, but will end up making things less ugly. in all JS i ever wrote, i doubt i started more than 2 lines or so with i also think that computed property names will be rare apart from |
Inside a class body, method definitions are declarations, not object property definitions. i.e. they aren't followed by a
Never. And I'm certainly not using computed names for method declarations. That's why this is so annoying. Because in order to use this feature I'll have to use semi-colons, which I literally never have to use anywhere else.
I'm referring to babel/babel#3225 where a change was recently made that causes an error to be thrown when class properties do not have a semi-colon. If they can be automatically inserted, that change should be reverted. But it looks like it was actually considered a bug in the way Babel interprets this spec. |
If removing semicolons from this proposal will slow down its shipping by TC39 - ill prefer to use semicolons. Its better to have good language feature, with small part of code style you dont like, than dont have anything at all. Javascript does not have consistent code style, such as Java's and its not so good to discard anything because you dont like its style. So if TC39 can accept this proposal without semicolons - its OK, if it will discard it - its better to stay with semicolons. Code style should not be the main criteria at all! |
it’s better to have a good language than to slap on features with no perspective or consideration. that would be how you get PHP and how JS got into the state it was in before ES2015. fucking variable hoisting, type coercion, all that bullshit. no, we don’t want to go back there. also if there is anything wrong with my proposal above i want to hear it. a good advice for any conversation is:
this thread has seen only rudimentary discussion but many people chiming in with emotion and little to say. SO. solutionsrated by cognitive load, inconsistency in language design, and code style compatibility. those three are to consider because 1. you don’t want programmers to think too much about syntax while coding. it’s a tool and should stay out of the way. 2. good language design is as internally consistent as possible. 3. people already write code in this language, so suddenly “blessing” keep semicolons
allow everything
complex grammar is apparently a deal-breaker, so -∞ points introduce special clarification rule (semicolon between class property and computed-name method)
in the end:
a winner by points is the special clarification rule i hope my point scheme was not too biased… but i wouldn’t have rooted for it if i didn’t weigh the pros and cons already and came to the conclusion that it was best |
I think @mjackson brings up an extremely important point:
This requirement adds complexity and edge cases to existing aspects of the language's ASI that we have to write books and tutorials about and make the language altogether more difficult to learn. I agree its important to require syntax lend itself to parsing performance, but this solution seems to be covering for awkward inconsistencies in the class syntax with the rest of the language. |
Another syntax error for you semicolon haters: class a {
a = b
*c(){}
} 😜 The "very short" list of problematic line start characters now has seven members. |
Exactly, I never intended to imply anything other than that the normal ASI rules apply, as quoted. Of course, I will continue to argue that this confusion is a very good reason for not having this be a keyworldless declaration form. It would be so much better if a keyword was required: class Foo {
field bar = baz
field [bing[
} |
@allenwb: A prefix keyword would help with properties that follow other properties, but given generator methods and any computed members we'll still have the same issues. @michaelficarra: Good catch. Main point is that it's a parse error, though |
@jeffmo |
My mistake, thanks for clearing it up everyone. |
for all you hipsters or 's, I'm not sure which is appropriate here, who are "smart" enough to remember all the use cases where it is a must to use semicolons, and then "smart" enough to avoid those use cases just to avoid typing an extra character, you should have a look at destructuring syntax, in particular "assignment without declaration"; Below is in reference to @flying-sheep #26 (comment) on how to easily avoid breaking syntax if not using semicolons. let bleep, bloop
const str = 'bleep-bloop'
const obj = str.split('-').reduce((acc, key) => ({...acc, [key]: key}), {})
({bleep, bloop} = obj)
//Cannot read property 'bleep' of undefined let bleep, bloop
const str = 'bleep-bloop'
const arr = str.split('-')
([bleep, bloop] = arr)
//Invalid attempt to destructure non-iterable instance Looks like there might be more uses cases now if you all enjoy writing es6, which is what I think this debate is about...better start exercising that pinky finger. |
Thanks for not breaking ASI, @jeffmo. 👍 |
@dtothefp don’t start calling people names. about your points:
|
For posterity, this discussion is missing a couple more cases, namely, uninitialized properties named class A {
get
x
} is a syntax error. class A {
static
x
} is a legal declaration of an uninitialized static property named So if you want to omit semicolons, the rule is something like "whenever starting a line with punctuation or the previous line was an uninitialized property named 'get', 'set', or 'static' ". 😐 |
no, that rule is always, i.e. if you put semicolons in your examples, that doesn’t change anything, it’ll be a syntax error / static initialization anyway. |
@flying-sheep, why? This looks like a syntactically valid class declaration with two uninitialized fields to me; am I missing something? class A {
get;
x;
} |
@bakkot I believe the examples you've given should be a syntax error. From the ASI rules,
Notice my emphasis. A semicolon would not be inserted after |
@michaelficarra, I agree; that's my point. (Well, except that my second example is a syntactically valid static field declaration.) But with semicolons inserted manually, they should be fine, I think. That is, I think this is valid syntax: class A {
get;
x;
} So, if I am understanding correctly, this is another place where you can't get away with skipping semicolons. |
Yes, that's right. |
sounds a lot like "Don't make beautiful and more readable code because if you use that clean style to write awful code that would confuse most people and any style guide should prohibit, it can end up confusing the runtime too." |
class A { x = 0; } Eww, gross. class A { x = 0 } Beautiful! Totally worth all the refactoring hazards. I only need to look ahead for 1 of 10 problematic tokens every time I make a change. |
In real code, in large files, yes all these crufty archaic semicolons are just unnecessary noise. Once you work in semicolon-free js for a couple days, the perspective switches from "why shouldn't we?" to "why should we?" with thoughts like: "Why are people adding these unnecessary semicolons everywhere when not one place here needs them?" Because you just don't run into these scenarios in real code and even if you did they're things that become immediately apparent and easily fixable. |
That's a very subjective opinion and controversial debate that we ABSOLUTELY MUST NOT get into in this thread. Just stop. ASI rules (whether using or omitting) both require memorizing a list of rules, and applying them in your code. A linter can help you remember, rendering the question of "which one's harder to remember" moot. Whichever you use, any additions to the language will not make this particular debate any worse by changing those ASI rules - that's simply a nonstarter. If a given approach (using vs omitting) ends up being trickier with a syntax addition when following the existing rules, TOUGH. Adapt, or change your approach. |
@ziemkowski I hope you think of me every time you look at class A {
x = 0
[Symbol.iterator]() { /* ... */ }
} or class A {
x = 0
*generatorMethod() { /* ... */ }
} and say to yourself, "what's wrong?". edit: I apologise to those who are following the thread for the noise caused by my responses to @ziemkowski's trolling. I was not in the mood for this kind of ASI snobbery. |
@allenwb, my reading of ASI is that an offending token must be one that is "not allowed by any production of the grammar", but here |
@bakkot oops, I think you're right. I forgot about the "any production" restriction. In that case ASI produces: class {
get
x; } which is a non-ASI-correctable syntax error. I really dislike these keyword free property declarations (same for keyword free privates). It would all work so much better if property declarations started with a contextual keyword such as class {
public get /* semi-inserted here*/
public x /*semi-inserted here */
} or class {
public get, y /* semi-inserted here */
} That was the sort of thing we were expecting when we made |
my JS style elides semicolons, and method literals in class declarations have no trailing delimiter neither.
therefore requiring semicolons results in a idiosyncracy compared to the rest of the language’s grammar.
i’m aware of #25 which was about a quick fix for ambiguities by requiring them temporarily. this bug is about the backwardds-compatible idiomatic final grammar which will allow eliding semicolons.
Updated status:
this was a misunderstanding. @jeffmo explained it here.
ASI is in effect here, and babel won’t require semicolons anymore soon
The text was updated successfully, but these errors were encountered: