From 8263f6ee3131cdea3c6041785c32771a6af0b24f Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 29 Nov 2019 18:06:24 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- doc/development/new_fe_guide/index.md | 4 - doc/development/new_fe_guide/style/html.md | 56 +------ doc/development/new_fe_guide/style/index.md | 18 +-- doc/development/new_fe_guide/style/javascript.md | 198 +---------------------- doc/development/new_fe_guide/style/prettier.md | 101 +----------- doc/development/new_fe_guide/style/scss.md | 3 - doc/development/new_fe_guide/style/vue.md | 3 - 7 files changed, 16 insertions(+), 367 deletions(-) delete mode 100644 doc/development/new_fe_guide/style/scss.md delete mode 100644 doc/development/new_fe_guide/style/vue.md (limited to 'doc/development/new_fe_guide') diff --git a/doc/development/new_fe_guide/index.md b/doc/development/new_fe_guide/index.md index 227d03bd86f..152ddcdae64 100644 --- a/doc/development/new_fe_guide/index.md +++ b/doc/development/new_fe_guide/index.md @@ -15,10 +15,6 @@ Learn about all the dependencies that make up our frontend, including some of ou Learn about all the internal JavaScript modules that make up our frontend. -## [Style guides](style/index.md) - -Style guides to keep our code consistent. - ## [Tips](tips.md) Tips from our frontend team to develop more efficiently and effectively. diff --git a/doc/development/new_fe_guide/style/html.md b/doc/development/new_fe_guide/style/html.md index 1445da3f0e1..0b4fce13d90 100644 --- a/doc/development/new_fe_guide/style/html.md +++ b/doc/development/new_fe_guide/style/html.md @@ -1,53 +1,5 @@ -# HTML style guide +--- +redirect_to: '../../fe_guide/style/html.md' +--- -## Buttons - -### Button type - -Button tags requires a `type` attribute according to the [W3C HTML specification](https://www.w3.org/TR/2011/WD-html5-20110525/the-button-element.html#dom-button-type). - -```html -// bad - - -// good - -``` - -### Button role - -If an HTML element has an `onClick` handler but is not a button, it should have `role="button"`. This is [more accessible](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/button_role). - -```html -// bad -
- -// good -
-``` - -## Links - -### Blank target - -Use `rel="noopener noreferrer"` whenever your links open in a new window, i.e. `target="_blank"`. This prevents a security vulnerability [documented by JitBit](https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/). - -```html -// bad - - -// good - -``` - -### Fake links - -**Do not use fake links.** Use a button tag if a link only invokes JavaScript click event handlers, which is more semantic. - -```html -// bad - - -// good - -``` +This document was moved to [another location](../../fe_guide/style/html.md). diff --git a/doc/development/new_fe_guide/style/index.md b/doc/development/new_fe_guide/style/index.md index f073dc56f1f..284862a2be9 100644 --- a/doc/development/new_fe_guide/style/index.md +++ b/doc/development/new_fe_guide/style/index.md @@ -1,15 +1,5 @@ -# Style guides +--- +redirect_to: '../../fe_guide/style/index.md' +--- -## [HTML style guide](html.md) - -## [SCSS style guide](scss.md) - -## [JavaScript style guide](javascript.md) - -## [Vue style guide](vue.md) - -## Tooling - -## [Prettier](prettier.md) - -Our code is automatically formatted with [Prettier](https://prettier.io) to follow our guidelines. +This document was moved to [another location](../../fe_guide/style/index.md). diff --git a/doc/development/new_fe_guide/style/javascript.md b/doc/development/new_fe_guide/style/javascript.md index d31edcb372d..003880c2592 100644 --- a/doc/development/new_fe_guide/style/javascript.md +++ b/doc/development/new_fe_guide/style/javascript.md @@ -1,195 +1,5 @@ -# JavaScript style guide +--- +redirect_to: '../../fe_guide/style/javascript.md' +--- -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. - -> **Tip:** -You can run eslint locally by running `yarn eslint` - -## 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 aligns with [Airbnb's style guide](https://github.com/airbnb/javascript#testing--for-real). - -```javascript -// bad -users.forEach((user, index) => { - user.id = index; -}); - -// good -const usersWithId = users.map((user, index) => { - return Object.assign({}, user, { id: index }); -}); -``` - -## 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) { - // ... -}; - -// good -function a(p) { - // ... -}; -``` - -## Avoid side effects in constructors - -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 -// bad -class myClass { - constructor(config) { - this.config = config; - axios.get(this.config.endpoint) - } -} - -// good -class myClass { - constructor(config) { - this.config = config; - } - - makeRequest() { - axios.get(this.config.endpoint) - } -} -const instance = new myClass(); -instance.makeRequest(); -``` - -## 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. - -```javascript -// bad -class myClass { - constructor(config) { - this.config = config; - } - - init() { - document.addEventListener('click', () => {}); - } -} - -// good - -const myFunction = () => { - document.addEventListener('click', () => { - // handle callback here - }); -} -``` - -## Pass element container to constructor - -When your class manipulates the DOM, receive the element container as a parameter. -This is more maintainable and performant. - -```javascript -// bad -class a { - constructor() { - document.querySelector('.b'); - } -} - -// good -class a { - constructor(options) { - options.container.querySelector('.b'); - } -} -``` - -## Use ParseInt - -Use `ParseInt` when converting a numeric string into a number. - -```javascript -// bad -Number('10') - -// good -parseInt('10', 10); -``` - -## CSS Selectors - 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-`. - -```html -// bad - - -// good - -``` - -## Absolute vs relative paths for modules - -Use relative paths if the module you are importing is less than two levels up. - -```javascript -// bad -import GitLabStyleGuide from '~/guides/GitLabStyleGuide'; - -// good -import GitLabStyleGuide from '../GitLabStyleGuide'; -``` - -If the module you are importing is two or more levels up, use an absolute path instead: - -```javascript -// bad -import GitLabStyleGuide from '../../../guides/GitLabStyleGuide'; - -// good -import GitLabStyleGuide from '~/GitLabStyleGuide'; -``` - -Additionally, **do not add to global namespace**. - -## 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. - -## Avoid XSS - -Do not use `innerHTML`, `append()` or `html()` to set content. It opens up too many -vulnerabilities. - -## 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 disable specific ESLint rules. Due to technical debt, you may disable the following -rules only if you are invoking/instantiating existing code modules. - -- [no-new](https://eslint.org/docs/rules/no-new) -- [class-method-use-this](https://eslint.org/docs/rules/class-methods-use-this) - -> 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`. +This document was moved to [another location](../../fe_guide/style/javascript.md). diff --git a/doc/development/new_fe_guide/style/prettier.md b/doc/development/new_fe_guide/style/prettier.md index 17b209d419e..9a95aa96dff 100644 --- a/doc/development/new_fe_guide/style/prettier.md +++ b/doc/development/new_fe_guide/style/prettier.md @@ -1,98 +1,5 @@ -# Formatting with Prettier +--- +redirect_to: '../../fe_guide/tooling.md#formatting-with-prettier' +--- -Our code is automatically formatted with [Prettier](https://prettier.io) to follow our style guides. Prettier is taking care of formatting .js, .vue, and .scss files based on the standard prettier rules. You can find all settings for Prettier in `.prettierrc`. - -## Editor - -The easiest way to include prettier in your workflow is by setting up your preferred editor (all major editors are supported) accordingly. We suggest setting up prettier to run automatically when each file is saved. Find [here](https://prettier.io/docs/en/editors.html) the best way to set it up in your preferred editor. - -Please take care that you only let Prettier format the same file types as the global Yarn script does (.js, .vue, and .scss). In VSCode by example you can easily exclude file formats in your settings file: - -``` - "prettier.disableLanguages": [ - "json", - "markdown" - ], -``` - -## Yarn Script - -The following yarn scripts are available to do global formatting: - -``` -yarn prettier-staged-save -``` - -Updates all currently staged files (based on `git diff`) with Prettier and saves the needed changes. - -``` -yarn prettier-staged -``` - -Checks all currently staged files (based on `git diff`) with Prettier and log which files would need manual updating to the console. - -``` -yarn prettier-all -``` - -Checks all files with Prettier and logs which files need manual updating to the console. - -``` -yarn prettier-all-save -``` - -Formats all files in the repository with Prettier. (This should only be used to test global rule updates otherwise you would end up with huge MR's). - -The source of these Yarn scripts can be found in `/scripts/frontend/prettier.js`. - -### Scripts during Conversion period - -``` -node ./scripts/frontend/prettier.js check-all ./vendor/ -``` - -This will go over all files in a specific folder check it. - -``` -node ./scripts/frontend/prettier.js save-all ./vendor/ -``` - -This will go over all files in a specific folder and save it. - -## VSCode Settings - -### Select Prettier as default formatter - -To select Prettier as a formatter, add the following properties to your User or Workspace Settings: - -```javascript -{ - "[html]": { - "editor.defaultFormatter": "esbenp.prettier-vscode" - }, - "[javascript]": { - "editor.defaultFormatter": "esbenp.prettier-vscode" - }, - "[vue]": { - "editor.defaultFormatter": "esbenp.prettier-vscode" - } -} -``` - -### Format on Save - -To automatically format your files with Prettier, add the following properties to your User or Workspace Settings: - -```javascript -{ - "[html]": { - "editor.formatOnSave": true - }, - "[javascript]": { - "editor.formatOnSave": true - }, - "[vue]": { - "editor.formatOnSave": true - }, -} -``` +This document was moved to [another location](../../fe_guide/tooling.md#formatting-with-prettier). diff --git a/doc/development/new_fe_guide/style/scss.md b/doc/development/new_fe_guide/style/scss.md deleted file mode 100644 index 6f5e818d7db..00000000000 --- a/doc/development/new_fe_guide/style/scss.md +++ /dev/null @@ -1,3 +0,0 @@ -# SCSS style guide - -> TODO: Add content diff --git a/doc/development/new_fe_guide/style/vue.md b/doc/development/new_fe_guide/style/vue.md deleted file mode 100644 index fd9353e0d3f..00000000000 --- a/doc/development/new_fe_guide/style/vue.md +++ /dev/null @@ -1,3 +0,0 @@ -# Vue style guide - -> TODO: Add content -- cgit v1.2.3