summaryrefslogtreecommitdiff
path: root/doc/development/fe_guide
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2020-07-20 12:26:25 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2020-07-20 12:26:25 +0000
commita09983ae35713f5a2bbb100981116d31ce99826e (patch)
tree2ee2af7bd104d57086db360a7e6d8c9d5d43667a /doc/development/fe_guide
parent18c5ab32b738c0b6ecb4d0df3994000482f34bd8 (diff)
downloadgitlab-ce-a09983ae35713f5a2bbb100981116d31ce99826e.tar.gz
Add latest changes from gitlab-org/gitlab@13-2-stable-ee
Diffstat (limited to 'doc/development/fe_guide')
-rw-r--r--doc/development/fe_guide/accessibility.md2
-rw-r--r--doc/development/fe_guide/development_process.md4
-rw-r--r--doc/development/fe_guide/frontend_faq.md17
-rw-r--r--doc/development/fe_guide/graphql.md2
-rw-r--r--doc/development/fe_guide/icons.md3
-rw-r--r--doc/development/fe_guide/index.md4
-rw-r--r--doc/development/fe_guide/tooling.md50
-rw-r--r--doc/development/fe_guide/vue.md14
-rw-r--r--doc/development/fe_guide/vuex.md66
9 files changed, 147 insertions, 15 deletions
diff --git a/doc/development/fe_guide/accessibility.md b/doc/development/fe_guide/accessibility.md
index 998c71135fb..669c93eb251 100644
--- a/doc/development/fe_guide/accessibility.md
+++ b/doc/development/fe_guide/accessibility.md
@@ -1,4 +1,4 @@
-# Accessibility
+# Accessibility & Readability
## Resources
diff --git a/doc/development/fe_guide/development_process.md b/doc/development/fe_guide/development_process.md
index 892e931bf5b..ff36b8b5c6a 100644
--- a/doc/development/fe_guide/development_process.md
+++ b/doc/development/fe_guide/development_process.md
@@ -73,8 +73,8 @@ With the purpose of being [respectful of others' time](https://about.gitlab.com/
- Before assigning to a maintainer, assign to a reviewer.
- If you assigned a merge request or pinged someone directly, be patient because we work in different timezones and asynchronously. Unless the merge request is urgent (like fixing a broken master), please don't DM or reassign the merge request before waiting for a 24-hour window.
- If you have a question regarding your merge request/issue, make it on the merge request/issue. When we DM each other, we no longer have a SSOT and [no one else is able to contribute](https://about.gitlab.com/handbook/values/#public-by-default).
-- When you have a big WIP merge request with many changes, you're advised to get the review started before adding/removing significant code. Make sure it is assigned well before the release cut-off, as the reviewer(s)/maintainer(s) would always prioritize reviewing finished MRs before WIP ones.
-- Make sure to remove the WIP title before the last round of review.
+- When you have a big **Draft** merge request with many changes, you're advised to get the review started before adding/removing significant code. Make sure it is assigned well before the release cut-off, as the reviewer(s)/maintainer(s) would always prioritize reviewing finished MRs before the **Draft** ones.
+- Make sure to remove the `Draft:` title before the last round of review.
### Share your work early
diff --git a/doc/development/fe_guide/frontend_faq.md b/doc/development/fe_guide/frontend_faq.md
index 4f814f3cdde..3c0845a9aaa 100644
--- a/doc/development/fe_guide/frontend_faq.md
+++ b/doc/development/fe_guide/frontend_faq.md
@@ -146,3 +146,20 @@ export const fetchFoos = ({ state }) => {
return axios.get(state.settings.fooPath);
};
```
+
+### 7. How can I test the production build locally?
+
+Sometimes it's necessary to test locally what the frontend production build would produce, to do so the steps are:
+
+1. Stop webpack: `gdk stop webpack`.
+1. Open `gitlab.yaml` located in your `gitlab` installation folder, scroll down to the `webpack` section and change `dev_server` to `enabled: false`.
+1. Run `yarn webpack-prod && gdk restart rails-web`.
+
+The production build takes a few minutes to be completed; any code change at this point will be
+displayed only after executing the item 3 above again.
+To return to the normal development mode:
+
+1. Open `gitlab.yaml` located in your `gitlab` installation folder, scroll down to the `webpack` section and change back `dev_server` to `enabled: true`.
+1. Run `yarn clean` to remove the production assets and free some space (optional).
+1. Start webpack again: `gdk start webpack`.
+1. Restart GDK: `gdk-restart rails-web`.
diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md
index 191ebd2ff58..3d74ec94ae4 100644
--- a/doc/development/fe_guide/graphql.md
+++ b/doc/development/fe_guide/graphql.md
@@ -646,7 +646,7 @@ defaultClient.query({ query })
.then(result => console.log(result));
```
-When [using Vuex](#Using-with-Vuex), disable the cache when:
+When [using Vuex](#using-with-vuex), disable the cache when:
- The data is being cached elsewhere
- The use case does not need caching
diff --git a/doc/development/fe_guide/icons.md b/doc/development/fe_guide/icons.md
index 131324e6479..7078b5e9b2f 100644
--- a/doc/development/fe_guide/icons.md
+++ b/doc/development/fe_guide/icons.md
@@ -61,6 +61,7 @@ export default {
<gl-icon
name="issues"
:size="24"
+ class="class-name"
/>
</template>
```
@@ -68,7 +69,7 @@ export default {
- **name** Name of the Icon in the SVG Sprite ([Overview is available here](https://gitlab-org.gitlab.io/gitlab-svgs)).
- **size (optional)** Number value for the size which is then mapped to a specific CSS class
(Available Sizes: 8, 12, 16, 18, 24, 32, 48, 72 are mapped to `sXX` CSS classes)
-- **css-classes (optional)** Additional CSS Classes to add to the SVG tag.
+- **class (optional)** Additional CSS Classes to add to the SVG tag.
### Usage in HTML/JS
diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md
index 8fd6ba459b9..3a2b3cac9bf 100644
--- a/doc/development/fe_guide/index.md
+++ b/doc/development/fe_guide/index.md
@@ -17,7 +17,9 @@ Working with our frontend assets requires Node (v10.13.0 or greater) and Yarn
For our currently-supported browsers, see our [requirements](../../install/requirements.md#supported-web-browsers).
-Use [BrowserStack](https://www.browserstack.com/) to test with our supported browsers. Login to BrowserStack with the credentials saved in GitLab's [shared 1Password account](https://about.gitlab.com/handbook/security/#1password-for-teams).
+Use [BrowserStack](https://www.browserstack.com/) to test with our supported browsers.
+Sign in to BrowserStack with the credentials saved in the **Engineering** vault of GitLab's
+[shared 1Password account](https://about.gitlab.com/handbook/security/#1password-guide).
## Initiatives
diff --git a/doc/development/fe_guide/tooling.md b/doc/development/fe_guide/tooling.md
index 585cd969c96..28deb7d95f9 100644
--- a/doc/development/fe_guide/tooling.md
+++ b/doc/development/fe_guide/tooling.md
@@ -4,6 +4,44 @@
We use ESLint to encapsulate and enforce frontend code standards. Our configuration may be found in the [`gitlab-eslint-config`](https://gitlab.com/gitlab-org/gitlab-eslint-config) project.
+### Yarn Script
+
+This section describes yarn scripts that are available to validate and apply automatic fixes to files using ESLint.
+
+To check all currently staged files (based on `git diff`) with ESLint, run the following script:
+
+```shell
+yarn eslint-staged
+```
+
+_A list of problems found will be logged to the console._
+
+To apply automatic ESLint fixes to all currently staged files (based on `git diff`), run the following script:
+
+```shell
+yarn eslint-staged-fix
+```
+
+_If manual changes are required, a list of changes will be sent to the console._
+
+To check **all** files in the repository with ESLint, run the following script:
+
+```shell
+yarn eslint
+```
+
+_A list of problems found will be logged to the console._
+
+To apply automatic ESLint fixes to **all** files in the repository, run the following script:
+
+```shell
+yarn eslint-fix
+```
+
+_If manual changes are required, a list of changes will be sent to the console._
+
+**Caution:** Limit use to global rule updates. Otherwise, the changes can lead to huge Merge Requests.
+
### Disabling ESLint in new files
Do not disable ESLint when creating new files. Existing files may have existing rules
@@ -56,13 +94,15 @@ When declaring multiple globals, always use one `/* global [name] */` line per v
## 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`.
+> Support for `.graphql` [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/227280) in GitLab 13.2.
+
+Our code is automatically formatted with [Prettier](https://prettier.io) to follow our style guides. Prettier is taking care of formatting `.js`, `.vue`, `.graphql`, 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:
+Please take care that you only let Prettier format the same file types as the global Yarn script does (`.js`, `.vue`, `.graphql`, and `.scss`). In VSCode by example you can easily exclude file formats in your settings file:
```json
"prettier.disableLanguages": [
@@ -131,6 +171,9 @@ To select Prettier as a formatter, add the following properties to your User or
},
"[vue]": {
"editor.defaultFormatter": "esbenp.prettier-vscode"
+ },
+ "[graphql]": {
+ "editor.defaultFormatter": "esbenp.prettier-vscode"
}
}
```
@@ -150,5 +193,8 @@ To automatically format your files with Prettier, add the following properties t
"[vue]": {
"editor.formatOnSave": true
},
+ "[graphql]": {
+ "editor.formatOnSave": true
+ },
}
```
diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md
index 0d77e4d129b..2a0556c6cda 100644
--- a/doc/development/fe_guide/vue.md
+++ b/doc/development/fe_guide/vue.md
@@ -53,14 +53,14 @@ Be sure to read about [page-specific JavaScript](./performance.md#page-specific-
#### Providing data from HAML to JavaScript
-While mounting a Vue application may be a need to provide data from Rails to JavaScript.
-To do that, provide the data through `data` attributes in the HTML element and query them while mounting the application.
+While mounting a Vue application, you might need to provide data from Rails to JavaScript.
+To do that, you can use the `data` attributes in the HTML element and query them while mounting the application.
_Note:_ You should only do this while initializing the application, because the mounted element will be replaced with Vue-generated DOM.
The advantage of providing data from the DOM to the Vue instance through `props` in the `render` function
-instead of querying the DOM inside the main Vue component is that makes tests easier by avoiding the need to
-create a fixture or an HTML element in the unit test. See the following example:
+instead of querying the DOM inside the main Vue component is avoiding the need to create a fixture or an HTML element in the unit test,
+which will make the tests easier. See the following example:
```javascript
// haml
@@ -134,7 +134,7 @@ This approach has a few benefits:
intermediate components being aware of it (c.f. passing the flag down via
props).
- Good testability, since the flag can be provided to `mount`/`shallowMount`
- from `vue-test-utils` as easily as a prop.
+ from `vue-test-utils` simply as a prop.
```javascript
import { shallowMount } from '@vue/test-utils';
@@ -155,7 +155,7 @@ 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
+A good rule of thumb 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
@@ -321,7 +321,7 @@ We should verify an event has been fired by asserting against the result of the
## Vue.js Expert Role
-One should apply to be a Vue.js expert by opening an MR when the Merge Request's they create and review show:
+You should only apply to be a Vue.js expert when your own merge requests and your reviews show:
- Deep understanding of Vue and Vuex reactivity
- Vue and Vuex code are structured according to both official and our guidelines
diff --git a/doc/development/fe_guide/vuex.md b/doc/development/fe_guide/vuex.md
index e7be67b8da5..02387c15951 100644
--- a/doc/development/fe_guide/vuex.md
+++ b/doc/development/fe_guide/vuex.md
@@ -201,6 +201,72 @@ By following this pattern we guarantee:
1. All data in the application follows the same lifecycle pattern
1. Unit tests are easier
+#### Updating complex state
+
+Sometimes, especially when the state is complex, is really hard to traverse the state to precisely update what the mutation needs to update.
+Ideally a `vuex` state should be as normalized/decoupled as possible but this is not always the case.
+
+It's important to remember that the code is much easier to read and maintain when the `portion of the mutated state` is selected and mutated in the mutation itself.
+
+Given this state:
+
+```javascript
+ export default () => ({
+ items: [
+ {
+ id: 1,
+ name: 'my_issue',
+ closed: false,
+ },
+ {
+ id: 2,
+ name: 'another_issue',
+ closed: false,
+ }
+ ]
+});
+```
+
+It may be tempting to write a mutation like so:
+
+```javascript
+// Bad
+export default {
+ [types.MARK_AS_CLOSED](state, item) {
+ Object.assign(item, {closed: true})
+ }
+}
+```
+
+While this approach works it has several dependencies:
+
+- Correct selection of `item` in the component/action.
+- The `item` property is already declared in the `closed` state.
+ - A new `confidential` property would not be reactive.
+- Noting that `item` is referenced by `items`
+
+A mutation written like this is harder to maintain and more error prone. We should rather write a mutation like this:
+
+```javascript
+// Good
+export default {
+ [types.MARK_AS_CLOSED](state, itemId) {
+ const item = state.items.find(i => i.id == itemId);
+ Vue.set(item, 'closed', true)
+
+ state.items.splice(index, 1, item)
+ }
+}
+```
+
+This approach is better because:
+
+- It selects and updates the state in the mutation, which is more maintainable.
+- It has no external dependencies, if the correct `itemId` is passed the state is correctly updated.
+- It does not have reactivity caveats, as we generate a new `item` to avoid coupling to the initial state.
+
+A mutation written like this is easier to maintain. In addition, we avoid errors due to the limitation of the reactivity system.
+
### `getters.js`
Sometimes we may need to get derived state based on store state, like filtering for a specific prop.