Adds numbered lists to easily point to documentation
This commit is contained in:
parent
9eab1db9a2
commit
f68f0cd1ea
2 changed files with 282 additions and 287 deletions
|
@ -11,207 +11,197 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns.
|
|||
|
||||
#### ESlint
|
||||
|
||||
- **Never** disable eslint rules unless you have a good reason. You may see a lot of legacy files with `/* eslint-disable some-rule, some-other-rule */` at the top, but legacy files are a special case. Any time you develop a new feature or refactor an existing one, you should abide by the eslint rules.
|
||||
|
||||
- **Never Ever EVER** disable eslint globally for a file
|
||||
1. **Never** disable eslint rules unless you have a good reason. You may see a lot of legacy files with `/* eslint-disable some-rule, some-other-rule */` at the top, but legacy files are a special case. Any time you develop a new feature or refactor an existing one, you should abide by the eslint rules.
|
||||
|
||||
1. **Never Ever EVER** disable eslint globally for a file
|
||||
```javascript
|
||||
// bad
|
||||
/* eslint-disable */
|
||||
// bad
|
||||
/* eslint-disable */
|
||||
|
||||
// better
|
||||
/* eslint-disable some-rule, some-other-rule */
|
||||
// better
|
||||
/* eslint-disable some-rule, some-other-rule */
|
||||
|
||||
// best
|
||||
// nothing :)
|
||||
// best
|
||||
// nothing :)
|
||||
```
|
||||
|
||||
- If you do need to disable a rule for a single violation, try to do it as locally as possible
|
||||
|
||||
1. If you do need to disable a rule for a single violation, try to do it as locally as possible
|
||||
```javascript
|
||||
// bad
|
||||
/* eslint-disable no-new */
|
||||
// bad
|
||||
/* eslint-disable no-new */
|
||||
|
||||
import Foo from 'foo';
|
||||
import Foo from 'foo';
|
||||
|
||||
new Foo();
|
||||
new Foo();
|
||||
|
||||
// better
|
||||
import Foo from 'foo';
|
||||
// better
|
||||
import Foo from 'foo';
|
||||
|
||||
// eslint-disable-next-line no-new
|
||||
new Foo();
|
||||
// eslint-disable-next-line no-new
|
||||
new Foo();
|
||||
```
|
||||
1. There are few rules that we need to disable due to technical debt. Which are:
|
||||
1. [no-new][eslint-new]
|
||||
1. [class-methods-use-this][eslint-this]
|
||||
|
||||
1. When they are needed _always_ place ESlint directive comment blocks on the first line of a script, followed by any global declarations, then a blank newline prior to any imports or code.
|
||||
```javascript
|
||||
// bad
|
||||
/* global Foo */
|
||||
/* eslint-disable no-new */
|
||||
import Bar from './bar';
|
||||
|
||||
// good
|
||||
/* eslint-disable no-new */
|
||||
/* global Foo */
|
||||
|
||||
import Bar from './bar';
|
||||
```
|
||||
|
||||
- When they are needed _always_ place ESlint directive comment blocks on the first line of a script, followed by any global declarations, then a blank newline prior to any imports or code.
|
||||
1. **Never** disable the `no-undef` rule. Declare globals with `/* global Foo */` instead.
|
||||
|
||||
1. When declaring multiple globals, always use one `/* global [name] */` line per variable.
|
||||
```javascript
|
||||
// bad
|
||||
/* global Foo */
|
||||
/* eslint-disable no-new */
|
||||
import Bar from './bar';
|
||||
// bad
|
||||
/* globals Flash, Cookies, jQuery */
|
||||
|
||||
// good
|
||||
/* eslint-disable no-new */
|
||||
/* global Foo */
|
||||
|
||||
import Bar from './bar';
|
||||
// good
|
||||
/* global Flash */
|
||||
/* global Cookies */
|
||||
/* global jQuery */
|
||||
```
|
||||
|
||||
- **Never** disable the `no-undef` rule. Declare globals with `/* global Foo */` instead.
|
||||
|
||||
- When declaring multiple globals, always use one `/* global [name] */` line per variable.
|
||||
|
||||
1. Use up to 3 parameters for a function or class. If you need more accept an Object instead.
|
||||
```javascript
|
||||
// bad
|
||||
/* globals Flash, Cookies, jQuery */
|
||||
// bad
|
||||
fn(p1, p2, p3, p4) {}
|
||||
|
||||
// good
|
||||
/* global Flash */
|
||||
/* global Cookies */
|
||||
/* global jQuery */
|
||||
```
|
||||
|
||||
- Use up to 3 parameters for a function or class. If you need more accept an Object instead.
|
||||
|
||||
```javascript
|
||||
// bad
|
||||
fn(p1, p2, p3, p4) {}
|
||||
|
||||
// good
|
||||
fn(options) {}
|
||||
// good
|
||||
fn(options) {}
|
||||
```
|
||||
|
||||
#### Modules, Imports, and Exports
|
||||
- Use ES module syntax to import modules
|
||||
|
||||
1. Use ES module syntax to import modules
|
||||
```javascript
|
||||
// bad
|
||||
require('foo');
|
||||
// bad
|
||||
require('foo');
|
||||
|
||||
// good
|
||||
import Foo from 'foo';
|
||||
// good
|
||||
import Foo from 'foo';
|
||||
|
||||
// bad
|
||||
module.exports = Foo;
|
||||
// bad
|
||||
module.exports = Foo;
|
||||
|
||||
// good
|
||||
export default Foo;
|
||||
// good
|
||||
export default Foo;
|
||||
```
|
||||
|
||||
- Relative paths
|
||||
1. Relative paths: Unless you are writing a test, always reference other scripts using relative paths instead of `~`
|
||||
* In **app/assets/javascripts**:
|
||||
|
||||
Unless you are writing a test, always reference other scripts using relative paths instead of `~`
|
||||
```javascript
|
||||
// bad
|
||||
import Foo from '~/foo'
|
||||
|
||||
In **app/assets/javascripts**:
|
||||
// good
|
||||
import Foo from '../foo';
|
||||
```
|
||||
* In **spec/javascripts**:
|
||||
|
||||
```javascript
|
||||
// bad
|
||||
import Foo from '../../app/assets/javascripts/foo'
|
||||
|
||||
// good
|
||||
import Foo from '~/foo';
|
||||
```
|
||||
|
||||
1. Avoid using IIFE. Although we have a lot of examples of files which wrap their contents in IIFEs (immediately-invoked function expressions), this is no longer necessary after the transition from Sprockets to webpack. Do not use them anymore and feel free to remove them when refactoring legacy code.
|
||||
|
||||
1. Avoid adding to the global namespace.
|
||||
```javascript
|
||||
// bad
|
||||
import Foo from '~/foo'
|
||||
// bad
|
||||
window.MyClass = class { /* ... */ };
|
||||
|
||||
// good
|
||||
import Foo from '../foo';
|
||||
// good
|
||||
export default class MyClass { /* ... */ }
|
||||
```
|
||||
|
||||
In **spec/javascripts**:
|
||||
1. Side effects are forbidden in any script which contains exports
|
||||
```javascript
|
||||
// bad
|
||||
import Foo from '../../app/assets/javascripts/foo'
|
||||
// bad
|
||||
export default class MyClass { /* ... */ }
|
||||
|
||||
// good
|
||||
import Foo from '~/foo';
|
||||
```
|
||||
|
||||
- Avoid using IIFE. Although we have a lot of examples of files which wrap their contents in IIFEs (immediately-invoked function expressions), this is no longer necessary after the transition from Sprockets to webpack. Do not use them anymore and feel free to remove them when refactoring legacy code.
|
||||
|
||||
- Avoid adding to the global namespace.
|
||||
|
||||
```javascript
|
||||
// bad
|
||||
window.MyClass = class { /* ... */ };
|
||||
|
||||
// good
|
||||
export default class MyClass { /* ... */ }
|
||||
```
|
||||
|
||||
- Side effects are forbidden in any script which contains exports
|
||||
|
||||
```javascript
|
||||
// bad
|
||||
export default class MyClass { /* ... */ }
|
||||
|
||||
document.addEventListener("DOMContentLoaded", function(event) {
|
||||
new MyClass();
|
||||
}
|
||||
document.addEventListener("DOMContentLoaded", function(event) {
|
||||
new MyClass();
|
||||
}
|
||||
```
|
||||
|
||||
|
||||
#### Data Mutation and Pure functions
|
||||
- Strive to write many small pure functions, and minimize where mutations occur.
|
||||
|
||||
1. Strive to write many small pure functions, and minimize where mutations occur.
|
||||
```javascript
|
||||
// bad
|
||||
const values = {foo: 1};
|
||||
// bad
|
||||
const values = {foo: 1};
|
||||
|
||||
function impureFunction(items) {
|
||||
const bar = 1;
|
||||
function impureFunction(items) {
|
||||
const bar = 1;
|
||||
|
||||
items.foo = items.a * bar + 2;
|
||||
items.foo = items.a * bar + 2;
|
||||
|
||||
return items.a;
|
||||
}
|
||||
return items.a;
|
||||
}
|
||||
|
||||
const c = impureFunction(values);
|
||||
const c = impureFunction(values);
|
||||
|
||||
// good
|
||||
var values = {foo: 1};
|
||||
// good
|
||||
var values = {foo: 1};
|
||||
|
||||
function pureFunction (foo) {
|
||||
var bar = 1;
|
||||
function pureFunction (foo) {
|
||||
var bar = 1;
|
||||
|
||||
foo = foo * bar + 2;
|
||||
foo = foo * bar + 2;
|
||||
|
||||
return foo;
|
||||
}
|
||||
return foo;
|
||||
}
|
||||
|
||||
var c = pureFunction(values.foo);
|
||||
var c = pureFunction(values.foo);
|
||||
```
|
||||
|
||||
- Avoid constructors with side-effects
|
||||
1. Avoid constructors with side-effects
|
||||
|
||||
- Prefer `.map`, `.reduce` or `.filter` over `.forEach`
|
||||
1. Prefer `.map`, `.reduce` or `.filter` over `.forEach`
|
||||
A forEach will cause side effects, it will be mutating the array being iterated. Prefer using `.map`,
|
||||
`.reduce` or `.filter`
|
||||
|
||||
```javascript
|
||||
const users = [ { name: 'Foo' }, { name: 'Bar' } ];
|
||||
const users = [ { name: 'Foo' }, { name: 'Bar' } ];
|
||||
|
||||
// bad
|
||||
users.forEach((user, index) => {
|
||||
user.id = index;
|
||||
});
|
||||
// bad
|
||||
users.forEach((user, index) => {
|
||||
user.id = index;
|
||||
});
|
||||
|
||||
// good
|
||||
const usersWithId = users.map((user, index) => {
|
||||
return Object.assign({}, user, { id: index });
|
||||
});
|
||||
// good
|
||||
const usersWithId = users.map((user, index) => {
|
||||
return Object.assign({}, user, { id: index });
|
||||
});
|
||||
```
|
||||
|
||||
#### Parse Strings into Numbers
|
||||
- `parseInt()` is preferable over `Number()` or `+`
|
||||
|
||||
1. `parseInt()` is preferable over `Number()` or `+`
|
||||
```javascript
|
||||
// bad
|
||||
+'10' // 10
|
||||
// bad
|
||||
+'10' // 10
|
||||
|
||||
// good
|
||||
Number('10') // 10
|
||||
// good
|
||||
Number('10') // 10
|
||||
|
||||
// better
|
||||
parseInt('10', 10);
|
||||
// better
|
||||
parseInt('10', 10);
|
||||
```
|
||||
|
||||
#### CSS classes used for JavaScript
|
||||
- If the class is being used in Javascript it needs to be prepend with `js-`
|
||||
1. If the class is being used in Javascript it needs to be prepend with `js-`
|
||||
```html
|
||||
// bad
|
||||
<button class="add-user">
|
||||
|
@ -226,234 +216,222 @@ A forEach will cause side effects, it will be mutating the array being iterated.
|
|||
|
||||
### Vue.js
|
||||
|
||||
|
||||
#### Basic Rules
|
||||
- Only include one Vue.js component per file.
|
||||
- Export components as plain objects:
|
||||
|
||||
1. Only include one Vue.js component per file.
|
||||
1. Export components as plain objects:
|
||||
```javascript
|
||||
export default {
|
||||
template: `<h1>I'm a component</h1>
|
||||
}
|
||||
export default {
|
||||
template: `<h1>I'm a component</h1>
|
||||
}
|
||||
```
|
||||
1.
|
||||
|
||||
#### Naming
|
||||
- **Extensions**: Use `.vue` extension for Vue components.
|
||||
- **Reference Naming**: Use PascalCase for Vue components and camelCase for their instances:
|
||||
|
||||
1. **Extensions**: Use `.vue` extension for Vue components.
|
||||
1. **Reference Naming**: Use PascalCase for Vue components and camelCase for their instances:
|
||||
```javascript
|
||||
// bad
|
||||
import cardBoard from 'cardBoard';
|
||||
// bad
|
||||
import cardBoard from 'cardBoard';
|
||||
|
||||
// good
|
||||
import CardBoard from 'cardBoard'
|
||||
// good
|
||||
import CardBoard from 'cardBoard'
|
||||
|
||||
// bad
|
||||
components: {
|
||||
CardBoard: CardBoard
|
||||
};
|
||||
// bad
|
||||
components: {
|
||||
CardBoard: CardBoard
|
||||
};
|
||||
|
||||
// good
|
||||
components: {
|
||||
cardBoard: CardBoard
|
||||
};
|
||||
// good
|
||||
components: {
|
||||
cardBoard: CardBoard
|
||||
};
|
||||
```
|
||||
|
||||
- **Props Naming:**
|
||||
- Avoid using DOM component prop names.
|
||||
- Use kebab-case instead of camelCase to provide props in templates.
|
||||
|
||||
1. **Props Naming:**
|
||||
1. Avoid using DOM component prop names.
|
||||
1. Use kebab-case instead of camelCase to provide props in templates.
|
||||
```javascript
|
||||
// bad
|
||||
<component class="btn">
|
||||
// bad
|
||||
<component class="btn">
|
||||
|
||||
// good
|
||||
<component css-class="btn">
|
||||
// good
|
||||
<component css-class="btn">
|
||||
|
||||
// bad
|
||||
<component myProp="prop" />
|
||||
// bad
|
||||
<component myProp="prop" />
|
||||
|
||||
// good
|
||||
<component my-prop="prop" />
|
||||
```
|
||||
// good
|
||||
<component my-prop="prop" />
|
||||
```
|
||||
|
||||
#### Alignment
|
||||
- Follow these alignment styles for the template method:
|
||||
|
||||
1. Follow these alignment styles for the template method:
|
||||
```javascript
|
||||
// bad
|
||||
<component v-if="bar"
|
||||
param="baz" />
|
||||
// bad
|
||||
<component v-if="bar"
|
||||
param="baz" />
|
||||
|
||||
<button class="btn">Click me</button>
|
||||
<button class="btn">Click me</button>
|
||||
|
||||
// good
|
||||
<component
|
||||
v-if="bar"
|
||||
param="baz"
|
||||
/>
|
||||
// good
|
||||
<component
|
||||
v-if="bar"
|
||||
param="baz"
|
||||
/>
|
||||
|
||||
<button class="btn">
|
||||
Click me
|
||||
</button>
|
||||
<button class="btn">
|
||||
Click me
|
||||
</button>
|
||||
|
||||
// if props fit in one line then keep it on the same line
|
||||
<component bar="bar" />
|
||||
// if props fit in one line then keep it on the same line
|
||||
<component bar="bar" />
|
||||
```
|
||||
|
||||
#### Quotes
|
||||
- Always use double quotes `"` inside templates and single quotes `'` for all other JS.
|
||||
|
||||
1. Always use double quotes `"` inside templates and single quotes `'` for all other JS.
|
||||
```javascript
|
||||
// bad
|
||||
template: `
|
||||
<button :class='style'>Button</button>
|
||||
`
|
||||
// bad
|
||||
template: `
|
||||
<button :class='style'>Button</button>
|
||||
`
|
||||
|
||||
// good
|
||||
template: `
|
||||
<button :class="style">Button</button>
|
||||
`
|
||||
// good
|
||||
template: `
|
||||
<button :class="style">Button</button>
|
||||
`
|
||||
```
|
||||
|
||||
#### Props
|
||||
- Props should be declared as an object
|
||||
|
||||
1. Props should be declared as an object
|
||||
```javascript
|
||||
// bad
|
||||
props: ['foo']
|
||||
// bad
|
||||
props: ['foo']
|
||||
|
||||
// good
|
||||
props: {
|
||||
foo: {
|
||||
type: String,
|
||||
required: false,
|
||||
default: 'bar'
|
||||
// good
|
||||
props: {
|
||||
foo: {
|
||||
type: String,
|
||||
required: false,
|
||||
default: 'bar'
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- Required key should always be provided when declaring a prop
|
||||
|
||||
1. Required key should always be provided when declaring a prop
|
||||
```javascript
|
||||
// bad
|
||||
props: {
|
||||
foo: {
|
||||
type: String,
|
||||
// bad
|
||||
props: {
|
||||
foo: {
|
||||
type: String,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// good
|
||||
props: {
|
||||
foo: {
|
||||
type: String,
|
||||
required: false,
|
||||
default: 'bar'
|
||||
// good
|
||||
props: {
|
||||
foo: {
|
||||
type: String,
|
||||
required: false,
|
||||
default: 'bar'
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- Default key should always be provided if the prop is not required:
|
||||
|
||||
1. Default key should always be provided if the prop is not required:
|
||||
```javascript
|
||||
// bad
|
||||
props: {
|
||||
foo: {
|
||||
type: String,
|
||||
required: false,
|
||||
// bad
|
||||
props: {
|
||||
foo: {
|
||||
type: String,
|
||||
required: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// good
|
||||
props: {
|
||||
foo: {
|
||||
type: String,
|
||||
required: false,
|
||||
default: 'bar'
|
||||
// good
|
||||
props: {
|
||||
foo: {
|
||||
type: String,
|
||||
required: false,
|
||||
default: 'bar'
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// good
|
||||
props: {
|
||||
foo: {
|
||||
type: String,
|
||||
required: true
|
||||
// good
|
||||
props: {
|
||||
foo: {
|
||||
type: String,
|
||||
required: true
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
#### Data
|
||||
- `data` method should always be a function
|
||||
1. `data` method should always be a function
|
||||
|
||||
```javascript
|
||||
// bad
|
||||
data: {
|
||||
foo: 'foo'
|
||||
}
|
||||
|
||||
// good
|
||||
data() {
|
||||
return {
|
||||
// bad
|
||||
data: {
|
||||
foo: 'foo'
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// good
|
||||
data() {
|
||||
return {
|
||||
foo: 'foo'
|
||||
};
|
||||
}
|
||||
```
|
||||
|
||||
#### Directives
|
||||
|
||||
- Shorthand `@` is preferable over `v-on`
|
||||
|
||||
1. Shorthand `@` is preferable over `v-on`
|
||||
```javascript
|
||||
// bad
|
||||
<component v-on:click="eventHandler"/>
|
||||
// bad
|
||||
<component v-on:click="eventHandler"/>
|
||||
|
||||
|
||||
// good
|
||||
<component @click="eventHandler"/>
|
||||
// good
|
||||
<component @click="eventHandler"/>
|
||||
```
|
||||
|
||||
- Shorthand `:` is preferable over `v-bind`
|
||||
|
||||
1. Shorthand `:` is preferable over `v-bind`
|
||||
```javascript
|
||||
// bad
|
||||
<component v-bind:class="btn"/>
|
||||
// bad
|
||||
<component v-bind:class="btn"/>
|
||||
|
||||
|
||||
// good
|
||||
<component :class="btn"/>
|
||||
// good
|
||||
<component :class="btn"/>
|
||||
```
|
||||
|
||||
#### Closing tags
|
||||
- Prefer self closing component tags
|
||||
|
||||
1. Prefer self closing component tags
|
||||
```javascript
|
||||
// bad
|
||||
<component></component>
|
||||
// bad
|
||||
<component></component>
|
||||
|
||||
// good
|
||||
<component />
|
||||
// good
|
||||
<component />
|
||||
```
|
||||
|
||||
#### Ordering
|
||||
- Order for a Vue Component:
|
||||
1. Order for a Vue Component:
|
||||
1. `name`
|
||||
2. `props`
|
||||
3. `data`
|
||||
4. `components`
|
||||
5. `computedProps`
|
||||
6. `methods`
|
||||
7. lifecycle methods
|
||||
1. `beforeCreate`
|
||||
2. `created`
|
||||
3. `beforeMount`
|
||||
4. `mounted`
|
||||
5. `beforeUpdate`
|
||||
6. `updated`
|
||||
7. `activated`
|
||||
8. `deactivated`
|
||||
9. `beforeDestroy`
|
||||
10. `destroyed`
|
||||
8. `template`
|
||||
1. `props`
|
||||
1. `mixins`
|
||||
1. `data`
|
||||
1. `components`
|
||||
1. `computedProps`
|
||||
1. `methods`
|
||||
1. `beforeCreate`
|
||||
1. `created`
|
||||
1. `beforeMount`
|
||||
1. `mounted`
|
||||
1. `beforeUpdate`
|
||||
1. `updated`
|
||||
1. `activated`
|
||||
1. `deactivated`
|
||||
1. `beforeDestroy`
|
||||
1. `destroyed`
|
||||
|
||||
|
||||
## SCSS
|
||||
|
@ -461,3 +439,5 @@ A forEach will cause side effects, it will be mutating the array being iterated.
|
|||
|
||||
[airbnb-js-style-guide]: https://github.com/airbnb/javascript
|
||||
[eslintrc]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.eslintrc
|
||||
[eslint-this]: http://eslint.org/docs/rules/class-methods-use-this
|
||||
[eslint-new]: http://eslint.org/docs/rules/no-new
|
||||
|
|
|
@ -387,6 +387,10 @@ describe('Todos App', () => {
|
|||
});
|
||||
});
|
||||
```
|
||||
#### Test the component's output
|
||||
The main return value of a Vue component is the rendered output. In order to test the component we
|
||||
need to test the rendered output. [Vue][vue-test] guide's to unit test show us exactly that:
|
||||
|
||||
|
||||
### Stubbing API responses
|
||||
[Vue Resource Interceptors][vue-resource-interceptor] allow us to add a interceptor with
|
||||
|
@ -419,6 +423,16 @@ the response we need:
|
|||
});
|
||||
```
|
||||
|
||||
1. Use `$.mount()` to mount the component
|
||||
```javascript
|
||||
// bad
|
||||
new Component({
|
||||
el: document.createElement('div')
|
||||
});
|
||||
|
||||
// good
|
||||
new Component().$mount();
|
||||
```
|
||||
|
||||
[vue-docs]: http://vuejs.org/guide/index.html
|
||||
[issue-boards]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/boards
|
||||
|
@ -429,5 +443,6 @@ the response we need:
|
|||
[one-way-data-flow]: https://vuejs.org/v2/guide/components.html#One-Way-Data-Flow
|
||||
[vue-resource-repo]: https://github.com/pagekit/vue-resource
|
||||
[vue-resource-interceptor]: https://github.com/pagekit/vue-resource/blob/develop/docs/http.md#interceptors
|
||||
[vue-test]: https://vuejs.org/v2/guide/unit-testing.html
|
||||
[issue-boards-service]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/boards/services/board_service.js.es6
|
||||
[flux]: https://facebook.github.io/flux
|
||||
|
|
Loading…
Reference in a new issue