Noissue - Add some more JavaScript coding guidlines

pages/coding-style.html

 title=Coding Style
 
 
 <h2 id="precedence">{{s1 Precedence}}</h2>
 
 <p>{{s2 The Adblock Plus coding style is based on others. Our rules (both general and language specific) always overrule referenced style guides. The language specific sections overrule the general section.}}</p>
 
 <h2 id="consistency">{{s3 Consistency}}</h2>
 
 <p>{{s4 Consistency is most important: Be consistent within functions, files, modules and projects. Making existing code conform with this style guide is fine, but it should happen in dedicated commits, preferably for a whole module or project at once.}}</p>
 
 <h2 id="general">{{s5 General}}</h2>
 <ul>
   <li>{{s6 Follow the Mozilla Coding Style's <a href="https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style#General_C.2FC.2B.2B_Practices">general practices</a> and its <a href="https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style#Naming_and_Formatting_code">naming and formatting rules</a>.}}</li>
   <li>{{s8 All files should have a <a href="license-headers">license header</a>, but no mode line comments.}}</li>
   <li>{{s9 Newline at end of file, otherwise no trailing whitespace.}}</li>
   <li>{{s10 Lines can be longer than the limit, if limiting line length would hurt readability in a particular case.}}</li>
   <li>{{s11 Opening braces always go on their own line.}}</li>
   <li>{{s12 No hungarian notation, no special variable name prefixes or suffixes denoting type or scope. All variable names start with a lower case letter.}}</li>
   <li>{{s13 Don't comment code out, delete it.}}</li>
   <li>{{todo-comments Don't add TODO comments, create an <a href="https://issues.adblockplus.org">issue</a> and reference it in a comment instead.}}</li>
 </ul>
 
 <h2 id="javascript">{{s14 JavaScript}}</h2>
 <ul>
   <li>{{s15 Follow the Mozilla Coding Style's <a href="https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style#JavaScript_practices">JavaScript practices</a>.}}</li>
   <li>{{s16 Opening braces of object literals in don't go on their own line when that would cause a syntax error.}}</li>
-  <li>{{s17 Use <code>bind()</code> to ensure the desired value of the <code>this</code> variable, don’t use temporary variables as a replacement.}}</li>
+  <li>{{javascript-this Use <code>bind()</code> (or arrow functions) to ensure the desired value of the <code>this</code> variable, don’t use temporary variables as a replacement.}}</li>

[Sebastian Noack, 2017/01/16 17:07:14]

For code snippets please also use the <fix> tag, see the Python section, for
reference. This makes sure they get not translated.

[Sebastian Noack, 2017/01/16 17:07:14]

I think this should go below the rule for arrow functions and clarify that you
should only use the function statement + bind() for compatibility with legacy
browsers if using arrow functions isn't an option.

[kzar, 2017/01/17 06:48:51]

Done.

[kzar, 2017/01/17 06:48:51]

Done.

   <li>{{s18 In classes, prefix private functions with a single underscore to make them pseudo-private.}}</li>
+  <li>{{javascript-let Use <code>let</code> or better yet <code>const</code> for variable definitions where possible instead of <code>var</code>.}}</li>

[Sebastian Noack, 2017/01/16 17:07:14]

I find this rule a little too vague. How about:

  If supported, use
  <a href="https://developer.mozilla.org/../Statements/let">block-scoping</a>,
  except when sharing global variables between scripts cannot be avoided.

As for const, this should could go into another rule, if we come up with any.
But keep in mind that we currently only use const for immutable types. If we
would use const wherever possible we would also have to use them for objects
which might change, as long as the variable itself isn't reassigned.

[kzar, 2017/01/17 06:48:51]

Done.

+  <li>{{javascript-arrow Use the <a href="https://developer.mozilla.org/docs/Web/JavaScript/Reference/Functions/Arrow_functions">arrow function</a> syntax for anonymous functions when the binding of <code>this</code> and <code>arguments</code> is not required.}}</li>

[Sebastian Noack, 2017/01/16 17:07:14]

Arrow functions automatically bind "this". So it seems your implications here
are the wrong way around. The reason to not use arrow functions would be because
NOT binding it would be required, right?

[kzar, 2017/01/17 06:48:51]

Well I think arrow functions simply close over the binding of `this` and
`arguments` from the parent scope, the same as functions normally do for other
variables. I think that's because they simply don't bind `this` and `arguments`
to something new like old style functions do, or am I wrong?

+  <li>{{javascript-method Use the <a href="https://developer.mozilla.org/docs/Web/JavaScript/Reference/Functions/Method_definitions">method definition</a> syntax where possible, even in preference to arrow functions.}}</li>

[Sebastian Noack, 2017/01/16 17:07:14]

I don't like how we overrule another rule here (i.e. "even in preference to
arrow functions"). Perhaps a better way to put it; "use the arrow function
syntax when passing anonymous functions" and "use the method definition
shorthand syntax when defining methods on an object". That way we have a clearer
distinction and mutually exclusive definition of the use cases, without need for
any exceptions.

[Sebastian Noack, 2017/01/16 17:07:14]

"method definition" is ambiguous as the old syntax (i.e. |{method: function() {
... }}|) arguably is a form of method definition as well. However, we a re
specifically talking about the method definition _shorthand_ syntax here.

[kzar, 2017/01/17 06:48:51]

Done.

[kzar, 2017/01/17 06:48:51]

Done.

[kzar, 2017/01/17 06:48:51]

Done.

+  <li>{{javascript-strict Where possible always use <a href="https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode">strict mode</a>.}}</li>

[Sebastian Noack, 2017/01/16 17:07:14]

Why the "where possible" restriction? Is there any legit use case for some
behavior that only exists outside of strict mode?

[kzar, 2017/01/17 06:48:51]

Done.

+  <li>{{javascript-if-else-braces When an <code>if</code>, <code>else</code> or <code>loop</code> statement spans over more than one line always enclose it with braces. When an <code>if</code> or <code>else</code> statement uses braces the other half should do too.}}</li>

[Sebastian Noack, 2017/01/16 17:07:14]

"other half" doesn't sound too technically accurrate, how about "opposing
block"?

[Sebastian Noack, 2017/01/16 17:07:14]

"loop" isn't a literal code snippet, so it shouldn't use the <code> tag. Also
"loop" is not a statement (but a group of statements).


  When an <code><fix>if</fix></code>, <code><fix>else</fix></code> or statement
  or loop spans over multiple lines ...

[kzar, 2017/01/17 06:48:51]

Done.

[kzar, 2017/01/17 06:48:52]

Done.

+  <li>{{javascript-for-of When iterating through Arrays and similar prefer <code>for (let item of array)</code> loops to traditional <code>for (let i=0; i < array.length; i++)</code> or even <code>array.forEach(...)</code> unless there is a good reason to do otherwise.}}</li>

[Sebastian Noack, 2017/01/16 17:07:14]

I think we are really only talking about arrays here, as there don't seem to be
any other objects which support both, length+indices as well as the iterator
protocol.

Also I wonder whether we can come up with a less vague rule. The only "good
reasons" to not use for..of would be if you need to either access the the loop
index, or have to iterate in non-standard order (skipping the first/last,
reversing order, etc.), or if it isn't supported by any targeted browser.

How about this:

  If supported, use for..of when iterating over arrays in canonical order,
  rather than using for(;;) or the forEach() method.

[kzar, 2017/01/17 06:48:51]

Done.

+  <li>{{javascript-object-proto Use <code>Object.create(null)</code> instead of <code>{__proto__: null}</code> and otherwise avoid the use of <code>__proto__</code>.}}</li>

[Sebastian Noack, 2017/01/16 17:07:14]

Well, by now, you should rather use Map or Set objects if you need a hash table,
rather than using Object.create(null) to avoid potential collisions.

[kzar, 2017/01/17 06:48:52]

Done.

 </ul>
 
 <h2 id="html-css">{{html-css HTML and CSS}}</h2>
 <ul>
   <li>{{html-css-general Follow the <a href="https://google.github.io/styleguide/htmlcssguide.xml">Google HTML/CSS Style Guide</a>.}}</li>
   <li>{{html-css-quotation Use double over single quotation marks.}}</li>
   <li>{{html-css-optional-tag Don't omit optional HTML tags.}}</li>
   <li>{{html-css-hexadecimal CSS color values should be specified in hexadecimal where possible.}}</li>
   <li>{{html-css-shorthand CSS shorthand properties usage is optional.}}</li>
   <li>{{html-css-order CSS rule declaration order should follow the <a href="https://make.wordpress.org/core/handbook/best-practices/coding-standards/css/#property-ordering">WordPress CSS Coding Standards</a>.}}</li>
@@ skipping 35 matching lines @@
 <ul>
   <li>{{objc-conventions Follow Apple's <a href="https://developer.apple.com/library/ios/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/Conventions/Conventions.html">conventions</a>}}.</li>
 </ul>
 
 <h2 id="puppet">{{s31 Puppet}}</h2>
 <ul>
   <li>{{s32 Follow the <a href="http://docs.puppetlabs.com/guides/style_guide.html">Puppet Style Guide</a>.}}</li>
   <li>{{s33 Opening braces don't go on their own line.}}</li>
   <li>{{s34 Arrows should not be aligned.}}</li>
 </ul>