-
Notifications
You must be signed in to change notification settings - Fork 3
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
Remove old-fashioned semicolons #2
base: main
Are you sure you want to change the base?
Conversation
I agree with that!
It's funny you mention this old vs modern, following the hype (although coffee-script also tried it years ago) :D Some background
A good explanation can be found on MDN, along with a bunch of use cases where things can go wrong. Some entertainment can be found in an old discussion on Github between Bootstrap and JSmin maintainer Douglas Crockford, and a response from Brendan Eich. Arguments for omitting semi's can be read here and here. They mostly highlight that experienced/real JS programmers should know the exact syntax rules so they know when to and not to use semi's, instead of just adding them everywhere to hope not run into ASI issues. ArgumentsSo, most discussions about semi-colons are about two arguments:
Prettier / standardFor the first argument, people mention modern tooling like prettier, or minifiers, that can automatically add them where needed. While this is true, it's actually more nuanced. It's best to read through prettier's and standard's documentation about it. Both tools add semicolons at the beginning of "dangerous lines" (lines beginning with if (shouldAddLines) {
;[-1, 1].forEach(delta => addLine(delta * 20))
}
;(function () {
window.alert('ok')
}())
;[1, 2, 3].forEach(bar)
;`hello`.indexOf('o') But more importantly:
console.log('Running a background task')
(async () => {
await doBackgroundWork()
})()
// If you feed this into Prettier, it will not alter the behavior of this code.
// instead, it will reformat it in a way that shows how this code will actually behave when ran.
console.log("Running a background task")(async () => {
await doBackgroundWork();
})(); or a = b + c
(d + e).print()
// turns into
a = b + c(d + e).print() Standard says (which is reasonable, but still):
So this means that you
And then there are still cases that prettier leaves alone, but look different from what they actually do, like: function getCheese() {
return
{
cheeseType: "Gouda"
}
} Which seems to return an object, but actually returns Luckily we have other tools that spot "dead code", so you would be notified about this one way or another. How code feelsI believe there are two main camps;
Personally I fall in the 2nd camp, especially when chaining code where newlines are missing (note, they aren't added by prettier in these cases): items
.map((x) => callSomethingWith(x))
.filter((x) => isXSomethingWeNeed(x))
.map((x) => doSomethingExpensive(x))
andHereWeCallAFunction(items)
andThis.is.another.line = someValue Even though there is indentation to indicate what part belongs to the chaining bit, when I'm scanning the right side of the code, I have no way of seeing which line is the last one of the statement. I also found an interesting comment:
ConclusionTo conclude I could probably get used to not having them, but I also see no need to get rid of them. We're already changing quite a lot of things, so I'm not sure if we should add this to the list, as it's mostly a stylistic change, but people have to put effort into:
And that second one is worrying me, that preventable errors can still slip in. So I would suggest only changing this when the (big) majority agrees we should get rid of them, and everyone understands the effort (and risk). |
This is always a controversial topic.
But yet again, here we are... 😅
Given the number of linters and formatters we have set up, we can safely get rid of semicolons.
A while ago, I used to be heavily opinionated about enforcing them everywhere until I worked on a modern codebase where their absence made absolutely no difference, and on top of that, the code somehow looked super-clean, modern and friendly (semis were not the only responsible for that feeling, but they definitely were part of it). I was pretty surprised by that, and since then, I removed them from every project I was in charge of without any issue.
(again, prettier will add them automatically when required).