frontend.md 19.1 KB
Newer Older
1
# Frontend Development Guidelines
2

3
This document describes various guidelines to ensure consistency and quality
4
across GitLab's frontend team.
5

6 7
## Overview

8 9 10
GitLab is built on top of [Ruby on Rails][rails] using [Haml][haml] with
[Hamlit][hamlit]. Be wary of [the limitations that come with using
Hamlit][hamlit-limits]. We also use [SCSS][scss] and plain JavaScript with
11 12 13 14 15 16 17 18
[ES6 by way of Babel][es6].

The asset pipeline is [Sprockets][sprockets], which handles the concatenation,
minification, and compression of our assets.

[jQuery][jquery] is used throughout the application's JavaScript, with
[Vue.js][vue] for particularly advanced, dynamic elements.

19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34
### Architecture

The Frontend Architect is an expert who makes high-level frontend design choices
and decides on technical standards, including coding standards, and frameworks.

When you are assigned a new feature that requires architectural design,
make sure it is discussed with one of the Frontend Architecture Experts.

This rule also applies if you plan to change the architecture of an existing feature.

These decisions should be accessible to everyone, so please document it on the Merge Request.

You can find the Frontend Architecture experts on the [team page][team-page].

You can find documentation about the desired architecture for a new feature built with Vue.js in [here][vue-section].

35 36 37 38 39 40 41 42
### Realtime

When writing code for realtime features we have to keep a couple of things in mind:
1. Do not overload the server with requests.
1. It should feel realtime. 

Thus, we must strike a balance between sending requests and the feeling of realtime. Use the following rules when creating realtime solutions.

43 44
1. The server will tell you how much to poll by sending `X-Poll-Interval` in the header. Use that as your polling interval. This way it is easy for system administrators to change the polling rate. A `X-Poll-Interval: -1` means disabled polling, and this must be implemented. 
1. A response of `HTTP 429 Too Many Requests`, should disable polling as well. This must also be implemented.
45
1. Use a common library for polling.
46 47
1. Poll on active tabs only. Use a common library to find out which tab currently has eyes on it. Please use [Focus](https://gitlab.com/andrewn/focus). Specifically [Eyeballs Detector](https://gitlab.com/andrewn/focus/blob/master/lib/eyeballs-detector.js).
1. Use regular polling intervals, do not use backoff polling, or jitter, as the interval will be controlled by the server.
48 49
1. The backend code will most likely be using etags. You do not and should not check for status `304 Not Modified`. The browser will transform it for you.

50 51 52
### Vue

For more complex frontend features, we recommend using Vue.js. It shares
53
some ideas with React.js as well as Angular.
54 55 56

To get started with Vue, read through [their documentation][vue-docs].

57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83
#### How to build a new feature with Vue.js
**Components, Stores and Services**

In some features implemented with Vue.js, like the [issue board][issue-boards]
or [environments table][environments-table]
you can find a clear separation of concerns:

```
new_feature
├── components
│   └── component.js.es6
│   └── ...
├── store
│  └── new_feature_store.js.es6
├── service
│  └── new_feature_service.js.es6
├── new_feature_bundle.js.es6
```
_For consistency purposes, we recommend you to follow the same structure._

Let's look into each of them:

**A `*_bundle.js` file**

This is the index file of your new feature. This is where the root Vue instance
of the new feature should be.

F
Filipa Lacerda 已提交
84
The Store and the Service should be imported and initialized in this file and provided as a prop to the main component.
85

N
Nur Rony 已提交
86
Don't forget to follow [these steps.][page_specific_javascript]
87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105

**A folder for Components**

This folder holds all components that are specific of this new feature.
If you need to use or create a component that will probably be used somewhere
else, please refer to `vue_shared/components`.

A good thumb rule to know when you should create a component is to think if
it will be reusable elsewhere.

For example, tables are used in a quite amount of places across GitLab, a table
would be a good fit for a component.
On the other hand, a table cell used only in on table, would not be a good use
of this pattern.

You can read more about components in Vue.js site, [Component System][component-system]

**A folder for the Store**

106
The Store is a class that allows us to manage the state in a single
107 108 109 110 111 112 113 114 115 116 117 118 119 120 121
source of truth.

The concept we are trying to follow is better explained by Vue documentation
itself, please read this guide: [State Management][state-management]

**A folder for the Service**

The Service is used only to communicate with the server.
It does not store or manipulate any data.
We use [vue-resource][vue-resource-repo] to
communicate with the server.

The [issue boards service][issue-boards-service]
is a good example of this pattern.

122 123
## Performance

124 125 126 127 128 129 130
### Resources

- [WebPage Test][web-page-test] for testing site loading time and size.
- [Google PageSpeed Insights][pagespeed-insights] grades web pages and provides feedback to improve the page.
- [Profiling with Chrome DevTools][google-devtools-profiling]
- [Browser Diet][browser-diet] is a community-built guide that catalogues practical tips for improving web page performance.

131
### Page-specific JavaScript
132

133 134 135 136
Certain pages may require the use of a third party library, such as [d3][d3] for
the User Activity Calendar and [Chart.js][chartjs] for the Graphs pages. These
libraries increase the page size significantly, and impact load times due to
bandwidth bottlenecks and the browser needing to parse more JavaScript.
137

138
In cases where libraries are only used on a few specific pages, we use
139
"page-specific JavaScript" to prevent the main `application.js` file from
140 141 142 143 144 145
becoming unnecessarily large.

Steps to split page-specific JavaScript from the main `application.js`:

1. Create a directory for the specific page(s), e.g. `graphs/`.
1. In that directory, create a `namespace_bundle.js` file, e.g. `graphs_bundle.js`.
146
1. In `graphs_bundle.js` add the line `//= require_tree .`, this adds all other files in the directory to the bundle.
147
1. Add any necessary libraries to `app/assets/javascripts/lib/`, all files directly descendant from this directory will be precompiled as separate assets, in this case `chart.js` would be added.
148 149 150
1. Add the new "bundle" file to the list of precompiled assets in
`config/application.rb`.
  - For example: `config.assets.precompile << "graphs/graphs_bundle.js"`.
151
1. Move code reliant on these libraries into the `graphs` directory.
152 153 154 155 156 157 158 159
1. In the relevant views, add the scripts to the page with the following:

```haml
- content_for :page_specific_javascripts do
  = page_specific_javascript_tag('lib/chart.js')
  = page_specific_javascript_tag('graphs/graphs_bundle.js')
```

160
The above loads `chart.js` and `graphs_bundle.js` for this page only. `chart.js`
161
is separated from the bundle file so it can be cached separately from the bundle
162 163
and reused for other pages that also rely on the library. For an example, see
[this Haml file][page-specific-js-example].
164 165 166 167

### Minimizing page size

A smaller page size means the page loads faster (especially important on mobile
168 169
and poor connections), the page is parsed more quickly by the browser, and less
data is used for users with capped data plans.
170 171 172

General tips:

173
- Don't add new fonts.
174
- Prefer font formats with better compression, e.g. WOFF2 is better than WOFF, which is better than TTF.
175
- Compress and minify assets wherever possible (For CSS/JS, Sprockets does this for us).
176 177
- If some functionality can reasonably be achieved without adding extra libraries, avoid them.
- Use page-specific JavaScript as described above to dynamically load libraries that are only needed on certain pages.
178 179 180

## Accessibility

181 182
### Resources

183
[Chrome Accessibility Developer Tools][chrome-accessibility-developer-tools]
184 185 186 187 188 189 190
are useful for testing for potential accessibility problems in GitLab.

Accessibility best-practices and more in-depth information is available on
[the Audit Rules page][audit-rules] for the Chrome Accessibility Developer Tools.

## Security

191 192
### Resources

193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 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 239 240 241 242 243 244 245 246 247 248
[Mozilla’s HTTP Observatory CLI][observatory-cli] and the
[Qualys SSL Labs Server Test][qualys-ssl] are good resources for finding
potential problems and ensuring compliance with security best practices.

<!-- Uncomment these sections when CSP/SRI are implemented.
### Content Security Policy (CSP)

Content Security Policy is a web standard that intends to mitigate certain
forms of Cross-Site Scripting (XSS) as well as data injection.

Content Security Policy rules should be taken into consideration when
implementing new features, especially those that may rely on connection with
external services.

GitLab's CSP is used for the following:

- Blocking plugins like Flash and Silverlight from running at all on our pages.
- Blocking the use of scripts and stylesheets downloaded from external sources.
- Upgrading `http` requests to `https` when possible.
- Preventing `iframe` elements from loading in most contexts.

Some exceptions include:

- Scripts from Google Analytics and Piwik if either is enabled.
- Connecting with GitHub, Bitbucket, GitLab.com, etc. to allow project importing.
- Connecting with Google, Twitter, GitHub, etc. to allow OAuth authentication.

We use [the Secure Headers gem][secure_headers] to enable Content
Security Policy headers in the GitLab Rails app.

Some resources on implementing Content Security Policy:

- [MDN Article on CSP][mdn-csp]
- [GitHub’s CSP Journey on the GitHub Engineering Blog][github-eng-csp]
- The Dropbox Engineering Blog's series on CSP: [1][dropbox-csp-1], [2][dropbox-csp-2], [3][dropbox-csp-3], [4][dropbox-csp-4]

### Subresource Integrity (SRI)

Subresource Integrity prevents malicious assets from being provided by a CDN by
guaranteeing that the asset downloaded is identical to the asset the server
is expecting.

The Rails app generates a unique hash of the asset, which is used as the
asset's `integrity` attribute. The browser generates the hash of the asset
on-load and will reject the asset if the hashes do not match.

All CSS and JavaScript assets should use Subresource Integrity. For implementation details,
see the documentation for [the Sprockets implementation of SRI][sprockets-sri].

Some resources on implementing Subresource Integrity:

- [MDN Article on SRI][mdn-sri]
- [Subresource Integrity on the GitHub Engineering Blog][github-eng-sri]

-->

249 250
### Including external resources

251 252 253 254 255
External fonts, CSS, and JavaScript should never be used with the exception of
Google Analytics and Piwik - and only when the instance has enabled it. Assets
should always be hosted and served locally from the GitLab instance. Embedded
resources via `iframes` should never be used except in certain circumstances
such as with ReCaptcha, which cannot be used without an `iframe`.
256 257 258

### Avoiding inline scripts and styles

259
In order to protect users from [XSS vulnerabilities][xss], we will disable inline scripts in the future using Content Security Policy.
260

261 262
While inline scripts can be useful, they're also a security concern. If
user-supplied content is unintentionally left un-sanitized, malicious users can
263
inject scripts into the web app.
264

265 266 267 268
Inline styles should be avoided in almost all cases, they should only be used
when no alternatives can be found. This allows reusability of styles as well as
readability.

269
## Style guides and linting
270

271
See the relevant style guides for our guidelines and for information on linting:
272 273

- [SCSS][scss-style-guide]
274 275
- JavaScript - We defer to [AirBnb][airbnb-js-style-guide] on most style-related
conventions and enforce them with eslint. See [our current .eslintrc][eslintrc]
276
for specific rules and patterns.
277

278 279
## Testing

280 281 282
Feature tests need to be written for all new features. Regression tests
also need to be written for all bug fixes to prevent them from occurring
again in the future.
283 284 285 286

See [the Testing Standards and Style Guidelines](testing.md) for more
information.

W
Winnie 已提交
287 288
### Running frontend tests

289
`rake karma` runs the frontend-only (JavaScript) tests.
W
Winnie 已提交
290 291
It consists of two subtasks:

292 293
- `rake karma:fixtures` (re-)generates fixtures
- `rake karma:tests` actually executes the tests
W
Winnie 已提交
294

295
As long as the fixtures don't change, `rake karma:tests` is sufficient
W
Winnie 已提交
296 297 298
(and saves you some time).

Please note: Not all of the frontend fixtures are generated. Some are still static
299
files. These will not be touched by `rake karma:fixtures`.
W
Winnie 已提交
300

301 302 303 304 305 306 307 308
## Design Patterns

### Singletons

When exactly one object is needed for a given task, prefer to define it as a
`class` rather than as an object literal. Prefer also to explicitly restrict
instantiation, unless flexibility is important (e.g. for testing).

309
```javascript
310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328
// bad

gl.MyThing = {
  prop1: 'hello',
  method1: () => {}
};

// good

class MyThing {
  constructor() {
    this.prop1 = 'hello';
  }
  method1() {}
}

gl.MyThing = new MyThing();

// best
B
Bryce Johnson 已提交
329

330 331 332 333 334
let singleton;

class MyThing {
  constructor() {
    if (!singleton) {
B
Bryce Johnson 已提交
335 336 337
      singleton = this;
      singleton.init();
    }
338 339 340 341 342 343 344 345 346 347 348
      return singleton;
  }

  init() {
    this.prop1 = 'hello';
  }

  method1() {}
}

gl.MyThing = MyThing;
B
Bryce Johnson 已提交
349

350 351
```

352 353 354 355 356 357
### Manipulating the DOM in a JS Class

When writing a class that needs to manipulate the DOM guarantee a container option is provided.
This is useful when we need that class to be instantiated more than once in the same page.

Bad:
358
```javascript
359 360 361 362 363 364 365 366 367
class Foo {
  constructor() {
    document.querySelector('.bar');
  }
}
new Foo();
```

Good:
368
```javascript
369 370
class Foo {
  constructor(opts) {
F
Filipa Lacerda 已提交
371
    document.querySelector(`${opts.container} .bar`);
372 373 374 375 376 377 378
  }
}

new Foo({ container: '.my-element' });
```
You can find an example of the above in this [class][container-class-example];

379 380 381 382
## Supported browsers

For our currently-supported browsers, see our [requirements][requirements].

383

384
## Gotchas
385

386 387
### Spec errors due to use of ES6 features in `.js` files

388
If you see very generic JavaScript errors (e.g. `jQuery is undefined`) being
389
thrown in Karma, Spinach, or Rspec tests but can't reproduce them manually,
390 391 392
you may have included `ES6`-style JavaScript in files that don't have the
`.js.es6` file extension. Either use ES5-friendly JavaScript or rename the file
you're working in (`git mv <file.js> <file.js.es6>`).
393 394 395

### Spec errors due to use of unsupported JavaScript

396
Similar errors will be thrown if you're using JavaScript features not yet
397 398 399 400 401 402 403 404 405 406 407
supported by our test runner's version of webkit, whether or not you've updated
the file extension. Examples of unsupported JavaScript features are:

- Array.from
- Array.find
- Array.first
- Object.assign
- Async functions
- Generators
- Array destructuring
- For Of
B
Bryce Johnson 已提交
408
- Symbol/Symbol.iterator
409 410
- Spread

411 412
Until these are polyfilled or transpiled appropriately, they should not be used.
Please update this list with additional unsupported features or when any of
413 414 415 416
these are made usable.

### Spec errors due to JavaScript not enabled

417
If, as a result of a change you've made, a feature now depends on JavaScript to
418
run correctly, you need to make sure a JavaScript web driver is enabled when
419 420
specs are run. If you don't you'll see vague error messages from the spec
runner, and an explosion of vague console errors in the HTML snapshot.
421

422 423 424
To enable a JavaScript driver in an `rspec` test, add `js: true` to the
individual spec or the context block containing multiple specs that need
JavaScript enabled:
425 426 427 428 429 430 431 432 433 434 435 436 437 438 439 440 441

```ruby

# For one spec
it 'presents information about abuse report', js: true do
    # assertions...
end

describe "Admin::AbuseReports", js: true do
   it 'presents information about abuse report' do
        # assertions...
    end
   it 'shows buttons for adding to abuse report' do
        # assertions...
    end
end
```
442

443 444
In Spinach, the JavaScript driver is enabled differently. In the `*.feature`
file for the failing spec, add the `@javascript` flag above the Scenario:
445

446 447 448 449 450 451 452 453 454
```
@javascript
Scenario: Developer can approve merge request
    Given I am a "Shop" developer
    And I visit project "Shop" merge requests page
    And merge request 'Bug NS-04' must be approved
    And I click link "Bug NS-04"
    When I click link "Approve"
    Then I should see approved merge request "Bug NS-04"
455

456
```
457 458 459 460 461 462 463 464 465 466 467 468 469 470 471 472 473 474 475 476 477 478 479 480 481 482 483 484 485 486 487 488 489 490 491 492 493 494 495 496 497 498 499


[rails]: http://rubyonrails.org/
[haml]: http://haml.info/
[hamlit]: https://github.com/k0kubun/hamlit
[hamlit-limits]: https://github.com/k0kubun/hamlit/blob/master/REFERENCE.md#limitations
[scss]: http://sass-lang.com/
[es6]: https://babeljs.io/
[sprockets]: https://github.com/rails/sprockets
[jquery]: https://jquery.com/
[vue]: http://vuejs.org/
[vue-docs]: http://vuejs.org/guide/index.html
[web-page-test]: http://www.webpagetest.org/
[pagespeed-insights]: https://developers.google.com/speed/pagespeed/insights/
[google-devtools-profiling]: https://developers.google.com/web/tools/chrome-devtools/profile/?hl=en
[browser-diet]: https://browserdiet.com/
[d3]: https://d3js.org/
[chartjs]: http://www.chartjs.org/
[page-specific-js-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/13bb9ed77f405c5f6ee4fdbc964ecf635c9a223f/app/views/projects/graphs/_head.html.haml#L6-8
[chrome-accessibility-developer-tools]: https://github.com/GoogleChrome/accessibility-developer-tools
[audit-rules]: https://github.com/GoogleChrome/accessibility-developer-tools/wiki/Audit-Rules
[observatory-cli]: https://github.com/mozilla/http-observatory-cli
[qualys-ssl]: https://www.ssllabs.com/ssltest/analyze.html
[secure_headers]: https://github.com/twitter/secureheaders
[mdn-csp]: https://developer.mozilla.org/en-US/docs/Web/Security/CSP
[github-eng-csp]: http://githubengineering.com/githubs-csp-journey/
[dropbox-csp-1]: https://blogs.dropbox.com/tech/2015/09/on-csp-reporting-and-filtering/
[dropbox-csp-2]: https://blogs.dropbox.com/tech/2015/09/unsafe-inline-and-nonce-deployment/
[dropbox-csp-3]: https://blogs.dropbox.com/tech/2015/09/csp-the-unexpected-eval/
[dropbox-csp-4]: https://blogs.dropbox.com/tech/2015/09/csp-third-party-integrations-and-privilege-separation/
[mdn-sri]: https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity
[github-eng-sri]: http://githubengineering.com/subresource-integrity/
[sprockets-sri]: https://github.com/rails/sprockets-rails#sri-support
[xss]: https://en.wikipedia.org/wiki/Cross-site_scripting
[scss-style-guide]: scss_styleguide.md
[requirements]: ../install/requirements.md#supported-web-browsers
[issue-boards]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/boards
[environments-table]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/app/assets/javascripts/environments
[page_specific_javascript]: https://docs.gitlab.com/ce/development/frontend.html#page-specific-javascript
[component-system]: https://vuejs.org/v2/guide/#Composing-with-Components
[state-management]: https://vuejs.org/v2/guide/state-management.html#Simple-State-Management-from-Scratch
[vue-resource-repo]: https://github.com/pagekit/vue-resource
[issue-boards-service]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/boards/services/board_service.js.es6
500
[airbnb-js-style-guide]: https://github.com/airbnb/javascript
501
[eslintrc]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.eslintrc
502 503
[team-page]: https://about.gitlab.com/team
[vue-section]: https://docs.gitlab.com/ce/development/frontend.html#how-to-build-a-new-feature-with-vue-js
504
[container-class-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/mini_pipeline_graph_dropdown.js