Commit 99c49164 authored by Marcia Ramos's avatar Marcia Ramos

Merge branch 'docs-javascript-style' into 'master'

Docs: Bring Javascript Style Guide in line with docs standards

See merge request gitlab-org/gitlab-ce!25026
parents db9ed1cf 8ed98b6b
# JavaScript style guide # JavaScript style guide
We use [Airbnb's JavaScript Style Guide][airbnb-style-guide] and it's accompanying linter to manage most of our JavaScript style guidelines. We use [Airbnb's JavaScript Style Guide](https://github.com/airbnb/javascript) and it's accompanying
linter to manage most of our JavaScript style guidelines.
In addition to the style guidelines set by Airbnb, we also have a few specific rules listed below. In addition to the style guidelines set by Airbnb, we also have a few specific rules
listed below.
> **Tip:** > **Tip:**
You can run eslint locally by running `yarn eslint` You can run eslint locally by running `yarn eslint`
## Arrays ## Avoid forEach
<a name="avoid-foreach"></a><a name="1.1"></a> Avoid forEach when mutating data. Use `map`, `reduce` or `filter` instead of `forEach`
when mutating data. This will minimize mutations in functions,
which aligns with [Airbnb's style guide](https://github.com/airbnb/javascript#testing--for-real).
- [1.1](#avoid-foreach) **Avoid ForEach when mutating data** Use `map`, `reduce` or `filter` instead of `forEach` when mutating data. This will minimize mutations in functions ([which is aligned with Airbnb's style guide][airbnb-minimize-mutations]) ```javascript
// bad
``` users.forEach((user, index) => {
// bad
users.forEach((user, index) => {
user.id = index; user.id = index;
}); });
// good // good
const usersWithId = users.map((user, index) => { const usersWithId = users.map((user, index) => {
return Object.assign({}, user, { id: index }); return Object.assign({}, user, { id: index });
}); });
``` ```
## Functions ## Limit number of parameters
<a name="limit-params"></a><a name="2.1"></a> If your function or method has more than 3 parameters, use an object as a parameter
instead.
- [2.1](#limit-params) **Limit number of parameters** If your function or method has more than 3 parameters, use an object as a parameter instead. ```javascript
// bad
``` function a(p1, p2, p3) {
// bad
function a(p1, p2, p3) {
// ... // ...
}; };
// good // good
function a(p) { function a(p) {
// ... // ...
}; };
``` ```
## Classes & constructors
<a name="avoid-constructor-side-effects"></a><a name="3.1"></a> ## Avoid side effects in constructors
- [3.1](#avoid-constructor-side-effects) **Avoid side effects in constructors** Avoid making some operations in the `constructor`, such as asynchronous calls, API requests and DOM manipulations. Prefer moving them into separate functions. This will make tests easier to write and code easier to maintain. Avoid making asynchronous calls, API requests or DOM manipulations in the `constructor`.
Move them into separate functions instead. This will make tests easier to write and
code easier to maintain.
```javascript ```javascript
// bad // bad
class myClass { class myClass {
constructor(config) { constructor(config) {
this.config = config; this.config = config;
axios.get(this.config.endpoint) axios.get(this.config.endpoint)
} }
} }
// good // good
class myClass { class myClass {
constructor(config) { constructor(config) {
this.config = config; this.config = config;
} }
...@@ -67,19 +68,20 @@ You can run eslint locally by running `yarn eslint` ...@@ -67,19 +68,20 @@ You can run eslint locally by running `yarn eslint`
makeRequest() { makeRequest() {
axios.get(this.config.endpoint) axios.get(this.config.endpoint)
} }
} }
const instance = new myClass(); const instance = new myClass();
instance.makeRequest(); instance.makeRequest();
``` ```
<a name="avoid-classes-to-handle-dom-events"></a><a name="3.2"></a> ## Avoid classes to handle DOM events
- [3.2](#avoid-classes-to-handle-dom-events) **Avoid classes to handle DOM events** If the only purpose of the class is to bind a DOM event and handle the callback, prefer using a function. If the only purpose of the class is to bind a DOM event and handle the callback, prefer
using a function.
``` ```javascript
// bad // bad
class myClass { class myClass {
constructor(config) { constructor(config) {
this.config = config; this.config = config;
} }
...@@ -87,122 +89,108 @@ You can run eslint locally by running `yarn eslint` ...@@ -87,122 +89,108 @@ You can run eslint locally by running `yarn eslint`
init() { init() {
document.addEventListener('click', () => {}); document.addEventListener('click', () => {});
} }
} }
// good // good
const myFunction = () => { const myFunction = () => {
document.addEventListener('click', () => { document.addEventListener('click', () => {
// handle callback here // handle callback here
}); });
} }
``` ```
<a name="element-container"></a><a name="3.3"></a> ## Pass element container to constructor
- [3.3](#element-container) **Pass element container to constructor** When your class manipulates the DOM, receive the element container as a parameter. When your class manipulates the DOM, receive the element container as a parameter.
This is more maintainable and performant. This is more maintainable and performant.
``` ```javascript
// bad // bad
class a { class a {
constructor() { constructor() {
document.querySelector('.b'); document.querySelector('.b');
} }
} }
// good // good
class a { class a {
constructor(options) { constructor(options) {
options.container.querySelector('.b'); options.container.querySelector('.b');
} }
} }
``` ```
## Type Casting & Coercion
<a name="use-parseint"></a><a name="4.1"></a>
- [4.1](#use-parseint) **Use ParseInt** Use `ParseInt` when converting a numeric string into a number.
```
// bad
Number('10')
// good
parseInt('10', 10);
```
## CSS Selectors
<a name="use-js-prefix"></a><a name="5.1"></a>
- [5.1](#use-js-prefix) **Use js prefix** If a CSS class is only being used in JavaScript as a reference to the element, prefix the class name with `js-`
```
// bad
<button class="add-user"></button>
// good ## Use ParseInt
<button class="js-add-user"></button>
```
## Modules Use `ParseInt` when converting a numeric string into a number.
<a name="use-absolute-paths"></a><a name="6.1"></a> ```javascript
// bad
Number('10')
- [6.1](#use-absolute-paths) **Use absolute paths for nearby modules** Use absolute paths if the module you are importing is less than two levels up. // good
parseInt('10', 10);
```
``` ## CSS Selectors - Use `js-` prefix
// bad
import GitLabStyleGuide from '~/guides/GitLabStyleGuide';
// good If a CSS class is only being used in JavaScript as a reference to the element, prefix
import GitLabStyleGuide from '../GitLabStyleGuide'; the class name with `js-`.
```
<a name="use-relative-paths"></a><a name="6.2"></a> ```html
// bad
<button class="add-user"></button>
- [6.2](#use-relative-paths) **Use relative paths for distant modules** If the module you are importing is two or more levels up, use a relative path instead of an absolute path. // good
<button class="js-add-user"></button>
```
``` ## Absolute vs relative paths for modules
// bad
import GitLabStyleGuide from '../../../guides/GitLabStyleGuide';
// good Use relative paths if the module you are importing is less than two levels up.
import GitLabStyleGuide from '~/GitLabStyleGuide';
```
<a name="global-namespace"></a><a name="6.3"></a> ```javascript
// bad
import GitLabStyleGuide from '~/guides/GitLabStyleGuide';
- [6.3](#global-namespace) **Do not add to global namespace** // good
import GitLabStyleGuide from '../GitLabStyleGuide';
```
<a name="domcontentloaded"></a><a name="6.4"></a> If the module you are importing is two or more levels up, use an absolute path instead:
- [6.4](#domcontentloaded) **Do not use DOMContentLoaded in non-page modules** Imported modules should act the same each time they are loaded. `DOMContentLoaded` events are only allowed on modules loaded in the `/pages/*` directory because those are loaded dynamically with webpack. ```javascript
// bad
import GitLabStyleGuide from '../../../guides/GitLabStyleGuide';
## Security // good
import GitLabStyleGuide from '~/GitLabStyleGuide';
```
<a name="avoid-xss"></a><a name="7.1"></a> Additionally, **do not add to global namespace**.
- [7.1](#avoid-xss) **Avoid XSS** Do not use `innerHTML`, `append()` or `html()` to set content. It opens up too many vulnerabilities. ## Do not use `DOMContentLoaded` in non-page modules
## ESLint Imported modules should act the same each time they are loaded. `DOMContentLoaded`
events are only allowed on modules loaded in the `/pages/*` directory because those
are loaded dynamically with webpack.
<a name="disable-eslint-file"></a><a name="8.1"></a> ## Avoid XSS
- [8.1](#disable-eslint-file) **Disabling ESLint in new files** Do not disable ESLint when creating new files. Existing files may have existing rules disabled due to legacy compatibility reasons but they are in the process of being refactored. Do not use `innerHTML`, `append()` or `html()` to set content. It opens up too many
vulnerabilities.
<a name="disable-eslint-rule"></a><a name="8.2"></a> ## Disabling ESLint in new files
- [8.2](#disable-eslint-rule) **Disabling ESLint rule** Do not disable specific ESLint rules. Due to technical debt, you may disable the following rules only if you are invoking/instantiating existing code modules Do not disable ESLint when creating new files. Existing files may have existing rules
disabled due to legacy compatibility reasons but they are in the process of being refactored.
- [no-new][no-new] Do not disable specific ESLint rules. Due to technical debt, you may disable the following
- [class-method-use-this][class-method-use-this] rules only if you are invoking/instantiating existing code modules.
> Note: Disable these rules on a per line basis. This makes it easier to refactor in the future. E.g. use `eslint-disable-next-line` or `eslint-disable-line` - [no-new](http://eslint.org/docs/rules/no-new)
- [class-method-use-this](http://eslint.org/docs/rules/class-methods-use-this)
[airbnb-style-guide]: https://github.com/airbnb/javascript > Note: Disable these rules on a per line basis. This makes it easier to refactor
[airbnb-minimize-mutations]: https://github.com/airbnb/javascript#testing--for-real in the future. E.g. use `eslint-disable-next-line` or `eslint-disable-line`.
[no-new]: http://eslint.org/docs/rules/no-new
[class-method-use-this]: http://eslint.org/docs/rules/class-methods-use-this
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment