8 ms·

A JavaScript Quality Guide

86 points by bevacqua 12y ago

netcraft 12y ago
Does this do something more than jshint?
- aikah 12y ago
  there is no code it's just an markdown text file.
weego 12y ago
The best quality/style guide I've ever been given was: "our code will be read many more times than it was written, don't do stupid shit". Having worked in that team for a while I really can't see any use for this kind of granular detail, other than if you goal really is over-zealous control.
nailer 12y ago
> Always put 'use strict'; at the top of your modules. 'use strict'; should be in the top of the scope you control. A global 'use strict'; would place the global scope into strict mode, so all your imported modules would be under strict mode too, and then third party code would break and you don't know why. IIRC jshint will pick this up. To be honest, the whole guide seems like a shorter, less comprehensive version of the JS community's own Idiomatic JS https://github.com/rwaldron/idiomatic.js/ https://github.com/rwaldron/idiomatic.js/
- bevacqua 12y ago
  That's what that means. In CommonJS the top of the module is the topmost scope, in systems like RequireJS its the first thing in your closures
gorhill 12y ago
I learned something: I didn't know that the difference between: function foo() { } and var foo = function() { }; Was that with the first form foo() will always be declared and available for use inside a scope, while the second form depends at which point it is declared. But I had read somewhere (can't remember where... jshint?) a long while ago that using the latter form inside scopes was better, so I've adopted that. I don't think I will go with the guide for this one, since not having the function defined automatically is often useful, to avoid overhead of instantiating the function for when it's not going to be used due to code flow.
- bevacqua 12y ago
  That's negligible, the code is going to get compiled in C++ anyways, and the overhead is minimal
  - gorhill 12y ago
    I don't understand why you bring up C++. With overhead, the approach I try to stick to is unless the code become more difficult to grasp, avoid overheads, however small they may be, whenever they can be avoided. Everything adds up in the end.
- warfangle 12y ago
  A lot of people prefer the latter form inside scope, because it disambiguates the ambiguity that hoisting introduces: define your function at the bottom of the scope, and it'll still be available at the top. By assigning it to var, you delineate "this function can be called by any line in this scope after this one, but not by any line before it." You also remove a named reference to it from the stack trace. I much prefer dealing with the "weird" "confusion" hoisting behavior than with not having a named function in my stack trace.
  - couchand 12y ago
    Fortunately they're not mutually exclusive, which is why the one true way (tm) is: var foo = function foo() { }; EDIT: The reason it's better to avoid hoisting is because global variables are almost always wrong.
    - delluminatus 12y ago
      Pretty sure hoisting is totally unrelated to global variables. If a function is defined in a scope, it'll only be hoisted to the top of the scope.
      - couchand 12y ago
        Ah, right. Should have left the original comment alone. Here's why you shouldn't rely on function hoisting: it gets weird. Weird enough that MDN has a section on it's "oddities" [0]. And since it doesn't add any expressive power, there's no compelling reason not to just treat all your code as data. [0]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Scope_Cheatsheet#function_oddities https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refe...
jasonkostempski 12y ago
I've been using JSLint with default options for years now. It's blissful not having to think about this stuff anymore.
yoanizer 12y ago
Don't sweat the small stuff. Really how you place your {} will never make or break your project. Focus on what really matters.
- bevacqua 12y ago
  Glad you got that. Some people are adamant of letting go of their obnoxious style checkers that do nothing but waste everyone's time https://github.com/bevacqua/js/issues/2 https://github.com/bevacqua/js/issues/2
- frik 12y ago
  See http://programmers.stackexchange.com/a/142114 http://programmers.stackexchange.com/a/142114
MrDosu 12y ago
Stopped reading after 2 spaces. 4 spaces is the law!
- Bahamut 12y ago
  I think you'll find that most frontend developers despise 4 spaces...at least that I've come upon anyhow. I only ever met one developer that liked 4, and that person is a tab guy to boot.
  - Raphmedia 12y ago
    I'm a front-end guy and I love my 4 spaces. Helps a lot with complicated HTML to have the extra spaces to easily see what is what. It's also very useful in SASS and Less, where you have nesting.
  - MrDosu 12y ago
    "I'll think you find that most..." is the start of no compelling argument ever 0o
- progx 12y ago
  Oh oh lets fight ;-) 2 Spaces, 4 Spaces, 1 Tab (view as 2 Spaces or 4 Spaces). Important is to configure once your favorite Editor and use it consequent.
  - madeofpalk 12y ago
    As someone who prefers tabs, but doesn't care enough so will happily use spaces, I've never understood the hatred around tabs. Not only does it remove the 2 vs 4 spaces debate (just set your browser preferences to display it as whatever!), it's more 'semantically correct' - it's literally saying "one level of indentation please". Anyway, not that I particularly care - I just always found it amusing that this is where developers have chosen against things being semantically correct and user-configurable.
    - titusjohnson 12y ago
mattfenwick 12y ago
I'm confused by what "Style checking" (https://github.com/bevacqua/js/#style-checking https://github.com/bevacqua/js/#style-checking) means. Aren't many of the other suggestions precisely "style checking" concerns? Semicolons, spacing, string quotes, conditional brackets, etc. ? Not to say that there isn't a difference -- just that I don't see what it is.
- bevacqua 12y ago
  Automated style checkers such as jscs [1]: https://www.npmjs.org/package/jscs https://www.npmjs.org/package/jscs
  - mattfenwick 12y ago
    That doesn't answer my question as far as I can tell, unless you're saying that: "Style checking" means "Automated style checkers such as jscs"
  - madeofpalk 12y ago
    what's the difference between a style checker like jscs and a linter like jshint/jslint?
    - akbar501 12y ago
      They are similar and have some overlap. However, the overlap is not 100%, so it's good practice to use both jshint and jscs together. jshint is a static code analysis tool to identify and flag errors/potential errors in JS code. jscs is purely a style checker.
progx 12y ago
Is this similar to https://github.com/airbnb/javascript https://github.com/airbnb/javascript ?
acconrad 12y ago
Not really sure we need another one of these: Idiomatic.JS[1] is far more complete, and if you want an opinion, just seek the style of Google[2]. [1] = https://github.com/rwaldron/idiomatic.js/ https://github.com/rwaldron/idiomatic.js/ [2] = https://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml https://google-styleguide.googlecode.com/svn/trunk/javascrip...
couchand 12y ago
I'm not convinced that this guide is very good. On many of the issues the actual decision is punted in favor of "just make sure it's consistent". Well obviously, that's why there's a guide. The purpose is to lay down the law so everyone knows HOW to be consistent. Moreover, a few of the suggestions introduce serious performance issues, notably around iteration and the treatment of the `arguments` object. If you haven't yet, please do check out Petka Antonov's Optimization Killers [0]. [0]: https://github.com/petkaantonov/bluebird/wiki/Optimization-killers https://github.com/petkaantonov/bluebird/wiki/Optimization-k...
- nailer 12y ago
  It really depends on whether performance is an issue, and the cost of the optimisation. A common example: lodash searches function toString() with regexps as an optimisation technique, underscore doesn't. Both are correct, maintaining lodash requires knowledge of how and why the hack works, but OTOH lodash is faster. Maintaining underscore is cognitively simpler but the resulting code is slower.
  - couchand 12y ago
    I'm all about making sure that code is optimized for humans first and computers second. Here is the specific remark I was referring to: Or even better, just use .forEach which doesn't have the same caveats as declaring functions in for loops. Blindly applying forEach is simply not the right answer here.
    - nailer 12y ago
      I'd suggest the opposite: forEach() is smaller that iterating over indexes and creates a scope by default which (although having overhead) will prevent a lot of common last-case-wins scenarios that frequently trap both new and experienced developers. Should we use a .forEach() for a 1 million item array? No. Performance would be a problem. For the typical use case of items like the demo [1, 2, 3]? Yes, this would be be simpler.
aikah 12y ago
var message = util.format('oh hai %s!', name); I thought it was a javascript quality guide ? there is no util.format in ES5 spec.
- bevacqua 12y ago
  It's a Node.js example
- iaskwhy 12y ago
  "Usually you'll be a happier JavaScript developer if you hack together a parameter-replacing method like util.format in Node." + "You could implement something similar using the piece of code below." I don't think you can ask more than this, everything is clearly laid out...
chdir 12y ago
> Keep regular expressions in variables, don't use them inline. This will vastly improve readability. Although it might introduce inadvertent bugs. I prefer not to follow that advice to avoid running into the 'lastIndex' problem described here [1]. There are other ways to avoid it like not using the 'g' flag or resetting lastIndex. However not storing regex in a variable is also useful to avoid that headache. You might argue about performance but for a good majority of the cases, it doesn't matter practically. [1] http://stackoverflow.com/questions/1520800/why-regexp-with-global-flag-in-javascript-give-wrong-results http://stackoverflow.com/questions/1520800/why-regexp-with-g... [2] http://stackoverflow.com/questions/13500519/difference-between-regexp-constructor-and-regex-literal-test-function http://stackoverflow.com/questions/13500519/difference-betwe...
kevincennis 12y ago
If you're going to have your team agree on any style rules at all, then automated style checking is the only way to go. Let the computer yell at people so you don't have to. Nobody wants to be the pedantic jerk who has to leave comments on a PR about whitespace or single vs double quotes. When you use an automated style checker, you presumably catch those errors before a PR gets submitted and nobody ever has to be the bad guy/girl.
- akbar501 12y ago
  > automated style checking is the only way to go Absolutely. We've found automated style checking also improves quality and efficiency. Even the "pedantic jerk" can miss small style issues that are easily caught by tooling. For anyone wondering what tools. The most common for JS are JSHint and JSCS. Then use Grunt or Gulp combined with a watch to check a file each time a change is saved. Lastly, use a precommit hook (or similar) to check again before code is committed into the repo.
okcoker 12y ago
You lost me at 2 spaces
EvanYou 12y ago
Don't confuse quality with style. If you want provide a quality guide, include only rules that actually improve the quality of the code, don't include style preferences. Style is subjective by nature, but quality not so much. Some rules being enforced in this guide are heavily opinionated and yet have no direct correlation with code quality. What does the indentation being 2 spaces or 4 spaces or tabs have to do with code quality? And a semicolon-less codebase can be as clean, readable and performant as one with semicolons, your arguments against ASI is simply FUD. As many have pointed out, for these stylistic disagreements the only rule should be "just be consistent".
frik 12y ago
Semicolons ; It seems to be fashionable recently to omit semicolons from Javascript. For example GitHub (internal) and zepto.js have jumped on the no-semicolon bandwagon, requiring their omission in any code. http://programmers.stackexchange.com/questions/142086/why-the-recent-shift-to-removing-omitting-semicolons-from-javascript http://programmers.stackexchange.com/questions/142086/why-th... OP's guide mentions "Avoid headaches, avoid ASI. Always add semicolons where needed", that's a good advice IMHO. Edit: I would add: http://programmers.stackexchange.com/a/142114 http://programmers.stackexchange.com/a/142114
- mnarayan01 12y ago
  > Always add semicolons where needed Perhaps overly pedantic, but this seems a little...vague.
  - EvanYou 12y ago
    That's why I don't like guides that suggest people to mindlessly add semicolons everywhere. If you don't really know how ASI works you are always living in the fear of missing a semicolon somewhere. Actually learning how ASI works liberates you from that fear and you can still opt to use or not use semicolons.
    - mnarayan01 12y ago
      I think I'm actually in total disagreement with this. I think using semicolons as statement terminators is quite easy and doesn't require any knowledge of the ASI in terms of _adding_ semicolons. The one (potential) issue with not being aware of the ASI when using semicolons as statement terminators is for something like: return { a: 0 }; since you can't disable ASI. Even in the above case, however, "standard" JS style would prevent the issue.
      - EvanYou 12y ago
        Relying on "standard JS style" is as reliable as relying on ASI without knowing how it works. If what you are saying is with "standard JS style" you will never add a newline after return, then I can also assume with "standard JS style" you will not: - start a line with ++ or -- - start a line with a regex literal - split a for statement into separate lines So, the only issue you need to be aware of when writing semicolon-less javascript is when you start a new line with ( or [. Easy enough to remember, no? I've personally been writing semicolon-less JS for almost two years now, and this one simple rule has been more than enough.

86 points by bevacqua 12y ago

netcraft 12y ago

Does this do something more than jshint?

aikah 12y ago
there is no code it's just an markdown text file.

weego 12y ago

The best quality/style guide I've ever been given was: "our code will be read many more times than it was written, don't do stupid shit". Having worked in that team for a while I really can't see any use for this kind of granular detail, other than if you goal really is over-zealous control.

nailer 12y ago

> Always put 'use strict'; at the top of your modules. 'use strict'; should be in the top of the scope you control. A global 'use strict'; would place the global scope into strict mode, so all your imported modules would be under strict mode too, and then third party code would break and you don't know why. IIRC jshint will pick this up. To be honest, the whole guide seems like a shorter, less comprehensive version of the JS community's own Idiomatic JS https://github.com/rwaldron/idiomatic.js/ https://github.com/rwaldron/idiomatic.js/

bevacqua 12y ago
That's what that means. In CommonJS the top of the module is the topmost scope, in systems like RequireJS its the first thing in your closures

gorhill 12y ago

I learned something: I didn't know that the difference between: function foo() { } and var foo = function() { }; Was that with the first form foo() will always be declared and available for use inside a scope, while the second form depends at which point it is declared. But I had read somewhere (can't remember where... jshint?) a long while ago that using the latter form inside scopes was better, so I've adopted that. I don't think I will go with the guide for this one, since not having the function defined automatically is often useful, to avoid overhead of instantiating the function for when it's not going to be used due to code flow.

bevacqua 12y ago
That's negligible, the code is going to get compiled in C++ anyways, and the overhead is minimal
- gorhill 12y ago
  I don't understand why you bring up C++. With overhead, the approach I try to stick to is unless the code become more difficult to grasp, avoid overheads, however small they may be, whenever they can be avoided. Everything adds up in the end.
warfangle 12y ago
A lot of people prefer the latter form inside scope, because it disambiguates the ambiguity that hoisting introduces: define your function at the bottom of the scope, and it'll still be available at the top. By assigning it to var, you delineate "this function can be called by any line in this scope after this one, but not by any line before it." You also remove a named reference to it from the stack trace. I much prefer dealing with the "weird" "confusion" hoisting behavior than with not having a named function in my stack trace.
- couchand 12y ago
  Fortunately they're not mutually exclusive, which is why the one true way (tm) is: var foo = function foo() { }; EDIT: The reason it's better to avoid hoisting is because global variables are almost always wrong.
  - delluminatus 12y ago
    Pretty sure hoisting is totally unrelated to global variables. If a function is defined in a scope, it'll only be hoisted to the top of the scope.
    - couchand 12y ago
      Ah, right. Should have left the original comment alone. Here's why you shouldn't rely on function hoisting: it gets weird. Weird enough that MDN has a section on it's "oddities" [0]. And since it doesn't add any expressive power, there's no compelling reason not to just treat all your code as data. [0]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Scope_Cheatsheet#function_oddities https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refe...

jasonkostempski 12y ago

I've been using JSLint with default options for years now. It's blissful not having to think about this stuff anymore.

yoanizer 12y ago

Don't sweat the small stuff. Really how you place your {} will never make or break your project. Focus on what really matters.

bevacqua 12y ago
Glad you got that. Some people are adamant of letting go of their obnoxious style checkers that do nothing but waste everyone's time https://github.com/bevacqua/js/issues/2 https://github.com/bevacqua/js/issues/2
frik 12y ago
See http://programmers.stackexchange.com/a/142114 http://programmers.stackexchange.com/a/142114

MrDosu 12y ago

Stopped reading after 2 spaces. 4 spaces is the law!

Bahamut 12y ago
I think you'll find that most frontend developers despise 4 spaces...at least that I've come upon anyhow. I only ever met one developer that liked 4, and that person is a tab guy to boot.
- Raphmedia 12y ago
  I'm a front-end guy and I love my 4 spaces. Helps a lot with complicated HTML to have the extra spaces to easily see what is what. It's also very useful in SASS and Less, where you have nesting.
- MrDosu 12y ago
  "I'll think you find that most..." is the start of no compelling argument ever 0o
progx 12y ago
Oh oh lets fight ;-) 2 Spaces, 4 Spaces, 1 Tab (view as 2 Spaces or 4 Spaces). Important is to configure once your favorite Editor and use it consequent.
- madeofpalk 12y ago
  As someone who prefers tabs, but doesn't care enough so will happily use spaces, I've never understood the hatred around tabs. Not only does it remove the 2 vs 4 spaces debate (just set your browser preferences to display it as whatever!), it's more 'semantically correct' - it's literally saying "one level of indentation please". Anyway, not that I particularly care - I just always found it amusing that this is where developers have chosen against things being semantically correct and user-configurable.
  - titusjohnson 12y ago

mattfenwick 12y ago

I'm confused by what "Style checking" (https://github.com/bevacqua/js/#style-checking https://github.com/bevacqua/js/#style-checking) means. Aren't many of the other suggestions precisely "style checking" concerns? Semicolons, spacing, string quotes, conditional brackets, etc. ? Not to say that there isn't a difference -- just that I don't see what it is.

bevacqua 12y ago
Automated style checkers such as jscs [1]: https://www.npmjs.org/package/jscs https://www.npmjs.org/package/jscs
- mattfenwick 12y ago
  That doesn't answer my question as far as I can tell, unless you're saying that: "Style checking" means "Automated style checkers such as jscs"
- madeofpalk 12y ago
  what's the difference between a style checker like jscs and a linter like jshint/jslint?
  - akbar501 12y ago
    They are similar and have some overlap. However, the overlap is not 100%, so it's good practice to use both jshint and jscs together. jshint is a static code analysis tool to identify and flag errors/potential errors in JS code. jscs is purely a style checker.

progx 12y ago

Is this similar to https://github.com/airbnb/javascript https://github.com/airbnb/javascript ?

acconrad 12y ago

Not really sure we need another one of these: Idiomatic.JS[1] is far more complete, and if you want an opinion, just seek the style of Google[2]. [1] = https://github.com/rwaldron/idiomatic.js/ https://github.com/rwaldron/idiomatic.js/ [2] = https://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml https://google-styleguide.googlecode.com/svn/trunk/javascrip...

couchand 12y ago

I'm not convinced that this guide is very good. On many of the issues the actual decision is punted in favor of "just make sure it's consistent". Well obviously, that's why there's a guide. The purpose is to lay down the law so everyone knows HOW to be consistent. Moreover, a few of the suggestions introduce serious performance issues, notably around iteration and the treatment of the `arguments` object. If you haven't yet, please do check out Petka Antonov's Optimization Killers [0]. [0]: https://github.com/petkaantonov/bluebird/wiki/Optimization-killers https://github.com/petkaantonov/bluebird/wiki/Optimization-k...

nailer 12y ago
It really depends on whether performance is an issue, and the cost of the optimisation. A common example: lodash searches function toString() with regexps as an optimisation technique, underscore doesn't. Both are correct, maintaining lodash requires knowledge of how and why the hack works, but OTOH lodash is faster. Maintaining underscore is cognitively simpler but the resulting code is slower.
- couchand 12y ago
  I'm all about making sure that code is optimized for humans first and computers second. Here is the specific remark I was referring to: Or even better, just use .forEach which doesn't have the same caveats as declaring functions in for loops. Blindly applying forEach is simply not the right answer here.
  - nailer 12y ago
    I'd suggest the opposite: forEach() is smaller that iterating over indexes and creates a scope by default which (although having overhead) will prevent a lot of common last-case-wins scenarios that frequently trap both new and experienced developers. Should we use a .forEach() for a 1 million item array? No. Performance would be a problem. For the typical use case of items like the demo [1, 2, 3]? Yes, this would be be simpler.

aikah 12y ago

var message = util.format('oh hai %s!', name); I thought it was a javascript quality guide ? there is no util.format in ES5 spec.

bevacqua 12y ago
It's a Node.js example
iaskwhy 12y ago
"Usually you'll be a happier JavaScript developer if you hack together a parameter-replacing method like util.format in Node." + "You could implement something similar using the piece of code below." I don't think you can ask more than this, everything is clearly laid out...

chdir 12y ago

> Keep regular expressions in variables, don't use them inline. This will vastly improve readability. Although it might introduce inadvertent bugs. I prefer not to follow that advice to avoid running into the 'lastIndex' problem described here [1]. There are other ways to avoid it like not using the 'g' flag or resetting lastIndex. However not storing regex in a variable is also useful to avoid that headache. You might argue about performance but for a good majority of the cases, it doesn't matter practically. [1] http://stackoverflow.com/questions/1520800/why-regexp-with-global-flag-in-javascript-give-wrong-results http://stackoverflow.com/questions/1520800/why-regexp-with-g... [2] http://stackoverflow.com/questions/13500519/difference-between-regexp-constructor-and-regex-literal-test-function http://stackoverflow.com/questions/13500519/difference-betwe...

kevincennis 12y ago

If you're going to have your team agree on any style rules at all, then automated style checking is the only way to go. Let the computer yell at people so you don't have to. Nobody wants to be the pedantic jerk who has to leave comments on a PR about whitespace or single vs double quotes. When you use an automated style checker, you presumably catch those errors before a PR gets submitted and nobody ever has to be the bad guy/girl.

akbar501 12y ago
> automated style checking is the only way to go Absolutely. We've found automated style checking also improves quality and efficiency. Even the "pedantic jerk" can miss small style issues that are easily caught by tooling. For anyone wondering what tools. The most common for JS are JSHint and JSCS. Then use Grunt or Gulp combined with a watch to check a file each time a change is saved. Lastly, use a precommit hook (or similar) to check again before code is committed into the repo.

okcoker 12y ago

You lost me at 2 spaces

EvanYou 12y ago

Don't confuse quality with style. If you want provide a quality guide, include only rules that actually improve the quality of the code, don't include style preferences. Style is subjective by nature, but quality not so much. Some rules being enforced in this guide are heavily opinionated and yet have no direct correlation with code quality. What does the indentation being 2 spaces or 4 spaces or tabs have to do with code quality? And a semicolon-less codebase can be as clean, readable and performant as one with semicolons, your arguments against ASI is simply FUD. As many have pointed out, for these stylistic disagreements the only rule should be "just be consistent".

frik 12y ago

Semicolons ; It seems to be fashionable recently to omit semicolons from Javascript. For example GitHub (internal) and zepto.js have jumped on the no-semicolon bandwagon, requiring their omission in any code. http://programmers.stackexchange.com/questions/142086/why-the-recent-shift-to-removing-omitting-semicolons-from-javascript http://programmers.stackexchange.com/questions/142086/why-th... OP's guide mentions "Avoid headaches, avoid ASI. Always add semicolons where needed", that's a good advice IMHO. Edit: I would add: http://programmers.stackexchange.com/a/142114 http://programmers.stackexchange.com/a/142114

mnarayan01 12y ago
> Always add semicolons where needed Perhaps overly pedantic, but this seems a little...vague.
- EvanYou 12y ago
  That's why I don't like guides that suggest people to mindlessly add semicolons everywhere. If you don't really know how ASI works you are always living in the fear of missing a semicolon somewhere. Actually learning how ASI works liberates you from that fear and you can still opt to use or not use semicolons.
  - mnarayan01 12y ago
    I think I'm actually in total disagreement with this. I think using semicolons as statement terminators is quite easy and doesn't require any knowledge of the ASI in terms of _adding_ semicolons. The one (potential) issue with not being aware of the ASI when using semicolons as statement terminators is for something like: return { a: 0 }; since you can't disable ASI. Even in the above case, however, "standard" JS style would prevent the issue.
    - EvanYou 12y ago
      Relying on "standard JS style" is as reliable as relying on ASI without knowing how it works. If what you are saying is with "standard JS style" you will never add a newline after return, then I can also assume with "standard JS style" you will not: - start a line with ++ or -- - start a line with a regex literal - split a for statement into separate lines So, the only issue you need to be aware of when writing semicolon-less javascript is when you start a new line with ( or [. Easy enough to remember, no? I've personally been writing semicolon-less JS for almost two years now, and this one simple rule has been more than enough.