style_guide_js.md 16 KB
Newer Older
1 2 3 4 5 6 7 8 9 10 11 12 13
# Style guides and linting
See the relevant style guides for our guidelines and for information on linting:

## JavaScript
We defer to [Airbnb][airbnb-js-style-guide] on most style-related
conventions and enforce them with eslint.

See [our current .eslintrc][eslintrc] for specific rules and patterns.

### Common

#### ESlint

Lin Jen-Shin's avatar
Lin Jen-Shin committed
14
1. **Never** disable eslint rules unless you have a good reason.
Filipa Lacerda's avatar
Filipa Lacerda committed
15 16 17
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.
18

19
1. **Never Ever EVER** disable eslint globally for a file
20
  ```javascript
21 22
    // bad
    /* eslint-disable */
23

24 25
    // better
    /* eslint-disable some-rule, some-other-rule */
26

27 28
    // best
    // nothing :)
29 30
  ```

31
1. If you do need to disable a rule for a single violation, try to do it as locally as possible
32
  ```javascript
33 34
    // bad
    /* eslint-disable no-new */
35

36
    import Foo from 'foo';
37

38
    new Foo();
39

40 41
    // better
    import Foo from 'foo';
42

43 44
    // eslint-disable-next-line no-new
    new Foo();
45
  ```
46 47 48
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]
49

Filipa Lacerda's avatar
Filipa Lacerda committed
50 51
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.
52
  ```javascript
53 54 55 56
    // bad
    /* global Foo */
    /* eslint-disable no-new */
    import Bar from './bar';
57

58 59 60
    // good
    /* eslint-disable no-new */
    /* global Foo */
61

62
    import Bar from './bar';
63 64
  ```

65
1. **Never** disable the `no-undef` rule. Declare globals with `/* global Foo */` instead.
66

67
1. When declaring multiple globals, always use one `/* global [name] */` line per variable.
68
  ```javascript
69 70
    // bad
    /* globals Flash, Cookies, jQuery */
71

72 73 74 75
    // good
    /* global Flash */
    /* global Cookies */
    /* global jQuery */
76
  ```
Fatih Acet's avatar
Fatih Acet committed
77

78
1. Use up to 3 parameters for a function or class. If you need more accept an Object instead.
Fatih Acet's avatar
Fatih Acet committed
79
  ```javascript
80 81
    // bad
    fn(p1, p2, p3, p4) {}
Fatih Acet's avatar
Fatih Acet committed
82

83 84
    // good
    fn(options) {}
Fatih Acet's avatar
Fatih Acet committed
85
  ```
86 87

#### Modules, Imports, and Exports
88
1. Use ES module syntax to import modules
89 90 91
    ```javascript
      // bad
      const SomeClass = require('some_class');
92

93 94
      // good
      import SomeClass from 'some_class';
95

96 97
      // bad
      module.exports = SomeClass;
98

99 100 101 102 103
      // good
      export default SomeClass;
    ```

    Import statements are following usual naming guidelines, for example object literals use camel case:
104

105 106 107 108 109
    ```javascript
      // some_object file
      export default {
        key: 'value',
      };
110

111 112
      // bad
      import ObjectLiteral from 'some_object';
113

114 115 116
      // good
      import objectLiteral from 'some_object';
    ```
117

Lin Jen-Shin's avatar
Lin Jen-Shin committed
118 119
1. Relative paths: when importing a module in the same directory, a child
directory, or an immediate parent directory prefer relative paths.  When
120
importing a module which is two or more levels up, prefer either `~/` or `ee/`.
121

122
    In **app/assets/javascripts/my-feature/subdir**:
123

124 125 126 127 128
    ```javascript
    // bad
    import Foo from '~/my-feature/foo';
    import Bar from '~/my-feature/subdir/bar';
    import Bin from '~/my-feature/subdir/lib/bin';
129

130 131 132 133 134
    // good
    import Foo from '../foo';
    import Bar from './bar';
    import Bin from './lib/bin';
    ```
135

136
    In **spec/javascripts**:
Lin Jen-Shin's avatar
Lin Jen-Shin committed
137

138 139 140
    ```javascript
    // bad
    import Foo from '../../app/assets/javascripts/my-feature/foo';
Lin Jen-Shin's avatar
Lin Jen-Shin committed
141

142 143 144
    // good
    import Foo from '~/my-feature/foo';
    ```
Lin Jen-Shin's avatar
Lin Jen-Shin committed
145

146
    When referencing an **EE component**:
Lin Jen-Shin's avatar
Lin Jen-Shin committed
147

148 149 150
    ```javascript
    // bad
    import Foo from '../../../../../ee/app/assets/javascripts/my-feature/ee-foo';
Lin Jen-Shin's avatar
Lin Jen-Shin committed
151

152 153 154
    // good
    import Foo from 'ee/my-feature/foo';
    ```
155

Filipa Lacerda's avatar
Filipa Lacerda committed
156 157 158 159
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.
160

161
1. Avoid adding to the global namespace.
162 163 164
    ```javascript
      // bad
      window.MyClass = class { /* ... */ };
165

166 167 168
      // good
      export default class MyClass { /* ... */ }
    ```
169

170
1. Side effects are forbidden in any script which contains exports
171 172 173
    ```javascript
      // bad
      export default class MyClass { /* ... */ }
174

175 176 177 178
      document.addEventListener("DOMContentLoaded", function(event) {
        new MyClass();
      }
    ```
179 180

#### Data Mutation and Pure functions
181
1. Strive to write many small pure functions, and minimize where mutations occur.
182
  ```javascript
183 184
    // bad
    const values = {foo: 1};
185

186 187
    function impureFunction(items) {
      const bar = 1;
188

189
      items.foo = items.a * bar + 2;
190

191 192
      return items.a;
    }
193

194
    const c = impureFunction(values);
195

196 197
    // good
    var values = {foo: 1};
198

199 200
    function pureFunction (foo) {
      var bar = 1;
201

202
      foo = foo * bar + 2;
203

204 205
      return foo;
    }
206

207
    var c = pureFunction(values.foo);
208 209
  ```

210 211 212
1. Avoid constructors with side-effects.
Although we aim for code without side-effects we need some side-effects for our code to run.

213
If the class won't do anything if we only instantiate it, it's ok to add side effects into the constructor (_Note:_ The following is just an example. If the only purpose of the class is to add an event listener and handle the callback a function will be more suitable.)
214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238

```javascript
// Bad
export class Foo {
  constructor() {
    this.init();
  }
  init() {
    document.addEventListener('click', this.handleCallback)
  },
  handleCallback() {

  }
}

// Good
export class Foo {
  constructor() {
    document.addEventListener()
  }
  handleCallback() {
  }
}
```

Pascal Borreli's avatar
Pascal Borreli committed
239
On the other hand, if a class only needs to extend a third party/add event listeners in some specific cases, they should be initialized outside of the constructor.
240

241
1. Prefer `.map`, `.reduce` or `.filter` over `.forEach`
242
A forEach will most likely cause side effects, it will be mutating the array being iterated. Prefer using `.map`,
243 244
`.reduce` or `.filter`
  ```javascript
245
    const users = [ { name: 'Foo' }, { name: 'Bar' } ];
246

247 248 249 250
    // bad
    users.forEach((user, index) => {
      user.id = index;
    });
251

252 253 254 255
    // good
    const usersWithId = users.map((user, index) => {
      return Object.assign({}, user, { id: index });
    });
256
  ```
257 258

#### Parse Strings into Numbers
259
1. `parseInt()` is preferable over `Number()` or `+`
260
  ```javascript
261 262
    // bad
    +'10' // 10
263

264 265
    // good
    Number('10') // 10
266

267 268
    // better
    parseInt('10', 10);
269 270
  ```

271
#### CSS classes used for JavaScript
272
1. If the class is being used in Javascript it needs to be prepend with `js-`
273 274 275 276 277 278 279 280 281 282 283
  ```html
    // bad
    <button class="add-user">
      Add User
    </button>

    // good
    <button class="js-add-user">
      Add User
    </button>
  ```
284 285 286

### Vue.js

287 288 289 290
#### `eslint-vue-plugin`
We default to [eslint-vue-plugin][eslint-plugin-vue], with the `plugin:vue/recommended`.
Please check this [rules][eslint-plugin-vue-rules] for more documentation.

291
#### Basic Rules
Filipa Lacerda's avatar
Filipa Lacerda committed
292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330
1. The service has it's own file
1. The store has it's own file
1. Use a function in the bundle file to instantiate the Vue component:
  ```javascript
    // bad
    class {
      init() {
        new Component({})
      }
    }

    // good
    document.addEventListener('DOMContentLoaded', () => new Vue({
      el: '#element',
      components: {
        componentName
      },
      render: createElement => createElement('component-name'),
    }));
  ```

1. Don not use a singleton for the service or the store
  ```javascript
    // bad
    class Store {
      constructor() {
        if (!this.prototype.singleton) {
          // do something
        }
      }
    }

    // good
    class Store {
      constructor() {
        // do something
      }
    }
  ```
331 332

#### Naming
333
1. **Extensions**: Use `.vue` extension for Vue components.
334
1. **Reference Naming**: Use PascalCase for their instances:
335
  ```javascript
336
    // bad
337
    import cardBoard from 'cardBoard.vue'
338 339

    components: {
340
      cardBoard,
341 342
    };

343
    // good
344
    import CardBoard from 'cardBoard.vue'
345

346
    components: {
347
      CardBoard,
348
    };
349
  ```
350

Filipa Lacerda's avatar
Filipa Lacerda committed
351 352
1. **Props Naming:**  Avoid using DOM component prop names.
1. **Props Naming:** Use kebab-case instead of camelCase to provide props in templates.
353
  ```javascript
354 355
    // bad
    <component class="btn">
356

357 358
    // good
    <component css-class="btn">
359

360 361
    // bad
    <component myProp="prop" />
362

363 364 365
    // good
    <component my-prop="prop" />
  ```
366 367

#### Alignment
368
1. Follow these alignment styles for the template method:
369
  1. With more than one attribute, all attributes should be on a new line:
370 371 372 373
      ```javascript
        // bad
        <component v-if="bar"
            param="baz" />
374

375
        <button class="btn">Click me</button>
376

377 378 379 380 381
        // good
        <component
          v-if="bar"
          param="baz"
        />
382

383 384 385 386
        <button class="btn">
          Click me
        </button>
      ```
387
  1. The tag can be inline if there is only one attribute:
388 389 390
      ```javascript
        // good
          <component bar="bar" />
391

392 393 394 395
        // good
          <component
            bar="bar"
            />
Filipa Lacerda's avatar
Filipa Lacerda committed
396 397 398 399

        // bad
         <component
            bar="bar" />
400
      ```
401 402

#### Quotes
403
1. Always use double quotes `"` inside templates and single quotes `'` for all other JS.
404
  ```javascript
405 406 407 408 409 410 411 412 413
    // bad
    template: `
      <button :class='style'>Button</button>
    `

    // good
    template: `
      <button :class="style">Button</button>
    `
414 415 416
  ```

#### Props
417
1. Props should be declared as an object
418
  ```javascript
419 420 421 422 423 424 425 426 427 428
    // bad
    props: ['foo']

    // good
    props: {
      foo: {
        type: String,
        required: false,
        default: 'bar'
      }
429 430 431
    }
  ```

432
1. Required key should always be provided when declaring a prop
433
  ```javascript
434 435 436 437 438
    // bad
    props: {
      foo: {
        type: String,
      }
439
    }
440 441 442 443 444 445 446 447

    // good
    props: {
      foo: {
        type: String,
        required: false,
        default: 'bar'
      }
448 449 450
    }
  ```

451 452 453
1. Default key should be provided if the prop is not required.
_Note:_ There are some scenarios where we need to check for the existence of the property.
On those a default key should not be provided.
454
  ```javascript
455
    // good
456 457 458 459 460
    props: {
      foo: {
        type: String,
        required: false,
      }
461
    }
462 463 464 465 466 467 468 469

    // good
    props: {
      foo: {
        type: String,
        required: false,
        default: 'bar'
      }
470 471
    }

472 473 474 475 476 477
    // good
    props: {
      foo: {
        type: String,
        required: true
      }
478 479 480 481
    }
  ```

#### Data
482
1. `data` method should always be a function
483

484 485 486
    ```javascript
      // bad
      data: {
487
        foo: 'foo'
488 489 490 491 492 493 494 495 496
      }

      // good
      data() {
        return {
          foo: 'foo'
        };
      }
    ```
497 498 499

#### Directives

500
1. Shorthand `@` is preferable over `v-on`
501
  ```javascript
502 503
    // bad
    <component v-on:click="eventHandler"/>
504 505


506 507
    // good
    <component @click="eventHandler"/>
508 509
  ```

510
1. Shorthand `:` is preferable over `v-bind`
511
  ```javascript
512 513
    // bad
    <component v-bind:class="btn"/>
514 515


516 517
    // good
    <component :class="btn"/>
518 519 520
  ```

#### Closing tags
521
1. Prefer self closing component tags
522
  ```javascript
523 524
    // bad
    <component></component>
525

526 527
    // good
    <component />
528 529 530
  ```

#### Ordering
531 532

1. Tag order in `.vue` file
533 534 535 536 537 538 539 540 541 542 543 544 545 546
    ```
    <script>
      // ...
    </script>

    <template>
      // ...
    </template>

    // We don't use scoped styles but there are few instances of this
    <style>
      // ...
    </style>
    ```
547 548

1. Properties in a Vue Component:
Filipa Lacerda's avatar
Filipa Lacerda committed
549
  Check [order of properties in components rule][vue-order].
550

551 552 553
#### `:key`
When using `v-for` you need to provide a *unique* `:key` attribute for each item.

Filipa Lacerda's avatar
Filipa Lacerda committed
554
1. If the elements of the array being iterated have an unique `id` it is advised to use it:
555
    ```html
Filipa Lacerda's avatar
Filipa Lacerda committed
556 557 558 559
      <div
        v-for="item in items"
        :key="item.id"
      >
560 561 562 563 564 565
        <!-- content -->
      </div>
    ```

1. When the elements being iterated don't have a unique id, you can use the array index as the `:key` attribute
    ```html
Filipa Lacerda's avatar
Filipa Lacerda committed
566 567 568 569
      <div
        v-for="(item, index) in items"
        :key="index"
      >
570 571 572 573 574
        <!-- content -->
      </div>
    ```


Filipa Lacerda's avatar
Filipa Lacerda committed
575
1. When using `v-for` with `template` and there is more than one child element, the `:key` values must be unique. It's advised to use `kebab-case` namespaces.
576 577 578 579 580 581 582
    ```html
      <template v-for="(item, index) in items">
        <span :key="`span-${index}`"></span>
        <button :key="`button-${index}`"></button>
      </template>
    ```

583 584 585 586 587 588 589 590 591 592 593 594 595 596 597 598
1. When dealing with nested `v-for` use the same guidelines as above.
      ```html
        <div
          v-for="item in items"
          :key="item.id"
        >
          <span
            v-for="element in array"
            :key="element.id"
          >
            <!-- content -->
          </span>
        </div>
      ```


599 600 601
Useful links:
1. [`key`](https://vuejs.org/v2/guide/list.html#key)
1. [Vue Style Guide: Keyed v-for](https://vuejs.org/v2/style-guide/#Keyed-v-for-essential )
602 603
#### Vue and Bootstrap

604
1. Tooltips: Do not rely on `has-tooltip` class name for Vue components
Lin Jen-Shin's avatar
Lin Jen-Shin committed
605
  ```javascript
Filipa Lacerda's avatar
Filipa Lacerda committed
606
    // bad
607 608 609
    <span
      class="has-tooltip"
      title="Some tooltip text">
Filipa Lacerda's avatar
Filipa Lacerda committed
610 611 612 613
      Text
    </span>

    // good
614 615 616
    <span
      v-tooltip
      title="Some tooltip text">
Filipa Lacerda's avatar
Filipa Lacerda committed
617 618 619 620
      Text
    </span>
  ```

621
1. Tooltips: When using a tooltip, include the tooltip directive, `./app/assets/javascripts/vue_shared/directives/tooltip.js`
Filipa Lacerda's avatar
Filipa Lacerda committed
622 623

1. Don't change `data-original-title`.
Filipa Lacerda's avatar
Filipa Lacerda committed
624 625 626 627 628 629 630 631 632
  ```javascript
    // bad
    <span data-original-title="tooltip text">Foo</span>

    // good
    <span title="tooltip text">Foo</span>

    $('span').tooltip('fixTitle');
  ```
633

634
### The Javascript/Vue Accord
635
The goal of this accord is to make sure we are all on the same page.
Filipa Lacerda's avatar
Filipa Lacerda committed
636

637
1. When writing Vue, you may not use jQuery in your application.
638
  1. If you need to grab data from the DOM, you may query the DOM 1 time while bootstrapping your application to grab data attributes using `dataset`. You can do this without jQuery.
639
  1. You may use a jQuery dependency in Vue.js following [this example from the docs](https://vuejs.org/v2/examples/select2.html).
640 641
  1. If an outside jQuery Event needs to be listen to inside the Vue application, you may use jQuery event listeners.
  1. We will avoid adding new jQuery events when they are not required. Instead of adding new jQuery events take a look at [different methods to do the same task](https://vuejs.org/v2/api/#vm-emit).
642 643 644 645 646
1. You may query the `window` object 1 time, while bootstrapping your application for application specific data (e.g. `scrollTo` is ok to access anytime). Do this access during the bootstrapping of your application.
1. You may have a temporary but immediate need to create technical debt by writing code that does not follow our standards, to be refactored later. Maintainers need to be ok with the tech debt in the first place. An issue should be created for that tech debt to evaluate it further and discuss. In the coming months you should fix that tech debt, with it's priority to be determined by maintainers.
1. When creating tech debt you must write the tests for that code before hand and those tests may not be rewritten. e.g. jQuery tests rewritten to Vue tests.
1. You may choose to use VueX as a centralized state management. If you choose not to use VueX, you must use the *store pattern* which can be found in the [Vue.js documentation](https://vuejs.org/v2/guide/state-management.html#Simple-State-Management-from-Scratch).
1. Once you have chosen a centralized state management solution you must use it for your entire application. i.e. Don't mix and match your state management solutions.
647 648 649 650 651 652

## SCSS
- [SCSS](style_guide_scss.md)

[airbnb-js-style-guide]: https://github.com/airbnb/javascript
[eslintrc]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.eslintrc
653 654
[eslint-this]: http://eslint.org/docs/rules/class-methods-use-this
[eslint-new]: http://eslint.org/docs/rules/no-new
655 656
[eslint-plugin-vue]: https://github.com/vuejs/eslint-plugin-vue
[eslint-plugin-vue-rules]: https://github.com/vuejs/eslint-plugin-vue#bulb-rules
Filipa Lacerda's avatar
Filipa Lacerda committed
657
[vue-order]: https://github.com/vuejs/eslint-plugin-vue/blob/master/docs/rules/order-in-components.md