Merge branch '208800-step-1-break-store-and-router-cyclical-dependency' into 'master'

Step 1.2 - Break  IDE store and router cyclical dependency

Closes #28717

See merge request gitlab-org/gitlab!33366

app/assets/javascripts/ide/ide_router.js

@@ -2,8 +2,8 @@ import Vue from 'vue';
 import IdeRouter from '~/ide/ide_router_extension';
 import { joinPaths } from '~/lib/utils/url_utility';
 import flash from '~/flash';
-import store from './stores';
 import { __ } from '~/locale';
+import { syncRouterAndStore } from './sync_router_and_store';
 
 Vue.use(IdeRouter);
 
@@ -33,7 +33,9 @@ const EmptyRouterComponent = {
   },
 };
 
-const router = new IdeRouter({
+// eslint-disable-next-line import/prefer-default-export
+export const createRouter = store => {
+  const router = new IdeRouter({
     mode: 'history',
     base: joinPaths(gon.relative_url_root || '', '/-/ide/'),
     routes: [
@@ -64,9 +66,9 @@ const router = new IdeRouter({
         ],
       },
     ],
-});
+  });
 
-router.beforeEach((to, from, next) => {
+  router.beforeEach((to, from, next) => {
     if (to.params.namespace && to.params.project) {
       store
         .dispatch('getProjectData', {
@@ -107,6 +109,9 @@ router.beforeEach((to, from, next) => {
     }
 
     next();
-});
+  });
 
-export default router;
+  syncRouterAndStore(router, store);
+
+  return router;
+};

app/assets/javascripts/ide/index.js

@@ -4,7 +4,7 @@ import Translate from '~/vue_shared/translate';
 import { identity } from 'lodash';
 import ide from './components/ide.vue';
 import store from './stores';
-import router from './ide_router';
+import { createRouter } from './ide_router';
 import { parseBoolean } from '../lib/utils/common_utils';
 import { resetServiceWorkersPublicPath } from '../lib/utils/webpack';
 import { DEFAULT_THEME } from './lib/themes';
@@ -32,6 +32,7 @@ export function initIde(el, options = {}) {
   if (!el) return null;
 
   const { rootComponent = ide, extendStore = identity } = options;
+  const router = createRouter(store);
 
   return new Vue({
     el,

app/assets/javascripts/ide/stores/actions.js

@@ -7,7 +7,6 @@ import * as types from './mutation_types';
 import { decorateFiles } from '../lib/files';
 import { stageKeys } from '../constants';
 import service from '../services';
-import router from '../ide_router';
 import eventHub from '../eventhub';
 
 export const redirectToUrl = (self, url) => visitUrl(url);
@@ -262,7 +261,7 @@ export const renameEntry = ({ dispatch, commit, state, getters }, { path, name,
     }
 
     if (newEntry.opened) {
-      router.push(`/project${newEntry.url}`);
+      dispatch('router/push', `/project${newEntry.url}`, { root: true });
     }
   }
 

app/assets/javascripts/ide/stores/actions/file.js

@@ -3,7 +3,6 @@ import { __ } from '~/locale';
 import eventHub from '../../eventhub';
 import service from '../../services';
 import * as types from '../mutation_types';
-import router from '../../ide_router';
 import { setPageTitleForFile } from '../utils';
 import { viewerTypes, stageKeys } from '../../constants';
 
@@ -30,10 +29,10 @@ export const closeFile = ({ commit, state, dispatch }, file) => {
         keyPrefix: nextFileToOpen.staged ? 'staged' : 'unstaged',
       });
     } else {
-      router.push(`/project${nextFileToOpen.url}`);
+      dispatch('router/push', `/project${nextFileToOpen.url}`, { root: true });
     }
   } else if (!state.openFiles.length) {
-    router.push(`/project/${file.projectId}/tree/${file.branchId}/`);
+    dispatch('router/push', `/project/${file.projectId}/tree/${file.branchId}/`, { root: true });
   }
 
   eventHub.$emit(`editor.update.model.dispose.${file.key}`);
@@ -226,7 +225,7 @@ export const discardFileChanges = ({ dispatch, state, commit, getters }, path) =
   if (!isDestructiveDiscard && file.path === getters.activeFile?.path) {
     dispatch('updateDelayViewerUpdated', true)
       .then(() => {
-        router.push(`/project${file.url}`);
+        dispatch('router/push', `/project${file.url}`, { root: true });
       })
       .catch(e => {
         throw e;
@@ -275,14 +274,16 @@ export const unstageChange = ({ commit, dispatch, getters }, path) => {
   }
 };
 
-export const openPendingTab = ({ commit, getters, state }, { file, keyPrefix }) => {
+export const openPendingTab = ({ commit, dispatch, getters, state }, { file, keyPrefix }) => {
   if (getters.activeFile && getters.activeFile.key === `${keyPrefix}-${file.key}`) return false;
 
   state.openFiles.forEach(f => eventHub.$emit(`editor.update.model.dispose.${f.key}`));
 
   commit(types.ADD_PENDING_TAB, { file, keyPrefix });
 
-  router.push(`/project/${file.projectId}/tree/${state.currentBranchId}/`);
+  dispatch('router/push', `/project/${file.projectId}/tree/${state.currentBranchId}/`, {
+    root: true,
+  });
 
   return true;
 };

app/assets/javascripts/ide/stores/actions/project.js

@@ -4,7 +4,6 @@ import { __, sprintf } from '~/locale';
 import service from '../../services';
 import api from '../../../api';
 import * as types from '../mutation_types';
-import router from '../../ide_router';
 
 export const getProjectData = ({ commit, state }, { namespace, projectId, force = false } = {}) =>
   new Promise((resolve, reject) => {
@@ -57,7 +56,7 @@ export const createNewBranchFromDefault = ({ state, dispatch, getters }, branch)
     })
     .then(() => {
       dispatch('setErrorMessage', null);
-      router.push(`${router.currentRoute.path}?${Date.now()}`);
+      window.location.reload();
     })
     .catch(() => {
       dispatch('setErrorMessage', {

app/assets/javascripts/ide/stores/index.js

@@ -11,6 +11,7 @@ import branches from './modules/branches';
 import fileTemplates from './modules/file_templates';
 import paneModule from './modules/pane';
 import clientsideModule from './modules/clientside';
+import routerModule from './modules/router';
 
 Vue.use(Vuex);
 
@@ -28,6 +29,7 @@ export const createStore = () =>
       fileTemplates: fileTemplates(),
       rightPane: paneModule(),
       clientside: clientsideModule(),
+      router: routerModule,
     },
   });
 

app/assets/javascripts/ide/stores/modules/branches/index.js

@@ -4,7 +4,7 @@ import mutations from './mutations';
 
 export default {
   namespaced: true,
-  state: state(),
+  state,
   actions,
   mutations,
 };

app/assets/javascripts/ide/stores/modules/commit/actions.js

@@ -3,7 +3,6 @@ import flash from '~/flash';
 import httpStatusCodes from '~/lib/utils/http_status';
 import * as rootTypes from '../../mutation_types';
 import { createCommitPayload, createNewMergeRequestUrl } from '../../utils';
-import router from '../../../ide_router';
 import service from '../../../services';
 import * as types from './mutation_types';
 import consts from './constants';
@@ -196,8 +195,10 @@ export const commitChanges = ({ commit, state, getters, dispatch, rootState, roo
             dispatch('updateViewer', 'editor', { root: true });
 
             if (rootGetters.activeFile) {
-              router.push(
+              dispatch(
+                'router/push',
                 `/project/${rootState.currentProjectId}/blob/${branchName}/-/${rootGetters.activeFile.path}`,
+                { root: true },
               );
             }
           }

app/assets/javascripts/ide/stores/modules/commit/index.js

@@ -5,7 +5,7 @@ import * as getters from './getters';
 
 export default {
   namespaced: true,
-  state: state(),
+  state,
   mutations,
   actions,
   getters,

spec/frontend/ide/components/branches/item_spec.js

 import { shallowMount } from '@vue/test-utils';
-import router from '~/ide/ide_router';
+import { createStore } from '~/ide/stores';
+import { createRouter } from '~/ide/ide_router';
 import Item from '~/ide/components/branches/item.vue';
 import Icon from '~/vue_shared/components/icon.vue';
 import Timeago from '~/vue_shared/components/time_ago_tooltip.vue';
@@ -13,6 +14,8 @@ const TEST_PROJECT_ID = projectData.name_with_namespace;
 
 describe('IDE branch item', () => {
   let wrapper;
+  let store;
+  let router;
 
   function createComponent(props = {}) {
     wrapper = shallowMount(Item, {
@@ -26,6 +29,11 @@ describe('IDE branch item', () => {
     });
   }
 
+  beforeEach(() => {
+    store = createStore();
+    router = createRouter(store);
+  });
+
   afterEach(() => {
     wrapper.destroy();
   });

spec/frontend/ide/components/commit_sidebar/list_item_spec.js

 import Vue from 'vue';
 import { trimText } from 'helpers/text_helper';
 import { createComponentWithStore } from 'helpers/vue_mount_component_helper';
-import store from '~/ide/stores';
+import { createStore } from '~/ide/stores';
 import listItem from '~/ide/components/commit_sidebar/list_item.vue';
-import router from '~/ide/ide_router';
-import { file, resetStore } from '../../helpers';
+import { createRouter } from '~/ide/ide_router';
+import { file } from '../../helpers';
 
 describe('Multi-file editor commit sidebar list item', () => {
   let vm;
   let f;
   let findPathEl;
+  let store;
+  let router;
 
   beforeEach(() => {
+    store = createStore();
+    router = createRouter(store);
+
     const Component = Vue.extend(listItem);
 
     f = file('test-file');
@@ -28,8 +33,6 @@ describe('Multi-file editor commit sidebar list item', () => {
 
   afterEach(() => {
     vm.$destroy();
-
-    resetStore(store);
   });
 
   const findPathText = () => trimText(findPathEl.textContent);

spec/frontend/ide/components/merge_requests/item_spec.js

-import { mount } from '@vue/test-utils';
-import router from '~/ide/ide_router';
+import Vuex from 'vuex';
+import { mount, createLocalVue } from '@vue/test-utils';
+import { createStore } from '~/ide/stores';
+import { createRouter } from '~/ide/ide_router';
 import Item from '~/ide/components/merge_requests/item.vue';
 
 const TEST_ITEM = {
@@ -9,7 +11,12 @@ const TEST_ITEM = {
 };
 
 describe('IDE merge request item', () => {
+  const localVue = createLocalVue();
+  localVue.use(Vuex);
+
   let wrapper;
+  let store;
+  let router;
 
   const createComponent = (props = {}) => {
     wrapper = mount(Item, {
@@ -21,11 +28,18 @@ describe('IDE merge request item', () => {
         currentProjectId: TEST_ITEM.projectPathWithNamespace,
         ...props,
       },
+      localVue,
       router,
+      store,
     });
   };
   const findIcon = () => wrapper.find('.ic-mobile-issue-close');
 
+  beforeEach(() => {
+    store = createStore();
+    router = createRouter(store);
+  });
+
   afterEach(() => {
     wrapper.destroy();
     wrapper = null;

spec/frontend/ide/components/repo_commit_section_spec.js

 import { mount } from '@vue/test-utils';
 import { createStore } from '~/ide/stores';
-import router from '~/ide/ide_router';
+import { createRouter } from '~/ide/ide_router';
 import RepoCommitSection from '~/ide/components/repo_commit_section.vue';
 import EmptyState from '~/ide/components/commit_sidebar/empty_state.vue';
 import { stageKeys } from '~/ide/constants';
@@ -10,6 +10,7 @@ const TEST_NO_CHANGES_SVG = 'nochangessvg';
 
 describe('RepoCommitSection', () => {
   let wrapper;
+  let router;
   let store;
 
   function createComponent() {
@@ -55,6 +56,7 @@ describe('RepoCommitSection', () => {
 
   beforeEach(() => {
     store = createStore();
+    router = createRouter(store);
 
     jest.spyOn(store, 'dispatch');
     jest.spyOn(router, 'push').mockImplementation();

spec/frontend/ide/components/repo_tab_spec.js

 import Vue from 'vue';
-import store from '~/ide/stores';
+import { createStore } from '~/ide/stores';
 import repoTab from '~/ide/components/repo_tab.vue';
-import router from '~/ide/ide_router';
-import { file, resetStore } from '../helpers';
+import { createRouter } from '~/ide/ide_router';
+import { file } from '../helpers';
 
 describe('RepoTab', () => {
   let vm;
+  let store;
+  let router;
 
   function createComponent(propsData) {
     const RepoTab = Vue.extend(repoTab);
@@ -17,13 +19,13 @@ describe('RepoTab', () => {
   }
 
   beforeEach(() => {
+    store = createStore();
+    router = createRouter(store);
     jest.spyOn(router, 'push').mockImplementation(() => {});
   });
 
   afterEach(() => {
     vm.$destroy();
-
-    resetStore(vm.$store);
   });
 
   it('renders a close link and a name link', () => {

spec/frontend/ide/ide_router_spec.js

-import router from '~/ide/ide_router';
-import store from '~/ide/stores';
+import { createRouter } from '~/ide/ide_router';
+import { createStore } from '~/ide/stores';
+import waitForPromises from 'helpers/wait_for_promises';
 
 describe('IDE router', () => {
   const PROJECT_NAMESPACE = 'my-group/sub-group';
   const PROJECT_NAME = 'my-project';
+  const TEST_PATH = `/project/${PROJECT_NAMESPACE}/${PROJECT_NAME}/merge_requests/2`;
 
-  afterEach(() => {
-    router.push('/');
-  });
+  let store;
+  let router;
 
-  afterAll(() => {
-    // VueRouter leaves this window.history at the "base" url. We need to clean this up.
+  beforeEach(() => {
     window.history.replaceState({}, '', '/');
+    store = createStore();
+    router = createRouter(store);
+    jest.spyOn(store, 'dispatch').mockReturnValue(new Promise(() => {}));
   });
 
   [
@@ -31,8 +34,6 @@ describe('IDE router', () => {
     `/project/${PROJECT_NAMESPACE}/${PROJECT_NAME}`,
   ].forEach(route => {
     it(`finds project path when route is "${route}"`, () => {
-      jest.spyOn(store, 'dispatch').mockReturnValue(new Promise(() => {}));
-
       router.push(route);
 
       expect(store.dispatch).toHaveBeenCalledWith('getProjectData', {
@@ -41,4 +42,22 @@ describe('IDE router', () => {
       });
     });
   });
+
+  it('keeps router in sync when store changes', async () => {
+    expect(router.currentRoute.fullPath).toBe('/');
+
+    store.state.router.fullPath = TEST_PATH;
+
+    await waitForPromises();
+
+    expect(router.currentRoute.fullPath).toBe(TEST_PATH);
+  });
+
+  it('keeps store in sync when router changes', () => {
+    expect(store.dispatch).not.toHaveBeenCalled();
+
+    router.push(TEST_PATH);
+
+    expect(store.dispatch).toHaveBeenCalledWith('router/push', TEST_PATH, { root: true });
+  });
 });

spec/frontend/ide/stores/actions/file_spec.js

@@ -5,7 +5,7 @@ import { createStore } from '~/ide/stores';
 import * as actions from '~/ide/stores/actions/file';
 import * as types from '~/ide/stores/mutation_types';
 import service from '~/ide/services';
-import router from '~/ide/ide_router';
+import { createRouter } from '~/ide/ide_router';
 import eventHub from '~/ide/eventhub';
 import { file } from '../../helpers';
 
@@ -16,6 +16,7 @@ describe('IDE store file actions', () => {
   let mock;
   let originalGon;
   let store;
+  let router;
 
   beforeEach(() => {
     mock = new MockAdapter(axios);
@@ -26,6 +27,7 @@ describe('IDE store file actions', () => {
     };
 
     store = createStore();
+    router = createRouter(store);
 
     jest.spyOn(store, 'commit');
     jest.spyOn(store, 'dispatch');

spec/frontend/ide/stores/actions/project_spec.js

 import MockAdapter from 'axios-mock-adapter';
 import axios from '~/lib/utils/axios_utils';
 import { createStore } from '~/ide/stores';
-import router from '~/ide/ide_router';
 import {
   refreshLastCommitData,
   showBranchNotFoundError,
@@ -13,7 +12,6 @@ import {
 } from '~/ide/stores/actions';
 import service from '~/ide/services';
 import api from '~/api';
-import { resetStore } from '../../helpers';
 import testAction from '../../../helpers/vuex_action_helper';
 
 const TEST_PROJECT_ID = 'abc/def';
@@ -33,8 +31,6 @@ describe('IDE store project actions', () => {
 
   afterEach(() => {
     mock.restore();
-
-    resetStore(store);
   });
 
   describe('refreshLastCommitData', () => {
@@ -122,7 +118,6 @@ describe('IDE store project actions', () => {
   describe('createNewBranchFromDefault', () => {
     beforeEach(() => {
       jest.spyOn(api, 'createBranch').mockResolvedValue();
-      jest.spyOn(router, 'push').mockImplementation();
     });
 
     it('calls API', done => {
@@ -175,6 +170,8 @@ describe('IDE store project actions', () => {
     });
 
     it('reloads window', done => {
+      jest.spyOn(window.location, 'reload').mockImplementation();
+
       createNewBranchFromDefault(
         {
           state: {
@@ -190,7 +187,7 @@ describe('IDE store project actions', () => {
         'new-branch-name',
       )
         .then(() => {
-          expect(router.push).toHaveBeenCalled();
+          expect(window.location.reload).toHaveBeenCalled();
         })
         .then(done)
         .catch(done.fail);

spec/frontend/ide/stores/actions/tree_spec.js

@@ -3,14 +3,16 @@ import testAction from 'helpers/vuex_action_helper';
 import { showTreeEntry, getFiles, setDirectoryData } from '~/ide/stores/actions/tree';
 import * as types from '~/ide/stores/mutation_types';
 import axios from '~/lib/utils/axios_utils';
-import store from '~/ide/stores';
+import { createStore } from '~/ide/stores';
 import service from '~/ide/services';
-import router from '~/ide/ide_router';
-import { file, resetStore, createEntriesFromPaths } from '../../helpers';
+import { createRouter } from '~/ide/ide_router';
+import { file, createEntriesFromPaths } from '../../helpers';
 
 describe('Multi-file store tree actions', () => {
   let projectTree;
   let mock;
+  let store;
+  let router;
 
   const basicCallParameters = {
     endpoint: 'rootEndpoint',
@@ -21,6 +23,8 @@ describe('Multi-file store tree actions', () => {
   };
 
   beforeEach(() => {
+    store = createStore();
+    router = createRouter(store);
     jest.spyOn(router, 'push').mockImplementation();
 
     mock = new MockAdapter(axios);
@@ -35,7 +39,6 @@ describe('Multi-file store tree actions', () => {
 
   afterEach(() => {
     mock.restore();
-    resetStore(store);
   });
 
   describe('getFiles', () => {

spec/frontend/ide/stores/actions_spec.js

 import MockAdapter from 'axios-mock-adapter';
 import { visitUrl } from '~/lib/utils/url_utility';
 import { createStore } from '~/ide/stores';
-import router from '~/ide/ide_router';
+import { createRouter } from '~/ide/ide_router';
 import {
   stageAllChanges,
   unstageAllChanges,
@@ -30,9 +30,11 @@ jest.mock('~/lib/utils/url_utility', () => ({
 
 describe('Multi-file store actions', () => {
   let store;
+  let router;
 
   beforeEach(() => {
     store = createStore();
+    router = createRouter(store);
 
     jest.spyOn(store, 'commit');
     jest.spyOn(store, 'dispatch');
@@ -339,10 +341,12 @@ describe('Multi-file store actions', () => {
       it('adds all files from changedFiles to stagedFiles', () => {
         stageAllChanges(store);
 
-        expect(store.commit.mock.calls).toEqual([
+        expect(store.commit.mock.calls).toEqual(
+          expect.arrayContaining([
             [types.SET_LAST_COMMIT_MSG, ''],
             [types.STAGE_CHANGE, expect.objectContaining({ path: file1.path })],
-        ]);
+          ]),
+        );
       });
 
       it('opens pending tab if a change exists in that file', () => {
@@ -371,9 +375,11 @@ describe('Multi-file store actions', () => {
       it('removes all files from stagedFiles after unstaging', () => {
         unstageAllChanges(store);
 
-        expect(store.commit.mock.calls).toEqual([
+        expect(store.commit.mock.calls).toEqual(
+          expect.arrayContaining([
             [types.UNSTAGE_CHANGE, expect.objectContaining({ path: file2.path })],
-        ]);
+          ]),
+        );
       });
 
       it('opens pending tab if a change exists in that file', () => {

spec/frontend/ide/stores/modules/commit/actions_spec.js

-import { resetStore, file } from 'jest/ide/helpers';
+import { file } from 'jest/ide/helpers';
 import axios from 'axios';
 import MockAdapter from 'axios-mock-adapter';
 import { visitUrl } from '~/lib/utils/url_utility';
 import { createStore } from '~/ide/stores';
 import service from '~/ide/services';
-import router from '~/ide/ide_router';
+import { createRouter } from '~/ide/ide_router';
 import eventHub from '~/ide/eventhub';
 import consts from '~/ide/stores/modules/commit/constants';
 import * as mutationTypes from '~/ide/stores/modules/commit/mutation_types';
@@ -18,12 +18,15 @@ jest.mock('~/lib/utils/url_utility', () => ({
 }));
 
 const TEST_COMMIT_SHA = '123456789';
-const store = createStore();
 
 describe('IDE commit module actions', () => {
   let mock;
+  let store;
+  let router;
 
   beforeEach(() => {
+    store = createStore();
+    router = createRouter(store);
     gon.api_version = 'v1';
     mock = new MockAdapter(axios);
     jest.spyOn(router, 'push').mockImplementation();
@@ -34,7 +37,6 @@ describe('IDE commit module actions', () => {
   afterEach(() => {
     delete gon.api_version;
     mock.restore();
-    resetStore(store);
   });
 
   describe('updateCommitMessage', () => {

spec/frontend/ide/sync_router_and_store_spec.js

-import Vue from 'vue';
 import VueRouter from 'vue-router';
-import Vuex from 'vuex';
-import routerModule from '~/ide/stores/modules/router';
+import { createStore } from '~/ide/stores';
 import { syncRouterAndStore } from '~/ide/sync_router_and_store';
 import waitForPromises from 'helpers/wait_for_promises';
 
 const TEST_ROUTE = '/test/lorem/ipsum';
 
-Vue.use(Vuex);
-
 describe('~/ide/sync_router_and_store', () => {
   let unsync;
   let router;
@@ -32,11 +28,7 @@ describe('~/ide/sync_router_and_store', () => {
 
   beforeEach(() => {
     router = new VueRouter();
-    store = new Vuex.Store({
-      modules: {
-        router: routerModule,
-      },
-    });
+    store = createStore();
     jest.spyOn(store, 'dispatch');
 
     onRouterChange = jest.fn();