Skip to content

Commit 25ddd6e

Browse files
Update JS Guidlines (#171)
* Update JS Guidlines - Add ESLint rule recommendations - Remove all the references to Drupal - Remove things that are obsolete in 2021. * Update javascript.md * Update docs/coding/javascript.md Co-authored-by: Steve Byerly <1393464+SteveByerly@users.noreply.github.com> * Update javascript.md Co-authored-by: Steve Byerly <1393464+SteveByerly@users.noreply.github.com>
1 parent 90095bb commit 25ddd6e

File tree

1 file changed

+43
-60
lines changed

1 file changed

+43
-60
lines changed

docs/coding/javascript.md

Lines changed: 43 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,13 @@ var string += 'Foo';
5555
string += bar;
5656
string += baz();
5757
```
58+
- :heavy_check_mark: DO configure ESLint rules `space-infix-ops` and `space-unary-ops` with option `"error"`.
5859

5960
## 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+
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.
62+
- :heavy_check_mark: DO configure ESLint rule `camelcase` with option `["error", {"properties": "always"}]`.
6163

6264
## Semi-colons
63-
- :heavy_check_mark: DO use a semi-colon except where it's not allowed
6465

6566
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`
6667

@@ -78,89 +79,72 @@ do {
7879
```
7980
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.
8081

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+
- :heavy_check_mark: DO use a semi-colon except where it's not allowed
83+
- :heavy_check_mark: DO configure ESLint rule `semi` with option `["error", "always"]`. If using TypeScript, use rule `@typescript-eslint/semi` instead and turn `semi` off.
8284

8385
## Control Structures
8486
These include `if`, `for`, `while`, `switch`, etc. Here is an example if statement, since it is the most complicated of them:
8587
```js
86-
if (condition1 || condition2)
87-
{
88+
if (condition1 || condition2) {
8889
action1();
8990
}
90-
else if (condition3 && condition4)
91-
{
91+
else if (condition3 && condition4) {
9292
action2();
9393
}
94-
else
95-
{
94+
else {
9695
defaultAction();
9796
}
9897
```
99-
Control statements should have a new line between the control keyword and opening parenthesis, to distinguish them from function calls.
98+
99+
Control statements should have single spaces between the control keyword, control condition, and opening parenthesis.
100100
- :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.
101101

102102
### switch
103103
For `switch` statements:
104104
```js
105-
switch (condition)
106-
{
105+
switch (condition) {
107106
case 1:
108-
action1();
109-
break;
107+
action1();
108+
break;
110109

111110
case 2:
112-
action2();
113-
break;
111+
action2();
112+
break;
114113

115114
default:
116-
defaultAction();
115+
defaultAction();
117116
}
118117
```
119118

120119
### try
121120
The try class of statements should have the following form:
122121
```js
123-
try
124-
{
125-
// Statements...
122+
try {
123+
// Statements...
126124
}
127-
catch (error)
128-
{
129-
// Error handling...
125+
catch (error) {
126+
// Error handling...
130127
}
131-
finally
132-
{
133-
// Statements...
128+
finally {
129+
// Statements...
134130
}
135131
```
132+
Omit the `catch` or `finally` clauses if they are not needed, but never omit both.
136133

137134
### for in
138135

139136
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:
140137
```js
141-
for (var variable in object) if (filter)
142-
{
143-
// Statements...
138+
for (var variable in object) {
139+
if (filter) {
140+
// Statements...
141+
}
144142
}
145143
```
146-
- :grey_question: CONSIDER avoiding `for in` in favor of `Object.keys(obj).forEach(` which doesn't include properties in the prototype chain
144+
- :grey_question: CONSIDER configuring ESLint rule `guard-for-in` with option `"error"`, or avoiding `for in` in favor of `Object.keys(obj).forEach(` which doesn't include properties in the prototype chain.
147145

148146
## Functions
149147

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-
164148
### Function Calls
165149
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:
166150
```js
@@ -172,6 +156,8 @@ div.onclick = function (e) {
172156
return false;
173157
};
174158
```
159+
- :heavy_check_mark: DO configure ESLint rule `func-call-spacing` with option `["error", "never"]`.
160+
- :heavy_check_mark: DO configure ESLint rule `space-before-function-paren` with option `["error", {"anonymous": "always", "named": "never", "asyncArrow": "always"}]`.
175161

176162
### Function Declarations
177163
```js
@@ -181,6 +167,7 @@ function funStuff(field) {
181167
}
182168
```
183169
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.
170+
- :heavy_check_mark: DO configure ESLint rule `default-param-last` with option `"error"`.
184171

185172
## Variables and Arrays
186173
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.
@@ -193,7 +180,7 @@ someArray = ['hello', 'world']
193180
```
194181

195182
## Comments
196-
Inline documentation for source files should follow the [Doxygen formatting conventions](http://drupal.org/node/1354).
183+
Inline documentation for source files should follow the [jsdoc formatting conventions](https://jsdoc.app/about-getting-started.html).
197184
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.
198185

199186
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:
@@ -226,6 +213,7 @@ let o = foo.bar.foobar;
226213
o.abc = true;
227214
o.xyz = true;
228215
```
216+
- :heavy_check_mark: DO configure ESLint rule `no-with` with option `"error"`.
229217

230218
## Operators
231219

@@ -239,20 +227,25 @@ to be true. This can mask type errors. When comparing to any of the following va
239227
0 '' undefined null false true
240228
```
241229
`!=` and `==` are good for handling `undefined | null`
230+
- :no_entry: AVOID the `==` and `!=` operators in favor of `===` or `!==`, unless checking against the literal `null` when checking against both `null` and `undefined` is desired.
231+
- :heavy_check_mark: DO - Configure ESLint rule `eqeqeq` with option `["error", "smart"]`.
242232

243233
### Comma Operator
244234
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:
245235
```js
246236
var x = (y = 3, z = 9);
247237
```
248238
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.)
239+
- :no_entry: AVOID the comma operator
240+
- :heavy_check_mark: DO - Configure ESLint rule `no-sequences` with option `"error"`.
249241

250242
### Avoiding unreachable code
251243
To prevent unreachable code, a `return`, `break`, `continue`, or `throw` statement should be followed by a } or `case` or `default`.
244+
- :heavy_check_mark: DO - Configure ESLint rule `no-unreachable` with option `"error"`.
252245

253246
## Constructors
254247
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.
248+
- :heavy_check_mark: DO use ES2015 class syntax, downleveling via Babel transpilation if obsolete browsers must be supported.
256249

257250
## Use literal expressions
258251
Use literal expressions instead of the new operator:
@@ -268,12 +261,14 @@ if (literalNum) { } // false because 0 is a false value, will not be executed.
268261
if (objectNum) { } // true because objectNum exists as an object, will be executed.
269262
if (objectNum.valueOf()) { } // false because the value of objectNum is 0.
270263
```
264+
- :heavy_check_mark: DO configure ESLint rules `no-new-wrappers`, `no-new-object`, and `no-array-constructor` with option `"error"`.
271265

272266
## 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()`.
267+
`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()`. NEVER pass untrusted strings to `eval()` or similar functions.
268+
- :heavy_check_mark: DO configure ESLint rules `no-implied-eval` and `no-new-func` with option `"error"`.
274269

275270
## 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.
271+
Rendering untrusted strings to HTML should be done through the appropriate mechanisms for the framework being used, if any. For example, `{{interpolation}}` for Vue and Angular, `{interpolation}` for React, or `data-bind="text: stringVariable"` for Knockout. If no framework is being used, untrusted input should be rendered by setting the [`textContent`](https://developer.mozilla.org/en-US/docs/Web/API/Node/textContent) of the desired DOM node or by creating a new text node with [`document.createTextNode()`](https://developer.mozilla.org/en-US/docs/Web/API/Document/createTextNode).
277272

278273
## Typeof
279274
When using a typeof check, don't use the parenthesis for the typeof. The following is the correct coding standard:
@@ -282,15 +277,3 @@ if (typeof myVariable == 'string') {
282277
// ...
283278
}
284279
```
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

Comments
 (0)