Merge branch 'tz-mr-incremental-rendering' into 'master'

MR Page incremental rendering

See merge request gitlab-org/gitlab-ce!21063

app/assets/javascripts/diffs/components/app.vue

@@ -114,11 +114,15 @@ export default {
     this.adjustView();
   },
   methods: {
-    ...mapActions('diffs', ['setBaseConfig', 'fetchDiffFiles']),
+    ...mapActions('diffs', ['setBaseConfig', 'fetchDiffFiles', 'startRenderDiffsQueue']),
     fetchData() {
-      this.fetchDiffFiles().catch(() => {
-        createFlash(__('Something went wrong on our end. Please try again!'));
-      });
+      this.fetchDiffFiles()
+        .then(() => {
+          requestIdleCallback(this.startRenderDiffsQueue, { timeout: 1000 });
+        })
+        .catch(() => {
+          createFlash(__('Something went wrong on our end. Please try again!'));
+        });
 
       if (!this.isNotesFetched) {
         eventHub.$emit('fetchNotesData');

app/assets/javascripts/diffs/components/diff_file.vue

@@ -46,16 +46,25 @@ export default {
     showExpandMessage() {
       return this.isCollapsed && !this.isLoadingCollapsedDiff && !this.file.tooLarge;
     },
+    showLoadingIcon() {
+      return this.isLoadingCollapsedDiff || (!this.file.renderIt && !this.isCollapsed);
+    },
   },
   methods: {
     ...mapActions('diffs', ['loadCollapsedDiff']),
     handleToggle() {
       const { collapsed, highlightedDiffLines, parallelDiffLines } = this.file;
 
-      if (collapsed && !highlightedDiffLines && !parallelDiffLines.length) {
+      if (
+        collapsed &&
+        !highlightedDiffLines &&
+        parallelDiffLines !== undefined &&
+        !parallelDiffLines.length
+      ) {
         this.handleLoadCollapsedDiff();
       } else {
         this.file.collapsed = !this.file.collapsed;
+        this.file.renderIt = true;
       }
     },
     handleLoadCollapsedDiff() {
@@ -65,6 +74,7 @@ export default {
         .then(() => {
           this.isLoadingCollapsedDiff = false;
           this.file.collapsed = false;
+          this.file.renderIt = true;
         })
         .catch(() => {
           this.isLoadingCollapsedDiff = false;
@@ -121,12 +131,12 @@ export default {
     </div>
 
     <diff-content
-      v-if="!isCollapsed"
+      v-if="!isCollapsed && file.renderIt"
       :class="{ hidden: isCollapsed || file.tooLarge }"
       :diff-file="file"
     />
     <loading-icon
-      v-if="isLoadingCollapsedDiff"
+      v-else-if="showLoadingIcon"
       class="diff-content loading"
     />
     <div

app/assets/javascripts/diffs/constants.js

@@ -25,3 +25,6 @@ export const CONTEXT_LINE_CLASS_NAME = 'diff-expanded';
 export const UNFOLD_COUNT = 20;
 export const COUNT_OF_AVATARS_IN_GUTTER = 3;
 export const LENGTH_OF_AVATAR_TOOLTIP = 17;
+
+export const LINES_TO_BE_RENDERED_DIRECTLY = 100;
+export const MAX_LINES_TO_BE_RENDERED = 2000;

app/assets/javascripts/diffs/store/actions.js

@@ -29,6 +29,27 @@ export const fetchDiffFiles = ({ state, commit }) => {
     .then(handleLocationHash);
 };
 
+export const startRenderDiffsQueue = ({ state, commit }) => {
+  const checkItem = () => {
+    const nextFile = state.diffFiles.find(
+      file => !file.renderIt && (!file.collapsed || !file.text),
+    );
+    if (nextFile) {
+      requestAnimationFrame(() => {
+        commit(types.RENDER_FILE, nextFile);
+      });
+      requestIdleCallback(
+        () => {
+          checkItem();
+        },
+        { timeout: 1000 },
+      );
+    }
+  };
+
+  checkItem();
+};
+
 export const setInlineDiffViewType = ({ commit }) => {
   commit(types.SET_DIFF_VIEW_TYPE, INLINE_DIFF_VIEW_TYPE);
 

app/assets/javascripts/diffs/store/mutation_types.js

@@ -8,3 +8,4 @@ export const REMOVE_COMMENT_FORM_LINE = 'REMOVE_COMMENT_FORM_LINE';
 export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES';
 export const ADD_COLLAPSED_DIFFS = 'ADD_COLLAPSED_DIFFS';
 export const EXPAND_ALL_FILES = 'EXPAND_ALL_FILES';
+export const RENDER_FILE = 'RENDER_FILE';

app/assets/javascripts/diffs/store/mutations.js

@@ -2,6 +2,7 @@ import Vue from 'vue';
 import _ from 'underscore';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
 import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } from './utils';
+import { LINES_TO_BE_RENDERED_DIRECTLY, MAX_LINES_TO_BE_RENDERED } from '../constants';
 import * as types from './mutation_types';
 
 export default {
@@ -15,8 +16,48 @@ export default {
   },
 
   [types.SET_DIFF_DATA](state, data) {
+    const diffData = convertObjectPropsToCamelCase(data, { deep: true });
+    let showingLines = 0;
+    const filesLength = diffData.diffFiles.length;
+    let i;
+    for (i = 0; i < filesLength; i += 1) {
+      const file = diffData.diffFiles[i];
+      if (file.parallelDiffLines) {
+        const linesLength = file.parallelDiffLines.length;
+        let u = 0;
+        for (u = 0; u < linesLength; u += 1) {
+          const line = file.parallelDiffLines[u];
+          if (line.left) delete line.left.text;
+          if (line.right) delete line.right.text;
+        }
+      }
+
+      if (file.highlightedDiffLines) {
+        const linesLength = file.highlightedDiffLines.length;
+        let u;
+        for (u = 0; u < linesLength; u += 1) {
+          const line = file.highlightedDiffLines[u];
+          delete line.text;
+        }
+      }
+
+      if (file.highlightedDiffLines) {
+        showingLines += file.parallelDiffLines.length;
+      }
+      Object.assign(file, {
+        renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY,
+        collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED,
+      });
+    }
+
     Object.assign(state, {
-      ...convertObjectPropsToCamelCase(data, { deep: true }),
+      ...diffData,
+    });
+  },
+
+  [types.RENDER_FILE](state, file) {
+    Object.assign(file, {
+      renderIt: true,
     });
   },
 

changelogs/unreleased/tz-mr-incremental-rendering.yml

+title: Incremental rendering with Vue on merge request page
+merge_request: 21063
+author:
+type: performance

spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb

@@ -28,7 +28,7 @@ describe 'Merge request > User sees MR with deleted source branch', :js do
     click_on 'Changes'
     wait_for_requests
 
-    expect(page).to have_selector('.diffs.tab-pane .nothing-here-block')
+    expect(page).to have_selector('.diffs.tab-pane .file-holder')
     expect(page).to have_content('Source branch does not exist.')
   end
 end

spec/javascripts/diffs/components/diff_file_spec.js

@@ -22,11 +22,18 @@ describe('DiffFile', () => {
 
       expect(el.id).toEqual(fileHash);
       expect(el.classList.contains('diff-file')).toEqual(true);
+
       expect(el.querySelectorAll('.diff-content.hidden').length).toEqual(0);
       expect(el.querySelector('.js-file-title')).toBeDefined();
       expect(el.querySelector('.file-title-name').innerText.indexOf(filePath) > -1).toEqual(true);
       expect(el.querySelector('.js-syntax-highlight')).toBeDefined();
-      expect(el.querySelectorAll('.line_content').length > 5).toEqual(true);
+
+      expect(vm.file.renderIt).toEqual(false);
+      vm.file.renderIt = true;
+
+      vm.$nextTick(() => {
+        expect(el.querySelectorAll('.line_content').length > 5).toEqual(true);
+      });
     });
 
     describe('collapsed', () => {
@@ -34,6 +41,7 @@ describe('DiffFile', () => {
         expect(vm.$el.querySelectorAll('.diff-content').length).toEqual(1);
         expect(vm.file.collapsed).toEqual(false);
         vm.file.collapsed = true;
+        vm.file.renderIt = true;
 
         vm.$nextTick(() => {
           expect(vm.$el.querySelectorAll('.diff-content').length).toEqual(0);

spec/javascripts/diffs/mock_data/diff_file.js

@@ -39,6 +39,7 @@ export default {
   viewPath: '/gitlab-org/gitlab-test/blob/spooky-stuff/CHANGELOG',
   replacedViewPath: null,
   collapsed: false,
+  renderIt: false,
   tooLarge: false,
   contextLinesPath:
     '/gitlab-org/gitlab-test/blob/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG/diff',

spec/javascripts/diffs/store/mutations_spec.js

 import mutations from '~/diffs/store/mutations';
 import * as types from '~/diffs/store/mutation_types';
 import { INLINE_DIFF_VIEW_TYPE } from '~/diffs/constants';
+import diffFileMockData from '../mock_data/diff_file';
 
 describe('DiffsStoreMutations', () => {
   describe('SET_BASE_CONFIG', () => {
@@ -24,6 +25,23 @@ describe('DiffsStoreMutations', () => {
     });
   });
 
+  describe('SET_DIFF_DATA', () => {
+    it('should set diff data type properly', () => {
+      const state = {};
+      const diffMock = {
+        diff_files: [diffFileMockData],
+      };
+
+      mutations[types.SET_DIFF_DATA](state, diffMock);
+
+      const firstLine = state.diffFiles[0].parallelDiffLines[0];
+
+      expect(firstLine.right.text).toBeUndefined();
+      expect(state.diffFiles[0].renderIt).toEqual(true);
+      expect(state.diffFiles[0].collapsed).toEqual(false);
+    });
+  });
+
   describe('SET_DIFF_VIEW_TYPE', () => {
     it('should set diff view type properly', () => {
       const state = {};