diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 35d1a564178f81ee771f6ef24ec691ab032ed8b0..c8ba4a1676da24933b740885f56ad41aadab248a 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -21,6 +21,7 @@ import PanelResizer from '~/vue_shared/components/panel_resizer.vue'; import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import notesEventHub from '~/notes/event_hub'; +import { DynamicScroller, DynamicScrollerItem } from 'vendor/vue-virtual-scroller'; import { TREE_LIST_WIDTH_STORAGE_KEY, INITIAL_TREE_WIDTH, @@ -53,15 +54,14 @@ import HiddenFilesWarning from './hidden_files_warning.vue'; import NoChanges from './no_changes.vue'; import TreeList from './tree_list.vue'; import VirtualScrollerScrollSync from './virtual_scroller_scroll_sync'; +import PreRenderer from './pre_renderer.vue'; export default { name: 'DiffsApp', components: { - DynamicScroller: () => - import('vendor/vue-virtual-scroller').then(({ DynamicScroller }) => DynamicScroller), - DynamicScrollerItem: () => - import('vendor/vue-virtual-scroller').then(({ DynamicScrollerItem }) => DynamicScrollerItem), - PreRenderer: () => import('./pre_renderer.vue').then((PreRenderer) => PreRenderer), + DynamicScroller, + DynamicScrollerItem, + PreRenderer, VirtualScrollerScrollSync, CompareVersions, DiffFile, diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 9f90de9abde40cc009ddb3e27cf83613af33bdb1..9d7a540570e0c170c49fa2a01d627542e33f41e4 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -14,6 +14,8 @@ import Poll from '~/lib/utils/poll'; import { mergeUrlParams, getLocationHash } from '~/lib/utils/url_utility'; import { __, s__ } from '~/locale'; import notesEventHub from '~/notes/event_hub'; +import { generateTreeList } from '~/diffs/utils/tree_worker_utils'; +import { sortTree } from '~/ide/stores/utils'; import { PARALLEL_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE, @@ -52,7 +54,6 @@ import { isCollapsed } from '../utils/diff_file'; import { markFileReview, setReviewsForMergeRequest } from '../utils/file_reviews'; import { getDerivedMergeRequestInformation } from '../utils/merge_request'; import { queueRedisHllEvents } from '../utils/queue_events'; -import TreeWorker from '../workers/tree_worker?worker'; import * as types from './mutation_types'; import { getDiffPositionByLineCode, @@ -199,21 +200,12 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => { }; export const fetchDiffFilesMeta = ({ commit, state }) => { - const worker = new TreeWorker(); const urlParams = { view: 'inline', w: state.showWhitespace ? '0' : '1', }; commit(types.SET_LOADING, true); - eventHub.$emit(EVT_PERF_MARK_FILE_TREE_START); - - worker.addEventListener('message', ({ data }) => { - commit(types.SET_TREE_DATA, data); - eventHub.$emit(EVT_PERF_MARK_FILE_TREE_END); - - worker.terminate(); - }); return axios .get(mergeUrlParams(urlParams, state.endpointMetadata)) @@ -225,18 +217,24 @@ export const fetchDiffFilesMeta = ({ commit, state }) => { commit(types.SET_MERGE_REQUEST_DIFFS, data.merge_request_diffs || []); commit(types.SET_DIFF_METADATA, strippedData); - worker.postMessage(data.diff_files); + eventHub.$emit(EVT_PERF_MARK_FILE_TREE_START); + const { treeEntries, tree } = generateTreeList(data.diff_files); + eventHub.$emit(EVT_PERF_MARK_FILE_TREE_END); + commit(types.SET_TREE_DATA, { + treeEntries, + tree: sortTree(tree), + }); return data; }) .catch((error) => { - worker.terminate(); - if (error.response.status === HTTP_STATUS_NOT_FOUND) { createAlert({ message: __('Building your merge request. Wait a few moments, then refresh this page.'), variant: VARIANT_WARNING, }); + } else { + throw error; } }); }; diff --git a/app/assets/javascripts/diffs/workers/tree_worker.js b/app/assets/javascripts/diffs/workers/tree_worker.js deleted file mode 100644 index 04010a99b52df02fb39541bf537d8e9213512029..0000000000000000000000000000000000000000 --- a/app/assets/javascripts/diffs/workers/tree_worker.js +++ /dev/null @@ -1,19 +0,0 @@ -import { sortTree } from '~/ide/stores/utils'; -import { generateTreeList } from '../utils/tree_worker_utils'; - -// eslint-disable-next-line no-restricted-globals -self.addEventListener('message', (e) => { - const { data } = e; - - if (data === undefined) { - return; - } - - const { treeEntries, tree } = generateTreeList(data); - - // eslint-disable-next-line no-restricted-globals - self.postMessage({ - treeEntries, - tree: sortTree(tree), - }); -}); diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js index 513e67ea247857e679b59f21390fa43b0486afa9..3048a40809ead1cc4c59f79a3b675dc33e882b66 100644 --- a/spec/frontend/diffs/components/app_spec.js +++ b/spec/frontend/diffs/components/app_spec.js @@ -71,12 +71,6 @@ describe('diffs/components/app', () => { }, provide, store, - stubs: { - DynamicScroller: { - template: `
`, - }, - DynamicScrollerItem: true, - }, }); } @@ -294,13 +288,13 @@ describe('diffs/components/app', () => { it('does not render empty state when diff files exist', () => { createComponent({}, ({ state }) => { - state.diffs.diffFiles.push({ - id: 1, - }); + state.diffs.diffFiles.push({ id: 1 }); }); expect(wrapper.findComponent(NoChanges).exists()).toBe(false); - expect(wrapper.findAllComponents(DiffFile).length).toBe(1); + expect(wrapper.findComponent({ name: 'DynamicScroller' }).props('items')).toBe( + store.state.diffs.diffFiles, + ); }); }); @@ -581,7 +575,10 @@ describe('diffs/components/app', () => { state.diffs.diffFiles.push({ sha: '123' }); }); - expect(wrapper.findComponent(DiffFile).exists()).toBe(true); + expect(wrapper.findComponent({ name: 'DynamicScroller' }).exists()).toBe(true); + expect(wrapper.findComponent({ name: 'DynamicScroller' }).props('items')).toBe( + store.state.diffs.diffFiles, + ); }); it("doesn't render tree list when no changes exist", () => { diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index 78765204322d9faba4c6ced5affe0af7bfd2ccac..614742d026f349d8babaa3eb694e03c11add5c97 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -236,13 +236,17 @@ describe('DiffsStoreActions', () => { it('should show no warning on any other status code', async () => { mock.onGet(endpointMetadata).reply(HTTP_STATUS_INTERNAL_SERVER_ERROR); - await testAction( - diffActions.fetchDiffFilesMeta, - {}, - { endpointMetadata, diffViewType: 'inline', showWhitespace: true }, - [{ type: types.SET_LOADING, payload: true }], - [], - ); + try { + await testAction( + diffActions.fetchDiffFilesMeta, + {}, + { endpointMetadata, diffViewType: 'inline', showWhitespace: true }, + [{ type: types.SET_LOADING, payload: true }], + [], + ); + } catch (error) { + expect(error.response.status).toBe(HTTP_STATUS_INTERNAL_SERVER_ERROR); + } expect(createAlert).not.toHaveBeenCalled(); });