512 lines
19 KiB
Markdown
512 lines
19 KiB
Markdown
# Frontend Development Guidelines
|
||
|
||
This document describes various guidelines to ensure consistency and quality
|
||
across GitLab's frontend team.
|
||
|
||
## Overview
|
||
|
||
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
|
||
[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.
|
||
|
||
### 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].
|
||
|
||
### 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.
|
||
|
||
1. The server will tell you how much to poll by sending `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 `Poll-Interval: -1` means you should disable polling, and this must be implemented.
|
||
1. A response with HTTP status `4XX` or `5XX` should disable polling as well.
|
||
1. Use a common library for polling.
|
||
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.
|
||
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.
|
||
|
||
### Vue
|
||
|
||
For more complex frontend features, we recommend using Vue.js. It shares
|
||
some ideas with React.js as well as Angular.
|
||
|
||
To get started with Vue, read through [their documentation][vue-docs].
|
||
|
||
#### 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.
|
||
|
||
The Store and the Service should be imported and initialized in this file and provided as a prop to the main component.
|
||
|
||
Don't forget to follow [these steps.][page_specific_javascript]
|
||
|
||
**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**
|
||
|
||
The Store is a class that allows us to manage the state in a single
|
||
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.
|
||
|
||
## Performance
|
||
|
||
### 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.
|
||
|
||
### Page-specific JavaScript
|
||
|
||
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.
|
||
|
||
In cases where libraries are only used on a few specific pages, we use
|
||
"page-specific JavaScript" to prevent the main `application.js` file from
|
||
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`.
|
||
1. In `graphs_bundle.js` add the line `//= require_tree .`, this adds all other files in the directory to the bundle.
|
||
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.
|
||
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"`.
|
||
1. Move code reliant on these libraries into the `graphs` directory.
|
||
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')
|
||
```
|
||
|
||
The above loads `chart.js` and `graphs_bundle.js` for this page only. `chart.js`
|
||
is separated from the bundle file so it can be cached separately from the bundle
|
||
and reused for other pages that also rely on the library. For an example, see
|
||
[this Haml file][page-specific-js-example].
|
||
|
||
### Minimizing page size
|
||
|
||
A smaller page size means the page loads faster (especially important on mobile
|
||
and poor connections), the page is parsed more quickly by the browser, and less
|
||
data is used for users with capped data plans.
|
||
|
||
General tips:
|
||
|
||
- Don't add new fonts.
|
||
- Prefer font formats with better compression, e.g. WOFF2 is better than WOFF, which is better than TTF.
|
||
- Compress and minify assets wherever possible (For CSS/JS, Sprockets does this for us).
|
||
- 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.
|
||
|
||
## Accessibility
|
||
|
||
### Resources
|
||
|
||
[Chrome Accessibility Developer Tools][chrome-accessibility-developer-tools]
|
||
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
|
||
|
||
### Resources
|
||
|
||
[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]
|
||
|
||
-->
|
||
|
||
### Including external resources
|
||
|
||
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`.
|
||
|
||
### Avoiding inline scripts and styles
|
||
|
||
In order to protect users from [XSS vulnerabilities][xss], we will disable inline scripts in the future using Content Security Policy.
|
||
|
||
While inline scripts can be useful, they're also a security concern. If
|
||
user-supplied content is unintentionally left un-sanitized, malicious users can
|
||
inject scripts into the web app.
|
||
|
||
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.
|
||
|
||
## Style guides and linting
|
||
|
||
See the relevant style guides for our guidelines and for information on linting:
|
||
|
||
- [SCSS][scss-style-guide]
|
||
- 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.
|
||
|
||
## Testing
|
||
|
||
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.
|
||
|
||
See [the Testing Standards and Style Guidelines](testing.md) for more
|
||
information.
|
||
|
||
### Running frontend tests
|
||
|
||
`rake karma` runs the frontend-only (JavaScript) tests.
|
||
It consists of two subtasks:
|
||
|
||
- `rake karma:fixtures` (re-)generates fixtures
|
||
- `rake karma:tests` actually executes the tests
|
||
|
||
As long as the fixtures don't change, `rake karma:tests` is sufficient
|
||
(and saves you some time).
|
||
|
||
Please note: Not all of the frontend fixtures are generated. Some are still static
|
||
files. These will not be touched by `rake karma:fixtures`.
|
||
|
||
## 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).
|
||
|
||
```javascript
|
||
// bad
|
||
|
||
gl.MyThing = {
|
||
prop1: 'hello',
|
||
method1: () => {}
|
||
};
|
||
|
||
// good
|
||
|
||
class MyThing {
|
||
constructor() {
|
||
this.prop1 = 'hello';
|
||
}
|
||
method1() {}
|
||
}
|
||
|
||
gl.MyThing = new MyThing();
|
||
|
||
// best
|
||
|
||
let singleton;
|
||
|
||
class MyThing {
|
||
constructor() {
|
||
if (!singleton) {
|
||
singleton = this;
|
||
singleton.init();
|
||
}
|
||
return singleton;
|
||
}
|
||
|
||
init() {
|
||
this.prop1 = 'hello';
|
||
}
|
||
|
||
method1() {}
|
||
}
|
||
|
||
gl.MyThing = MyThing;
|
||
|
||
```
|
||
|
||
### 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:
|
||
```javascript
|
||
class Foo {
|
||
constructor() {
|
||
document.querySelector('.bar');
|
||
}
|
||
}
|
||
new Foo();
|
||
```
|
||
|
||
Good:
|
||
```javascript
|
||
class Foo {
|
||
constructor(opts) {
|
||
document.querySelector(`${opts.container} .bar`);
|
||
}
|
||
}
|
||
|
||
new Foo({ container: '.my-element' });
|
||
```
|
||
You can find an example of the above in this [class][container-class-example];
|
||
|
||
## Supported browsers
|
||
|
||
For our currently-supported browsers, see our [requirements][requirements].
|
||
|
||
|
||
## Gotchas
|
||
|
||
### Spec errors due to use of ES6 features in `.js` files
|
||
|
||
If you see very generic JavaScript errors (e.g. `jQuery is undefined`) being
|
||
thrown in Karma, Spinach, or Rspec tests but can't reproduce them manually,
|
||
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>`).
|
||
|
||
### Spec errors due to use of unsupported JavaScript
|
||
|
||
Similar errors will be thrown if you're using JavaScript features not yet
|
||
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
|
||
- Symbol/Symbol.iterator
|
||
- Spread
|
||
|
||
Until these are polyfilled or transpiled appropriately, they should not be used.
|
||
Please update this list with additional unsupported features or when any of
|
||
these are made usable.
|
||
|
||
### Spec errors due to JavaScript not enabled
|
||
|
||
If, as a result of a change you've made, a feature now depends on JavaScript to
|
||
run correctly, you need to make sure a JavaScript web driver is enabled when
|
||
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.
|
||
|
||
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:
|
||
|
||
```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
|
||
```
|
||
|
||
In Spinach, the JavaScript driver is enabled differently. In the `*.feature`
|
||
file for the failing spec, add the `@javascript` flag above the Scenario:
|
||
|
||
```
|
||
@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"
|
||
|
||
```
|
||
|
||
|
||
[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
|
||
[airbnb-js-style-guide]: https://github.com/airbnb/javascript
|
||
[eslintrc]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/.eslintrc
|
||
[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
|
||
[container-class-example]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/mini_pipeline_graph_dropdown.js
|