Merge branch 'leipert-improve-startup-js' into 'master'

Improve startup.js

See merge request gitlab-org/gitlab!42079

app/assets/javascripts/lib/utils/axios_startup_calls.js

@@ -10,6 +10,32 @@ const getFullUrl = req => {
   return mergeUrlParams(req.params || {}, url);
 };
 
+const handleStartupCall = async ({ fetchCall }, req) => {
+  const res = await fetchCall;
+  if (!res.ok) {
+    throw new Error(res.statusText);
+  }
+
+  const fetchHeaders = {};
+  res.headers.forEach((val, key) => {
+    fetchHeaders[key] = val;
+  });
+
+  const data = await res.clone().json();
+
+  Object.assign(req, {
+    adapter: () =>
+      Promise.resolve({
+        data,
+        status: res.status,
+        statusText: res.statusText,
+        headers: fetchHeaders,
+        config: req,
+        request: req,
+      }),
+  });
+};
+
 const setupAxiosStartupCalls = axios => {
   const { startup_calls: startupCalls } = window.gl || {};
 
@@ -17,38 +43,28 @@ const setupAxiosStartupCalls = axios => {
     return;
   }
 
-  // TODO: To save performance of future axios calls, we can
-  // remove this interceptor once the "startupCalls" have been loaded
-  axios.interceptors.request.use(req => {
+  const remainingCalls = new Map(Object.entries(startupCalls));
+
+  const interceptor = axios.interceptors.request.use(async req => {
     const fullUrl = getFullUrl(req);
 
-    const existing = startupCalls[fullUrl];
-
-    if (existing) {
-      // eslint-disable-next-line no-param-reassign
-      req.adapter = () =>
-        existing.fetchCall.then(res => {
-          const fetchHeaders = {};
-          res.headers.forEach((val, key) => {
-            fetchHeaders[key] = val;
-          });
-
-          // We can delete it as it anyhow should only be called once
-          delete startupCalls[fullUrl];
-
-          // eslint-disable-next-line promise/no-nesting
-          return res
-            .clone()
-            .json()
-            .then(data => ({
-              data,
-              status: res.status,
-              statusText: res.statusText,
-              headers: fetchHeaders,
-              config: req,
-              request: req,
-            }));
-        });
+    const startupCall = remainingCalls.get(fullUrl);
+
+    if (!startupCall?.fetchCall) {
+      return req;
+    }
+
+    try {
+      await handleStartupCall(startupCall, req);
+    } catch (e) {
+      // eslint-disable-next-line no-console
+      console.warn(`[gitlab] Something went wrong with the startup call for "${fullUrl}"`, e);
+    }
+
+    remainingCalls.delete(fullUrl);
+
+    if (remainingCalls.size === 0) {
+      axios.interceptors.request.eject(interceptor);
     }
 
     return req;

spec/frontend/lib/utils/axios_startup_calls_spec.js

+import axios from 'axios';
+import MockAdapter from 'axios-mock-adapter';
+import setupAxiosStartupCalls from '~/lib/utils/axios_startup_calls';
+
+describe('setupAxiosStartupCalls', () => {
+  const AXIOS_RESPONSE = { text: 'AXIOS_RESPONSE' };
+  const STARTUP_JS_RESPONSE = { text: 'STARTUP_JS_RESPONSE' };
+  let mock;
+
+  function mockFetchCall(status) {
+    const p = {
+      ok: status >= 200 && status < 300,
+      status,
+      headers: new Headers({ 'Content-Type': 'application/json' }),
+      statusText: `MOCK-FETCH ${status}`,
+      clone: () => p,
+      json: () => Promise.resolve(STARTUP_JS_RESPONSE),
+    };
+    return Promise.resolve(p);
+  }
+
+  function mockConsoleWarn() {
+    jest.spyOn(console, 'warn').mockImplementation();
+  }
+
+  function expectConsoleWarn(path) {
+    // eslint-disable-next-line no-console
+    expect(console.warn).toHaveBeenCalledWith(expect.stringMatching(path), expect.any(Error));
+  }
+
+  beforeEach(() => {
+    window.gl = {};
+    mock = new MockAdapter(axios);
+    mock.onGet('/non-startup').reply(200, AXIOS_RESPONSE);
+    mock.onGet('/startup').reply(200, AXIOS_RESPONSE);
+    mock.onGet('/startup-failing').reply(200, AXIOS_RESPONSE);
+  });
+
+  afterEach(() => {
+    delete window.gl;
+    axios.interceptors.request.handlers = [];
+    mock.restore();
+  });
+
+  it('if no startupCalls are registered: does not register a request interceptor', () => {
+    setupAxiosStartupCalls(axios);
+
+    expect(axios.interceptors.request.handlers.length).toBe(0);
+  });
+
+  describe('if startupCalls are registered', () => {
+    beforeEach(() => {
+      window.gl.startup_calls = {
+        '/startup': {
+          fetchCall: mockFetchCall(200),
+        },
+        '/startup-failing': {
+          fetchCall: mockFetchCall(400),
+        },
+      };
+      setupAxiosStartupCalls(axios);
+    });
+
+    it('registers a request interceptor', () => {
+      expect(axios.interceptors.request.handlers.length).toBe(1);
+    });
+
+    it('detaches the request interceptor if every startup call has been made', async () => {
+      expect(axios.interceptors.request.handlers[0]).not.toBeNull();
+
+      await axios.get('/startup');
+      mockConsoleWarn();
+      await axios.get('/startup-failing');
+
+      // Axios sets the interceptor to null
+      expect(axios.interceptors.request.handlers[0]).toBeNull();
+    });
+
+    it('delegates to startup calls if URL is registered and call is successful', async () => {
+      const { headers, data, status, statusText } = await axios.get('/startup');
+
+      expect(headers).toEqual({ 'content-type': 'application/json' });
+      expect(status).toBe(200);
+      expect(statusText).toBe('MOCK-FETCH 200');
+      expect(data).toEqual(STARTUP_JS_RESPONSE);
+      expect(data).not.toEqual(AXIOS_RESPONSE);
+    });
+
+    it('delegates to startup calls exactly once', async () => {
+      await axios.get('/startup');
+      const { data } = await axios.get('/startup');
+
+      expect(data).not.toEqual(STARTUP_JS_RESPONSE);
+      expect(data).toEqual(AXIOS_RESPONSE);
+    });
+
+    it('does not delegate to startup calls if the call is failing', async () => {
+      mockConsoleWarn();
+      const { data } = await axios.get('/startup-failing');
+
+      expect(data).not.toEqual(STARTUP_JS_RESPONSE);
+      expect(data).toEqual(AXIOS_RESPONSE);
+      expectConsoleWarn('/startup-failing');
+    });
+
+    it('does not delegate to startup call if URL is not registered', async () => {
+      const { data } = await axios.get('/non-startup');
+
+      expect(data).toEqual(AXIOS_RESPONSE);
+      expect(data).not.toEqual(STARTUP_JS_RESPONSE);
+    });
+  });
+
+  it('removes GitLab Base URL from startup call', async () => {
+    const oldGon = window.gon;
+    window.gon = { gitlab_url: 'https://example.org/gitlab' };
+
+    window.gl.startup_calls = {
+      '/startup': {
+        fetchCall: mockFetchCall(200),
+      },
+    };
+    setupAxiosStartupCalls(axios);
+
+    const { data } = await axios.get('https://example.org/gitlab/startup');
+
+    expect(data).toEqual(STARTUP_JS_RESPONSE);
+
+    window.gon = oldGon;
+  });
+});