|
| 1 | +# JavaScript Guidelines |
| 2 | +Minimize the danger caused by a scripting (not strongly typed) language. |
| 3 | + |
| 4 | +## Sections |
| 5 | + |
| 6 | +- [JavaScript Guidelines](#javascript-guidelines) |
| 7 | + - [Sections](#sections) |
| 8 | +- [Guidelines](#guidelines) |
| 9 | + - [Indenting](#indenting) |
| 10 | + - [String Concatenation](#string-concatenation) |
| 11 | + - [CamelCasing](#camelcasing) |
| 12 | + - [Semi-colons](#semi-colons) |
| 13 | + - [Control Structures](#control-structures) |
| 14 | + - [switch](#switch) |
| 15 | + - [try](#try) |
| 16 | + - [for in](#for-in) |
| 17 | + - [Functions](#functions) |
| 18 | + - [Functions and Methods](#functions-and-methods) |
| 19 | + - [Function Calls](#function-calls) |
| 20 | + - [Function Declarations](#function-declarations) |
| 21 | + - [Variables and Arrays](#variables-and-arrays) |
| 22 | + - [Arrays](#arrays) |
| 23 | + - [Comments](#comments) |
| 24 | + - [JS code placement](#js-code-placement) |
| 25 | + - ["with" statement](#with-statement) |
| 26 | + - [Operators](#operators) |
| 27 | + - [True or false comparisons](#true-or-false-comparisons) |
| 28 | + - [Comma Operator](#comma-operator) |
| 29 | + - [Avoiding unreachable code](#avoiding-unreachable-code) |
| 30 | + - [Constructors](#constructors) |
| 31 | + - [Use literal expressions](#use-literal-expressions) |
| 32 | + - [eval is evil](#eval-is-evil) |
| 33 | + - [Preventing XSS](#preventing-xss) |
| 34 | + - [Typeof](#typeof) |
| 35 | + - [Modifying the DOM](#modifying-the-dom) |
| 36 | + |
| 37 | +# Guidelines |
| 38 | + |
| 39 | +## Indenting |
| 40 | + |
| 41 | +Use an indent of 2 spaces, with no tabs. No trailing whitespace. |
| 42 | + |
| 43 | +## String Concatenation |
| 44 | +- :heavy_check_mark: DO prefer template literals to string concatenation. |
| 45 | +- :heavy_check_mark: DO use a space between the `+` and the concatenated parts to improve readability. |
| 46 | +```js |
| 47 | +var string = 'Foo' + bar; |
| 48 | +string = bar + 'foo'; |
| 49 | +string = bar() + 'foo'; |
| 50 | +string = 'foo' + 'bar'; |
| 51 | +``` |
| 52 | +- :heavy_check_mark: WHEN using the concatenating assignment operator ('+='), use a space on each side as with the assignment operator: |
| 53 | +```js |
| 54 | +var string += 'Foo'; |
| 55 | +string += bar; |
| 56 | +string += baz(); |
| 57 | +``` |
| 58 | + |
| 59 | +## CamelCasing |
| 60 | +Unlike the variables and functions defined in Drupal's PHP, multi-word variables and functions in JavaScript should be lowerCamelCased. The first letter of each variable or function should be lowercase, while the first letter of subsequent words should be capitalized. There should be no underscores between the words. |
| 61 | + |
| 62 | +## Semi-colons |
| 63 | +- :heavy_check_mark: DO use a semi-colon except where it's not allowed |
| 64 | + |
| 65 | +JavaScript allows any expression to be used as a statement and uses semi-colons to mark the end of a statement. However, it attempts to make this optional with "semi-colon insertion", which can mask some errors and will also cause JS aggregation to fail. All statements should be followed by `;` except for the following: `for`, `function`, `if`, `switch`, `try`, `while` |
| 66 | + |
| 67 | +The exceptions to this are functions declared like |
| 68 | +```js |
| 69 | +Drupal.behaviors.tableSelect = function (context) { |
| 70 | + // Statements... |
| 71 | +}; |
| 72 | +``` |
| 73 | +and |
| 74 | +```js |
| 75 | +do { |
| 76 | + // Statements... |
| 77 | +} while (condition); |
| 78 | +``` |
| 79 | +These should all be followed by a semi-colon. In addition the `return` value expression must start on the same line as the return keyword in order to avoid semi-colon insertion. |
| 80 | + |
| 81 | +If the "Optimize JavaScript files" performance option in Drupal 6 is enabled, and there are missing semi-colons, then the JS aggregation will fail. It is therefore very important that semi-colons are used. |
| 82 | + |
| 83 | +## Control Structures |
| 84 | +These include `if`, `for`, `while`, `switch`, etc. Here is an example if statement, since it is the most complicated of them: |
| 85 | +```js |
| 86 | +if (condition1 || condition2) |
| 87 | +{ |
| 88 | + action1(); |
| 89 | +} |
| 90 | +else if (condition3 && condition4) |
| 91 | +{ |
| 92 | + action2(); |
| 93 | +} |
| 94 | +else |
| 95 | +{ |
| 96 | + defaultAction(); |
| 97 | +} |
| 98 | +``` |
| 99 | +Control statements should have a new line between the control keyword and opening parenthesis, to distinguish them from function calls. |
| 100 | +- :grey_question: STRONGLY CONSIDER using curly braces even in situations where they are technically optional. Having them increases readability and decreases the likelihood of logic errors being introduced when new lines are added. |
| 101 | + |
| 102 | +### switch |
| 103 | +For `switch` statements: |
| 104 | +```js |
| 105 | +switch (condition) |
| 106 | +{ |
| 107 | + case 1: |
| 108 | + action1(); |
| 109 | + break; |
| 110 | + |
| 111 | + case 2: |
| 112 | + action2(); |
| 113 | + break; |
| 114 | + |
| 115 | + default: |
| 116 | + defaultAction(); |
| 117 | +} |
| 118 | +``` |
| 119 | + |
| 120 | +### try |
| 121 | +The try class of statements should have the following form: |
| 122 | +```js |
| 123 | +try |
| 124 | +{ |
| 125 | + // Statements... |
| 126 | +} |
| 127 | +catch (error) |
| 128 | +{ |
| 129 | + // Error handling... |
| 130 | +} |
| 131 | +finally |
| 132 | +{ |
| 133 | + // Statements... |
| 134 | +} |
| 135 | +``` |
| 136 | + |
| 137 | +### for in |
| 138 | + |
| 139 | +The `for in` statement allows for looping through the names of all of the properties of an object. Unfortunately, all of the members which were inherited through the prototype chain will also be included in the loop. This has the disadvantage of serving up method functions when the interest is in data members. To prevent this, the body of every for in statement should be wrapped in an if statement that does filtering. It can select for a particular type or range of values, or it can exclude functions, or it can exclude properties from the prototype. For example: |
| 140 | +```js |
| 141 | +for (var variable in object) if (filter) |
| 142 | +{ |
| 143 | + // Statements... |
| 144 | +} |
| 145 | +``` |
| 146 | +- :grey_question: CONSIDER avoiding `for in` in favor of `Object.keys(obj).forEach(` which doesn't include properties in the prototype chain |
| 147 | + |
| 148 | +## Functions |
| 149 | + |
| 150 | +### Functions and Methods |
| 151 | + |
| 152 | +Functions and methods should be named using lowerCamelCase. Function names should begin with the name of the module or theme declaring the function, to avoid collisions. Named function expressions are generally preferable, though not very commonly used in jquery. |
| 153 | +```js |
| 154 | +Drupal.behaviors.tableDrag = function (context) { |
| 155 | + for (var base in Drupal.settings.tableDrag) { |
| 156 | + if (!$('#' + base + '.tabledrag-processed', context).size()) { |
| 157 | + $('#' + base).filter(':not(.tabledrag-processed)').each(addBehavior); |
| 158 | + $('#' + base).addClass('tabledrag-processed'); |
| 159 | + } |
| 160 | + } |
| 161 | +}; |
| 162 | +``` |
| 163 | + |
| 164 | +### Function Calls |
| 165 | +Functions should be called with no spaces between the function name, the opening parenthesis, and the first parameter; spaces between commas and each parameter, and no space between the last parameter, the closing parenthesis, and the semicolon. Here's an example: |
| 166 | +```js |
| 167 | +foobar = foo(bar, baz, quux); |
| 168 | +``` |
| 169 | +If a function literal is anonymous, there should be one space between the word function and the left parenthesis. If the space is omitted, then it can appear that the function's name is actually "function". |
| 170 | +```js |
| 171 | +div.onclick = function (e) { |
| 172 | + return false; |
| 173 | +}; |
| 174 | +``` |
| 175 | + |
| 176 | +### Function Declarations |
| 177 | +```js |
| 178 | +function funStuff(field) { |
| 179 | + alert("This JS file does fun message popups."); |
| 180 | + return field; |
| 181 | +} |
| 182 | +``` |
| 183 | +Arguments with default values go at the end of the argument list. Always attempt to return a meaningful value from a function if one is appropriate. Please note the special notion of anonymous functions explained above. |
| 184 | + |
| 185 | +## Variables and Arrays |
| 186 | +All variables should be declared with `const` unless you need to use `let` before they are used and should only be declared once. Doing this makes the program easier to read and makes it easier to detect undeclared variables that may become implied globals. |
| 187 | +- :no_entry: AVOID variables in global scope. The only time something should be made global is when setting the namespace for the module. |
| 188 | + |
| 189 | +### Arrays |
| 190 | +Arrays should be formatted with a space separating each element and assignment operator, if applicable: |
| 191 | +```js |
| 192 | +someArray = ['hello', 'world'] |
| 193 | +``` |
| 194 | + |
| 195 | +## Comments |
| 196 | +Inline documentation for source files should follow the [Doxygen formatting conventions](http://drupal.org/node/1354). |
| 197 | +Non-documentation comments are strongly encouraged. A general rule of thumb is that if you look at a section of code and think "Wow, I don't want to try and describe that", you need to comment it before you forget how it works. Comments can be removed by JS compression utilities later, so they don't negatively impact on the file download size. |
| 198 | + |
| 199 | +Non-documentation comments should use capitalized sentences with punctuation. All caps are used in comments only when referencing constants, e.g., TRUE. Comments should be on a separate line immediately before the code line or block they reference. For example: |
| 200 | +```js |
| 201 | +// Unselect all other checkboxes. |
| 202 | +``` |
| 203 | + |
| 204 | +If each line of a list needs a separate comment, the comments may be given on the same line and may be formatted to a uniform indent for readability. |
| 205 | +C style comments (`/* */`) and standard C++ comments (`//`) are both fine. |
| 206 | + |
| 207 | +## JS code placement |
| 208 | +JavaScript code should not be embedded in the HTML where possible, as it adds significantly to page weight with no opportunity for mitigation by caching and compression. |
| 209 | + |
| 210 | +## "with" statement |
| 211 | +The `with` statement was intended to provide a shorthand for accessing members in deeply nested objects. For example, it is possible to use the following shorthand (but not recommended) to access foo.bar.foobar.abc, etc: |
| 212 | +```js |
| 213 | +with (foo.bar.foobar) { |
| 214 | + const abc = true; |
| 215 | + const xyz = true; |
| 216 | +} |
| 217 | +``` |
| 218 | +However it's impossible to know by looking at the above code which abc and xyz will get modified. Does `foo.bar.foobar` get modified? Or is it the global variables abc and xyz? Instead you should use the explicit longer version: |
| 219 | +```js |
| 220 | +foo.bar.foobar.abc = true; |
| 221 | +foo.bar.foobar.xyz = true; |
| 222 | +``` |
| 223 | +or if you really want to use a shorthand, use the following alternative method: |
| 224 | +```js |
| 225 | +let o = foo.bar.foobar; |
| 226 | +o.abc = true; |
| 227 | +o.xyz = true; |
| 228 | +``` |
| 229 | + |
| 230 | +## Operators |
| 231 | + |
| 232 | +### True or false comparisons |
| 233 | +The `==` and `!=` operators do type coercion before comparing. This is bad because it causes: |
| 234 | +```js |
| 235 | +' \t\r\n' == 0 |
| 236 | +``` |
| 237 | +to be true. This can mask type errors. When comparing to any of the following values, use the `===` or `!==` operators, which do not do type coercion: |
| 238 | +```js |
| 239 | +0 '' undefined null false true |
| 240 | +``` |
| 241 | +`!=` and `==` are good for handling `undefined | null` |
| 242 | + |
| 243 | +### Comma Operator |
| 244 | +The comma operator causes the expressions on either side of it to be executed in left-to-right order, and returns the value of the expression on the right, and should be avoided. Example usage is: |
| 245 | +```js |
| 246 | +var x = (y = 3, z = 9); |
| 247 | +``` |
| 248 | +This sets x to 9. This can be confusing for users not familiar with the syntax and makes the code more difficult to read and understand. So avoid the use of the comma operator except for in the control part of for statements. This does not apply to the comma separator (used in object literals, array literals, etc.) |
| 249 | + |
| 250 | +### Avoiding unreachable code |
| 251 | +To prevent unreachable code, a `return`, `break`, `continue`, or `throw` statement should be followed by a } or `case` or `default`. |
| 252 | + |
| 253 | +## Constructors |
| 254 | +Constructors are functions that are designed to be used with the `new` prefix. The `new` prefix creates a new object based on the function's prototype, and binds that object to the function's implied this parameter. JavaScript doesn't issue compile-time warning or run-time warnings if a required `new` is omitted. If you neglect to use the `new` prefix, no new object will be made and this will be bound to the global object (bad). Constructor functions should be given names with an initial uppercase and a function with an initial uppercase name should not be called unless it has the `new` prefix. |
| 255 | +- :heavy_check_mark: DO - constructor should also share the function name. |
| 256 | + |
| 257 | +## Use literal expressions |
| 258 | +Use literal expressions instead of the new operator: |
| 259 | +- :heavy_check_mark: Instead of `new Array()` use `[]` |
| 260 | +- :heavy_check_mark: Instead of `new Object()` use `{}` |
| 261 | +- :no_entry: Don't use the wrapper forms `new Number`, `new String`, `new Boolean`. |
| 262 | + |
| 263 | +In most cases, the wrapper forms should be the same as the literal expressions. However, this isn't always the case, take the following as an example: |
| 264 | +```js |
| 265 | +var literalNum = 0; |
| 266 | +var objectNum = new Number(0); |
| 267 | +if (literalNum) { } // false because 0 is a false value, will not be executed. |
| 268 | +if (objectNum) { } // true because objectNum exists as an object, will be executed. |
| 269 | +if (objectNum.valueOf()) { } // false because the value of objectNum is 0. |
| 270 | +``` |
| 271 | + |
| 272 | +## eval is evil |
| 273 | +`eval()` is evil. It effectively requires the browser to create an entirely new scripting environment (just like creating a new web page), import all variables from the current scope, execute the script, collect the garbage, and export the variables back into the original environment. Additionally, the code cannot be cached for optimization purposes. It is probably the most powerful and most misused method in JavaScript. It also has aliases. So do not use the `Function` constructor and do not pass strings to `setTimeout()` or `setInterval()`. |
| 274 | + |
| 275 | +## Preventing XSS |
| 276 | +All output to the browser that has been provided by a user should be run through the `Drupal.checkPlain()` function first. This is similar to Drupal's PHP `check_plain()` and encodes special characters in a plain-text string for display as HTML. |
| 277 | + |
| 278 | +## Typeof |
| 279 | +When using a typeof check, don't use the parenthesis for the typeof. The following is the correct coding standard: |
| 280 | +```js |
| 281 | +if (typeof myVariable == 'string') { |
| 282 | + // ... |
| 283 | +} |
| 284 | +``` |
| 285 | + |
| 286 | +## Modifying the DOM |
| 287 | +When adding new HTML elements to the DOM, don't use `document.createElement()`. For cross-browser compatibility reasons and also in an effort to reduce file size, you should use the jQuery equivalent. |
| 288 | +- :no_entry: Dont: |
| 289 | +```js |
| 290 | +this.popup = document.createElement('div'); |
| 291 | +this.popup.id = 'autocomplete'; |
| 292 | +``` |
| 293 | +- :heavy_check_mark: Do: |
| 294 | +```js |
| 295 | +this.popup = $('<div id="autocomplete"></div>')[0]; |
| 296 | +``` |
0 commit comments