3.4 Don't Forget the Semicolon about nodebestpractices HOT 22 CLOSED

ASI is a flamable issue. IMHO semicolons are a style choice, so I wouldn't be opinionated about usage.
I'd call that section Respect the Semicolon put two code snippets that exemplify the pitfalls of both, something like:

function aaa() {
    return
    {
        test: 1
    };
}
aaa() === undefined;

and

var m = new Map()
var a = [1,2,3]
[...m.values()].forEach(console.log)

> [...m.values()].forEach(console.log)
>  ^^^
> SyntaxError: Unexpected token ...

And the tl;dr should be use a linter

from nodebestpractices.

I'd argue that since they are a style choice they should not be included in a guide like this. No site really has advantages, and there is a reason why projects like semistandard are successful next to projects like standard.

from nodebestpractices.

I was in favour of manual semi insertion.

Could come up with code examples when js would fail if not for the manual semi insertion.

But then, some 6-7 years ago on some hackaton, during 30+ th hour of coding (2am or so),
my friend had put , instead ; and we wasted half an hour debugging...

Since then I tried ASI (automatic semi) , first on some small projects, then on big enterprise projects.
Never had any issues around it.
Never.

My advise: just try it!
After one week with ASI you will not want to go back to manual.

Its not just about that code is more readable with ASI...
Its about having bug free codebase.

And those errors that only manual semi can fix?
Again: because of eslint we never, ever encountered them.

from nodebestpractices.

I agree that this rule doesn't make any sense today. This is a just personal taste, with no influence on the readability of the source code: omitting semicolons is only a matter of preference. In addition to that, ESLint has specific rules to avoid multiline expressions mistakes. https://eslint.org/docs/rules/semi and https://eslint.org/docs/rules/no-unexpected-multiline
I think that would be better to remove this rule and update eslint section to include this references.

from nodebestpractices.

I proposed a PR. Hope it unites all the opinions here in this thread.

from nodebestpractices.

IMHO, usage of semicolons itself is not enough to make code error-proof.

Only combination of several factors like block style, linter usage make code much safer even without semicolons.

from nodebestpractices.

This is a fantastic thread and was one of the things that made me go 🤔 in terms of best practices. I fully support dropping the section recommending semicolons while strongly advocating the use of a linter.

Also giving explicit examples such as standardjs, prettier or even AirBnB config I feel would be helpful.

from nodebestpractices.

@refack based on all of the above, it seems to be too opinionated for our guide. If you and @idori agree, let's drop

from nodebestpractices.

How about “3.4 Be Consistent With Your Semicolons?” (i.e. it doesn’t matter if you pick semicolons or no semicolons, as long as you’re aware of the pitfalls of each and stay consistent)

from nodebestpractices.

I agree. I would say "use a coding style" is important than following none at all. But using semicolons or not is very opinionated. However, I hope that standardjs will be more wide-spread used and adopted in the future.

from nodebestpractices.

Title for 3.4 can be changed to something like "Don't forget about proper statement separation" and describe some side-effects of not using explicit semicolons and following ASI. ASI itself is not bad. Main threat is not omitting semicolons itself, but problems with run-time separation of different statements.

Samples from this thread can make some wiki page with details.

from nodebestpractices.

Hi @idori

Sure, but my point is: I am not sure that a rule like "Don't Forget the Semicolon" is revelant, because the ecmascript engine is not hardly based on this "feature".

I could understand that this is more readable. But it cannot avoid some side issues.

I would suggest a warning more a use case like that:

if (aCondition) 
   doSomething();

Because on various languages, I have resolved a lot of big bugs because people do:

if (aCondition) 
   doSomething();
  doSomethingElse();

Where always use brackets on conditions avoid side issue.

Again, I understand your point of view.

My two cents

Regards

from nodebestpractices.

On the https://standardjs.com/ site there is a section that explains why no semi is the default choice using 3 links:
http://blog.izs.me/post/2353458699/an-open-letter-to-javascript-leaders-regarding
http://inimino.org/~inimino/blog/javascript_semicolons
https://www.youtube.com/watch?v=gsfbh17Ax9I
and also here:
https://standardjs.com/rules.html#semicolons

from nodebestpractices.

@refack @idori @j-f1 I think we should contain only definitive guidelines which includes verbs (do that, drop this, include these) and not general piece of information (you should now that).

Hence, I would combine @refack and @j-f1 suggestions into something like: Be consistent and mindful about semi-color + @refack text and examples.

@idori u lead this issue

from nodebestpractices.

Hi @rochejul and thanks for the input!
Please note that the issue you raised it explained in the previews point (3.3 Start a Codeblock's Curly Braces in the Same Line)
We might need to make it more clear though, as the example is inside the link to Stackoverflow

from nodebestpractices.

@refack what do you think?

from nodebestpractices.

P.S. we could ask @feross why he picked no-extra-semi for standard?

from nodebestpractices.

P.P.S. - ASI = Automatic Semicolon Insertion
From the spec:

Automatic Semicolon Insertion

Most ECMAScript statements and declarations must be terminated with a semicolon. Such semicolons may always appear explicitly in the source text. For convenience, however, such semicolons may be omitted from the source text in certain situations. These situations are described by saying that semicolons are automatically inserted into the source code token stream in those situations.

from nodebestpractices.

I concur with @refack . Semicolons are a style choice. And one option requires writing less code!

from nodebestpractices.

I do like @refack's definition, but I agree to take if out completely if it's a too opinionated subject for a style guide.

from nodebestpractices.

@syzer : exactly!

I never miss any semi in my code - ESLint make code safer, i always have some tricky bug eliminated only by working out some eslint issues (sure you have to configure it to your style).

So - semi is not mandatory, imho

from nodebestpractices.

Hello there! 👋
This issue has gone silent. Eerily silent. ⏳
We currently close issues after 100 days of inactivity. It has been 90 days since the last update here.
If needed, you can keep it open by replying here.
Thanks for being a part of the Node.js Best Practices community! 💚

from nodebestpractices.