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

The semicolon is YAGNI #1573

Closed
wants to merge 2 commits into from
Closed

The semicolon is YAGNI #1573

wants to merge 2 commits into from

Conversation

thepeoplesbourgeois
Copy link
Contributor

Since JavaScript is already interpreting newlines as the ends of developers' statements, and since developers shouldn't write more than one statement per line, semicolons are fundamentally unnecessary to a working JavaScript program. Much like the cape on a superhero's back, they are a holdover from a bygone era that serve only as a means for modern developers to trip over themselves.

If anyone has an example of any instance when the lack of a semicolon has caused unintended behavior in a modern browser, I might reconsider my stance, if Babel ever ceases to insert its own semicolons into its transpiled code

Since JavaScript is already interpreting newlines as the ends of developers' statements, semicolons are fundamentally unnecessary to a working JavaScript program. Much like how [the cape on a superhero's back is a liability](https://www.youtube.com/watch?v=4R2aW03pwL0&t=1s), they serve only as a means for modern JavaScript developers to trip over themselves.
Submitted for your approval, evidence that forbidding semicolons makes JavaScript more readable and less prone to developer typos
@kesne
Copy link
Contributor

kesne commented Oct 1, 2017

This has been brought up a few times in the past: #394 and #1567 (comment) come to mind.

To quote @ljharb, "semicolons are so necessary the language puts them in for you if you forget".

Copy link
Collaborator

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relying on ASI has caused many bugs; anyone who omits semicolons and claims not to have run into bugs as a result just hasn't been programming long enough.

Your stance is fine, for yourself. It's irrelevant to Airbnb's choice, however, nor to the majority of JS developers - all of which use semicolons as the language requires.

Explicit > implicit.

@ljharb ljharb closed this Oct 1, 2017
@ljharb ljharb added the wontfix label Oct 1, 2017
@thepeoplesbourgeois
Copy link
Contributor Author

thepeoplesbourgeois commented Oct 1, 2017

Relying on ASI has caused many bugs; anyone who omits semicolons and claims not to have run into bugs as a result just hasn't been programming long enough.

Respectfully, it would benefit not just your argument, but JavaScript developers too new to have encountered any of the dire errors you vaguely warn of, to have any examples of such errors. Don't just hand-wave someone you don't know as not knowing as much as you; guide them to where you are.

Reading the discussions linked (thanks @kesne). Array literal getting confused for an index reference is troubling, but IMO should be regarded as a bug in ASI as opposed to being considered "not explicit enough". I did say I'd let the argument go if anyone had an example, though, so, thank you for providing them

@goatslacker
Copy link
Collaborator

TBH I've wasted more time adding/removing unnecessary semicolons than I've ever wasted debugging ASI issues.

Examples:

class Foo {
}; // <-- does a semicolon go here (it doesn't)

const elements = path.node.elements.map(element => (
  wrapStatement(
    blockStatement([returnStatement(element)])
  ); // <-- oops!
));

More examples: https://gist.github.com/ryanflorence/61935031ff729f072d9b

There's FUD on both sides of the argument, and it's a matter of preference. Airbnb chooses to use semicolons. You're free to use our style guide and omit the semicolons though! I regularly do it myself.

@ljharb
Copy link
Collaborator

ljharb commented Oct 2, 2017

It's more than just that; ASI rules are so problematic that class fields are being held up on the question of whether or not to forbid ASI in field definitions.

@thepeoplesbourgeois
Copy link
Contributor Author

After reading a number of blogs defending the semicolon a voice I'd call "blindingly furious", I finally stumbled upon the language creator's opinion as well as a defense of letting your team decide its own best approach based on how comfortable they are with ASI.

Had I known there were such painful eccentricities to the way JavaScript regards what would clearly be expression closures and array/list literals in whitespace-terminated languages, I would probably not have written this pull request. But, since I am here now, given my new understanding of why Airbnb enforces semicolons, maybe it would be constructive of me to contribute a "why" explaining the gotchas and the "explicit > implicit" preference? Ideally, it would prevent similarly cavalier outsiders from suggesting the change again :P

@lencioni
Copy link
Member

lencioni commented Oct 4, 2017

@thepeoplesbourgeois that sounds like a good idea to me. It is always worthwhile to clarify reasoning.

@ljharb
Copy link
Collaborator

ljharb commented Oct 4, 2017

That sounds great (please note, isaac's post there does not include new ASI hazards in ES6, nor upcoming ones in class fields).

I'd be happy to review a PR that explains rationales.

@thepeoplesbourgeois
Copy link
Contributor Author

(for my money, I think the class field problem could be solved without arbitary lookaheads, if infix operators could only appear before line terminators while the most recently-entered closure was a class, but I am not here to fix ASI, and there are almost definitely still other complexities I didn't account for anyway)

I'll write up a paragraph later this afternoon!

@ljharb
Copy link
Collaborator

ljharb commented Oct 4, 2017

If you have ideas about that for class fields, it's not too late - we're discussing it in November. Please feel free to comment on tc39/proposal-class-fields#7.

Look forward to the prose PR!

@thepeoplesbourgeois
Copy link
Contributor Author

Likely because I rewrote history on my master branch, I needed to open a new PR for the updates: #1578

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

Successfully merging this pull request may close these issues.

None yet

5 participants