From c7b3ba834b80d81521a7e6619276e0920d4af94c Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 20 Apr 2017 11:24:40 +0000 Subject: Adds vue js example application and documentation --- doc/development/fe_guide/style_guide_js.md | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) (limited to 'doc/development/fe_guide/style_guide_js.md') diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index abd241c0bc8..ed656476a96 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -58,7 +58,7 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns. import Bar from './bar'; ``` -- **Never** disable the `no-undef` rule. Declare globals with `/* global Foo */` instead. +- **Never** disable the `no-undef` rule. Declare globals with `/* global Foo */` instead. - When declaring multiple globals, always use one `/* global [name] */` line per variable. @@ -183,6 +183,19 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns. parseInt('10', 10); ``` +#### CSS classes used for JavaScript +- If the class is being used in Javascript it needs to be prepend with `js-` + ```html + // bad + + + // good + + ``` ### Vue.js @@ -200,6 +213,7 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns. #### Naming - **Extensions**: Use `.vue` extension for Vue components. - **Reference Naming**: Use PascalCase for Vue components and camelCase for their instances: + ```javascript // bad import cardBoard from 'cardBoard'; @@ -217,6 +231,7 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns. cardBoard: CardBoard }; ``` + - **Props Naming:** - Avoid using DOM component prop names. - Use kebab-case instead of camelCase to provide props in templates. @@ -243,12 +258,18 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns. + + // good + + // if props fit in one line then keep it on the same line ``` -- cgit v1.2.1 From c7049ed07f411337f5f0569197f2b05649ec6038 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 20 Apr 2017 15:52:00 +0000 Subject: Adds documentation entry: Don't user forEach, aim for code without side effects --- doc/development/fe_guide/style_guide_js.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'doc/development/fe_guide/style_guide_js.md') diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index ed656476a96..038a689c09a 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -168,6 +168,23 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns. - Avoid constructors with side-effects +- 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' } ]; + + // bad + users.forEach((user, index) => { + 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 `+` -- cgit v1.2.1 From c49a8fb1be1d03292ec8974f2f96ce7b46139dd1 Mon Sep 17 00:00:00 2001 From: Fatih Acet Date: Thu, 20 Apr 2017 16:39:56 +0000 Subject: Update style_guide_js.md --- doc/development/fe_guide/style_guide_js.md | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'doc/development/fe_guide/style_guide_js.md') diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index 038a689c09a..1d2b0558948 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -71,6 +71,16 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns. /* 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) {} + ``` #### Modules, Imports, and Exports - Use ES module syntax to import modules -- cgit v1.2.1 From f68f0cd1eacbd9137b014c486c6f6cd6b6378e58 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 4 May 2017 14:53:00 +0100 Subject: Adds numbered lists to easily point to documentation --- doc/development/fe_guide/style_guide_js.md | 544 ++++++++++++++--------------- 1 file changed, 262 insertions(+), 282 deletions(-) (limited to 'doc/development/fe_guide/style_guide_js.md') diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index 1d2b0558948..c8a77f67607 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -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] -- 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. 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'; + // bad + /* global Foo */ + /* eslint-disable no-new */ + import Bar from './bar'; - // good - /* eslint-disable no-new */ - /* global Foo */ + // good + /* eslint-disable no-new */ + /* global Foo */ - import Bar from './bar'; + import Bar from './bar'; ``` -- **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. **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 - /* globals Flash, Cookies, jQuery */ + // bad + /* globals Flash, Cookies, jQuery */ - // good - /* global Flash */ - /* global Cookies */ - /* global jQuery */ + // 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. +1. 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) {} + // 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**: - ```javascript - // bad - import Foo from '~/foo' + // good + import Foo from '../foo'; + ``` + * In **spec/javascripts**: - // good - import Foo from '../foo'; - ``` - - In **spec/javascripts**: - ```javascript - // bad - import Foo from '../../app/assets/javascripts/foo' - - // good - import Foo from '~/foo'; - ``` + ```javascript + // bad + import Foo from '../../app/assets/javascripts/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. + // good + import Foo from '~/foo'; + ``` -- Avoid adding to the global namespace. +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 - window.MyClass = class { /* ... */ }; + // bad + window.MyClass = class { /* ... */ }; - // good - export default class MyClass { /* ... */ } + // good + export default class MyClass { /* ... */ } ``` -- Side effects are forbidden in any script which contains exports - +1. Side effects are forbidden in any script which contains exports ```javascript - // bad - export default class MyClass { /* ... */ } + // 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 + - // good - + // good + - + - // if props fit in one line then keep it on the same line - + // if props fit in one line then keep it on the same line + ``` #### 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: ` - - ` - - // good - template: ` - - ` + // bad + template: ` + + ` + + // good + template: ` + + ` ``` #### Props -- Props should be declared as an object - +1. Props should be declared as an object ```javascript - // bad - props: ['foo'] - - // good - props: { - foo: { - type: String, - required: false, - default: 'bar' + // bad + props: ['foo'] + + // 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 - + // bad + - // good - + // good + ``` -- Shorthand `:` is preferable over `v-bind` - +1. Shorthand `:` is preferable over `v-bind` ```javascript - // bad - + // bad + - // good - + // good + ``` #### Closing tags -- Prefer self closing component tags - +1. Prefer self closing component tags ```javascript - // bad - + // bad + - // good - + // good + ``` #### 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 -- cgit v1.2.1 From 1848ddf6562f1c0a7a53bd65c79e251f0726a532 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 4 May 2017 15:13:52 +0100 Subject: Add missing points --- doc/development/fe_guide/style_guide_js.md | 78 +++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 8 deletions(-) (limited to 'doc/development/fe_guide/style_guide_js.md') diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index c8a77f67607..a3f806c1713 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -11,7 +11,10 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns. #### ESlint -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** 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 @@ -44,7 +47,8 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns. 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. +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 */ @@ -96,7 +100,8 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns. export default Foo; ``` -1. Relative paths: Unless you are writing a test, always reference other scripts using relative paths instead of `~` +1. Relative paths: Unless you are writing a test, always reference other scripts using +relative paths instead of `~` * In **app/assets/javascripts**: ```javascript @@ -116,7 +121,10 @@ See [our current .eslintrc][eslintrc] for specific rules and patterns. 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 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 @@ -224,7 +232,45 @@ A forEach will cause side effects, it will be mutating the array being iterated. template: `

I'm a component

} ``` -1. +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 + } + } + ``` #### Naming 1. **Extensions**: Use `.vue` extension for Vue components. @@ -247,9 +293,8 @@ A forEach will cause side effects, it will be mutating the array being iterated. }; ``` -1. **Props Naming:** -1. Avoid using DOM component prop names. -1. Use kebab-case instead of camelCase to provide props in templates. +1. **Props Naming:** Avoid using DOM component prop names. +1. **Props Naming:** Use kebab-case instead of camelCase to provide props in templates. ```javascript // bad @@ -433,6 +478,23 @@ A forEach will cause side effects, it will be mutating the array being iterated. 1. `beforeDestroy` 1. `destroyed` +#### Vue and Boostrap +1. Tooltips: Do not rely on `has-tooltip` class name for vue components + ```javascript + // bad + + Text + + + // good + + Text + + ``` + +1. Tooltips: When using a tooltip, include the tooltip mixin + +1. Don't change `data-original-title`. ## SCSS - [SCSS](style_guide_scss.md) -- cgit v1.2.1 From 6f040230d2307d980dc3ebd134caebb1945a3903 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Thu, 4 May 2017 17:56:46 +0100 Subject: Changes after review --- doc/development/fe_guide/style_guide_js.md | 32 ++++++++++++------------------ 1 file changed, 13 insertions(+), 19 deletions(-) (limited to 'doc/development/fe_guide/style_guide_js.md') diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index a3f806c1713..d2d89517241 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -225,13 +225,6 @@ A forEach will cause side effects, it will be mutating the array being iterated. ### Vue.js #### Basic Rules -1. Only include one Vue.js component per file. -1. Export components as plain objects: - ```javascript - export default { - template: `

I'm a component

- } - ``` 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: @@ -274,22 +267,13 @@ A forEach will cause side effects, it will be mutating the array being iterated. #### Naming 1. **Extensions**: Use `.vue` extension for Vue components. -1. **Reference Naming**: Use PascalCase for Vue components and camelCase for their instances: +1. **Reference Naming**: Use camelCase for their instances: ```javascript - // bad - import cardBoard from 'cardBoard'; - // good - import CardBoard from 'cardBoard' + import cardBoard from 'cardBoard' - // bad components: { - CardBoard: CardBoard - }; - - // good - components: { - cardBoard: CardBoard + cardBoard: }; ``` @@ -495,6 +479,16 @@ A forEach will cause side effects, it will be mutating the array being iterated. 1. Tooltips: When using a tooltip, include the tooltip mixin 1. Don't change `data-original-title`. + ```javascript + // bad + Foo + + // good + Foo + + $('span').tooltip('fixTitle'); + ``` + ## SCSS - [SCSS](style_guide_scss.md) -- cgit v1.2.1