diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-12-17 11:59:07 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-12-17 11:59:07 +0000 |
| commit | 8b573c94895dc0ac0e1d9d59cf3e8745e8b539ca (patch) | |
| tree | 544930fb309b30317ae9797a9683768705d664c4 /doc/development/fe_guide | |
| parent | 4b1de649d0168371549608993deac953eb692019 (diff) | |
| download | gitlab-ce-8b573c94895dc0ac0e1d9d59cf3e8745e8b539ca.tar.gz | |
Add latest changes from gitlab-org/gitlab@13-7-stable-eev13.7.0-rc42
Diffstat (limited to 'doc/development/fe_guide')
34 files changed, 829 insertions, 391 deletions
diff --git a/doc/development/fe_guide/accessibility.md b/doc/development/fe_guide/accessibility.md index 8cb61019556..92730e8139f 100644 --- a/doc/development/fe_guide/accessibility.md +++ b/doc/development/fe_guide/accessibility.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Accessibility & Readability diff --git a/doc/development/fe_guide/architecture.md b/doc/development/fe_guide/architecture.md index 4b45fb97c76..964837dc5f7 100644 --- a/doc/development/fe_guide/architecture.md +++ b/doc/development/fe_guide/architecture.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Architecture diff --git a/doc/development/fe_guide/axios.md b/doc/development/fe_guide/axios.md index 856b03e4b47..cf5a4970c04 100644 --- a/doc/development/fe_guide/axios.md +++ b/doc/development/fe_guide/axios.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Axios diff --git a/doc/development/fe_guide/dependencies.md b/doc/development/fe_guide/dependencies.md index bbe4cdc285d..0ec10399ae0 100644 --- a/doc/development/fe_guide/dependencies.md +++ b/doc/development/fe_guide/dependencies.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Frontend dependencies @@ -32,7 +32,7 @@ updated using renovate are: We discourage installing some dependencies in [GitLab repository](https://gitlab.com/gitlab-org/gitlab) because they can create conflicts in the dependency tree. Blocked dependencies are declared in the -`blockDependencies` property of GitLab’s [`package.json` file](https://gitlab.com/gitlab-org/gitlab/-/blob/master/package.json). +`blockDependencies` property of the GitLab [`package.json` file](https://gitlab.com/gitlab-org/gitlab/-/blob/master/package.json). ## Dependency notes diff --git a/doc/development/fe_guide/design_patterns.md b/doc/development/fe_guide/design_patterns.md index ed4d91f34bd..784612682f8 100644 --- a/doc/development/fe_guide/design_patterns.md +++ b/doc/development/fe_guide/design_patterns.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Design Patterns diff --git a/doc/development/fe_guide/development_process.md b/doc/development/fe_guide/development_process.md index 915dab06ac2..d122459f51c 100644 --- a/doc/development/fe_guide/development_process.md +++ b/doc/development/fe_guide/development_process.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Frontend Development Process @@ -85,7 +85,7 @@ With the purpose of being [respectful of others' time](https://about.gitlab.com/ ### Share your work early 1. Before writing code, ensure your vision of the architecture is aligned with - GitLab's architecture. + GitLab architecture. 1. Add a diagram to the issue and ask a frontend maintainer in the Slack channel `#frontend_maintainers` about it.  diff --git a/doc/development/fe_guide/droplab/droplab.md b/doc/development/fe_guide/droplab/droplab.md index fe0f07b3019..8f1ecc115fe 100644 --- a/doc/development/fe_guide/droplab/droplab.md +++ b/doc/development/fe_guide/droplab/droplab.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # DropLab @@ -22,7 +22,7 @@ The value is irrelevant. The DropLab class has no side effects, so you must always call `.init` when the DOM is ready. `DropLab.prototype.init` takes the same arguments as `DropLab.prototype.addHook`. -If you don't provide any arguments, it will globally query and instantiate all +If you don't provide any arguments, it globally queries and instantiates all DropLab-compatible dropdowns. ```html @@ -103,7 +103,7 @@ droplab.addHook(trigger, list); ### Dynamic data -Adding `data-dynamic` to your dropdown element will enable dynamic list +Adding `data-dynamic` to your dropdown element enables dynamic list rendering. You can template a list item using the keys of the data object provided. Use the @@ -111,7 +111,7 @@ handlebars syntax `{{ value }}` to HTML escape the value. Use the `<%= value %>` syntax to interpolate the value. Use the `<%= value %>` syntax to evaluate the value. -Passing an array of objects to `DropLab.prototype.addData` will render that data +Passing an array of objects to `DropLab.prototype.addData` renders that data for all `data-dynamic` dropdown lists tracked by that DropLab instance. ```html @@ -227,14 +227,14 @@ provides some potentially useful data. Plugins are objects that are registered to be executed when a hook is added (when a DropLab trigger and dropdown are instantiated). -If no modules API is detected, the library will fall back as it does with -`window.DropLab` and will add `window.DropLab.plugins.PluginName`. +If no modules API is detected, the library falls back as it does with +`window.DropLab` and adds `window.DropLab.plugins.PluginName`. ### Usage To use plugins, you can pass them in an array as the third argument of `DropLab.prototype.init` or `DropLab.prototype.addHook`. Some plugins require -configuration values; the config object can be passed as the fourth argument. +configuration values; the configuration object can be passed as the fourth argument. ```html <a href="#" id="trigger" data-dropdown-trigger="#list">Toggle</a> diff --git a/doc/development/fe_guide/droplab/plugins/ajax.md b/doc/development/fe_guide/droplab/plugins/ajax.md index 3ac876ad062..f12f8f260c7 100644 --- a/doc/development/fe_guide/droplab/plugins/ajax.md +++ b/doc/development/fe_guide/droplab/plugins/ajax.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Ajax plugin diff --git a/doc/development/fe_guide/droplab/plugins/filter.md b/doc/development/fe_guide/droplab/plugins/filter.md index 9ab4946d21d..79f10cdb6c1 100644 --- a/doc/development/fe_guide/droplab/plugins/filter.md +++ b/doc/development/fe_guide/droplab/plugins/filter.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Filter plugin @@ -49,7 +49,7 @@ droplab.addData('trigger', [{ In the previous code, the input string is compared against the `test` key of the passed data objects. -Optionally you can set `filterFunction` to a function. This function will be +Optionally you can set `filterFunction` to a function. This function is then used instead of `Filter`'s built-in string search. `filterFunction` is passed two arguments: the first is one of the data objects, and the second is the current input value. diff --git a/doc/development/fe_guide/droplab/plugins/index.md b/doc/development/fe_guide/droplab/plugins/index.md index d44670bfb3c..c7a2865ca83 100644 --- a/doc/development/fe_guide/droplab/plugins/index.md +++ b/doc/development/fe_guide/droplab/plugins/index.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments description: A list of DropLab plugins. --- diff --git a/doc/development/fe_guide/droplab/plugins/input_setter.md b/doc/development/fe_guide/droplab/plugins/input_setter.md index ab8a5cebd08..a3c073520cb 100644 --- a/doc/development/fe_guide/droplab/plugins/input_setter.md +++ b/doc/development/fe_guide/droplab/plugins/input_setter.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # InputSetter plugin @@ -67,6 +67,6 @@ element's `data-selected-id` to `1`. Optionally, you can set `inputAttribute` to a string that's the name of an attribute on your `input` element that you want to update. If you don't provide -an `inputAttribute`, `InputSetter` will update the `value` of the `input` +an `inputAttribute`, `InputSetter` updates the `value` of the `input` element if it's an `INPUT` element, or the `textContent` of the `input` element if it isn't an `INPUT` element. diff --git a/doc/development/fe_guide/editor_lite.md b/doc/development/fe_guide/editor_lite.md index eb5852d258d..465d64ff63c 100644 --- a/doc/development/fe_guide/editor_lite.md +++ b/doc/development/fe_guide/editor_lite.md @@ -1,7 +1,7 @@ --- -stage: none -group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +stage: Create +group: Editor +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Editor Lite @@ -58,7 +58,7 @@ The editor follows the same public API as [provided by Monaco editor](https://mi | Function | Arguments | Description | ----- | ----- | ----- | | `updateModelLanguage` | `path`: String | Updates the instance's syntax highlighting to follow the extension of the passed `path`. Available only on _instance_ level| -| `use` | Array of objects | Array of **extensions** to apply to the instance. Accepts only the array of _objects_, which means that the extensions' ES6 modules should be fetched and resolved in your views/components before being passed to `use`. This prop is available on _instance_ (applies extension to this particular instance) and _global edtor_ (applies the same extension to all instances) levels. | +| `use` | Array of objects | Array of **extensions** to apply to the instance. Accepts only the array of _objects_, which means that the extensions' ES6 modules should be fetched and resolved in your views/components before being passed to `use`. This prop is available on _instance_ (applies extension to this particular instance) and _global editor_ (applies the same extension to all instances) levels. | | Monaco Editor options | See [documentation](https://microsoft.github.io/monaco-editor/api/interfaces/monaco.editor.istandalonecodeeditor.html) | Default Monaco editor options | ## Tips @@ -68,7 +68,7 @@ The editor follows the same public API as [provided by Monaco editor](https://mi Editor Lite comes with the loading state built-in, making spinners and loaders rarely needed in HTML. To benefit the built-in loading state, set the `data-editor-loading` property on the HTML element that is supposed to contain the editor. Editor Lite will show the loader automatically while it's bootstrapping.  -1. Update syntax highlighting if the file name changes. +1. Update syntax highlighting if the filename changes. ```javascript // fileNameEl here is the HTML input element that contains the file name diff --git a/doc/development/fe_guide/emojis.md b/doc/development/fe_guide/emojis.md index 9b1abaa14ae..7151e2ffeee 100644 --- a/doc/development/fe_guide/emojis.md +++ b/doc/development/fe_guide/emojis.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Emojis diff --git a/doc/development/fe_guide/event_tracking.md b/doc/development/fe_guide/event_tracking.md index 79ea80a52ea..24e83ffc524 100644 --- a/doc/development/fe_guide/event_tracking.md +++ b/doc/development/fe_guide/event_tracking.md @@ -3,3 +3,6 @@ redirect_to: '../product_analytics/index.md' --- This document was moved to [another location](../product_analytics/index.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/fe_guide/frontend_faq.md b/doc/development/fe_guide/frontend_faq.md index 22a48c8f9f9..9612f604b56 100644 --- a/doc/development/fe_guide/frontend_faq.md +++ b/doc/development/fe_guide/frontend_faq.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Frontend FAQ @@ -82,13 +82,13 @@ follow up issue and attach it to the component implementation epic found within ### 4. My submit form button becomes disabled after submitting -If you are using a submit button inside a form and you attach an `onSubmit` event listener on the form element, [this piece of code](https://gitlab.com/gitlab-org/gitlab/blob/794c247a910e2759ce9b401356432a38a4535d49/app/assets/javascripts/main.js#L225) will add a `disabled` class selector to the submit button when the form is submitted. +If you are using a submit button inside a form and you attach an `onSubmit` event listener on the form element, [this piece of code](https://gitlab.com/gitlab-org/gitlab/blob/794c247a910e2759ce9b401356432a38a4535d49/app/assets/javascripts/main.js#L225) adds a `disabled` class selector to the submit button when the form is submitted. To avoid this behavior, add the class `js-no-auto-disable` to the button. ### 5. Should I use a full URL (i.e. `gon.gitlab_url`) or a full path (i.e. `gon.relative_url_root`) when referencing backend endpoints? -It's preferred to use a **full path** over a **full URL** because the URL will use the hostname configured with -GitLab which may not match the request. This will cause [CORS issues like this Web IDE one](https://gitlab.com/gitlab-org/gitlab/-/issues/36810). +It's preferred to use a **full path** over a **full URL** because the URL uses the hostname configured with +GitLab which may not match the request. This causes [CORS issues like this Web IDE one](https://gitlab.com/gitlab-org/gitlab/-/issues/36810). Example: @@ -161,8 +161,9 @@ Sometimes it's necessary to test locally what the frontend production build woul 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 +The production build takes a few minutes to be completed; any code changes at this point are 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`. @@ -181,7 +182,7 @@ we're using that our target browsers don't support. You don't need to add `core- polyfills manually. GitLab adds non-`core-js` polyfills for extending browser features (such as -GitLab's SVG polyfill), which allow us to reference SVGs by using `<use xlink:href>`. +the GitLab SVG polyfill), which allow us to reference SVGs by using `<use xlink:href>`. Be sure to add these polyfills to `app/assets/javascripts/commons/polyfills.js`. To see what polyfills are being used: diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index cae2435e4ff..b1896863af9 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -143,7 +143,7 @@ More about fragments: ## Global IDs -GitLab's GraphQL API expresses `id` fields as Global IDs rather than the PostgreSQL +The GitLab GraphQL API expresses `id` fields as Global IDs rather than the PostgreSQL primary key `id`. Global ID is [a convention](https://graphql.org/learn/global-object-identification/) used for caching and fetching in client-side libraries. @@ -187,7 +187,7 @@ As shown in the code example by using `produce`, we can perform any kind of dire `draftState`. Besides, `immer` guarantees that a new state which includes the changes to `draftState` will be generated. Finally, to verify whether the immutable cache update is working properly, we need to change -`assumeImmutableResults` to `true` in the `default client config` (see [Apollo Client](#apollo-client) for more info). +`assumeImmutableResults` to `true` in the default client configuration (see [Apollo Client](#apollo-client) for more information). If everything is working properly `assumeImmutableResults` should remain set to `true`. @@ -411,7 +411,7 @@ handleClick() { ### Working with pagination -GitLab's GraphQL API uses [Relay-style cursor pagination](https://www.apollographql.com/docs/react/data/pagination/#cursor-based) +The GitLab GraphQL API uses [Relay-style cursor pagination](https://www.apollographql.com/docs/react/pagination/overview/#cursor-based) for connection types. This means a "cursor" is used to keep track of where in the data set the next items should be fetched from. [GraphQL Ruby Connection Concepts](https://graphql-ruby.org/pagination/connection_concepts.html) is a good overview and introduction to connections. @@ -439,9 +439,11 @@ parameter, indicating a starting or ending point of our pagination. They should followed with `first` or `last` parameter respectively to indicate _how many_ items we want to fetch after or before a given endpoint. -For example, here we're fetching 10 designs after a cursor: +For example, here we're fetching 10 designs after a cursor (let us call this `projectQuery`): ```javascript +#import "~/graphql_shared/fragments/pageInfo.fragment.graphql" + query { project(fullPath: "root/my-project") { id @@ -453,6 +455,9 @@ query { id } } + pageInfo { + ...PageInfo + } } } } @@ -460,21 +465,31 @@ query { } ``` +Note that we are using the [`pageInfo.fragment.graphql`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/assets/javascripts/graphql_shared/fragments/pageInfo.fragment.graphql) to populate the `pageInfo` information. + #### Using `fetchMore` method in components +This approach makes sense to use with user-handled pagination (e.g. when the scrolls to fetch more data or explicitly clicks a "Next Page"-button). +When we need to fetch all the data initially, it is recommended to use [a (non-smart) query, instead](#using-a-recursive-query-in-components). + When making an initial fetch, we usually want to start a pagination from the beginning. In this case, we can either: - Skip passing a cursor. - Pass `null` explicitly to `after`. -After data is fetched, we should save a `pageInfo` object. Let's assume we're storing -it to Vue component `data`: +After data is fetched, we can use the `update`-hook as an opportunity [to customize +the data that is set in the Vue component property](https://apollo.vuejs.org/api/smart-query.html#options), getting a hold of the `pageInfo` object among other data. + +In the `result`-hook, we can inspect the `pageInfo` object to see if we need to fetch +the next page. Note that we also keep a `requestCount` to ensure that the application +does not keep requesting the next page, indefinitely: ```javascript data() { return { pageInfo: null, + requestCount: 0, } }, apollo: { @@ -482,13 +497,29 @@ apollo: { query: projectQuery, variables() { return { - // rest of design variables - ... + // ... The rest of the design variables first: 10, }; }, - result(res) { - this.pageInfo = res.data?.project?.issue?.designCollection?.designs?.pageInfo; + update(data) { + const { id = null, issue = {} } = data.project || {}; + const { edges = [], pageInfo } = issue.designCollection?.designs || {}; + + return { + id, + edges, + pageInfo, + }; + }, + result() { + const { pageInfo } = this.designs; + + // Increment the request count with each new result + this.requestCount += 1; + // Only fetch next page if we have more requests and there is a next page to fetch + if (this.requestCount < MAX_REQUEST_COUNT && pageInfo?.hasNextPage) { + this.fetchNextPage(pageInfo.endCursor); + } }, }, }, @@ -497,34 +528,154 @@ apollo: { When we want to move to the next page, we use an Apollo `fetchMore` method, passing a new cursor (and, optionally, new variables) there. In the `updateQuery` hook, we have to return a result we want to see in the Apollo cache after fetching the next page. +[`Immer`s `produce`](#immutability-and-cache-updates)-function can help us with the immutability here: ```javascript -fetchNextPage() { - // as a first step, we're checking if we have more pages to move forward - if (this.pageInfo?.hasNextPage) { - this.$apollo.queries.designs.fetchMore({ - variables: { - // rest of design variables - ... - first: 10, - after: this.pageInfo?.endCursor, - }, - updateQuery(previousResult, { fetchMoreResult }) { - // here we can implement the logic of adding new designs to fetched one (for example, if we use infinite scroll) - // or replacing old result with the new one if we use numbered pages +fetchNextPage(endCursor) { + this.$apollo.queries.designs.fetchMore({ + variables: { + // ... The rest of the design variables + first: 10, + after: endCursor, + }, + updateQuery(previousResult, { fetchMoreResult }) { + // Here we can implement the logic of adding new designs to existing ones + // (for example, if we use infinite scroll) or replacing old result + // with the new one if we use numbered pages + + const { designs: previousDesigns } = previousResult.project.issue.designCollection; + const { designs: newDesigns } = fetchMoreResult.project.issue.designCollection + + return produce(previousResult, draftData => { + // `produce` gives us a working copy, `draftData`, that we can modify + // as we please and from it will produce the next immutable result for us + draftData.project.issue.designCollection.designs = [...previousDesigns, ...newDesigns]; + }); + }, + }); +} +``` - const newDesigns = fetchMoreResult.project.issue.designCollection.designs; - previousResult.project.issue.designCollection.designs.push(...newDesigns) +#### Using a recursive query in components - return previousResult; - }, - }); +When it is necessary to fetch all paginated data initially an Apollo query can do the trick for us. +If we need to fetch the next page based on user interactions, it is recommend to use a [`smartQuery`](https://apollo.vuejs.org/api/smart-query.html) along with the [`fetchMore`-hook](#using-fetchmore-method-in-components). + +When the query resolves we can update the component data and inspect the `pageInfo` object +to see if we need to fetch the next page, i.e. call the method recursively. + +Note that we also keep a `requestCount` to ensure that the application does not keep +requesting the next page, indefinitely. + +```javascript +data() { + return { + requestCount: 0, + isLoading: false, + designs: { + edges: [], + pageInfo: null, + }, + } +}, +created() { + this.fetchDesigns(); +}, +methods: { + handleError(error) { + this.isLoading = false; + // Do something with `error` + }, + fetchDesigns(endCursor) { + this.isLoading = true; + + return this.$apollo + .query({ + query: projectQuery, + variables() { + return { + // ... The rest of the design variables + first: 10, + endCursor, + }; + }, + }) + .then(({ data }) => { + const { id = null, issue = {} } = data.project || {}; + const { edges = [], pageInfo } = issue.designCollection?.designs || {}; + + // Update data + this.designs = { + id, + edges: [...this.designs.edges, ...edges]; + pageInfo: pageInfo; + }; + + // Increment the request count with each new result + this.requestCount += 1; + // Only fetch next page if we have more requests and there is a next page to fetch + if (this.requestCount < MAX_REQUEST_COUNT && pageInfo?.hasNextPage) { + this.fetchDesigns(pageInfo.endCursor); + } else { + this.isLoading = false; + } + }) + .catch(this.handleError); + }, +}, +``` + +#### Pagination and optimistic updates + +When Apollo caches paginated data client-side, it includes `pageInfo` variables in the cache key. +If you wanted to optimistically update that data, you'd have to provide `pageInfo` variables +when interacting with the cache via [`.readQuery()`](https://www.apollographql.com/docs/react/v2/api/apollo-client/#ApolloClient.readQuery) +or [`.writeQuery()`](https://www.apollographql.com/docs/react/v2/api/apollo-client/#ApolloClient.writeQuery). +This can be tedious and counter-intuitive. + +To make it easier to deal with cached paginated queries, Apollo provides the `@connection` directive. +The directive accepts a `key` parameter that will be used as a static key when caching the data. +You'd then be able to retrieve the data without providing any pagination-specific variables. + +Here's an example of a query using the `@connection` directive: + +```graphql +#import "~/graphql_shared/fragments/pageInfo.fragment.graphql" + +query DastSiteProfiles($fullPath: ID!, $after: String, $before: String, $first: Int, $last: Int) { + project(fullPath: $fullPath) { + siteProfiles: dastSiteProfiles(after: $after, before: $before, first: $first, last: $last) + @connection(key: "dastSiteProfiles") { + pageInfo { + ...PageInfo + } + edges { + cursor + node { + id + # ... + } + } + } } } ``` -Please note we don't have to save `pageInfo` one more time; `fetchMore` triggers a query -`result` hook as well. +In this example, Apollo will store the data with the stable `dastSiteProfiles` cache key. + +To retrieve that data from the cache, you'd then only need to provide the `$fullPath` variable, +omitting pagination-specific variables like `after` or `before`: + +```javascript +const data = store.readQuery({ + query: dastSiteProfilesQuery, + variables: { + fullPath: 'namespace/project', + }, +}); +``` + +Read more about the `@connection` directive in [Apollo's documentation](https://www.apollographql.com/docs/react/v2/caching/cache-interaction/#the-connection-directive). ### Managing performance @@ -561,7 +712,7 @@ it('tests apollo component', () => { const vm = shallowMount(App); vm.setData({ - ...mock data + ...mockData }); }); ``` @@ -633,7 +784,7 @@ function createComponent(props = {}) { ApolloMutation, }, mocks: { - $apollo: + $apollo, } }); } @@ -666,34 +817,51 @@ it('calls mutation on submitting form ', () => { To test the logic of Apollo cache updates, we might want to mock an Apollo Client in our unit tests. We use [`mock-apollo-client`](https://www.npmjs.com/package/mock-apollo-client) library to mock Apollo client and [`createMockApollo` helper](https://gitlab.com/gitlab-org/gitlab/-/blob/master/spec/frontend/helpers/mock_apollo_helper.js) we created on top of it. -To separate tests with mocked client from 'usual' unit tests, it's recommended to create an additional component factory. This way we only create Apollo Client instance when it's necessary: - -```javascript -function createComponent() {...} - -function createComponentWithApollo() {...} -``` +To separate tests with mocked client from 'usual' unit tests, it's recommended to create an additional factory and pass the created `mockApollo` as an option to the `createComponent`-factory. This way we only create Apollo Client instance when it's necessary. -Then we need to inject `VueApollo` to Vue local instance (`localVue.use()` can also be called within `createComponentWithApollo()`) +We need to inject `VueApollo` to the Vue local instance and, likewise, it is recommended to call `localVue.use()` within `createMockApolloProvider()` to only load it when it is necessary. ```javascript import VueApollo from 'vue-apollo'; import { createLocalVue } from '@vue/test-utils'; const localVue = createLocalVue(); -localVue.use(VueApollo); + +function createMockApolloProvider() { + localVue.use(VueApollo); + + return createMockApollo(requestHandlers); +} + +function createComponent(options = {}) { + const { mockApollo } = options; + ... + return shallowMount(..., { + localVue, + apolloProvider: mockApollo, + ... + }); +} ``` -After this, on the global `describe`, we should create a variable for `fakeApollo`: +After this, you can control whether you need a variable for `mockApollo` and assign it in the appropriate `describe`-scope: ```javascript -describe('Some component with Apollo mock', () => { +describe('Some component', () => { let wrapper; - let fakeApollo -}) + + describe('with Apollo mock', () => { + let mockApollo; + + beforeEach(() => { + mockApollo = createMockApolloProvider(); + wrapper = createComponent({ mockApollo }); + }); + }); +}); ``` -Within component factory, we need to define an array of _handlers_ for every query or mutation: +Within `createMockApolloProvider`-factory, we need to define an array of _handlers_ for every query or mutation: ```javascript import getDesignListQuery from '~/design_management/graphql/queries/get_design_list.query.graphql'; @@ -702,13 +870,16 @@ import moveDesignMutation from '~/design_management/graphql/mutations/move_desig describe('Some component with Apollo mock', () => { let wrapper; - let fakeApollo; + let mockApollo; + + function createMockApolloProvider() { + Vue.use(VueApollo); - function createComponentWithApollo() { const requestHandlers = [ [getDesignListQuery, jest.fn().mockResolvedValue(designListQueryResponse)], [permissionsQuery, jest.fn().mockResolvedValue(permissionsQueryResponse)], ]; + ... } }) ``` @@ -718,23 +889,38 @@ After this, we need to create a mock Apollo Client instance using a helper: ```javascript import createMockApollo from 'jest/helpers/mock_apollo_helper'; -describe('Some component with Apollo mock', () => { +describe('Some component', () => { let wrapper; - let fakeApollo; - function createComponentWithApollo() { + function createMockApolloProvider() { + Vue.use(VueApollo); + const requestHandlers = [ [getDesignListQuery, jest.fn().mockResolvedValue(designListQueryResponse)], [permissionsQuery, jest.fn().mockResolvedValue(permissionsQueryResponse)], ]; - fakeApollo = createMockApollo(requestHandlers); - wrapper = shallowMount(Index, { + return createMockApollo(requestHandlers); + } + + function createComponent(options = {}) { + const { mockApollo } = options; + + return shallowMount(Index, { localVue, - apolloProvider: fakeApollo, + apolloProvider: mockApollo, }); } -}) + + describe('with Apollo mock', () => { + let mockApollo; + + beforeEach(() => { + mockApollo = createMockApolloProvider(); + wrapper = createComponent({ mockApollo }); + }); + }); +}); ``` When mocking resolved values, ensure the structure of the response is the same @@ -744,13 +930,15 @@ When testing queries, please keep in mind they are promises, so they need to be ```javascript it('renders a loading state', () => { - createComponentWithApollo(); + const mockApollo = createMockApolloProvider(); + const wrapper = createComponent({ mockApollo }); expect(wrapper.find(LoadingSpinner).exists()).toBe(true) }); it('renders designs list', async () => { - createComponentWithApollo(); + const mockApollo = createMockApolloProvider(); + const wrapper = createComponent({ mockApollo }); jest.runOnlyPendingTimers(); await wrapper.vm.$nextTick(); @@ -762,7 +950,7 @@ it('renders designs list', async () => { If we need to test a query error, we need to mock a rejected value as request handler: ```javascript -function createComponentWithApollo() { +function createMockApolloProvider() { ... const requestHandlers = [ [getDesignListQuery, jest.fn().mockRejectedValue(new Error('GraphQL error')], @@ -772,7 +960,7 @@ function createComponentWithApollo() { ... it('renders error if query fails', async () => { - createComponent() + const wrapper = createComponent(); jest.runOnlyPendingTimers(); await wrapper.vm.$nextTick(); @@ -786,9 +974,11 @@ Request handlers can also be passed to component factory as a parameter. Mutations could be tested the same way with a few additional `nextTick`s to get the updated result: ```javascript -function createComponentWithApollo({ +function createMockApolloProvider({ moveHandler = jest.fn().mockResolvedValue(moveDesignMutationResponse), }) { + Vue.use(VueApollo); + moveDesignHandler = moveHandler; const requestHandlers = [ @@ -797,15 +987,21 @@ function createComponentWithApollo({ [moveDesignMutation, moveDesignHandler], ]; - fakeApollo = createMockApollo(requestHandlers); - wrapper = shallowMount(Index, { + return createMockApollo(requestHandlers); +} + +function createComponent(options = {}) { + const { mockApollo } = options; + + return shallowMount(Index, { localVue, - apolloProvider: fakeApollo, + apolloProvider: mockApollo, }); } ... it('calls a mutation with correct parameters and reorders designs', async () => { - createComponentWithApollo({}); + const mockApollo = createMockApolloProvider({}); + const wrapper = createComponent({ mockApollo }); wrapper.find(VueDraggable).vm.$emit('change', { moved: { @@ -828,14 +1024,100 @@ it('calls a mutation with correct parameters and reorders designs', async () => #### Testing `@client` queries -If your application contains `@client` queries, most probably you will have an Apollo Client warning saying that you have a local query but no resolvers are defined. In order to fix it, you need to pass resolvers to the mocked client with a second parameter (bare minimum is an empty object): +##### Using mock resolvers + +If your application contains `@client` queries, you get +the following Apollo Client warning when passing only handlers: + +```shell +Unexpected call of console.warn() with: + +Warning: mock-apollo-client - The query is entirely client-side (using @client directives) and resolvers have been configured. The request handler will not be called. +``` + +To fix this you should define mock `resolvers` instead of +mock `handlers`. For example, given the following `@client` query: + +```graphql +query getBlobContent($path: String, $ref: String!) { + blobContent(path: $path, ref: $ref) @client { + rawData + } +} +``` + +And its actual client-side resolvers: ```javascript -import createMockApollo from 'jest/helpers/mock_apollo_helper'; -... -fakeApollo = createMockApollo(requestHandlers, {}); +import Api from '~/api'; + +export const resolvers = { + Query: { + blobContent(_, { path, ref }) { + return { + __typename: 'BlobContent', + rawData: Api.getRawFile(path, { ref }).then(({ data }) => { + return data; + }), + }; + }, + }, +}; + +export default resolvers; +``` + +We can use a **mock resolver** that returns data with the +same shape, while mock the result with a mock function: + +```javascript +let mockApollo; +let mockBlobContentData; // mock function, jest.fn(); + +const mockResolvers = { + Query: { + blobContent() { + return { + __typename: 'BlobContent', + rawData: mockBlobContentData(), // the mock function can resolve mock data + }; + }, + }, +}; + +const createComponentWithApollo = ({ props = {} } = {}) => { + mockApollo = createMockApollo([], mockResolvers); // resolvers are the second parameter + + wrapper = shallowMount(MyComponent, { + localVue, + propsData: {}, + apolloProvider: mockApollo, + // ... + }) +}; + ``` +After which, you can resolve or reject the value needed. + +```javascript +beforeEach(() => { + mockBlobContentData = jest.fn(); +}); + +it('shows data', async() => { + mockBlobContentData.mockResolvedValue(data); // you may resolve or reject to mock the result + + createComponentWithApollo(); + + await waitForPromises(); // wait on the resolver mock to execute + + expect(findContent().text()).toBe(mockCiYml); +}); +``` + +##### Using `cache.writeQuery` + Sometimes we want to test a `result` hook of the local query. In order to have it triggered, we need to populate a cache with correct data to be fetched with this query: ```javascript @@ -849,14 +1131,16 @@ query fetchLocalUser { ```javascript import fetchLocalUserQuery from '~/design_management/graphql/queries/fetch_local_user.query.graphql'; -function createComponentWithApollo() { +function createMockApolloProvider() { + Vue.use(VueApollo); + const requestHandlers = [ [getDesignListQuery, jest.fn().mockResolvedValue(designListQueryResponse)], [permissionsQuery, jest.fn().mockResolvedValue(permissionsQueryResponse)], ]; - fakeApollo = createMockApollo(requestHandlers, {}); - fakeApollo.clients.defaultClient.cache.writeQuery({ + const mockApollo = createMockApollo(requestHandlers, {}); + mockApollo.clients.defaultClient.cache.writeQuery({ query: fetchLocalUserQuery, data: { fetchLocalUser: { @@ -864,18 +1148,111 @@ function createComponentWithApollo() { name: 'Test', }, }, - }) + }); + + return mockApollo; +} + +function createComponent(options = {}) { + const { mockApollo } = options; - wrapper = shallowMount(Index, { + return shallowMount(Index, { localVue, - apolloProvider: fakeApollo, + apolloProvider: mockApollo, + }); +} +``` + +Sometimes it is necessary to control what the local resolver returns and inspect how it is called by the component. This can be done by mocking your local resolver: + +```javascript +import fetchLocalUserQuery from '~/design_management/graphql/queries/fetch_local_user.query.graphql'; + +function createMockApolloProvider(options = {}) { + Vue.use(VueApollo); + const { fetchLocalUserSpy } = options; + + const mockApollo = createMockApollo([], { + Query: { + fetchLocalUser: fetchLocalUserSpy, + }, + }); + + // Necessary for local resolvers to be activated + mockApollo.clients.defaultClient.cache.writeQuery({ + query: fetchLocalUserQuery, + data: {}, }); + + return mockApollo; } ``` +In the test you can then control what the spy is supposed to do and inspect the component after the request have returned: + +```javascript +describe('My Index test with `createMockApollo`', () => { + let wrapper; + let fetchLocalUserSpy; + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + fetchLocalUserSpy = null; + }); + + describe('when loading', () => { + beforeEach(() => { + const mockApollo = createMockApolloProvider(); + wrapper = createComponent({ mockApollo }); + }); + + it('displays the loader', () => { + // Assess that the loader is present + }); + }); + + describe('with data', () => { + beforeEach(async () => { + fetchLocalUserSpy = jest.fn().mockResolvedValue(localUserQueryResponse); + const mockApollo = createMockApolloProvider(fetchLocalUserSpy); + wrapper = createComponent({ mockApollo }); + await waitForPromises(); + }); + + it('should fetch data once', () => { + expect(fetchLocalUserSpy).toHaveBeenCalledTimes(1); + }); + + it('displays data', () => { + // Assess that data is present + }); + }); + + describe('with error', () => { + const error = 'Error!'; + + beforeEach(async () => { + fetchLocalUserSpy = jest.fn().mockRejectedValueOnce(error); + const mockApollo = createMockApolloProvider(fetchLocalUserSpy); + wrapper = createComponent({ mockApollo }); + await waitForPromises(); + }); + + it('should fetch data once', () => { + expect(fetchLocalUserSpy).toHaveBeenCalledTimes(1); + }); + + it('displays the error', () => { + // Assess that the error is displayed + }); + }); +}); +``` + ## Handling errors -GitLab's GraphQL mutations currently have two distinct error modes: [Top-level](#top-level-errors) and [errors-as-data](#errors-as-data). +The GitLab GraphQL mutations currently have two distinct error modes: [Top-level](#top-level-errors) and [errors-as-data](#errors-as-data). When utilising a GraphQL mutation, we must consider handling **both of these error modes** to ensure that the user receives the appropriate feedback when an error occurs. diff --git a/doc/development/fe_guide/icons.md b/doc/development/fe_guide/icons.md index 994a3aa36fc..1468e886220 100644 --- a/doc/development/fe_guide/icons.md +++ b/doc/development/fe_guide/icons.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Icons and SVG Illustrations @@ -22,7 +22,7 @@ Our goal is to replace one by one all inline SVG Icons (as those currently bloat ### Usage in HAML/Rails -To use a sprite Icon in HAML or Rails we use a specific helper function : +To use a sprite Icon in HAML or Rails we use a specific helper function: ```ruby sprite_icon(icon_name, size: nil, css_class: '') @@ -31,7 +31,7 @@ sprite_icon(icon_name, size: nil, css_class: '') - **icon_name**: Use the icon_name for the SVG sprite in the list of ([GitLab SVGs](https://gitlab-org.gitlab.io/gitlab-svgs)). - **size (optional)**: Use one of the following sizes : 16, 24, 32, 48, 72 (this - will be translated into a `s16` class) + is translated into a `s16` class) - **css_class (optional)**: If you want to add additional CSS classes. **Example** @@ -90,7 +90,7 @@ Please use the following function inside JS to render an icon: ### Usage in HAML/Rails -DANGER: **Warning:** +WARNING: Do not use the `spinner` or `icon('spinner spin')` rails helpers to insert loading icons. These helpers rely on the Font Awesome icon library which is deprecated. diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 9f98876bc8e..84c1623f8c0 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -1,13 +1,13 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Frontend Development Guidelines This document describes various guidelines to ensure consistency and quality -across GitLab's frontend team. +across the GitLab frontend team. ## Overview @@ -24,7 +24,7 @@ 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. -Sign in to BrowserStack with the credentials saved in the **Engineering** vault of GitLab's +Sign in to BrowserStack with the credentials saved in the **Engineering** vault of the GitLab [shared 1Password account](https://about.gitlab.com/handbook/security/#1password-guide). ## Initiatives @@ -41,7 +41,7 @@ How we [plan and execute](development_process.md) the work on the frontend. ## Architecture -How we go about [making fundamental design decisions](architecture.md) in GitLab's frontend team +How we go about [making fundamental design decisions](architecture.md) in the GitLab frontend team or make changes to our frontend development guidelines. ## Testing @@ -56,7 +56,7 @@ Reusable components with technical and usage guidelines can be found in our ## Design Patterns -Common JavaScript [design patterns](design_patterns.md) in GitLab's codebase. +Common JavaScript [design patterns](design_patterns.md) in the GitLab codebase. ## Vue.js Best Practices diff --git a/doc/development/fe_guide/keyboard_shortcuts.md b/doc/development/fe_guide/keyboard_shortcuts.md index 227b5cfd22c..e50e9ec65df 100644 --- a/doc/development/fe_guide/keyboard_shortcuts.md +++ b/doc/development/fe_guide/keyboard_shortcuts.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Implementing keyboard shortcuts diff --git a/doc/development/fe_guide/performance.md b/doc/development/fe_guide/performance.md index de9a9f5cb14..7825c89b7cf 100644 --- a/doc/development/fe_guide/performance.md +++ b/doc/development/fe_guide/performance.md @@ -1,11 +1,205 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Performance +Performance is an essential part and one of the main areas of concern for any modern application. + +## User Timing API + +[User Timing API](https://developer.mozilla.org/en-US/docs/Web/API/User_Timing_API) is a web API +[available in all modern browsers](https://caniuse.com/?search=User%20timing). It allows measuring +custom times and durations in your applications by placing special marks in your +code. You can use the User Timing API in GitLab to measure any timing, regardless of the framework, +including Rails, Vue, or vanilla JavaScript environments. For consistency and +convenience of adoption, GitLab offers several ways to enable custom user timing metrics in +your code. + +User Timing API introduces two important paradigms: `mark` and `measure`. + +**Mark** is the timestamp on the performance timeline. For example, +`performance.mark('my-component-start');` makes a browser note the time this code +is met. Then, you can obtain information about this mark by querying the global +performance object again. For example, in your DevTools console: + +```javascript +performance.getEntriesByName('my-component-start') +``` + +**Measure** is the duration between either: + +- Two marks +- The start of navigation and a mark +- The start of navigation and the moment the measurement is taken + +It takes several arguments of which the measurement’s name is the only one required. Examples: + +- Duration between the start and end marks: + + ```javascript + performance.measure('My component', 'my-component-start', 'my-component-end') + ``` + +- Duration between a mark and the moment the measurement is taken. The end mark is omitted in + this case. + + ```javascript + performance.measure('My component', 'my-component-start') + ``` + +- Duration between [the navigation start](https://developer.mozilla.org/en-US/docs/Web/API/Performance/timeOrigin) + and the moment the actual measurement is taken. + + ```javascript + performance.measure('My component') + ``` + +- Duration between [the navigation start](https://developer.mozilla.org/en-US/docs/Web/API/Performance/timeOrigin) + and a mark. You cannot omit the start mark in this case but you can set it to `undefined`. + + ```javascript + performance.measure('My component', undefined, 'my-component-end') + ``` + +To query a particular `measure`, You can use the same API, as for `mark`: + +```javascript +performance.getEntriesByName('My component') +``` + +You can also query for all captured marks and measurements: + +```javascript +performance.getEntriesByType('mark'); +performance.getEntriesByType('measure'); +``` + +Using `getEntriesByName()` or `getEntriesByType()` returns an Array of [the PerformanceMeasure +objects](https://developer.mozilla.org/en-US/docs/Web/API/PerformanceMeasure) which contain +information about the measurement's start time and duration. + +### User Timing API utility + +You can use the `performanceMarkAndMeasure` utility anywhere in GitLab, as it's not tied to any +particular environment. + +`performanceMarkAndMeasure` takes an object as an argument, where: + +| Attribute | Type | Required | Description | +|:------------|:---------|:---------|:----------------------| +| `mark` | `String` | no | The name for the mark to set. Used for retrieving the mark later. If not specified, the mark is not set. | +| `measures` | `Array` | no | The list of the measurements to take at this point. | + +In return, the entries in the `measures` array are objects with the following API: + +| Attribute | Type | Required | Description | +|:------------|:---------|:---------|:----------------------| +| `name` | `String` | yes | The name for the measurement. Used for retrieving the mark later. Must be specified for every measure object, otherwise JavaScript fails. | +| `start` | `String` | no | The name of a mark **from** which the measurement should be taken. | +| `end` | `String` | no | The name of a mark **to** which the measurement should be taken. | + +Example: + +```javascript +import { performanceMarkAndMeasure } from '~/performance/utils'; +... +performanceMarkAndMeasure({ + mark: MR_DIFFS_MARK_DIFF_FILES_END, + measures: [ + { + name: MR_DIFFS_MEASURE_DIFF_FILES_DONE, + start: MR_DIFFS_MARK_DIFF_FILES_START, + end: MR_DIFFS_MARK_DIFF_FILES_END, + }, + ], +}); +``` + +### Vue performance plugin + +The plugin captures and measures the performance of the specified Vue components automatically +leveraging the Vue lifecycle and the User Timing API. + +To use the Vue performance plugin: + +1. Import the plugin: + + ```javascript + import PerformancePlugin from '~/performance/vue_performance_plugin'; + ``` + +1. Use it before initializing your Vue application: + + ```javascript + Vue.use(PerformancePlugin, { + components: [ + 'IdeTreeList', + 'FileTree', + 'RepoEditor', + ] + }); + ``` + +The plugin accepts the list of components, performance of which should be measured. The components +should be specified by their `name` option. + +You might need to explicitly set this option on the needed components, as +most components in the codebase don't have this option set: + +```javascript +export default { + name: 'IdeTreeList', + components: { + ... + ... +} +``` + +The plugin captures and stores the following: + +- The start **mark** for when the component has been initialized (in `beforeCreate()` hook) +- The end **mark** of the component when it has been rendered (next animation frame after `nextTick` + in `mounted()` hook). In most cases, this event does not wait for all sub-components to be + bootstrapped. To measure the sub-components, you should include those into the + plugin options. +- **Measure** duration between the two marks above. + +### Access stored measurements + +To access stored measurements, you can use either: + +- **Performance bar**. If you have it enabled (`P` + `B` key-combo), you can see the metrics + output in your DevTools console. +- **"Performance" tab** of the DevTools. You can get the measurements (not the marks, though) in + this tab when profiling performance. +- **DevTools console**. As mentioned above, you can query for the entries: + + ```javascript + performance.getEntriesByType('mark'); + performance.getEntriesByType('measure'); + ``` + +### Naming convention + +All the marks and measures should be instantiated with the constants from +`app/assets/javascripts/performance/constants.js`. When you’re ready to add a new mark’s or +measurement’s label, you can follow the pattern. + +NOTE: +This pattern is a recommendation and not a hard rule. + +```javascript +app-*-start // for a start ‘mark’ +app-*-end // for an end ‘mark’ +app-* // for ‘measure’ +``` + +For example, `'webide-init-editor-start`, `mr-diffs-mark-file-tree-end`, and so on. We do it to +help identify marks and measures coming from the different apps on the same page. + ## Best Practices ### Realtime Components @@ -18,29 +212,29 @@ When writing code for realtime features we have to keep a couple of things in mi 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](../../administration/polling.md). +1. The server tells you how much to poll by sending `Poll-Interval` in the header. + Use that as your polling interval. This enables system administrators to change the + [polling rate](../../administration/polling.md). A `Poll-Interval: -1` means you should disable polling, and this must be implemented. 1. A response with HTTP status different from 2XX should disable polling as well. 1. Use a common library for polling. 1. Poll on active tabs only. Please use [Visibility](https://github.com/ai/visibilityjs). -1. Use regular polling intervals, do not use backoff polling, or jitter, as the interval will be +1. Use regular polling intervals, do not use backoff polling, or jitter, as the interval is 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. +1. The backend code is likely to be using etags. You do not and should not check for status + `304 Not Modified`. The browser transforms it for you. ### Lazy Loading Images To improve the time to first render we are using lazy loading for images. This works by setting the actual image source on the `data-src` attribute. After the HTML is rendered and JavaScript is loaded, -the value of `data-src` will be moved to `src` automatically if the image is in the current viewport. +the value of `data-src` is moved to `src` automatically if the image is in the current viewport. - Prepare images in HTML for lazy loading by renaming the `src` attribute to `data-src` AND adding the class `lazy`. -- If you are using the Rails `image_tag` helper, all images will be lazy-loaded by default unless `lazy: false` is provided. +- If you are using the Rails `image_tag` helper, all images are lazy-loaded by default unless `lazy: false` is provided. If you are asynchronously adding content which contains lazy images then you need to call the function -`gl.lazyLoader.searchLazyImages()` which will search for lazy images and load them if needed. +`gl.lazyLoader.searchLazyImages()` which searches for lazy images and loads them if needed. But in general it should be handled automatically through a `MutationObserver` in the lazy loading function. ### Animations @@ -49,14 +243,14 @@ Only animate `opacity` & `transform` properties. Other properties (such as `top` Layout to be recalculated, which is much more expensive. For details on this, see "Styles that Affect Layout" in [High Performance Animations](https://www.html5rocks.com/en/tutorials/speed/high-performance-animations/). -If you _do_ need to change layout (e.g. a sidebar that pushes main content over), prefer [FLIP](https://aerotwist.com/blog/flip-your-animations/) to change expensive +If you _do_ need to change layout (for example, a sidebar that pushes main content over), prefer [FLIP](https://aerotwist.com/blog/flip-your-animations/) to change expensive properties once, and handle the actual animation with transforms. ## Reducing Asset Footprint ### Universal code -Code that is contained within `main.js` and `commons/index.js` are loaded and +Code that is contained in `main.js` and `commons/index.js` is loaded and run on _all_ pages. **DO NOT ADD** anything to these files unless it is truly needed _everywhere_. These bundles include ubiquitous libraries like `vue`, `axios`, and `jQuery`, as well as code for the main navigation and sidebar. @@ -66,26 +260,26 @@ code footprint. ### Page-specific JavaScript Webpack has been configured to automatically generate entry point bundles based -on the file structure within `app/assets/javascripts/pages/*`. The directories -within the `pages` directory correspond to Rails controllers and actions. These -auto-generated bundles will be automatically included on the corresponding +on the file structure in `app/assets/javascripts/pages/*`. The directories +in the `pages` directory correspond to Rails controllers and actions. These +auto-generated bundles are automatically included on the corresponding pages. For example, if you were to visit <https://gitlab.com/gitlab-org/gitlab/-/issues>, you would be accessing the `app/controllers/projects/issues_controller.rb` controller with the `index` action. If a corresponding file exists at -`pages/projects/issues/index/index.js`, it will be compiled into a webpack +`pages/projects/issues/index/index.js`, it is compiled into a webpack bundle and included on the page. Previously, GitLab encouraged the use of -`content_for :page_specific_javascripts` within HAML files, along with +`content_for :page_specific_javascripts` in HAML files, along with manually generated webpack bundles. However under this new system you should not ever need to manually add an entry point to the `webpack.config.js` file. -TIP: **Tip:** +NOTE: If you are unsure what controller and action corresponds to a given page, you -can find this out by inspecting `document.body.dataset.page` within your -browser's developer console while on any page within GitLab. +can find this out by inspecting `document.body.dataset.page` in your +browser's developer console while on any page in GitLab. #### Important Considerations @@ -97,7 +291,7 @@ browser's developer console while on any page within GitLab. instantiate, and nothing else. - **`DOMContentLoaded` should not be used:** - All of GitLab's JavaScript files are added with the `defer` attribute. + All GitLab JavaScript files are added with the `defer` attribute. According to the [Mozilla documentation](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/script#attr-defer), this implies that "the script is meant to be executed after the document has been parsed, but before firing `DOMContentLoaded`". Since the document is already @@ -119,21 +313,21 @@ browser's developer console while on any page within GitLab. ``` Note that `waitForCSSLoaded()` methods supports receiving the action in different ways: - + - With a callback: - + ```javascript waitForCSSLoaded(action) ``` - + - With `then()`: - + ```javascript waitForCSSLoaded().then(action); ``` - + - With `await` followed by `action`: - + ```javascript await waitForCSSLoaded; action(); @@ -150,21 +344,21 @@ browser's developer console while on any page within GitLab. - **Supporting Module Placement:** - If a class or a module is _specific to a particular route_, try to locate - it close to the entry point it will be used. For instance, if - `my_widget.js` is only imported within `pages/widget/show/index.js`, you + it close to the entry point in which it is used. For instance, if + `my_widget.js` is only imported in `pages/widget/show/index.js`, you should place the module at `pages/widget/show/my_widget.js` and import it - with a relative path (e.g. `import initMyWidget from './my_widget';`). - - If a class or module is _used by multiple routes_, place it within a + with a relative path (for example, `import initMyWidget from './my_widget';`). + - If a class or module is _used by multiple routes_, place it in a shared directory at the closest common parent directory for the entry - points that import it. For example, if `my_widget.js` is imported within + points that import it. For example, if `my_widget.js` is imported in both `pages/widget/show/index.js` and `pages/widget/run/index.js`, then place the module at `pages/widget/shared/my_widget.js` and import it with - a relative path if possible (e.g. `../shared/my_widget`). + a relative path if possible (for example, `../shared/my_widget`). - **Enterprise Edition Caveats:** - For GitLab Enterprise Edition, page-specific entry points will override their + For GitLab Enterprise Edition, page-specific entry points override their Community Edition counterparts with the same name, so if - `ee/app/assets/javascripts/pages/foo/bar/index.js` exists, it will take + `ee/app/assets/javascripts/pages/foo/bar/index.js` exists, it takes precedence over `app/assets/javascripts/pages/foo/bar/index.js`. If you want to minimize duplicate code, you can import one entry point from the other. This is not done automatically to allow for flexibility in overriding @@ -172,10 +366,10 @@ browser's developer console while on any page within GitLab. ### Code Splitting -For any code that does not need to be run immediately upon page load, (e.g. +For any code that does not need to be run immediately upon page load, (for example, modals, dropdowns, and other behaviors that can be lazy-loaded), you can split your module into asynchronous chunks with dynamic import statements. These -imports return a Promise which will be resolved once the script has loaded: +imports return a Promise which is resolved after the script has loaded: ```javascript import(/* webpackChunkName: 'emoji' */ '~/emoji') @@ -184,7 +378,7 @@ import(/* webpackChunkName: 'emoji' */ '~/emoji') ``` Please try to use `webpackChunkName` when generating these dynamic imports as -it will provide a deterministic filename for the chunk which can then be cached +it provides a deterministic filename for the chunk which can then be cached the browser across GitLab versions. More information is available in [webpack's code splitting documentation](https://webpack.js.org/guides/code-splitting/#dynamic-imports). @@ -198,7 +392,7 @@ 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. +- Prefer font formats with better compression, for example, WOFF2 is better than WOFF, which is better than TTF. - Compress and minify assets wherever possible (For CSS/JS, Sprockets and webpack do this for us). - If some functionality can reasonably be achieved without adding extra libraries, avoid them. - Use page-specific JavaScript as described above to load libraries that are only needed on certain pages. diff --git a/doc/development/fe_guide/principles.md b/doc/development/fe_guide/principles.md index 75954a5f988..4952d023d90 100644 --- a/doc/development/fe_guide/principles.md +++ b/doc/development/fe_guide/principles.md @@ -1,12 +1,12 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Principles -These principles will ensure that your frontend contribution starts off in the right direction. +These principles ensure that your frontend contribution starts off in the right direction. ## Discuss architecture before implementation @@ -14,7 +14,7 @@ Discuss your architecture design in an issue before writing code. This helps dec ## Be consistent -There are multiple ways of writing code to accomplish the same results. We should be as consistent as possible in how we write code across our codebases. This will make it easier for us to maintain our code across GitLab. +There are multiple ways of writing code to accomplish the same results. We should be as consistent as possible in how we write code across our codebases. This makes it easier for us to maintain our code across GitLab. ## Improve code [iteratively](https://about.gitlab.com/handbook/values/#iteration) diff --git a/doc/development/fe_guide/security.md b/doc/development/fe_guide/security.md index a82c315032f..627c5f4d12f 100644 --- a/doc/development/fe_guide/security.md +++ b/doc/development/fe_guide/security.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Security @@ -31,7 +31,7 @@ GitLab's CSP is used for the following: Some exceptions include: -- Scripts from Google Analytics and Piwik if either is enabled. +- Scripts from Google Analytics and Matomo 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. @@ -66,14 +66,14 @@ Some resources on implementing Subresource Integrity: ## 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 +Google Analytics and Matomo - 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](https://en.wikipedia.org/wiki/Cross-site_scripting), we will disable +In order to protect users from [XSS vulnerabilities](https://en.wikipedia.org/wiki/Cross-site_scripting), we intend to disable inline scripts in the future using Content Security Policy. While inline scripts can be useful, they're also a security concern. If diff --git a/doc/development/fe_guide/style/html.md b/doc/development/fe_guide/style/html.md index 61a724cf3c7..7fedbc6ce0d 100644 --- a/doc/development/fe_guide/style/html.md +++ b/doc/development/fe_guide/style/html.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # HTML style guide diff --git a/doc/development/fe_guide/style/index.md b/doc/development/fe_guide/style/index.md index 0d73b16b5d3..89a3d874184 100644 --- a/doc/development/fe_guide/style/index.md +++ b/doc/development/fe_guide/style/index.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # GitLab development style guides diff --git a/doc/development/fe_guide/style/javascript.md b/doc/development/fe_guide/style/javascript.md index b8e7429eb2c..8e3538e891d 100644 --- a/doc/development/fe_guide/style/javascript.md +++ b/doc/development/fe_guide/style/javascript.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments disqus_identifier: 'https://docs.gitlab.com/ee/development/fe_guide/style_guide_js.html' --- @@ -13,13 +13,13 @@ 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: **Tip:** +NOTE: 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, +when mutating data. This minimizes mutations in functions, which aligns with [Airbnb's style guide](https://github.com/airbnb/javascript#testing--for-real). ```javascript @@ -237,7 +237,7 @@ document.addEventListener("DOMContentLoaded", function(event) { ### 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 +Move them into separate functions instead. This makes tests easier to write and avoids violating the [Single Responsibility Principle](https://en.wikipedia.org/wiki/Single_responsibility_principle). ```javascript diff --git a/doc/development/fe_guide/style/scss.md b/doc/development/fe_guide/style/scss.md index c6ee1e64a80..a4cae12c4f3 100644 --- a/doc/development/fe_guide/style/scss.md +++ b/doc/development/fe_guide/style/scss.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments disqus_identifier: 'https://docs.gitlab.com/ee/development/fe_guide/style_guide_scss.html' --- @@ -12,7 +12,7 @@ easy to maintain, and performant for the end-user. ## Rules -Our CSS is a mixture of current and legacy approaches. That means sometimes it may be difficult to follow this guide to the letter; it means you will definitely run into exceptions, where following the guide is difficult to impossible without outsized effort. In those cases, you may work with your reviewers and maintainers to identify an approach that does not fit these rules. Please endeavor to limit these cases. +Our CSS is a mixture of current and legacy approaches. That means sometimes it may be difficult to follow this guide to the letter; it means you are likely to run into exceptions, where following the guide is difficult to impossible without outsized effort. In those cases, you may work with your reviewers and maintainers to identify an approach that does not fit these rules. Please endeavor to limit these cases. ### Utility Classes @@ -26,7 +26,7 @@ Classes in [`utilities.scss`](https://gitlab.com/gitlab-org/gitlab/blob/master/a Avoid [Bootstrap's Utility Classes](https://getbootstrap.com/docs/4.3/utilities/). -NOTE: **Note:** +NOTE: While migrating [Bootstrap's Utility Classes](https://getbootstrap.com/docs/4.3/utilities/) to the [GitLab UI](https://gitlab.com/gitlab-org/gitlab-ui/-/blob/master/doc/css.md#utilities) utility classes, note both the classes for margin and padding differ. The size scale used at @@ -79,10 +79,8 @@ CSS classes should use the `lowercase-hyphenated` format rather than ``` Class names should be used instead of tag name selectors. -Using tag name selectors are discouraged in CSS because -they can affect unintended elements in the hierarchy. -Also, since they are not meaningful names, they do not -add meaning to the code. +Using tag name selectors is discouraged because they can affect +unintended elements in the hierarchy. ```scss // Bad @@ -94,136 +92,23 @@ ul { .class-name { color: #fff; } -``` - -### Formatting - -You should always use a space before a brace, braces should be on the same -line, each property should each get its own line, and there should be a space -between the property and its value. - -```scss -// Bad -.container-item { - width: 100px; height: 100px; - margin-top: 0; -} - -// Bad -.container-item -{ - width: 100px; - height: 100px; - margin-top: 0; -} - -// Bad -.container-item{ - width:100px; - height:100px; - margin-top:0; -} - -// Good -.container-item { - width: 100px; - height: 100px; - margin-top: 0; -} -``` - -Note that there is an exception for single-line rulesets, although these are -not typically recommended. - -```scss -p { margin: 0; padding: 0; } -``` - -### Colors - -HEX (hexadecimal) colors should use shorthand where possible, and should use -lower case letters to differentiate between letters and numbers, e.g. `#E3E3E3` -vs. `#e3e3e3`. - -```scss -// Bad -p { - color: #ffffff; -} - -// Bad -p { - color: #FFFFFF; -} - -// Good -p { - color: #fff; -} -``` - -### Indentation - -Indentation should always use two spaces for each indentation level. - -```scss -// Bad, four spaces -p { - color: #f00; -} - -// Good -p { - color: #f00; -} -``` - -### Semicolons - -Always include semicolons after every property. When the stylesheets are -minified, the semicolons will be removed automatically. - -```scss -// Bad -.container-item { - width: 100px; - height: 100px -} - -// Good -.container-item { - width: 100px; - height: 100px; -} -``` -### Shorthand +// Best +// prefer an existing utility class over adding existing styles +```0 -The shorthand form should be used for properties that support it. +Class names are also preferable to IDs. Rules that use IDs +are not-reusable, as there can only be one affected element on +the page. ```scss // Bad -margin: 10px 15px 10px 15px; -padding: 10px 10px 10px 10px; - -// Good -margin: 10px 15px; -padding: 10px; -``` - -### Zero Units - -Omit length units on zero values, they're unnecessary and not including them -is slightly more performant. - -```scss -// Bad -.item-with-padding { - padding: 0px; +#my-element { + padding: 0; } // Good -.item-with-padding { +.my-element { padding: 0; } ``` @@ -234,27 +119,11 @@ Do not use any selector prefixed with `js-` for styling purposes. These selectors are intended for use only with JavaScript to allow for removal or renaming without breaking styling. -### IDs - -Don't use ID selectors in CSS. - -```scss -// Bad -#my-element { - padding: 0; -} - -// Good -.my-element { - padding: 0; -} -``` - ### Variables Before adding a new variable for a color or a size, guarantee: -- There isn't already one +- There isn't an existing one. - There isn't a similar one we can use instead. ## Linting @@ -263,8 +132,8 @@ We use [SCSS Lint](https://github.com/sds/scss-lint) to check for style guide co ruleset in `.scss-lint.yml`, which is located in the home directory of the project. -To check if any warnings will be produced by your changes, you can run `rake -scss_lint` in the GitLab directory. SCSS Lint will also run in GitLab CI/CD to +To check if any warnings are produced by your changes, run `rake +scss_lint` in the GitLab directory. SCSS Lint also runs in GitLab CI/CD to catch any warnings. If the Rake task is throwing warnings you don't understand, SCSS Lint's @@ -278,23 +147,4 @@ the SCSS style guide, you can use [CSSComb](https://github.com/csscomb/csscomb.j CSSComb globally (system-wide). Run it in the GitLab directory with `csscomb app/assets/stylesheets` to automatically fix issues with CSS/SCSS. -Note that this won't fix every problem, but it should fix a majority. - -### Ignoring issues - -If you want a line or set of lines to be ignored by the linter, you can use -`// scss-lint:disable RuleName` ([more information](https://github.com/sds/scss-lint#disabling-linters-via-source)): - -```scss -// This lint rule is disabled because it is supported only in Chrome/Safari -// scss-lint:disable PropertySpelling -body { - text-decoration-skip: ink; -} -// scss-lint:enable PropertySpelling -``` - -Make sure a comment is added on the line above the `disable` rule, otherwise the -linter will throw a warning. `DisableLinterReason` is enabled to make sure the -style guide isn't being ignored, and to communicate to others why the style -guide is ignored in this instance. +Note that this doesn't fix every problem, but it should fix a majority. diff --git a/doc/development/fe_guide/style/vue.md b/doc/development/fe_guide/style/vue.md index 6b6ca9a6c71..b85c1b1de35 100644 --- a/doc/development/fe_guide/style/vue.md +++ b/doc/development/fe_guide/style/vue.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Vue.js style guide @@ -98,7 +98,7 @@ Please check this [rules](https://github.com/vuejs/eslint-plugin-vue#bulb-rules) We discourage the use of the spread operator in this specific case in order to keep our codebase explicit, discoverable, and searchable. - This applies in any place where we'll benefit from the above, such as + This applies in any place where we would benefit from the above, such as when [initializing Vuex state](../vuex.md#why-not-just-spread-the-initial-state). The pattern above also enables us to easily parse non scalar values during instantiation. @@ -667,7 +667,7 @@ The goal of this accord is to make sure we are all on the same page. 1. If you need to grab data from the DOM, you may query the DOM 1 time while bootstrapping your application to grab data attributes using `dataset`. You can do this without jQuery. 1. You may use a jQuery dependency in Vue.js following [this example from the docs](https://vuejs.org/v2/examples/select2.html). 1. If an outside jQuery Event needs to be listen to inside the Vue application, you may use jQuery event listeners. - 1. We will avoid adding new jQuery events when they are not required. Instead of adding new jQuery events take a look at [different methods to do the same task](https://vuejs.org/v2/api/#vm-emit). + 1. We avoid adding new jQuery events when they are not required. Instead of adding new jQuery events take a look at [different methods to do the same task](https://vuejs.org/v2/api/#vm-emit). 1. You may query the `window` object one time, while bootstrapping your application for application specific data (e.g. `scrollTo` is ok to access anytime). Do this access during the bootstrapping of your application. 1. You may have a temporary but immediate need to create technical debt by writing code that does not follow our standards, to be refactored later. Maintainers need to be ok with the tech debt in the first place. An issue should be created for that tech debt to evaluate it further and discuss. In the coming months you should fix that tech debt, with its priority to be determined by maintainers. 1. When creating tech debt you must write the tests for that code before hand and those tests may not be rewritten. e.g. jQuery tests rewritten to Vue tests. diff --git a/doc/development/fe_guide/style_guide_js.md b/doc/development/fe_guide/style_guide_js.md index f3fa80325ef..73a5bea189d 100644 --- a/doc/development/fe_guide/style_guide_js.md +++ b/doc/development/fe_guide/style_guide_js.md @@ -3,3 +3,6 @@ redirect_to: 'style/javascript.md' --- This document was moved to [another location](style/javascript.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/fe_guide/style_guide_scss.md b/doc/development/fe_guide/style_guide_scss.md index 2b4e6427a18..eaa6d33fb43 100644 --- a/doc/development/fe_guide/style_guide_scss.md +++ b/doc/development/fe_guide/style_guide_scss.md @@ -3,3 +3,6 @@ redirect_to: 'style/scss.md' --- This document was moved to [another location](style/scss.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/fe_guide/testing.md b/doc/development/fe_guide/testing.md index b23e37d1eef..457d15235ae 100644 --- a/doc/development/fe_guide/testing.md +++ b/doc/development/fe_guide/testing.md @@ -3,3 +3,6 @@ redirect_to: '../testing_guide/frontend_testing.md' --- This document was moved to [another location](../testing_guide/frontend_testing.md). + +<!-- This redirect file can be deleted after February 1, 2021. --> +<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page --> diff --git a/doc/development/fe_guide/tooling.md b/doc/development/fe_guide/tooling.md index 809e05ea61f..d33022b9355 100644 --- a/doc/development/fe_guide/tooling.md +++ b/doc/development/fe_guide/tooling.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Tooling @@ -14,21 +14,21 @@ We use ESLint to encapsulate and enforce frontend code standards. Our configurat 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: +To check all 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. +A list of problems found are logged to the console. -To apply automatic ESLint fixes to all currently staged files (based on `git diff`), run the following script: +To apply automatic ESLint fixes to all 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. +If manual changes are required, a list of changes are sent to the console. To check **all** files in the repository with ESLint, run the following script: @@ -36,7 +36,7 @@ To check **all** files in the repository with ESLint, run the following script: yarn eslint ``` -A list of problems found will be logged to the console. +A list of problems found are logged to the console. To apply automatic ESLint fixes to **all** files in the repository, run the following script: @@ -44,9 +44,9 @@ To apply automatic ESLint fixes to **all** files in the repository, run the foll yarn eslint-fix ``` -If manual changes are required, a list of changes will be sent to the console. +If manual changes are required, a list of changes are sent to the console. -CAUTION: **Caution:** +WARNING: Limit use to global rule updates. Otherwise, the changes can lead to huge Merge Requests. ### Disabling ESLint in new files @@ -156,13 +156,13 @@ The source of these Yarn scripts can be found in `/scripts/frontend/prettier.js` node ./scripts/frontend/prettier.js check-all ./vendor/ ``` -This will go over all files in a specific folder check it. +This iterates over all files in a specific folder, and checks them. ```shell node ./scripts/frontend/prettier.js save-all ./vendor/ ``` -This will go over all files in a specific folder and save it. +This iterates over all files in a specific folder and saves them. ### VSCode Settings diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index 77bdadfe8da..41fbd128631 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Vue @@ -62,11 +62,11 @@ Be sure to read about [page-specific JavaScript](performance.md#page-specific-ja 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. -You should only do this while initializing the application, because the mounted element will be replaced with Vue-generated DOM. +You should only do this while initializing the application, because the mounted element is replaced with a 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 avoiding the need to create a fixture or an HTML element in the unit test, -which will make the tests easier. +which makes the tests easier. See the following example, also, please refer to our [Vue style guide](style/vue.md#basic-rules) for additional information on why we explicitly declare the data being passed into the Vue app; @@ -91,15 +91,15 @@ return new Vue({ }, }); }, -} +}); ``` > When adding an `id` attribute to mount a Vue application, please make sure this `id` is unique across the codebase #### Accessing the `gl` object -When we need to query the `gl` object for data that won't change during the application's life cycle, we should do it in the same place where we query the DOM. -By following this practice, we can avoid the need to mock the `gl` object, which will make tests easier. +When we need to query the `gl` object for data that doesn't change during the application's life cycle, we should do it in the same place where we query the DOM. +By following this practice, we can avoid the need to mock the `gl` object, which makes tests easier. It should be done while initializing our Vue instance, and the data should be provided as `props` to the main component: ```javascript @@ -162,12 +162,12 @@ This approach has a few benefits: ### 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 +This folder holds all components that are specific to this new feature. +If you need to use or create a component that is likely to be used somewhere else, please refer to `vue_shared/components`. A good rule of thumb to know when you should create a component is to think if -it will be reusable elsewhere. +it could 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 @@ -192,7 +192,7 @@ Check this [page](vuex.md) for more details. In the [Vue documentation](https://vuejs.org/v2/api/#Options-Data) the Data function/object is defined as follows: -> The data object for the Vue instance. Vue will recursively convert its properties into getter/setters to make it “reactive”. The object must be plain: native objects such as browser API objects and prototype properties are ignored. A rule of thumb is that data should just be data - it is not recommended to observe objects with their own stateful behavior. +> The data object for the Vue instance. Vue recursively converts its properties into getter/setters to make it “reactive”. The object must be plain: native objects such as browser API objects and prototype properties are ignored. A rule of thumb is that data should just be data - it is not recommended to observe objects with their own stateful behavior. Based on the Vue guidance: diff --git a/doc/development/fe_guide/vue3_migration.md b/doc/development/fe_guide/vue3_migration.md index 46437f39dbe..9d2f3b27968 100644 --- a/doc/development/fe_guide/vue3_migration.md +++ b/doc/development/fe_guide/vue3_migration.md @@ -1,7 +1,7 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Migration to Vue 3 @@ -82,7 +82,7 @@ const FunctionalComp = (props, slots) => { } ``` -It is not recommended to replace stateful components with functional components unless you absolutely need a performance improvement right now. In Vue 3, performance gains for functional components will be negligible. +It is not recommended to replace stateful components with functional components unless you absolutely need a performance improvement right now. In Vue 3, performance gains for functional components are negligible. ## Old slots syntax with `slot` attribute diff --git a/doc/development/fe_guide/vuex.md b/doc/development/fe_guide/vuex.md index 7765ba04d40..1d83335291a 100644 --- a/doc/development/fe_guide/vuex.md +++ b/doc/development/fe_guide/vuex.md @@ -1,19 +1,19 @@ --- stage: none group: unassigned -info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#designated-technical-writers +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Vuex -When there's a clear benefit to separating state management from components (e.g. due to state complexity) we recommend using [Vuex](https://vuex.vuejs.org) over any other Flux pattern. Otherwise, feel free to manage state within the components. +When there's a clear benefit to separating state management from components (for example, due to state complexity) we recommend using [Vuex](https://vuex.vuejs.org) over any other Flux pattern. Otherwise, feel free to manage state in the components. Vuex should be strongly considered when: -- You expect multiple parts of the application to react to state changes -- There's a need to share data between multiple components -- There are complex interactions with Backend, e.g. multiple API calls -- The app involves interacting with backend via both traditional REST API and GraphQL (especially when moving the REST API over to GraphQL is a pending backend task) +- You expect multiple parts of the application to react to state changes. +- There's a need to share data between multiple components. +- There are complex interactions with Backend, for example, multiple API calls. +- The app involves interacting with backend via both traditional REST API and GraphQL (especially when moving the REST API over to GraphQL is a pending backend task). The information included in this page is explained in more detail in the official [Vuex documentation](https://vuex.vuejs.org). @@ -22,7 +22,7 @@ official [Vuex documentation](https://vuex.vuejs.org). Vuex is composed of State, Getters, Mutations, Actions, and Modules. -When a user clicks on an action, we need to `dispatch` it. This action will `commit` a mutation that will change the state. The action itself will not update the state; only a mutation should update the state. +When a user clicks on an action, we need to `dispatch` it. This action `commits` a mutation that changes the state. The action itself does not update the state; only a mutation should update the state. ## File structure @@ -92,7 +92,7 @@ An action is a payload of information to send data from our application to our s An action is usually composed by a `type` and a `payload` and they describe what happened. Unlike [mutations](#mutationsjs), actions can contain asynchronous operations - that's why we always need to handle asynchronous logic in actions. -In this file, we will write the actions that will call mutations for handling a list of users: +In this file, we write the actions that call mutations for handling a list of users: ```javascript import * as types from './mutation_types'; @@ -163,15 +163,15 @@ Instead of creating an mutation to toggle the loading state, we should: - `PUT`: `updateSomething` - `DELETE`: `deleteSomething` -As a result, we can dispatch the `fetchNamespace` action from the component and it will be responsible to commit `REQUEST_NAMESPACE`, `RECEIVE_NAMESPACE_SUCCESS` and `RECEIVE_NAMESPACE_ERROR` mutations. +As a result, we can dispatch the `fetchNamespace` action from the component and it is responsible to commit `REQUEST_NAMESPACE`, `RECEIVE_NAMESPACE_SUCCESS` and `RECEIVE_NAMESPACE_ERROR` mutations. -> Previously, we were dispatching actions from the `fetchNamespace` action instead of committing mutation, so please don't be confused if you find a different pattern in the older parts of the codebase. However, we encourage leveraging a new pattern whenever you write new Vuex stores +> Previously, we were dispatching actions from the `fetchNamespace` action instead of committing mutation, so please don't be confused if you find a different pattern in the older parts of the codebase. However, we encourage leveraging a new pattern whenever you write new Vuex stores. By following this pattern we guarantee: -1. All applications follow the same pattern, making it easier for anyone to maintain the code -1. All data in the application follows the same lifecycle pattern -1. Unit tests are easier +1. All applications follow the same pattern, making it easier for anyone to maintain the code. +1. All data in the application follows the same lifecycle pattern. +1. Unit tests are easier. #### Updating complex state @@ -215,7 +215,7 @@ 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` +- 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: @@ -245,7 +245,7 @@ A mutation written like this is easier to maintain. In addition, we avoid errors ### `getters.js` Sometimes we may need to get derived state based on store state, like filtering for a specific prop. -Using a getter will also cache the result based on dependencies due to [how computed props work](https://vuejs.org/v2/guide/computed.html#Computed-Caching-vs-Methods) +Using a getter also caches the result based on dependencies due to [how computed props work](https://vuejs.org/v2/guide/computed.html#Computed-Caching-vs-Methods) This can be done through the `getters`: ```javascript @@ -272,7 +272,10 @@ import { mapGetters } from 'vuex'; ### `mutation_types.js` From [vuex mutations docs](https://vuex.vuejs.org/guide/mutations.html): -> It is a commonly seen pattern to use constants for mutation types in various Flux implementations. This allows the code to take advantage of tooling like linters, and putting all constants in a single file allows your collaborators to get an at-a-glance view of what mutations are possible in the entire application. +> It is a commonly seen pattern to use constants for mutation types in various Flux implementations. +> This allows the code to take advantage of tooling like linters, and putting all constants in a +> single file allows your collaborators to get an at-a-glance view of what mutations are possible +> in the entire application. ```javascript export const ADD_USER = 'ADD_USER'; @@ -346,7 +349,7 @@ export default ({ #### Why not just ...spread the initial state? -The astute reader will see an opportunity to cut out a few lines of code from +The astute reader sees an opportunity to cut out a few lines of code from the example above: ```javascript @@ -359,17 +362,17 @@ export default initialState => ({ }); ``` -We've made the conscious decision to avoid this pattern to aid in the -discoverability and searchability of our frontend codebase. The same applies +We made the conscious decision to avoid this pattern to improve the ability to +discover and search our frontend codebase. The same applies when [providing data to a Vue app](vue.md#providing-data-from-haml-to-javascript). The reasoning for this is described in [this discussion](https://gitlab.com/gitlab-org/frontend/rfcs/-/issues/56#note_302514865): > Consider a `someStateKey` is being used in the store state. You _may_ not be > able to grep for it directly if it was provided only by `el.dataset`. Instead, -> you'd have to grep for `some_state_key`, since it could have come from a rails +> you'd have to grep for `some_state_key`, because it could have come from a Rails > template. The reverse is also true: if you're looking at a rails template, you > might wonder what uses `some_state_key`, but you'd _have_ to grep for -> `someStateKey` +> `someStateKey`. ### Communicating with the Store @@ -426,12 +429,12 @@ export default { #### Testing Vuex concerns -Refer to [vuex docs](https://vuex.vuejs.org/guide/testing.html) regarding testing Actions, Getters and Mutations. +Refer to [Vuex docs](https://vuex.vuejs.org/guide/testing.html) regarding testing Actions, Getters and Mutations. #### Testing components that need a store -Smaller components might use `store` properties to access the data. -In order to write unit tests for those components, we need to include the store and provide the correct state: +Smaller components might use `store` properties to access the data. To write unit tests for those +components, we need to include the store and provide the correct state: ```javascript //component_spec.js @@ -482,8 +485,9 @@ describe('component', () => { ### Two way data binding -When storing form data in Vuex, it is sometimes necessary to update the value stored. The store should never be mutated directly, and an action should be used instead. -In order to still use `v-model` in our code, we need to create computed properties in this form: +When storing form data in Vuex, it is sometimes necessary to update the value stored. The store +should never be mutated directly, and an action should be used instead. +To use `v-model` in our code, we need to create computed properties in this form: ```javascript export default { @@ -521,7 +525,7 @@ export default { }; ``` -Adding a few of these properties becomes cumbersome, and makes the code more repetitive with more tests to write. To simplify this there is a helper in `~/vuex_shared/bindings.js` +Adding a few of these properties becomes cumbersome, and makes the code more repetitive with more tests to write. To simplify this there is a helper in `~/vuex_shared/bindings.js`. The helper can be used like so: @@ -568,4 +572,4 @@ export default { } ``` -`mapComputed` will then generate the appropriate computed properties that get the data from the store and dispatch the correct action when updated. +`mapComputed` then generates the appropriate computed properties that get the data from the store and dispatch the correct action when updated. |