diff options
Diffstat (limited to 'doc/development/fe_guide')
20 files changed, 591 insertions, 690 deletions
diff --git a/doc/development/fe_guide/accessibility.md b/doc/development/fe_guide/accessibility.md index af45603782f..b00131b12f3 100644 --- a/doc/development/fe_guide/accessibility.md +++ b/doc/development/fe_guide/accessibility.md @@ -345,7 +345,7 @@ Keep in mind that: - When you add `:hover` styles, in most cases you should add `:focus` styles too so that the styling is applied for both mouse **and** keyboard users. - If you remove an interactive element's `outline`, make sure you maintain visual focus state in another way such as with `box-shadow`. -See the [Pajamas Keyboard-only page](https://design.gitlab.com/accessibility-audits/keyboard-only/) for more detail. +See the [Pajamas Keyboard-only page](https://design.gitlab.com/accessibility/keyboard-only) for more detail. ## `tabindex` @@ -516,6 +516,55 @@ GitLab-specific examples are assignee and label dropdowns. Building such widgets require ARIA to make them understandable to screen readers. Proper research and testing should be done to ensure compliance with [WCAG](https://www.w3.org/WAI/standards-guidelines/wcag/). +## Automated accessibility testing with axe + +You can use [axe-core](https://github.com/dequelabs/axe-core) [gems](https://github.com/dequelabs/axe-core-gems) +to run automated accessibility tests in feature tests. + +Axe provides the custom matcher `be_axe_clean`, which can be used like the following: + +```ruby +# spec/features/settings_spec.rb +it 'passes axe automated accessibility testing', :js do + visit_settings_page + + wait_for_requests # ensures page is fully loaded + + expect(page).to be_axe_clean +end +``` + +If needed, you can scope testing to a specific area of the page by using `within`. + +Axe also provides specific [clauses](https://github.com/dequelabs/axe-core-gems/blob/develop/packages/axe-core-rspec/README.md#clauses), +for example: + +```ruby +expect(page).to be_axe_clean.within '[data-testid="element"]' + +# run only WCAG 2.0 Level AA rules +expect(page).to be_axe_clean.according_to :wcag2aa + +# specifies which rule to skip +expect(page).to be_axe_clean.skipping :'link-in-text-block' + +# clauses can be chained +expect(page).to be_axe_clean.within('[data-testid="element"]') + .according_to(:wcag2aa) +``` + +Axe does not test hidden regions, such as inactive menus or modal windows. To test +hidden regions for accessibility, write tests that activate or render the regions visible +and run the matcher again. + +### Known accessibility violations + +This section documents violations where a recommendation differs with the [design system](https://design.gitlab.com/): + +- `link-in-text-block`: For now, use the `skipping` clause to skip `:'link-in-text-block'` + rule to fix the violation. After this is fixed as part of [issue 1444](https://gitlab.com/gitlab-org/gitlab-services/design.gitlab.com/-/issues/1444) + and underline is added to the `GlLink` component, this clause can be removed. + ## Resources ### Viewing the browser accessibility tree diff --git a/doc/development/fe_guide/content_editor.md b/doc/development/fe_guide/content_editor.md index 6d13f419430..25140a067ca 100644 --- a/doc/development/fe_guide/content_editor.md +++ b/doc/development/fe_guide/content_editor.md @@ -1,6 +1,6 @@ --- -stage: Create -group: Editor +stage: Plan +group: Knowledge info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- @@ -64,7 +64,7 @@ Instead, you should obtain an instance of the `ContentEditor` class by listening ```html <script> -import { createAlert } from '~/flash'; +import { createAlert } from '~/alert'; import { __ } from '~/locale'; export default { diff --git a/doc/development/fe_guide/customizable_dashboards.md b/doc/development/fe_guide/customizable_dashboards.md index 26c73b26126..9e45c660745 100644 --- a/doc/development/fe_guide/customizable_dashboards.md +++ b/doc/development/fe_guide/customizable_dashboards.md @@ -6,7 +6,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Customizable dashboards -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/98610) in GitLab 15.5 as an [Alpha feature](../../policy/alpha-beta-support.md#alpha-features). +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/98610) in GitLab 15.5 as an [Experiment](../../policy/alpha-beta-support.md#experiment). Customizable dashboards provide a dashboard structure that allows users to create their own dashboards and commit the structure to a repository. diff --git a/doc/development/fe_guide/emojis.md b/doc/development/fe_guide/emojis.md index 3296783a616..c93e1bb34c5 100644 --- a/doc/development/fe_guide/emojis.md +++ b/doc/development/fe_guide/emojis.md @@ -30,7 +30,7 @@ when your platform does not support it. - Negative intent should be set to `1.5`. 1. Ensure you see new individual images copied into `app/assets/images/emoji/` 1. Ensure you can see the new emojis and their aliases in the GitLab Flavored Markdown (GLFM) Autocomplete - 1. Ensure you can see the new emojis and their aliases in the award emoji menu + 1. Ensure you can see the new emojis and their aliases in the emoji reactions menu 1. You might need to add new emoji Unicode support checks and rules for platforms that do not support a certain emoji and we need to fallback to an image. See `app/assets/javascripts/emoji/support/is_emoji_unicode_supported.js` diff --git a/doc/development/fe_guide/frontend_faq.md b/doc/development/fe_guide/frontend_faq.md index 3b349d880c0..995730796b4 100644 --- a/doc/development/fe_guide/frontend_faq.md +++ b/doc/development/fe_guide/frontend_faq.md @@ -69,7 +69,7 @@ banner on top of the component examples indicates that: > component. For example, at the time of writing, this type of warning can be observed for -[all form components](https://design.gitlab.com/components/form/). It, however, +all form components, such as the [checkbox](https://design.gitlab.com/components/checkbox). It, however, doesn't imply that the component should not be used. GitLab always asks to use `<gl-*>` components whenever a suitable component exists. @@ -192,7 +192,7 @@ To see what polyfills are being used: 1. Select the [`compile-production-assets`](https://gitlab.com/gitlab-org/gitlab/-/jobs/641770154) job. 1. In the right-hand sidebar, scroll to **Job Artifacts**, and select **Browse**. 1. Select the **webpack-report** folder to open it, and select **index.html**. -1. In the upper left corner of the page, select the right arrow **{chevron-lg-right}** +1. In the upper-left corner of the page, select the right arrow (**{chevron-lg-right}**) to display the explorer. 1. In the **Search modules** field, enter `gitlab/node_modules/core-js` to see which polyfills are being loaded and where: diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index 8ae0458e47c..da3a6eff79d 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -63,17 +63,17 @@ the GraphQL extension, follow these steps: 1. Add an `apollo.config.js` file to the root of your `gitlab` local directory. 1. Populate the file with the following content: - ```javascript - module.exports = { - client: { - includes: ['./app/assets/javascripts/**/*.graphql', './ee/app/assets/javascripts/**/*.graphql'], - service: { - name: 'GitLab', - localSchemaFile: './tmp/tests/graphql/gitlab_schema.graphql', - }, - }, - }; - ``` + ```javascript + module.exports = { + client: { + includes: ['./app/assets/javascripts/**/*.graphql', './ee/app/assets/javascripts/**/*.graphql'], + service: { + name: 'GitLab', + localSchemaFile: './tmp/tests/graphql/gitlab_schema.graphql', + }, + }, + }; + ``` 1. Restart VS Code. @@ -84,10 +84,8 @@ Our GraphQL API can be explored via GraphiQL at your instance's [GitLab GraphQL API Reference documentation](../../api/graphql/reference) where needed. -You can check all existing queries and mutations on the right side -of GraphiQL in its **Documentation explorer**. You can also -write queries and mutations directly on the left tab and check -their execution by selecting **Execute query** in the upper left: +To check all existing queries and mutations, on the right side of GraphiQL, select **Documentation explorer**. +To check the execution of the queries and mutations you've written, in the upper-left corner, select **Execute query**.  @@ -107,7 +105,9 @@ Default client accepts two parameters: `resolvers` and `config`. ### Multiple client queries for the same object -If you are making multiple queries to the same Apollo client object you might encounter the following error: `Cache data may be lost when replacing the someProperty field of a Query object. To address this problem, either ensure all objects of SomeEntityhave an id or a custom merge function`. We are already checking `ID` presence for every GraphQL type that has an `ID`, so this shouldn't be the case. Most likely, the `SomeEntity` type doesn't have an `ID` property, and to fix this warning we need to define a custom merge function. +If you are making multiple queries to the same Apollo client object you might encounter the following error: `Cache data may be lost when replacing the someProperty field of a Query object. To address this problem, either ensure all objects of SomeEntityhave an id or a custom merge function`. We are already checking `id` presence for every GraphQL type that has an `id`, so this shouldn't be the case (unless you see this warning when running unit tests; in this case please ensure your mocked responses contain an `id` whenever it's requested). + +When `SomeEntity` type doesn't have an `id` property in the GraphQL schema, to fix this warning we need to define a custom merge function. We have some client-wide types with `merge: true` defined in the default client as [`typePolicies`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/assets/javascripts/lib/graphql.js) (this means that Apollo will merge existing and incoming responses in the case of subsequent queries). Consider adding `SomeEntity` there or defining a custom merge function for it. @@ -264,9 +264,7 @@ Read more about [Vue Apollo](https://github.com/vuejs/vue-apollo) in the [Vue Ap ### Local state with Apollo -It is possible to manage an application state with Apollo by using [client-site resolvers](#using-client-side-resolvers) -or [type policies with reactive variables](#using-type-policies-with-reactive-variables) when creating your default -client. +It is possible to manage an application state with Apollo when creating your default client. #### Using client-side resolvers @@ -326,6 +324,32 @@ export default { } ``` +Instead of using `writeQuery`, we can create a type policy that will return `user` on every attempt of reading the `userQuery` from the cache: + +```javascript +const defaultClient = createDefaultClient({}, { + cacheConfig: { + typePolicies: { + Query: { + fields: { + user: { + read(data) { + return data || { + user: { + name: 'John', + surname: 'Doe', + age: 30 + }, + } + } + } + } + } + } + } +}); +``` + Along with creating local data, we can also extend existing GraphQL types with `@client` fields. This is extremely helpful when we need to mock an API response for fields not yet added to our GraphQL API. ##### Mocking API response with local Apollo cache @@ -390,122 +414,9 @@ For each attempt to fetch a version, our client fetches `id` and `sha` from the Read more about local state management with Apollo in the [Vue Apollo documentation](https://vue-apollo.netlify.app/guide/local-state.html#local-state). -#### Using type policies with reactive variables - -Apollo Client 3 offers an alternative to [client-side resolvers](#using-client-side-resolvers) by using -[reactive variables to store client state](https://www.apollographql.com/docs/react/local-state/reactive-variables/). - -**NOTE:** -We are still learning the best practices for both **type policies** and **reactive vars**. -Take a moment to improve this guide or [leave a comment](https://gitlab.com/gitlab-org/frontend/rfcs/-/issues/100) -if you use it! - -In the example below we define a `@client` query and its `typedefs`: - -```javascript -// ./graphql/typedefs.graphql -extend type Query { - localData: String! -} -``` - -```javascript -// ./graphql/get_local_data.query.graphql -query getLocalData { - localData @client -} -``` - -Similar to resolvers, your `typePolicies` execute when the `@client` query is used. However, -using `makeVar` triggers every relevant active Apollo query to reactively update when the state -mutates. - -```javascript -// ./graphql/local_state.js - -import { makeVar } from '@apollo/client/core'; -import typeDefs from './typedefs.graphql'; - -export const createLocalState = () => { - // set an initial value - const localDataVar = makeVar(''); - - const cacheConfig = { - typePolicies: { - Query: { - fields: { - localData() { - // obtain current value - // triggers when `localDataVar` is updated - return localDataVar(); - }, - }, - }, - }, - }; - - // methods that update local state - const localMutations = { - setLocalData(newData) { - localDataVar(newData); - }, - clearData() { - localDataVar(''); - }, - }; - - return { - cacheConfig, - typeDefs, - localMutations, - }; -}; -``` - -Pass the cache configuration to your Apollo Client: - -```javascript -// index.js - -// ... -import createDefaultClient from '~/lib/graphql'; -import { createLocalState } from './graphql/local_state'; - -const { cacheConfig, typeDefs, localMutations } = createLocalState(); - -const apolloProvider = new VueApollo({ - defaultClient: createDefaultClient({}, { cacheConfig, typeDefs }), -}); - -return new Vue({ - el, - apolloProvider, - provide: { - // inject local state mutations to your app - localMutations, - }, - render(h) { - return h(MyApp); - }, -}); -``` - -Wherever used, the local query updates as the state updates thanks to the **reactive variable**. - ### Using with Vuex -When the Apollo Client is used in Vuex and fetched data is stored in the Vuex store, the Apollo Client cache does not need to be enabled. Otherwise we would have data from the API stored in two places - Vuex store and Apollo Client cache. With Apollo's default settings, a subsequent fetch from the GraphQL API could result in fetching data from Apollo cache (in the case where we have the same query and variables). To prevent this behavior, we need to disable Apollo Client cache by passing a valid `fetchPolicy` option to its constructor: - -```javascript -import fetchPolicies from '~/graphql_shared/fetch_policy_constants'; - -export const gqClient = createGqClient( - {}, - { - fetchPolicy: fetchPolicies.NO_CACHE, - }, -); -``` +We do not recommend creating new applications with Vuex and Apollo Client combined ### Working on GraphQL-based features when frontend and backend are not in sync @@ -1218,53 +1129,7 @@ We use [subscriptions](https://www.apollographql.com/docs/react/data/subscriptio **NOTE:** We cannot test subscriptions using GraphiQL, because they require an ActionCable client, which GraphiQL does not support at the moment. -Subscriptions don't require any additional configuration of Apollo Client instance, you can use them in the application right away. To distinguish subscriptions from queries and mutations, we recommend naming them with `.subscription.graphql` extension: - -```graphql -// ~/sidebar/queries/issuable_assignees.subscription.graphql - -subscription issuableAssigneesUpdated($issuableId: IssuableID!) { - issuableAssigneesUpdated(issuableId: $issuableId) { - ... on Issue { - assignees { - nodes { - ...User - status { - availability - } - } - } - } - } -} -``` - -When using GraphQL subscriptions in Vue application, we recommend updating existing Apollo query results with [subscribeToMore](https://apollo.vuejs.org/guide/apollo/subscriptions.html#subscribe-to-more) option: - -```javascript -import issuableAssigneesSubscription from '~/sidebar/queries/issuable_assignees.subscription.graphql' - -apollo: { - issuable: { - query() { - return assigneesQueries[this.issuableType].query; - }, - subscribeToMore: { - // Specify the subscription that will update the query - document() { - return issuableAssigneesSubscription; - }, - variables() { - return { - issuableId: convertToGraphQLId(this.issuableClass, this.issuableId), - }; - }, - }, - }, -}, -``` - -We would need also to define a field policy similarly like we do it for the [paginated queries](#defining-field-merge-policy) +Refer to the [Real-time widgets developer guide](../real_time.md) for a comprehensive introduction to subscriptions. ### Best Practices @@ -1314,73 +1179,6 @@ If you use the RubyMine IDE, and have marked the `tmp` directory as `gitlab/tmp/tests/graphql`. This will allow the **JS GraphQL** plugin to automatically find and index the schema. -#### Testing Apollo components - -If we use `ApolloQuery` or `ApolloMutation` in our components, to test their functionality we need to add a stub first: - -```javascript -import { ApolloMutation } from 'vue-apollo'; - -function createComponent(props = {}) { - wrapper = shallowMount(MyComponent, { - sync: false, - propsData: { - ...props, - }, - stubs: { - ApolloMutation, - }, - }); -} -``` - -`ApolloMutation` component exposes `mutate` method via scoped slot. If we want to test this method, we need to add it to mocks: - -```javascript -const mutate = jest.fn().mockResolvedValue(); -const $apollo = { - mutate, -}; - -function createComponent(props = {}) { - wrapper = shallowMount(MyComponent, { - sync: false, - propsData: { - ...props, - }, - stubs: { - ApolloMutation, - }, - mocks: { - $apollo, - } - }); -} -``` - -Then we can check if `mutate` is called with correct variables: - -```javascript -const mutationVariables = { - mutation: createNoteMutation, - update: expect.anything(), - variables: { - input: { - noteableId: 'noteable-id', - body: 'test', - discussionId: '0', - }, - }, -}; - -it('calls mutation on submitting form ', () => { - createComponent() - findReplyForm().vm.$emit('submitForm'); - - expect(mutate).toHaveBeenCalledWith(mutationVariables); -}); -``` - #### Mocking Apollo Client To test the components with Apollo operations, we need 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. @@ -1393,38 +1191,40 @@ import Vue from 'vue'; Vue.use(VueApollo); -function createMockApolloProvider() { - return createMockApollo(requestHandlers); -} +describe('Some component with Apollo mock', () => { + let wrapper; -function createComponent(options = {}) { - const { mockApollo } = options; - ... - return shallowMount(..., { - apolloProvider: mockApollo, - ... - }); -} + function createComponent(options = {}) { + wrapper = shallowMount(...); + } +}) ``` -After this, you can control whether you need a variable for `mockApollo` and assign it in the appropriate `describe`-scope: +After this, we need to create a mocked Apollo provider: ```javascript -describe('Some component', () => { +import createMockApollo from 'helpers/mock_apollo_helper'; + +describe('Some component with Apollo mock', () => { let wrapper; + let mockApollo; - describe('with Apollo mock', () => { - let mockApollo; + function createComponent(options = {}) { + mockApollo = createMockApollo(...) - beforeEach(() => { - mockApollo = createMockApolloProvider(); - wrapper = createComponent({ mockApollo }); + wrapper = shallowMount(SomeComponent, { + apolloProvider: mockApollo }); - }); -}); + } + + afterEach(() => { + // we need to ensure we don't have provider persisted between tests + mockApollo = null + }) +}) ``` -In the `createMockApolloProvider`-factory, we need to define an array of _handlers_ for every query or mutation: +Now, we need to define an array of _handlers_ for every query or mutation. Handlers should be mock functions that return either a correct query response, or an error: ```javascript import getDesignListQuery from '~/design_management/graphql/queries/get_design_list.query.graphql'; @@ -1435,72 +1235,70 @@ describe('Some component with Apollo mock', () => { let wrapper; let mockApollo; - function createMockApolloProvider() { - Vue.use(VueApollo); - - const requestHandlers = [ - [getDesignListQuery, jest.fn().mockResolvedValue(designListQueryResponse)], - [permissionsQuery, jest.fn().mockResolvedValue(permissionsQueryResponse)], - ]; - ... + function createComponent(options = { + designListHandler: jest.fn().mockResolvedValue(designListQueryResponse) + }) { + mockApollo = createMockApollo([ + [getDesignListQuery, options.designListHandler], + [permissionsQuery, jest.fn().mockResolvedValue(permissionsQueryResponse)], + [moveDesignMutation, jest.fn().mockResolvedValue(moveDesignMutationResponse)], + ]) + + wrapper = shallowMount(SomeComponent, { + apolloProvider: mockApollo + }); } }) ``` -After this, we need to create a mock Apollo Client instance using a helper: +When mocking resolved values, ensure the structure of the response is the same +as the actual API response. For example, root property should be `data`: ```javascript -import createMockApollo from 'helpers/mock_apollo_helper'; - -describe('Some component', () => { - let wrapper; - - function createMockApolloProvider() { - Vue.use(VueApollo); - - const requestHandlers = [ - [getDesignListQuery, jest.fn().mockResolvedValue(designListQueryResponse)], - [permissionsQuery, jest.fn().mockResolvedValue(permissionsQueryResponse)], - ]; - - return createMockApollo(requestHandlers); - } - - function createComponent(options = {}) { - const { mockApollo } = options; - - return shallowMount(Index, { - apolloProvider: mockApollo, - }); - } - - describe('with Apollo mock', () => { - let mockApollo; - - beforeEach(() => { - mockApollo = createMockApolloProvider(); - wrapper = createComponent({ mockApollo }); - }); - }); -}); +const designListQueryResponse = { + data: { + project: { + id: '1', + issue: { + id: 'issue-1', + designCollection: { + copyState: 'READY', + designs: { + nodes: [ + { + id: '3', + event: 'NONE', + filename: 'fox_3.jpg', + notesCount: 1, + image: 'image-3', + imageV432x230: 'image-3', + currentUserTodos: { + nodes: [], + }, + }, + ], + }, + versions: { + nodes: [], + }, + }, + }, + }, + }, +}; ``` -When mocking resolved values, ensure the structure of the response is the same -as the actual API response. For example, root property should be `data`. - When testing queries, keep in mind they are promises, so they need to be _resolved_ to render a result. Without resolving, we can check the `loading` state of the query: ```javascript it('renders a loading state', () => { - const mockApollo = createMockApolloProvider(); - const wrapper = createComponent({ mockApollo }); + const wrapper = createComponent(); expect(wrapper.findComponent(LoadingSpinner).exists()).toBe(true) }); it('renders designs list', async () => { - const mockApollo = createMockApolloProvider(); - const wrapper = createComponent({ mockApollo }); + const wrapper = createComponent(); await waitForPromises() @@ -1511,17 +1309,10 @@ 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 createMockApolloProvider() { - ... - const requestHandlers = [ - [getDesignListQuery, jest.fn().mockRejectedValue(new Error('GraphQL error')], - ]; - ... -} -... - it('renders error if query fails', async () => { - const wrapper = createComponent(); + const wrapper = createComponent({ + designListHandler: jest.fn.mockRejectedValue('Houston, we have a problem!') + }); await waitForPromises() @@ -1529,38 +1320,28 @@ it('renders error if query fails', async () => { }) ``` -Request handlers can also be passed to component factory as a parameter. - Mutations could be tested the same way: ```javascript -function createMockApolloProvider({ - moveHandler = jest.fn().mockResolvedValue(moveDesignMutationResponse), -}) { - Vue.use(VueApollo); - - moveDesignHandler = moveHandler; - - const requestHandlers = [ - [getDesignListQuery, jest.fn().mockResolvedValue(designListQueryResponse)], - [permissionsQuery, jest.fn().mockResolvedValue(permissionsQueryResponse)], - [moveDesignMutation, moveDesignHandler], - ]; - - return createMockApollo(requestHandlers); -} - -function createComponent(options = {}) { - const { mockApollo } = options; + const moveDesignHandlerSuccess = jest.fn().mockResolvedValue(moveDesignMutationResponse) + + function createComponent(options = { + designListHandler: jest.fn().mockResolvedValue(designListQueryResponse), + moveDesignHandler: moveDesignHandlerSuccess + }) { + mockApollo = createMockApollo([ + [getDesignListQuery, options.designListHandler], + [permissionsQuery, jest.fn().mockResolvedValue(permissionsQueryResponse)], + [moveDesignMutation, moveDesignHandler], + ]) + + wrapper = shallowMount(SomeComponent, { + apolloProvider: mockApollo + }); + } - return shallowMount(Index, { - apolloProvider: mockApollo, - }); -} -... it('calls a mutation with correct parameters and reorders designs', async () => { - const mockApollo = createMockApolloProvider({}); - const wrapper = createComponent({ mockApollo }); + const wrapper = createComponent(); wrapper.find(VueDraggable).vm.$emit('change', { moved: { @@ -1569,7 +1350,7 @@ it('calls a mutation with correct parameters and reorders designs', async () => }, }); - expect(moveDesignHandler).toHaveBeenCalled(); + expect(moveDesignHandlerSuccess).toHaveBeenCalled(); await waitForPromises(); @@ -1619,6 +1400,24 @@ describe('when query times out', () => { }); ``` +Previously, we've used `{ mocks: { $apollo ...}}` on `mount` to test Apollo functionality. This approach is discouraged - proper `$apollo` mocking leaks a lot of implementation details to the tests. Consider replacing it with mocked Apollo provider + +```javascript +wrapper = mount(SomeComponent, { + mocks: { + // avoid! Mock real graphql queries and mutations instead + $apollo: { + mutate: jest.fn(), + queries: { + groups: { + loading, + }, + }, + }, + }, +}); +``` + #### Testing `@client` queries ##### Using mock resolvers @@ -1726,121 +1525,29 @@ query fetchLocalUser { ```javascript import fetchLocalUserQuery from '~/design_management/graphql/queries/fetch_local_user.query.graphql'; -function createMockApolloProvider() { - Vue.use(VueApollo); - - const requestHandlers = [ - [getDesignListQuery, jest.fn().mockResolvedValue(designListQueryResponse)], - [permissionsQuery, jest.fn().mockResolvedValue(permissionsQueryResponse)], - ]; - - const mockApollo = createMockApollo(requestHandlers, {}); - mockApollo.clients.defaultClient.cache.writeQuery({ - query: fetchLocalUserQuery, - data: { - fetchLocalUser: { - __typename: 'User', - name: 'Test', - }, - }, - }); - - return mockApollo; -} - -function createComponent(options = {}) { - const { mockApollo } = options; - - return shallowMount(Index, { - 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`', () => { +describe('Some component with Apollo mock', () => { let wrapper; - let fetchLocalUserSpy; - - afterEach(() => { - wrapper.destroy(); - 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(); - }); + let mockApollo; - it('should fetch data once', () => { - expect(fetchLocalUserSpy).toHaveBeenCalledTimes(1); + function createComponent(options = { + designListHandler: jest.fn().mockResolvedValue(designListQueryResponse) + }) { + mockApollo = createMockApollo([...]) + mockApollo.clients.defaultClient.cache.writeQuery({ + query: fetchLocalUserQuery, + data: { + fetchLocalUser: { + __typename: 'User', + name: 'Test', + }, + }, }); - it('displays the error', () => { - // Assess that the error is displayed + wrapper = shallowMount(SomeComponent, { + apolloProvider: mockApollo }); - }); -}); + } +}) ``` When you need to configure the mocked apollo client's caching behavior, @@ -1854,21 +1561,15 @@ const defaultCacheOptions = { ``` ```javascript -function createMockApolloProvider({ props = {}, requestHandlers } = {}) { - Vue.use(VueApollo); - - const mockApollo = createMockApollo( - requestHandlers, - {}, - { - dataIdFromObject: (object) => - // eslint-disable-next-line no-underscore-dangle - object.__typename === 'Requirement' ? object.iid : defaultDataIdFromObject(object), - }, - ); - - return mockApollo; -} +mockApollo = createMockApollo( + requestHandlers, + {}, + { + dataIdFromObject: (object) => + // eslint-disable-next-line no-underscore-dangle + object.__typename === 'Requirement' ? object.iid : defaultDataIdFromObject(object), + }, +); ``` ## Handling errors diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 6403cff549f..3d05d395ef1 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -11,7 +11,7 @@ across the GitLab frontend team. ## Overview -GitLab is built on top of [Ruby on Rails](https://rubyonrails.org). It uses [Haml](https://haml.info/) and a JavaScript-based frontend with [Vue.js](https://vuejs.org). +GitLab is built on top of [Ruby on Rails](https://rubyonrails.org). It uses [Haml](https://haml.info/) and a JavaScript-based frontend with [Vue.js](https://vuejs.org). If you are not sure when to use Vue on top of Haml-page, please read [this explanation](vue.md#when-to-add-vue-application). <!-- vale gitlab.Spelling = NO --> @@ -24,7 +24,7 @@ modern ECMAScript standards supported through [Babel](https://babeljs.io/) and E Working with our frontend assets requires Node (v12.22.1 or greater) and Yarn (v1.10.0 or greater). You can find information on how to install these on our -[installation guide](../../install/installation.md#4-node). +[installation guide](../../install/installation.md#5-node). ### Browser Support diff --git a/doc/development/fe_guide/merge_request_widget_extensions.md b/doc/development/fe_guide/merge_request_widget_extensions.md index 5ad918d466b..07029aec015 100644 --- a/doc/development/fe_guide/merge_request_widget_extensions.md +++ b/doc/development/fe_guide/merge_request_widget_extensions.md @@ -329,7 +329,6 @@ To generate these known events for a single widget: 1. `product_section` = `dev` 1. `product_stage` = `create` 1. `product_group` = `code_review` - 1. `product_category` = `code_review` 1. `introduced_by_url` = `'[your MR]'` 1. `options.events` = (the event in the command from above that generated this file, like `i_code_review_merge_request_widget_test_reports_count_view`) - This value is how the telemetry events are linked to "metrics" so this is probably one of the more important values. @@ -352,7 +351,6 @@ To generate these known events for a single widget: 1. Repeat step 6, but change the `data_source` to `redis_hll`. 1. Add each of the HLL metrics to `lib/gitlab/usage_data_counters/known_events/code_review_events.yml`: 1. `name` = (the event) - 1. `redis_slot` = `code_review` 1. `category` = `code_review` 1. `aggregation` = `weekly` 1. Add each event (those listed in the command in step 7, replacing `test_reports` diff --git a/doc/development/fe_guide/performance.md b/doc/development/fe_guide/performance.md index 3aa901093f0..432e66bee33 100644 --- a/doc/development/fe_guide/performance.md +++ b/doc/development/fe_guide/performance.md @@ -142,21 +142,21 @@ To use the Vue performance plugin: 1. Import the plugin: - ```javascript - import PerformancePlugin from '~/performance/vue_performance_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', - ] - }); - ``` + ```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. @@ -474,4 +474,4 @@ General tips: - [WebPage Test](https://www.webpagetest.org) for testing site loading time and size. - [Google PageSpeed Insights](https://pagespeed.web.dev/) grades web pages and provides feedback to improve the page. - [Profiling with Chrome DevTools](https://developer.chrome.com/docs/devtools/) -- [Browser Diet](https://browserdiet.com/) is a community-built guide that catalogues practical tips for improving web page performance. +- [Browser Diet](https://github.com/zenorocha/browser-diet) was a community-built guide that cataloged practical tips for improving web page performance. diff --git a/doc/development/fe_guide/registry_architecture.md b/doc/development/fe_guide/registry_architecture.md index d86f8416db6..cf267e80619 100644 --- a/doc/development/fe_guide/registry_architecture.md +++ b/doc/development/fe_guide/registry_architecture.md @@ -14,7 +14,7 @@ Existing registries: - Package Registry - Container Registry -- Infrastructure Registry +- Terraform Module Registry - Dependency Proxy ## Frontend architecture diff --git a/doc/development/fe_guide/source_editor.md b/doc/development/fe_guide/source_editor.md index 5f2e8c1e029..45ec3ba1464 100644 --- a/doc/development/fe_guide/source_editor.md +++ b/doc/development/fe_guide/source_editor.md @@ -1,6 +1,6 @@ --- stage: Create -group: Editor +group: Source Code info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- @@ -17,6 +17,14 @@ GitLab features use it, including: - [Web Editor](../../user/project/repository/web_editor.md) - [Security Policies](../../user/application_security/policies/index.md) +## When to use Source Editor + +Use Source Editor only when users need to edit the file content. +If you only need to display source code, consider using the [`BlobContent`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/assets/javascripts/blob/components/blob_content.vue) component. + +If the page you're working on is already loading the Source Editor, +displaying read-only content in the Source Editor is still a valid option. + ## How to use Source Editor Source Editor is framework-agnostic and can be used in any application, including both @@ -35,7 +43,7 @@ Vue component, but the integration of Source Editor is generally straightforward const editor = new SourceEditor({ // Editor Options. // The list of all accepted options can be found at - // https://microsoft.github.io/monaco-editor/api/enums/monaco.editor.EditorOption.html + // https://microsoft.github.io/monaco-editor/docs.html }); ``` @@ -56,19 +64,19 @@ An instance of Source Editor accepts the following configuration options: | `blobContent` | `false` | `String`: The initial content to render in the editor. | | `extensions` | `false` | `Array`: Extensions to use in this instance. | | `blobGlobalId` | `false` | `String`: An auto-generated property.<br>**Note:** This property may go away in the future. Do not pass `blobGlobalId` unless you know what you're doing.| -| Editor Options | `false` | `Object(s)`: Any property outside of the list above is treated as an Editor Option for this particular instance. Use this field to override global Editor Options on the instance level. A full [index of Editor Options](https://microsoft.github.io/monaco-editor/api/enums/monaco.editor.EditorOption.html) is available. | +| Editor Options | `false` | `Object(s)`: Any property outside of the list above is treated as an Editor Option for this particular instance. Use this field to override global Editor Options on the instance level. A full [index of Editor Options](https://microsoft.github.io/monaco-editor/docs.html#enums/editor.EditorOption.html) is available. | ## API The editor uses the same public API as -[provided by Monaco editor](https://microsoft.github.io/monaco-editor/api/interfaces/monaco.editor.IStandaloneCodeEditor.html) +[provided by Monaco editor](https://microsoft.github.io/monaco-editor/docs.html) with additional functions on the instance level: | Function | Arguments | Description | --------------------- | ----- | ----- | | `updateModelLanguage` | `path`: String | Updates the instance's syntax highlighting to follow the extension of the passed `path`. Available only on the instance level. | | `use` | Array of objects | Array of extensions to apply to the instance. Accepts only an array of **objects**. The extensions' ES6 modules must be fetched and resolved in your views or components before they're passed to `use`. Available on the instance and global editor (all instances) levels. | -| Monaco Editor options | See [documentation](https://microsoft.github.io/monaco-editor/api/interfaces/monaco.editor.IStandaloneCodeEditor.html) | Default Monaco editor options. | +| Monaco Editor options | See [documentation](https://microsoft.github.io/monaco-editor/docs.html#interfaces/editor.IStandaloneCodeEditor.html) | Default Monaco editor options. | ## Tips @@ -202,7 +210,7 @@ export default { In the code example, `this` refers to the instance. By referring to the instance, we can access the complete underlying -[Monaco editor API](https://microsoft.github.io/monaco-editor/api/interfaces/monaco.editor.IStandaloneCodeEditor.html), +[Monaco editor API](https://microsoft.github.io/monaco-editor/api/), which includes functions like `getValue()`. Now let's use our extension: diff --git a/doc/development/fe_guide/storybook.md b/doc/development/fe_guide/storybook.md index e57c117bc39..8e0814ad96b 100644 --- a/doc/development/fe_guide/storybook.md +++ b/doc/development/fe_guide/storybook.md @@ -16,15 +16,15 @@ To build and launch Storybook locally, in the root directory of the `gitlab` pro 1. Install Storybook dependencies: - ```shell - yarn storybook:install - ``` + ```shell + yarn storybook:install + ``` 1. Build the Storybook site: - ```shell - yarn storybook:start - ``` + ```shell + yarn storybook:start + ``` ## Adding components to Storybook @@ -35,14 +35,14 @@ To add a story: 1. Create a new `.stories.js` file in the same directory as the Vue component. The filename should have the same prefix as the Vue component. - ```txt - vue_shared/ - ├─ components/ - │ ├─ sidebar - │ | ├─ todo_toggle - │ | | ├─ todo_button.vue - │ │ | ├─ todo_button.stories.js - ``` + ```txt + vue_shared/ + ├─ components/ + │ ├─ sidebar + │ | ├─ todo_toggle + │ | | ├─ todo_button.vue + │ │ | ├─ todo_button.stories.js + ``` 1. Write the story as per the [official Storybook instructions](https://storybook.js.org/docs/vue/writing-stories/introduction/) diff --git a/doc/development/fe_guide/style/html.md b/doc/development/fe_guide/style/html.md index b1cce29bc61..c92f77e9033 100644 --- a/doc/development/fe_guide/style/html.md +++ b/doc/development/fe_guide/style/html.md @@ -58,7 +58,7 @@ Button tags requires a `type` attribute according to the [W3C HTML specification ### Blank target -Arbitrarily opening links in a new tab is not recommended, so refer to the [Pajamas guidelines on links](https://design.gitlab.com/product-foundations/interaction/#links) when considering adding `target="_blank"` to links. +Arbitrarily opening links in a new tab is not recommended, so refer to the [Pajamas guidelines on links](https://design.gitlab.com/components/link) when considering adding `target="_blank"` to links. When using `target="_blank"` with `a` tags, you must also add the `rel="noopener noreferrer"` attribute. This prevents a security vulnerability [documented by JitBit](https://www.jitbit.com/alexblog/256-targetblank---the-most-underestimated-vulnerability-ever/). diff --git a/doc/development/fe_guide/style/index.md b/doc/development/fe_guide/style/index.md index 94ed9544cf5..552b769d6f6 100644 --- a/doc/development/fe_guide/style/index.md +++ b/doc/development/fe_guide/style/index.md @@ -4,7 +4,7 @@ group: unassigned info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# GitLab development style guides +# Frontend style guides See below for the relevant style guides, guidelines, linting, and other information for developing GitLab. diff --git a/doc/development/fe_guide/style/javascript.md b/doc/development/fe_guide/style/javascript.md index 3e3a79dd7bb..987543642f0 100644 --- a/doc/development/fe_guide/style/javascript.md +++ b/doc/development/fe_guide/style/javascript.md @@ -2,7 +2,6 @@ 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/product/ux/technical-writing/#assignments -disqus_identifier: 'https://docs.gitlab.com/ee/development/fe_guide/style_guide_js.html' --- # JavaScript style guide @@ -329,3 +328,26 @@ Only export the constants as a collection (array, or object) when there is a nee // good, if the constants need to be iterated over export const VARIANTS = [VARIANT_WARNING, VARIANT_ERROR]; ``` + +## Error handling + +When catching a server-side error, you should use the error message +utility function contained in `app/assets/javascripts/lib/utils/error_message.js`. +This utility accepts two parameters: the error object received from the server response and a +default error message. The utility examines the message in the error object for a prefix that +indicates whether the message is meant to be user-facing or not. If the message is intended +to be user-facing, the utility returns it as is. Otherwise, it returns the default error +message passed as a parameter. + +```javascript +import { parseErrorMessage } from '~/lib/utils/error_message'; + +onError(error) { + const errorMessage = parseErrorMessage(error, genericErrorText); +} +``` + +To benefit from this parsing mechanism, the utility user should ensure that the server-side +code is aware of this utility's usage and prefixes the error messages where appropriate +before sending them back to the user. See +[Error handling for API](../../api_styleguide.md#error-handling) for more information. diff --git a/doc/development/fe_guide/style/scss.md b/doc/development/fe_guide/style/scss.md index aed7310e95d..e760b0adaaa 100644 --- a/doc/development/fe_guide/style/scss.md +++ b/doc/development/fe_guide/style/scss.md @@ -2,7 +2,6 @@ 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/product/ux/technical-writing/#assignments -disqus_identifier: 'https://docs.gitlab.com/ee/development/fe_guide/style_guide_scss.html' --- # SCSS style guide @@ -77,7 +76,7 @@ These mixins should be used to replace _magic values_ in our code. For example a `margin-top: 8px` is a good candidate for the `@include gl-mt-3` mixin replacement. Avoid using utility mixins for [pre-defined CSS keywords](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Values_and_Units#pre-defined_keyword_values). -For example prefer `display: flex` over `@include gl-display-flex`. +For example prefer `display: flex` over `@include gl-display-flex`. Utility mixins are particularly useful for encapsulating our design system but there is no need to encapsulate simple properties. ```scss // Bad diff --git a/doc/development/fe_guide/style/vue.md b/doc/development/fe_guide/style/vue.md index a21d7c4577b..a3ab1c1af30 100644 --- a/doc/development/fe_guide/style/vue.md +++ b/doc/development/fe_guide/style/vue.md @@ -59,63 +59,66 @@ Check the [rules](https://github.com/vuejs/eslint-plugin-vue#bulb-rules) for mor 1. Explicitly define data being passed into the Vue app - ```javascript - // bad - return new Vue({ - el: '#element', - components: { - componentName - }, - provide: { - ...someDataset - }, - props: { - ...anotherDataset - }, - render: createElement => createElement('component-name'), - })); - - // good - const { foobar, barfoo } = someDataset; - const { foo, bar } = anotherDataset; - - return new Vue({ - el: '#element', - components: { - componentName - }, - provide: { - foobar, - barfoo - }, - props: { - foo, - bar - }, - render: createElement => createElement('component-name'), - })); - ``` - - 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 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. - - ```javascript - return new Vue({ - el: '#element', - components: { - componentName - }, - props: { - foo, - bar: parseBoolean(bar) - }, - render: createElement => createElement('component-name'), - })); - ``` + ```javascript + // bad + return new Vue({ + el: '#element', + name: 'ComponentNameRoot', + components: { + componentName + }, + provide: { + ...someDataset + }, + props: { + ...anotherDataset + }, + render: createElement => createElement('component-name'), + })); + + // good + const { foobar, barfoo } = someDataset; + const { foo, bar } = anotherDataset; + + return new Vue({ + el: '#element', + name: 'ComponentNameRoot', + components: { + componentName + }, + provide: { + foobar, + barfoo + }, + props: { + foo, + bar + }, + render: createElement => createElement('component-name'), + })); + ``` + + 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 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. + + ```javascript + return new Vue({ + el: '#element', + name: 'ComponentNameRoot', + components: { + componentName + }, + props: { + foo, + bar: parseBoolean(bar) + }, + render: createElement => createElement('component-name'), + })); + ``` ## Naming @@ -404,7 +407,7 @@ When using `v-for` you need to provide a *unique* `:key` attribute for each item ``` 1. When using `v-for` with `template` and there is more than one child element, the `:key` values -must be unique. It's advised to use `kebab-case` namespaces. + must be unique. It's advised to use `kebab-case` namespaces. ```html <template v-for="(item, index) in items"> @@ -467,8 +470,9 @@ Creating a global, mutable wrapper provides a number of advantages, including th ``` - Use a `beforeEach` block to mount the component (see -[the `createComponent` factory](#the-createcomponent-factory) for more information). -- Use an `afterEach` block to destroy the component, for example, `wrapper.destroy()`. + [the `createComponent` factory](#the-createcomponent-factory) for more information). +- Automatically destroy the component after the test is run with [`enableAutoDestroy`](https://v1.test-utils.vuejs.org/api/#enableautodestroy-hook) + set in [`shared_test_setup.js`](https://gitlab.com/gitlab-org/gitlab/-/blob/d0bdc8370ef17891fd718a4578e41fef97cf065d/spec/frontend/__helpers__/shared_test_setup.js#L20). #### The `createComponent` factory @@ -538,42 +542,42 @@ describe('MyComponent', () => { #### `createComponent` best practices 1. Consider using a single (or a limited number of) object arguments over many arguments. - Defining single parameters for common data like `props` is okay, - but keep in mind our [JavaScript style guide](javascript.md#limit-number-of-parameters) and - stay within the parameter number limit: + Defining single parameters for common data like `props` is okay, + but keep in mind our [JavaScript style guide](javascript.md#limit-number-of-parameters) and + stay within the parameter number limit: - ```javascript - // bad - function createComponent(data, props, methods, isLoading, mountFn) { } + ```javascript + // bad + function createComponent(data, props, methods, isLoading, mountFn) { } - // good - function createComponent({ data, props, methods, stubs, isLoading } = {}) { } + // good + function createComponent({ data, props, methods, stubs, isLoading } = {}) { } - // good - function createComponent(props = {}, { data, methods, stubs, isLoading } = {}) { } - ``` + // good + function createComponent(props = {}, { data, methods, stubs, isLoading } = {}) { } + ``` 1. If you require both `mount` _and_ `shallowMount` within the same set of tests, it -can be useful define a `mountFn` parameter for the `createComponent` factory that accepts -the mounting function (`mount` or `shallowMount`) to be used to mount the component: + can be useful define a `mountFn` parameter for the `createComponent` factory that accepts + the mounting function (`mount` or `shallowMount`) to be used to mount the component: - ```javascript - import { shallowMount } from '@vue/test-utils'; + ```javascript + import { shallowMount } from '@vue/test-utils'; - function createComponent({ mountFn = shallowMount } = {}) { } - ``` + function createComponent({ mountFn = shallowMount } = {}) { } + ``` 1. Use the `mountExtended` and `shallowMountExtended` helpers to expose `wrapper.findByTestId()`: - ```javascript - import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; - import { SomeComponent } from 'components/some_component.vue'; + ```javascript + import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; + import { SomeComponent } from 'components/some_component.vue'; - let wrapper; + let wrapper; - const createWrapper = () => { wrapper = shallowMountExtended(SomeComponent); }; - const someButton = () => wrapper.findByTestId('someButtonTestId'); - ``` + const createWrapper = () => { wrapper = shallowMountExtended(SomeComponent); }; + const someButton = () => wrapper.findByTestId('someButtonTestId'); + ``` ### Setting component state @@ -581,70 +585,82 @@ the mounting function (`mount` or `shallowMount`) to be used to mount the compon component state wherever possible. Instead, set the component's [`propsData`](https://v1.test-utils.vuejs.org/api/options.html#propsdata) when mounting the component: - ```javascript - // bad - wrapper = shallowMount(MyComponent); - wrapper.setProps({ - myProp: 'my cool prop' - }); + ```javascript + // bad + wrapper = shallowMount(MyComponent); + wrapper.setProps({ + myProp: 'my cool prop' + }); - // good - wrapper = shallowMount({ propsData: { myProp: 'my cool prop' } }); - ``` + // good + wrapper = shallowMount({ propsData: { myProp: 'my cool prop' } }); + ``` - The exception here is when you wish to test component reactivity in some way. - For example, you may want to test the output of a component when after a particular watcher has - executed. Using `setProps` to test such behavior is okay. + The exception here is when you wish to test component reactivity in some way. + For example, you may want to test the output of a component when after a particular watcher has + executed. Using `setProps` to test such behavior is okay. ### Accessing component state 1. When accessing props or attributes, prefer the `wrapper.props('myProp')` syntax over `wrapper.props().myProp` or `wrapper.vm.myProp`: - ```javascript - // good - expect(wrapper.props().myProp).toBe(true); - expect(wrapper.attributes().myAttr).toBe(true); + ```javascript + // good + expect(wrapper.props().myProp).toBe(true); + expect(wrapper.attributes().myAttr).toBe(true); - // better - expect(wrapper.props('myProp').toBe(true); - expect(wrapper.attributes('myAttr')).toBe(true); - ``` + // better + expect(wrapper.props('myProp').toBe(true); + expect(wrapper.attributes('myAttr')).toBe(true); + ``` 1. When asserting multiple props, check the deep equality of the `props()` object with [`toEqual`](https://jestjs.io/docs/expect#toequalvalue): - ```javascript - // good - expect(wrapper.props('propA')).toBe('valueA'); - expect(wrapper.props('propB')).toBe('valueB'); - expect(wrapper.props('propC')).toBe('valueC'); - - // better - expect(wrapper.props()).toEqual({ - propA: 'valueA', - propB: 'valueB', - propC: 'valueC', - }); - ``` + ```javascript + // good + expect(wrapper.props('propA')).toBe('valueA'); + expect(wrapper.props('propB')).toBe('valueB'); + expect(wrapper.props('propC')).toBe('valueC'); + + // better + expect(wrapper.props()).toEqual({ + propA: 'valueA', + propB: 'valueB', + propC: 'valueC', + }); + ``` 1. If you are only interested in some of the props, you can use [`toMatchObject`](https://jestjs.io/docs/expect#tomatchobjectobject). Prefer `toMatchObject` over [`expect.objectContaining`](https://jestjs.io/docs/expect#expectobjectcontainingobject): - ```javascript - // good - expect(wrapper.props()).toEqual(expect.objectContaining({ - propA: 'valueA', - propB: 'valueB', - })); + ```javascript + // good + expect(wrapper.props()).toEqual(expect.objectContaining({ + propA: 'valueA', + propB: 'valueB', + })); + + // better + expect(wrapper.props()).toMatchObject({ + propA: 'valueA', + propB: 'valueB', + }); + ``` + +### Testing props validation + +1. When checking component props use `assertProps` helper. Props validation failures will be thrown as errors - // better - expect(wrapper.props()).toMatchObject({ - propA: 'valueA', - propB: 'valueB', - }); - ``` +```javascript +import { assertProps } from 'helpers/assert_props' + +// ... + +expect(() => assertProps(SomeComponent, { invalidPropValue: '1', someOtherProp: 2 })).toThrow() +``` ## The JavaScript/Vue Accord @@ -659,16 +675,16 @@ The goal of this accord is to make sure we are all on the same page. 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://v2.vuejs.org/v2/api/#vm-emit). 1. You may query the `window` object one time, while bootstrapping your application for application -specific data (for example, `scrollTo` is ok to access anytime). Do this access during the -bootstrapping of your application. + specific data (for example, `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. + 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. For example, jQuery tests rewritten to Vue tests. + not be rewritten. For example, jQuery tests rewritten to Vue tests. 1. You may choose to use VueX as a centralized state management. If you choose not to use VueX, you -must use the *store pattern* which can be found in the -[Vue.js documentation](https://v2.vuejs.org/v2/guide/state-management.html#Simple-State-Management-from-Scratch). + must use the *store pattern* which can be found in the + [Vue.js documentation](https://v2.vuejs.org/v2/guide/state-management.html#Simple-State-Management-from-Scratch). 1. Once you have chosen a centralized state-management solution you must use it for your entire -application. Don't mix and match your state-management solutions. + application. Don't mix and match your state-management solutions. diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index cea47bc0e4c..7ba774392a1 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -16,11 +16,23 @@ What is described in the following sections can be found in these examples: - [Security products](https://gitlab.com/gitlab-org/gitlab/-/tree/master/ee/app/assets/javascripts/vue_shared/security_reports) - [Registry](https://gitlab.com/gitlab-org/gitlab-foss/tree/master/app/assets/javascripts/registry/stores) +## When to add Vue application + +Sometimes, HAML page is enough to satisfy requirements. This statement is correct primarily for the static pages or pages that have very little logic. How do we know it's worth adding a Vue application to the page? The answer is "when we need to maintain application state and synchronize the rendered page with it". + +To better explain this, let's imagine the page that has one toggle, and toggling it sends an API request. This case does not involve any state we want to maintain, we send the request and switch the toggle. However, if we add one more toggle that should always be the opposite to the first one, we need a _state_: one toggle should be "aware" about the state of another one. When written in plain JavaScript, this logic usually involves listening to DOM event and reacting with modifying DOM. Cases like this are much easier to handle with Vue.js so we should create a Vue application here. + +### What are some flags signaling that you might need Vue application? + +- when you need to define complex conditionals based on multiple factors and update them on user interaction; +- when you have to maintain any form of application state and share it between tags/elements; +- when you expect complex logic to be added in the future - it's easier to start with basic Vue application than having to rewrite JS/HAML to Vue on the next step. + ## Vue architecture -All new features built with Vue.js must follow a [Flux architecture](https://facebook.github.io/flux/). +All new features built with Vue.js must follow a [Flux architecture](https://facebookarchive.github.io/flux/). The main goal we are trying to achieve is to have only one data flow, and only one data entry. -To achieve this goal we use [Vuex](#vuex). +To achieve this goal we use [Vuex](#vuex) or [Apollo Client](graphql.md#libraries) You can also read about this architecture in Vue documentation about [state management](https://v2.vuejs.org/v2/guide/state-management.html#Simple-State-Management-from-Scratch) @@ -76,7 +88,13 @@ component, is that you avoid creating a fixture or an HTML element in the unit t `initSimpleApp` is a helper function that streamlines the process of mounting a component in Vue.js. It accepts two arguments: a selector string representing the mount point in the HTML, and a Vue component. -To use `initSimpleApp`, include the HTML element in the page with the appropriate selector and add a data-view-model attribute containing a JSON object. Then, import the desired Vue component and pass it along with the selector to `initSimpleApp`. This mounts the component at the specified location. +To use `initSimpleApp`: + +1. Include an HTML element in the page with an ID or unique class. +1. Add a data-view-model attribute containing a JSON object. +1. Import the desired Vue component, and pass it along with a valid CSS selector string + that selects the HTML element to `initSimpleApp`. This string mounts the component + at the specified location. `initSimpleApp` automatically retrieves the content of the data-view-model attribute as a JSON object and passes it as props to the mounted Vue component. This can be used to pre-populate the component with data. @@ -115,7 +133,7 @@ export default { ```javascript //index.js import MyComponent from './my_component.vue' -import initSimpleApp from '~/helpers/init_simple_app_helper' +import { initSimpleApp } from '~/helpers/init_simple_app_helper' initSimpleApp('#js-my-element', MyComponent) ``` @@ -138,6 +156,7 @@ const { endpoint } = el.dataset; return new Vue({ el, + name: 'MyComponentRoot', render(createElement) { return createElement('my-component', { provide: { @@ -198,6 +217,7 @@ const { endpoint } = el.dataset; return new Vue({ el, + name: 'MyComponentRoot', render(createElement) { return createElement('my-component', { props: { @@ -252,6 +272,7 @@ export const initUserForm = () => { return new Vue({ el, + name: 'UserFormRoot', render(h) { return h(UserForm, { props: { @@ -305,6 +326,7 @@ initializing our Vue instance, and the data should be provided as `props` to the ```javascript return new Vue({ el: '.js-vue-app', + name: 'MyComponentRoot', render(createElement) { return createElement('my-component', { props: { @@ -381,6 +403,87 @@ You can read more about components in Vue.js site, [Component System](https://v2 Check this [page](vuex.md) for more details. +### Vue Router + +To add [Vue Router](https://router.vuejs.org/) to a page: + +1. Add a catch-all route to the Rails route file using a wildcard named `*vueroute`: + + ```ruby + # example from ee/config/routes/project.rb + + resources :iteration_cadences, path: 'cadences(/*vueroute)', action: :index + ``` + + The above example serves the `index` page from `iteration_cadences` controller to any route + matching the start of the `path`, for example `groupname/projectname/-/cadences/123/456/`. +1. Pass the base route (everything before `*vueroute`) to the frontend to use as the `base` parameter to initialize Vue Router: + + ```haml + .js-my-app{ data: { base_path: project_iteration_cadences_path(project) } } + ``` + +1. Initialize the router: + + ```javascript + Vue.use(VueRouter); + + export function createRouter(basePath) { + return new VueRouter({ + routes: createRoutes(), + mode: 'history', + base: basePath, + }); + } + ``` + +1. Add a fallback for unrecognised routes with `path: '*'`. Either: + - Add a redirect to the end of your routes array: + + ```javascript + const routes = [ + { + path: '/', + name: 'list-page', + component: ListPage, + }, + { + path: '*', + redirect: '/', + }, + ]; + ``` + + - Add a fallback component to the end of your routes array: + + ```javascript + const routes = [ + { + path: '/', + name: 'list-page', + component: ListPage, + }, + { + path: '*', + component: NotFound, + }, + ]; + ``` + +1. Optional. To also allow using the path helper for child routes, add `controller` and `action` + parameters to use the parent controller. + + ```ruby + resources :iteration_cadences, path: 'cadences(/*vueroute)', action: :index do + resources :iterations, only: [:index, :new, :edit, :show], constraints: { id: /\d+/ }, controller: :iteration_cadences, action: :index + end + ``` + + This means routes like `/cadences/123/iterations/456/edit` can be validated on the backend, + for example to check group or project membership. + It also means we can use the `_path` helper, which means we can load the page in feature specs + without manually building the `*vueroute` part of the path.. + ### Mixing Vue and jQuery - Mixing Vue and jQuery is not recommended. @@ -443,6 +546,22 @@ Composition API allows you to place the logic in the `<script>` section of the c </script> ``` +### `v-bind` limitations + +Avoid using `v-bind="$attrs"` unless absolutely necessary. You might need this when +developing a native control wrapper. (This is a good candidate for a `gitlab-ui` component.) +In any other cases, always prefer using `props` and explicit data flow. + +Using `v-bind="$attrs"` leads to: + +1. A loss in component's contract. The `props` were designed specifically + to address this problem. +1. High maintenance cost for each component in the tree. `v-bind="$attrs"` is specifically + hard to debug because you must scan the whole hierarchy of components to understand + the data flow. +1. Problems during migration to Vue 3. `$attrs` in Vue 3 include event listeners which + could cause unexpected side-effects after Vue 3 migration is completed. + ### Aim to have one API style per component When adding `setup()` property to Vue component, consider refactoring it to Composition API entirely. It's not always feasible, especially for large components, but we should aim to have one API style per component for readability and maintainability. @@ -608,8 +727,7 @@ describe('~/todos/app.vue', () => { }); afterEach(() => { - // IMPORTANT: Clean up the component instance and axios mock adapter - wrapper.destroy(); + // IMPORTANT: Clean up the axios mock adapter mock.restore(); }); diff --git a/doc/development/fe_guide/vuex.md b/doc/development/fe_guide/vuex.md index 01ee50fb6ca..52278c94e9f 100644 --- a/doc/development/fe_guide/vuex.md +++ b/doc/development/fe_guide/vuex.md @@ -6,14 +6,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Vuex -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, 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). +[Vuex](https://vuex.vuejs.org) should no longer be considered a preferred path to store management and is currently in its legacy phase. This means it is acceptable to add upon existing `Vuex` stores, but we strongly recommend reducing store sizes over time and eventually migrating fully to [Apollo](https://www.apollographql.com/) whenever it's possible. Before adding any new `Vuex` store to an application, first ensure that the `Vue` application you plan to add it into **does not use** `Apollo`. `Vuex` and `Apollo` should not be combined unless absolutely necessary. Please consider reading through [our GraphQL documentation](../fe_guide/graphql.md) for more guidelines on how you can build `Apollo` based applications. The information included in this page is explained in more detail in the official [Vuex documentation](https://vuex.vuejs.org). @@ -97,7 +90,7 @@ In this file, we write the actions that call mutations for handling a list of us ```javascript import * as types from './mutation_types'; import axios from '~/lib/utils/axios_utils'; - import { createAlert } from '~/flash'; + import { createAlert } from '~/alert'; export const fetchUsers = ({ state, dispatch }) => { commit(types.REQUEST_USERS); @@ -305,6 +298,7 @@ export default () => { return new Vue({ el, + name: 'AwesomeVueRoot', store: createStore(el.dataset), render: h => h(AwesomeVueApp) }); @@ -462,10 +456,6 @@ describe('component', () => { createComponent(); }); - afterEach(() => { - wrapper.destroy(); - }); - it('should show a user', async () => { const user = { name: 'Foo', diff --git a/doc/development/fe_guide/widgets.md b/doc/development/fe_guide/widgets.md index edb8559da48..6cd8e6c091c 100644 --- a/doc/development/fe_guide/widgets.md +++ b/doc/development/fe_guide/widgets.md @@ -62,11 +62,11 @@ Because we need different GraphQL queries and mutations for different sidebars, ```javascript export const assigneesQueries = { - [IssuableType.Issue]: { + [TYPE_ISSUE]: { query: getIssueParticipants, mutation: updateAssigneesMutation, }, - [IssuableType.MergeRequest]: { + [TYPE_MERGE_REQUEST]: { query: getMergeRequestParticipants, mutation: updateMergeRequestParticipantsMutation, }, |