From 9400ed3b3e3bf1c1ce20b9748f4b6e4c0e5b5db7 Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Mon, 20 Nov 2017 09:57:08 +0000 Subject: Use axios instead of vue resource - step 1 --- .../clusters/services/clusters_service.js | 8 +-- .../javascripts/jobs/job_details_mediator.js | 6 +- .../javascripts/jobs/services/job_service.js | 9 +-- app/assets/javascripts/lib/utils/axios_utils.js | 22 ++++++- app/assets/javascripts/lib/utils/poll.js | 4 +- doc/development/fe_guide/axios.md | 68 ++++++++++++++++++++ doc/development/fe_guide/index.md | 4 +- doc/development/fe_guide/vue.md | 70 ++++----------------- doc/development/fe_guide/vue_resource.md | 72 ---------------------- package.json | 1 + spec/javascripts/jobs/job_details_mediator_spec.js | 20 +++--- yarn.lock | 6 ++ 12 files changed, 128 insertions(+), 162 deletions(-) create mode 100644 doc/development/fe_guide/axios.md delete mode 100644 doc/development/fe_guide/vue_resource.md diff --git a/app/assets/javascripts/clusters/services/clusters_service.js b/app/assets/javascripts/clusters/services/clusters_service.js index 0ac8e68187d..ce14c9a9945 100644 --- a/app/assets/javascripts/clusters/services/clusters_service.js +++ b/app/assets/javascripts/clusters/services/clusters_service.js @@ -1,10 +1,7 @@ -import axios from 'axios'; -import setAxiosCsrfToken from '../../lib/utils/axios_utils'; +import axios from '../../lib/utils/axios_utils'; export default class ClusterService { constructor(options = {}) { - setAxiosCsrfToken(); - this.options = options; this.appInstallEndpointMap = { helm: this.options.installHelmEndpoint, @@ -18,7 +15,6 @@ export default class ClusterService { } installApplication(appId) { - const endpoint = this.appInstallEndpointMap[appId]; - return axios.post(endpoint); + return axios.post(this.appInstallEndpointMap[appId]); } } diff --git a/app/assets/javascripts/jobs/job_details_mediator.js b/app/assets/javascripts/jobs/job_details_mediator.js index 3e2658f9fc1..5a216f8fae2 100644 --- a/app/assets/javascripts/jobs/job_details_mediator.js +++ b/app/assets/javascripts/jobs/job_details_mediator.js @@ -29,8 +29,8 @@ export default class JobMediator { this.poll = new Poll({ resource: this.service, method: 'getJob', - successCallback: this.successCallback.bind(this), - errorCallback: this.errorCallback.bind(this), + successCallback: response => this.successCallback(response), + errorCallback: () => this.errorCallback(), }); if (!Visibility.hidden()) { @@ -57,7 +57,7 @@ export default class JobMediator { successCallback(response) { this.state.isLoading = false; - return response.json().then(data => this.store.storeJob(data)); + return this.store.storeJob(response.data); } errorCallback() { diff --git a/app/assets/javascripts/jobs/services/job_service.js b/app/assets/javascripts/jobs/services/job_service.js index eaf1c6e500a..b746489c45c 100644 --- a/app/assets/javascripts/jobs/services/job_service.js +++ b/app/assets/javascripts/jobs/services/job_service.js @@ -1,14 +1,11 @@ -import Vue from 'vue'; -import VueResource from 'vue-resource'; - -Vue.use(VueResource); +import axios from '../../lib/utils/axios_utils'; export default class JobService { constructor(endpoint) { - this.job = Vue.resource(endpoint); + this.job = endpoint; } getJob() { - return this.job.get(); + return axios.get(this.job); } } diff --git a/app/assets/javascripts/lib/utils/axios_utils.js b/app/assets/javascripts/lib/utils/axios_utils.js index 45bff245827..7aeeca3b283 100644 --- a/app/assets/javascripts/lib/utils/axios_utils.js +++ b/app/assets/javascripts/lib/utils/axios_utils.js @@ -1,6 +1,22 @@ import axios from 'axios'; import csrf from './csrf'; -export default function setAxiosCsrfToken() { - axios.defaults.headers.common[csrf.headerKey] = csrf.token; -} +axios.defaults.headers.common[csrf.headerKey] = csrf.token; + +// Maintain a global counter for active requests +// see: spec/support/wait_for_requests.rb +axios.interceptors.request.use((config) => { + window.activeVueResources = window.activeVueResources || 0; + window.activeVueResources += 1; + + return config; +}); + +// Remove the global counter +axios.interceptors.response.use((config) => { + window.activeVueResources -= 1; + + return config; +}); + +export default axios; diff --git a/app/assets/javascripts/lib/utils/poll.js b/app/assets/javascripts/lib/utils/poll.js index 65a8cf2c891..7fca80c2fdb 100644 --- a/app/assets/javascripts/lib/utils/poll.js +++ b/app/assets/javascripts/lib/utils/poll.js @@ -3,7 +3,9 @@ import { normalizeHeaders } from './common_utils'; /** * Polling utility for handling realtime updates. - * Service for vue resouce and method need to be provided as props + * Requirements: Promise based HTTP client + * + * Service for promise based http client and method need to be provided as props * * @example * new Poll({ diff --git a/doc/development/fe_guide/axios.md b/doc/development/fe_guide/axios.md new file mode 100644 index 00000000000..962fe3dcec9 --- /dev/null +++ b/doc/development/fe_guide/axios.md @@ -0,0 +1,68 @@ +# Axios +We use [axios][axios] to communicate with the server in Vue applications and most new code. + +In order to guarantee all defaults are set you *should not use `axios` directly*, you should import `axios` from `axios_utils`. + +## CSRF token +All our request require a CSRF token. +To guarantee this token is set, we are importing [axios][axios], setting the token, and exporting `axios` . + +This exported module should be used instead of directly using `axios` to ensure the token is set. + +## Usage +```javascript + import axios from '~/lib/utils/axios_utils'; + + axios.get(url) + .then((response) => { + // `data` is the response that was provided by the server + const data = response.data; + + // `headers` the headers that the server responded with + // All header names are lower cased + const paginationData = response.headers; + }) + .catch(() => { + //handle the error + }); +``` + +## Mock axios response on tests + +To help us mock the responses we need we use [axios-mock-adapter][axios-mock-adapter] + + +```javascript + import axios from '~/lib/utils/axios_utils'; + import MockAdapter from 'axios-mock-adapter'; + + let mock; + beforeEach(() => { + // This sets the mock adapter on the default instance + mock = new MockAdapter(axios); + // Mock any GET request to /users + // arguments for reply are (status, data, headers) + mock.onGet('/users').reply(200, { + users: [ + { id: 1, name: 'John Smith' } + ] + }); + }); + + afterEach(() => { + mock.reset(); + }); +``` + +### Mock poll requests on tests with axios + +Because polling function requires an header object, we need to always include an object as the third argument: + +```javascript + mock.onGet('/users').reply(200, { foo: 'bar' }, {}); +``` + +[axios]: https://github.com/axios/axios +[axios-instance]: #creating-an-instance +[axios-interceptors]: https://github.com/axios/axios#interceptors +[axios-mock-adapter]: https://github.com/ctimmerm/axios-mock-adapter diff --git a/doc/development/fe_guide/index.md b/doc/development/fe_guide/index.md index 73a03c07812..72cb557d054 100644 --- a/doc/development/fe_guide/index.md +++ b/doc/development/fe_guide/index.md @@ -71,8 +71,8 @@ Vue specific design patterns and practices. --- -## [Vue Resource](vue_resource.md) -Vue resource specific practices and gotchas. +## [Axios](axios.md) +Axios specific practices and gotchas. ## [Icons](icons.md) How we use SVG for our Icons. diff --git a/doc/development/fe_guide/vue.md b/doc/development/fe_guide/vue.md index f88f0753687..6e9f18dd1c3 100644 --- a/doc/development/fe_guide/vue.md +++ b/doc/development/fe_guide/vue.md @@ -178,16 +178,13 @@ itself, please read this guide: [State Management][state-management] The Service is a class used only to communicate with the server. It does not store or manipulate any data. It is not aware of the store or the components. -We use [vue-resource][vue-resource-repo] to communicate with the server. -Refer to [vue resource](vue_resource.md) for more details. +We use [axios][axios] to communicate with the server. +Refer to [axios](axios.md) for more details. -Vue Resource should only be imported in the service file. +Axios instance should only be imported in the service file. ```javascript - import Vue from 'vue'; - import VueResource from 'vue-resource'; - - Vue.use(VueResource); + import axios from 'javascripts/lib/utils/axios_utils'; ``` ### End Result @@ -230,15 +227,14 @@ export default class Store { } // service.js -import Vue from 'vue'; -import VueResource from 'vue-resource'; -import 'vue_shared/vue_resource_interceptor'; - -Vue.use(VueResource); +import axios from 'javascripts/lib/utils/axios_utils' export default class Service { constructor(options) { - this.todos = Vue.resource(endpoint.todosEndpoint); + this.todos = axios.create({ + baseURL: endpoint.todosEndpoint + }); + } getTodos() { @@ -477,50 +473,8 @@ The main return value of a Vue component is the rendered output. In order to tes need to test the rendered output. [Vue][vue-test] guide's to unit test show us exactly that: ### Stubbing API responses -[Vue Resource Interceptors][vue-resource-interceptor] allow us to add a interceptor with -the response we need: - - ```javascript - // Mock the service to return data - const interceptor = (request, next) => { - next(request.respondWith(JSON.stringify([{ - title: 'This is a todo', - body: 'This is the text' - }]), { - status: 200, - })); - }; +Refer to [mock axios](axios.md#mock-axios-response-on-tests) - beforeEach(() => { - Vue.http.interceptors.push(interceptor); - }); - - afterEach(() => { - Vue.http.interceptors = _.without(Vue.http.interceptors, interceptor); - }); - - it('should do something', (done) => { - setTimeout(() => { - // Test received data - done(); - }, 0); - }); - ``` - -1. Headers interceptor -Refer to [this section](vue.md#headers) - -1. Use `$.mount()` to mount the component - -```javascript -// bad -new Component({ - el: document.createElement('div') -}); - -// good -new Component().$mount(); -``` ## Vuex To manage the state of an application you may use [Vuex][vuex-docs]. @@ -721,7 +675,6 @@ describe('component', () => { [component-system]: https://vuejs.org/v2/guide/#Composing-with-Components [state-management]: https://vuejs.org/v2/guide/state-management.html#Simple-State-Management-from-Scratch [one-way-data-flow]: https://vuejs.org/v2/guide/components.html#One-Way-Data-Flow -[vue-resource-interceptor]: https://github.com/pagekit/vue-resource/blob/develop/docs/http.md#interceptors [vue-test]: https://vuejs.org/v2/guide/unit-testing.html [issue-boards-service]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/boards/services/board_service.js.es6 [flux]: https://facebook.github.io/flux @@ -729,3 +682,6 @@ describe('component', () => { [vuex-structure]: https://vuex.vuejs.org/en/structure.html [vuex-mutations]: https://vuex.vuejs.org/en/mutations.html [vuex-testing]: https://vuex.vuejs.org/en/testing.html +[axios]: https://github.com/axios/axios +[axios-interceptors]: https://github.com/axios/axios#interceptors + diff --git a/doc/development/fe_guide/vue_resource.md b/doc/development/fe_guide/vue_resource.md deleted file mode 100644 index c376c5c32bf..00000000000 --- a/doc/development/fe_guide/vue_resource.md +++ /dev/null @@ -1,72 +0,0 @@ -# Vue Resouce -In Vue applications we use [vue-resource][vue-resource-repo] to communicate with the server. - -## HTTP Status Codes - -### `.json()` -When making a request to the server, you will most likely need to access the body of the response. -Use `.json()` to convert. Because `.json()` returns a Promise the follwoing structure should be used: - - ```javascript - service.get('url') - .then(resp => resp.json()) - .then((data) => { - this.store.storeData(data); - }) - .catch(() => new Flash('Something went wrong')); - ``` - - -When using `Poll` (`app/assets/javascripts/lib/utils/poll.js`), the `successCallback` needs to handle `.json()` as a Promise: - ```javascript - successCallback: (response) => { - return response.json().then((data) => { - // handle the response - }); - } - ``` - -### 204 -Some endpoints - usually `delete` endpoints - return `204` as the success response. -When handling `204 - No Content` responses, we cannot use `.json()` since it tries to parse the non-existant body content. - -When handling `204` responses, do not use `.json`, otherwise the promise will throw an error and will enter the `catch` statement: - -```javascript - Vue.http.delete('path') - .then(() => { - // success! - }) - .catch(() => { - // handle error - }) -``` - -## Headers -Headers are being parsed into a plain object in an interceptor. -In Vue-resource 1.x `headers` object was changed into an `Headers` object. In order to not change all old code, an interceptor was added. - -If you need to write a unit test that takes the headers in consideration, you need to include an interceptor to parse the headers after your test interceptor. -You can see an example in `spec/javascripts/environments/environment_spec.js`: - ```javascript - import { headersInterceptor } from './helpers/vue_resource_helper'; - - beforeEach(() => { - Vue.http.interceptors.push(myInterceptor); - Vue.http.interceptors.push(headersInterceptor); - }); - - afterEach(() => { - Vue.http.interceptors = _.without(Vue.http.interceptors, myInterceptor); - Vue.http.interceptors = _.without(Vue.http.interceptors, headersInterceptor); - }); - ``` - -## CSRF token -We use a Vue Resource interceptor to manage the CSRF token. -`app/assets/javascripts/vue_shared/vue_resource_interceptor.js` holds all our common interceptors. -Note: You don't need to load `app/assets/javascripts/vue_shared/vue_resource_interceptor.js` -since it's already being loaded by `common_vue.js`. - - -[vue-resource-repo]: https://github.com/pagekit/vue-resource diff --git a/package.json b/package.json index cbcce40ffb9..21e04724441 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,7 @@ "dependencies": { "autosize": "^4.0.0", "axios": "^0.16.2", + "axios-mock-adapter": "^1.10.0", "babel-core": "^6.22.1", "babel-eslint": "^7.2.1", "babel-loader": "^7.1.1", diff --git a/spec/javascripts/jobs/job_details_mediator_spec.js b/spec/javascripts/jobs/job_details_mediator_spec.js index 1d7fa7e12fc..3069a0cd60e 100644 --- a/spec/javascripts/jobs/job_details_mediator_spec.js +++ b/spec/javascripts/jobs/job_details_mediator_spec.js @@ -1,39 +1,35 @@ -import Vue from 'vue'; +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; import JobMediator from '~/jobs/job_details_mediator'; import job from './mock_data'; describe('JobMediator', () => { let mediator; + let mock; beforeEach(() => { - mediator = new JobMediator({ endpoint: 'foo' }); + mediator = new JobMediator({ endpoint: 'jobs/40291672.json' }); + mock = new MockAdapter(axios); }); it('should set defaults', () => { expect(mediator.store).toBeDefined(); expect(mediator.service).toBeDefined(); - expect(mediator.options).toEqual({ endpoint: 'foo' }); + expect(mediator.options).toEqual({ endpoint: 'jobs/40291672.json' }); expect(mediator.state.isLoading).toEqual(false); }); describe('request and store data', () => { - const interceptor = (request, next) => { - next(request.respondWith(JSON.stringify(job), { - status: 200, - })); - }; - beforeEach(() => { - Vue.http.interceptors.push(interceptor); + mock.onGet().reply(200, job, {}); }); afterEach(() => { - Vue.http.interceptors = _.without(Vue.http.interceptor, interceptor); + mock.restore(); }); it('should store received data', (done) => { mediator.fetchJob(); - setTimeout(() => { expect(mediator.store.state.job).toEqual(job); done(); diff --git a/yarn.lock b/yarn.lock index 1271c8a1ee3..a73aebbf180 100644 --- a/yarn.lock +++ b/yarn.lock @@ -264,6 +264,12 @@ aws4@^1.2.1: version "1.6.0" resolved "https://registry.yarnpkg.com/aws4/-/aws4-1.6.0.tgz#83ef5ca860b2b32e4a0deedee8c771b9db57471e" +axios-mock-adapter@^1.10.0: + version "1.10.0" + resolved "https://registry.yarnpkg.com/axios-mock-adapter/-/axios-mock-adapter-1.10.0.tgz#3ccee65466439a2c7567e932798fc0377d39209d" + dependencies: + deep-equal "^1.0.1" + axios@^0.16.2: version "0.16.2" resolved "https://registry.yarnpkg.com/axios/-/axios-0.16.2.tgz#ba4f92f17167dfbab40983785454b9ac149c3c6d" -- cgit v1.2.1