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

typo: "No semicolons" should read "Semicolons" #5

Closed
ajschumacher opened this issue Jan 27, 2015 · 19 comments

Comments

@ajschumacher
Copy link

commented Jan 27, 2015

No description provided.

@aharris

This comment has been minimized.

Copy link

commented Jan 27, 2015

+1

@feross feross closed this Jan 27, 2015

@KrisSiegel

This comment has been minimized.

Copy link

commented Jan 27, 2015

Issues like this means you can't simply have one style standard to rule them all without options like jshint otherwise you won't get the highest adoption rate.

For me and the code bases I manage I won't allow eliding of semicolons in JavaScript. Yes it works without them but you can really screw yourself over by simply omitting a part of the language's syntax in some, admittedly rare, situations. You can't elide them in all situations (again, rare, but I'd rather have a rule that covers the most cases and not just some due to someone wanting their code to look slightly nicer).

@jprichardson

This comment has been minimized.

Copy link
Member

commented Jan 27, 2015

Yes it works without them but you can really screw yourself over by simply omitting a part of the language's syntax in some, admittedly rare, situations.

@KrisSiegel Can you provide a situation where you can "really screw yourself" by omitting semicolons? The absence of them in any of the three required cases before [, before ( (assuming they start a line) and for loops will causes an interpreter error. Are there cases where things silently fail?

@feross

This comment has been minimized.

Copy link
Member

commented Jan 27, 2015

@KrisSiegel @jprichardson

There are actual (albeit VERY rare) gotchas with omitting semicolons. However, this module detects those cases and prints an error. You can't footgun yourself if you add standard to your npm test script.

Here's one specific case to watch for:

alert('hi')
(function () { var x = 1 })()

Gets treated as:

alert('hi')(function () { var x = 1 })()

The rule is: "If not inserting a semicolon where there is a newline would cause a parse error, then automatically insert one."

@feross

This comment has been minimized.

Copy link
Member

commented Jan 27, 2015

@KrisSiegel The goal of this module is not to get the highest adoption rate.

The goal is to save time in two ways:

  • Be the easiest way to enforce style checking in your module/project. No configuration; just drop it in.
  • Save precious code review time by catching style errors before they're submitted as PRs. This eliminates back-and-forth and increases the chances of a successful merge.
@KrisSiegel

This comment has been minimized.

Copy link

commented Jan 27, 2015

I understand the goal isn't the highest adoption rate (sorry, I worded that poorly earlier and didn't really mean exactly that). I'm just not sure who the target audience is for a strict style-enforcing module with no options (granted I was given this link by a friend, perhaps this isn't really meant for wide use I don't know).

I like the other rules but wouldn't use the module due to the no semicolon enforcement (I really feel uncomfortable ignoring a part of the syntax especially when there are issues, even rare, when omitted).

Either way it works out, good luck!

@feross

This comment has been minimized.

Copy link
Member

commented Jan 27, 2015

@KrisSiegel: I really feel uncomfortable ignoring a part of the syntax especially when there are issues, even rare, when omitted

I just wanted to clarify one thing in case others read this thread: automatic semicolon insertion is well-understood and exactly specified in the ECMAScript spec. It's not like C's "undefined" or "implimentation defined" behavior. The behavior is completely predictable if you just take the time to understand it.

@feross

This comment has been minimized.

Copy link
Member

commented Jan 27, 2015

I'm almost tempted to add a --semicolon or maybe --no-semicolon option, but the main appeal of this module is that it's zero-configuration. Going to avoid any options for the time being and see how people use this.

@mattdesl

This comment has been minimized.

Copy link

commented Jan 27, 2015

Yeah, I would avoid options. Otherwise you may as well just make another jshint.

If somebody really wants semicolons, or 6 spaces, or tighter whitespace, etc they can make their own fork.

@Flet

This comment has been minimized.

Copy link
Member

commented Jan 27, 2015

I'll just leave this here https://github.com/flet/semistandard :trollface:

@feross

This comment has been minimized.

Copy link
Member

commented Jan 27, 2015

@Flet nice one!

@ungoldman

This comment has been minimized.

Copy link
Member

commented Mar 29, 2015

@Flet++

If you really want semicolons just use https://github.com/flet/semistandard, please no bike-shedding!

@KrisSiegel for me the whole point of adopting something like standard or semistandard into my workflow is to never have to argue about really trivial style considerations ever again. Code consistency is enforced at the package level through tests, people can focus on the project, nobody has to have a conversation about ASI or comma-first, everybody wins.

(Thanks @feross)

@callumacrae

This comment has been minimized.

Copy link

commented Mar 29, 2015

@ngoldman Are you telling flet to use flet/semistandard? 😝

@maxogden

This comment has been minimized.

Copy link
Contributor

commented Mar 29, 2015

@callumacrae What is the point of calling someone out like that?

Also if you re-read the comment you'll see it was a general statement directed at bike shedders.

@ungoldman

This comment has been minimized.

Copy link
Member

commented Mar 29, 2015

Sorry I probably should have said nothing. Everyone's doing great work

@mearns

This comment has been minimized.

Copy link

commented Jan 12, 2018

Given that TC39 has been coming out pretty hard against relying on ASI, is this project going to consider revising this rule at all? I use standardjs for the reason cited by @ungoldman above, to avoid bikeshedding. However, recent discussions about ASI seem to be more about the structural integrity of the shed, rather than the color, and might warrant some further consideration.

@bcomnes

This comment has been minimized.

Copy link
Member

commented Jan 12, 2018

If anything, we might take brendaneich’s recomendation to use semicolons in class bodies as statement separators (think for loops). In terms of outside class bodies, it’s yet to be seen what class of asi errors and hazards tc-39 is willing to add. Doesn’t sound good, but I also haven’t seen anything bad yet. It’s an unfortunate situation because their semicolon statement is probably one of the worst options on the table.

As always the goal is to make writing js fun and leightwheight and less error prone, so standard will evolve as ECMA script does.

@feross

This comment has been minimized.

Copy link
Member

commented Jan 15, 2018

As always the goal is to make writing js fun and leightwheight and less error prone, so standard will evolve as ECMA script does.

Exactly.

Right now, there's nothing concretely bad, i.e. no new ASI hazards. We'll keep an eye on the discussions and advocate for no-semi users. If we have to evolve, we'll happily do it. But we're not there yet.

@lock lock bot locked as resolved and limited conversation to collaborators May 25, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.