root
/
gitlab-ce
mirror of https://gitlab.com/free-releases/gitlab-ce.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Porting MR Vue Memory Fixes to current master

This commit is contained in:
Tim Zallmann 2018-08-08 02:50:01 +00:00 committed by Mike Greiling
parent b507d93a3b
commit 8daf9db68f
18 changed files with 303 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/assets/javascripts/commons/polyfills.js
View File

@ -19,3 +19,4 @@ import './polyfills/custom_event';
import './polyfills/element';
import './polyfills/event';
import './polyfills/nodelist';
import './polyfills/request_idle_callback';

17
app/assets/javascripts/commons/polyfills/request_idle_callback.js Normal file
View File

@ -0,0 +1,17 @@
window.requestIdleCallback =
window.requestIdleCallback ||
function requestShim(cb) {
const start = Date.now();
return setTimeout(() => {
cb({
didTimeout: false,
timeRemaining: () => Math.max(0, 50 - (Date.now() - start)),
});
}, 1);
};
window.cancelIdleCallback =
window.cancelIdleCallback ||
function cancelShim(id) {
clearTimeout(id);
};

28
app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
View File

@ -71,13 +71,23 @@ export default {
required: false,
default: false,
},
isHover: {
type: Boolean,
required: false,
default: false,
},
discussions: {
type: Array,
required: false,
default: () => [],
},
},
computed: {
...mapState({
diffViewType: state => state.diffs.diffViewType,
diffFiles: state => state.diffs.diffFiles,
}),
...mapGetters(['isLoggedIn', 'discussionsByLineCode']),
...mapGetters(['isLoggedIn']),
lineHref() {
return this.lineCode ? `#${this.lineCode}` : '#';
},
@ -85,26 +95,22 @@ export default {
return (
this.isLoggedIn &&
this.showCommentButton &&
this.isHover &&
!this.isMatchLine &&
!this.isContextLine &&
!this.hasDiscussions &&
!this.isMetaLine
!this.isMetaLine &&
!this.hasDiscussions
);
},
discussions() {
return this.discussionsByLineCode[this.lineCode] || [];
},
hasDiscussions() {
return this.discussions.length > 0;
},
shouldShowAvatarsOnGutter() {
let render = this.hasDiscussions && this.showCommentButton;
if (!this.lineType && this.linePosition === LINE_POSITION_RIGHT) {
render = false;
return false;
}
return render;
return this.showCommentButton && this.hasDiscussions;
},
},
methods: {
@ -176,7 +182,7 @@ export default {
v-else
>
<button
v-show="shouldShowCommentButton"
v-if="shouldShowCommentButton"
type="button"
class="add-diff-note js-add-diff-note-button"
title="Add a comment to this line"

7
app/assets/javascripts/diffs/components/diff_table_cell.vue
View File

@ -67,6 +67,11 @@ export default {
required: false,
default: false,
},
discussions: {
type: Array,
required: false,
default: () => [],
},
},
computed: {
...mapGetters(['isLoggedIn']),
@ -132,10 +137,12 @@ export default {
:line-number="lineNumber"
:meta-data="normalizedLine.metaData"
:show-comment-button="showCommentButton"
:is-hover="isHover"
:is-bottom="isBottom"
:is-match-line="isMatchLine"
:is-context-line="isContentLine"
:is-meta-line="isMetaLine"
:discussions="discussions"
/>
</td>
</template>

11
app/assets/javascripts/diffs/components/inline_diff_comment_row.vue
View File

@ -1,5 +1,5 @@
<script>
import { mapState, mapGetters } from 'vuex';
import { mapState } from 'vuex';
import diffDiscussions from './diff_discussions.vue';
import diffLineNoteForm from './diff_line_note_form.vue';
@ -21,15 +21,16 @@ export default {
type: Number,
required: true,
},
discussions: {
type: Array,
required: false,
default: () => [],
},
},
computed: {
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
...mapGetters(['discussionsByLineCode']),
discussions() {
return this.discussionsByLineCode[this.line.lineCode] || [];
},
className() {
return this.discussions.length ? '' : 'js-temp-notes-holder';
},

7
app/assets/javascripts/diffs/components/inline_diff_table_row.vue
View File

@ -33,6 +33,11 @@ export default {
required: false,
default: false,
},
discussions: {
type: Array,
required: false,
default: () => [],
},
},
data() {
return {
@ -89,6 +94,7 @@ export default {
:is-bottom="isBottom"
:is-hover="isHover"
:show-comment-button="true"
:discussions="discussions"
class="diff-line-num old_line"
/>
<diff-table-cell
@ -98,6 +104,7 @@ export default {
:line-type="newLineType"
:is-bottom="isBottom"
:is-hover="isHover"
:discussions="discussions"
class="diff-line-num new_line"
/>
<td

22
app/assets/javascripts/diffs/components/inline_diff_view.vue
View File

@ -20,8 +20,11 @@ export default {
},
},
computed: {
...mapGetters('diffs', ['commitId']),
...mapGetters(['discussionsByLineCode']),
...mapGetters('diffs', [
'commitId',
'shouldRenderInlineCommentRow',
'singleDiscussionByLineCode',
]),
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
@ -36,15 +39,8 @@ export default {
},
},
methods: {
shouldRenderCommentRow(line) {
if (this.diffLineCommentForms[line.lineCode]) return true;
const lineDiscussions = this.discussionsByLineCode[line.lineCode];
if (lineDiscussions === undefined) {
return false;
}
return lineDiscussions.every(discussion => discussion.expanded);
discussionsList(line) {
return line.lineCode !== undefined ? this.singleDiscussionByLineCode(line.lineCode) : [];
},
},
};
@ -65,13 +61,15 @@ export default {
:line="line"
:is-bottom="index + 1 === diffLinesLength"
:key="line.lineCode"
:discussions="discussionsList(line)"
/>
<inline-diff-comment-row
v-if="shouldRenderCommentRow(line)"
v-if="shouldRenderInlineCommentRow(line)"
:diff-file-hash="diffFile.fileHash"
:line="line"
:line-index="index"
:key="index"
:discussions="discussionsList(line)"
/>
</template>
</tbody>

51
app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue
View File

@ -1,5 +1,5 @@
<script>
import { mapState, mapGetters } from 'vuex';
import { mapState } from 'vuex';
import diffDiscussions from './diff_discussions.vue';
import diffLineNoteForm from './diff_line_note_form.vue';
@ -21,30 +21,34 @@ export default {
type: Number,
required: true,
},
leftDiscussions: {
type: Array,
required: false,
default: () => [],
},
rightDiscussions: {
type: Array,
required: false,
default: () => [],
},
},
computed: {
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
...mapGetters(['discussionsByLineCode']),
leftLineCode() {
return this.line.left.lineCode;
},
rightLineCode() {
return this.line.right.lineCode;
},
hasDiscussion() {
const discussions = this.discussionsByLineCode;
return discussions[this.leftLineCode] || discussions[this.rightLineCode];
},
hasExpandedDiscussionOnLeft() {
const discussions = this.discussionsByLineCode[this.leftLineCode];
const discussions = this.leftDiscussions;
return discussions ? discussions.every(discussion => discussion.expanded) : false;
},
hasExpandedDiscussionOnRight() {
const discussions = this.discussionsByLineCode[this.rightLineCode];
const discussions = this.rightDiscussions;
return discussions ? discussions.every(discussion => discussion.expanded) : false;
},
@ -52,17 +56,18 @@ export default {
return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight;
},
shouldRenderDiscussionsOnLeft() {
return this.discussionsByLineCode[this.leftLineCode] && this.hasExpandedDiscussionOnLeft;
return this.leftDiscussions && this.hasExpandedDiscussionOnLeft;
},
shouldRenderDiscussionsOnRight() {
return (
this.discussionsByLineCode[this.rightLineCode] &&
this.hasExpandedDiscussionOnRight &&
this.line.right.type
);
return this.rightDiscussions && this.hasExpandedDiscussionOnRight && this.line.right.type;
},
showRightSideCommentForm() {
return this.line.right.type && this.diffLineCommentForms[this.rightLineCode];
},
className() {
return this.hasDiscussion ? '' : 'js-temp-notes-holder';
return this.leftDiscussions.length > 0 || this.rightDiscussions.length > 0
? ''
: 'js-temp-notes-holder';
},
},
};
@ -80,13 +85,12 @@ export default {
class="content"
>
<diff-discussions
v-if="discussionsByLineCode[leftLineCode].length"
:discussions="discussionsByLineCode[leftLineCode]"
v-if="leftDiscussions.length"
:discussions="leftDiscussions"
/>
</div>
<diff-line-note-form
v-if="diffLineCommentForms[leftLineCode] &&
diffLineCommentForms[leftLineCode]"
v-if="diffLineCommentForms[leftLineCode]"
:diff-file-hash="diffFileHash"
:line="line.left"
:note-target-line="line.left"
@ -100,13 +104,12 @@ export default {
class="content"
>
<diff-discussions
v-if="discussionsByLineCode[rightLineCode].length"
:discussions="discussionsByLineCode[rightLineCode]"
v-if="rightDiscussions.length"
:discussions="rightDiscussions"
/>
</div>
<diff-line-note-form
v-if="diffLineCommentForms[rightLineCode] &&
diffLineCommentForms[rightLineCode] && line.right.type"
v-if="showRightSideCommentForm"
:diff-file-hash="diffFileHash"
:line="line.right"
:note-target-line="line.right"

12
app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
View File

@ -36,6 +36,16 @@ export default {
required: false,
default: false,
},
leftDiscussions: {
type: Array,
required: false,
default: () => [],
},
rightDiscussions: {
type: Array,
required: false,
default: () => [],
},
},
data() {
return {
@ -116,6 +126,7 @@ export default {
:is-hover="isLeftHover"
:show-comment-button="true"
:diff-view-type="parallelDiffViewType"
:discussions="leftDiscussions"
class="diff-line-num old_line"
/>
<td
@ -136,6 +147,7 @@ export default {
:is-hover="isRightHover"
:show-comment-button="true"
:diff-view-type="parallelDiffViewType"
:discussions="rightDiscussions"
class="diff-line-num new_line"
/>
<td

39
app/assets/javascripts/diffs/components/parallel_diff_view.vue
View File

@ -21,8 +21,11 @@ export default {
},
},
computed: {
...mapGetters('diffs', ['commitId']),
...mapGetters(['discussionsByLineCode']),
...mapGetters('diffs', [
'commitId',
'singleDiscussionByLineCode',
'shouldRenderParallelCommentRow',
]),
...mapState({
diffLineCommentForms: state => state.diffs.diffLineCommentForms,
}),
@ -53,29 +56,9 @@ export default {
},
},
methods: {
shouldRenderCommentRow(line) {
const leftLineCode = line.left.lineCode;
const rightLineCode = line.right.lineCode;
const discussions = this.discussionsByLineCode;
const leftDiscussions = discussions[leftLineCode];
const rightDiscussions = discussions[rightLineCode];
const hasDiscussion = leftDiscussions || rightDiscussions;
const hasExpandedDiscussionOnLeft = leftDiscussions
? leftDiscussions.every(discussion => discussion.expanded)
: false;
const hasExpandedDiscussionOnRight = rightDiscussions
? rightDiscussions.every(discussion => discussion.expanded)
: false;
if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) {
return true;
}
const hasCommentFormOnLeft = this.diffLineCommentForms[leftLineCode];
const hasCommentFormOnRight = this.diffLineCommentForms[rightLineCode];
return hasCommentFormOnLeft || hasCommentFormOnRight;
discussionsByLine(line, leftOrRight) {
return line[leftOrRight] && line[leftOrRight].lineCode !== undefined ?
this.singleDiscussionByLineCode(line[leftOrRight].lineCode) : [];
},
},
};
@ -98,13 +81,17 @@ export default {
:line="line"
:is-bottom="index + 1 === diffLinesLength"
:key="index"
:left-discussions="discussionsByLine(line, 'left')"
:right-discussions="discussionsByLine(line, 'right')"
/>
<parallel-diff-comment-row
v-if="shouldRenderCommentRow(line)"
v-if="shouldRenderParallelCommentRow(line)"
:key="`dcr-${index}`"
:line="line"
:diff-file-hash="diffFile.fileHash"
:line-index="index"
:left-discussions="discussionsByLine(line, 'left')"
:right-discussions="discussionsByLine(line, 'right')"
/>
</template>
</tbody>

41
app/assets/javascripts/diffs/store/getters.js
View File

@ -64,6 +64,47 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) =
discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash),
) || [];
export const singleDiscussionByLineCode = (state, getters, rootState, rootGetters) => lineCode => {
if (!lineCode || lineCode === undefined) return [];
const discussions = rootGetters.discussionsByLineCode;
return discussions[lineCode] || [];
};
export const shouldRenderParallelCommentRow = (state, getters) => line => {
const leftLineCode = line.left.lineCode;
const rightLineCode = line.right.lineCode;
const leftDiscussions = getters.singleDiscussionByLineCode(leftLineCode);
const rightDiscussions = getters.singleDiscussionByLineCode(rightLineCode);
const hasDiscussion = leftDiscussions.length || rightDiscussions.length;
const hasExpandedDiscussionOnLeft = leftDiscussions.length
? leftDiscussions.every(discussion => discussion.expanded)
: false;
const hasExpandedDiscussionOnRight = rightDiscussions.length
? rightDiscussions.every(discussion => discussion.expanded)
: false;
if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) {
return true;
}
const hasCommentFormOnLeft = state.diffLineCommentForms[leftLineCode];
const hasCommentFormOnRight = state.diffLineCommentForms[rightLineCode];
return hasCommentFormOnLeft || hasCommentFormOnRight;
};
export const shouldRenderInlineCommentRow = (state, getters) => line => {
if (state.diffLineCommentForms[line.lineCode]) return true;
const lineDiscussions = getters.singleDiscussionByLineCode(line.lineCode);
if (lineDiscussions.length === 0) {
return false;
}
return lineDiscussions.every(discussion => discussion.expanded);
};
// prevent babel-plugin-rewire from generating an invalid default during karma∂ tests
export const getDiffFileByHash = state => fileHash =>
state.diffFiles.find(file => file.fileHash === fileHash);

21
app/assets/javascripts/diffs/store/utils.js
View File

@ -173,3 +173,24 @@ export function trimFirstCharOfLineContent(line = {}) {
return parsedLine;
}
export function getDiffRefsByLineCode(diffFiles) {
return diffFiles.reduce((acc, diffFile) => {
const { baseSha, headSha, startSha } = diffFile.diffRefs;
const { newPath, oldPath } = diffFile;
// We can only use highlightedDiffLines to create the map of diff lines because
// highlightedDiffLines will also include every parallel diff line in it.
if (diffFile.highlightedDiffLines) {
diffFile.highlightedDiffLines.forEach(line => {
const { lineCode, oldLine, newLine } = line;
if (lineCode) {
acc[lineCode] = { baseSha, headSha, startSha, newPath, oldPath, oldLine, newLine };
}
});
}
return acc;
}, {});
}

18
app/assets/javascripts/lazy_loader.js
View File

@ -19,11 +19,17 @@ export default class LazyLoader {
scrollContainer.addEventListener('load', () => this.loadCheck());
}
searchLazyImages() {
this.lazyImages = [].slice.call(document.querySelectorAll('.lazy'));
const that = this;
requestIdleCallback(
() => {
that.lazyImages = [].slice.call(document.querySelectorAll('.lazy'));
if (this.lazyImages.length) {
this.checkElementsInView();
}
if (that.lazyImages.length) {
that.checkElementsInView();
}
},
{ timeout: 500 },
);
}
startContentObserver() {
const contentNode = document.querySelector(this.observerNode) || document.querySelector('body');
@ -56,7 +62,9 @@ export default class LazyLoader {
const imgBound = imgTop + imgBoundRect.height;
if (scrollTop < imgBound && visHeight > imgTop) {
LazyLoader.loadImage(selectedImage);
requestAnimationFrame(() => {
LazyLoader.loadImage(selectedImage);
});
return false;
}

3
app/assets/javascripts/vue_shared/components/content_viewer/viewers/image_viewer.vue
View File

@ -42,6 +42,9 @@ export default {
},
methods: {
onImgLoad() {
requestIdleCallback(this.calculateImgSize, { timeout: 1000 });
},
calculateImgSize() {
const { contentImg } = this.$refs;
if (contentImg) {

5
changelogs/unreleased/tz-mr-port-memory-fixes.yml Normal file
View File

@ -0,0 +1,5 @@
---
title: Improve performance and memory footprint of Changes tab of Merge Requests
merge_request: 21028
author:
type: performance

6
spec/javascripts/diffs/components/diff_line_gutter_content_spec.js
View File

@ -48,7 +48,11 @@ describe('DiffLineGutterContent', () => {
it('should return discussions for the given lineCode', () => {
const { lineCode } = getDiffFileMock().highlightedDiffLines[1];
const component = createComponent({ lineCode, showCommentButton: true });
const component = createComponent({
lineCode,
showCommentButton: true,
discussions: getDiscussionsMockData(),
});
setDiscussions(component);

98
spec/javascripts/diffs/store/getters_spec.js
View File

@ -184,6 +184,104 @@ describe('Diffs Module Getters', () => {
});
});
describe('singleDiscussionByLineCode', () => {
it('returns found discussion per line Code', () => {
const discussionsMock = {};
discussionsMock.ABC = discussionMock;
expect(
getters.singleDiscussionByLineCode(localState, {}, null, {
discussionsByLineCode: () => discussionsMock,
})('DEF'),
).toEqual([]);
});
it('returns empty array when no discussions match', () => {
expect(
getters.singleDiscussionByLineCode(localState, {}, null, {
discussionsByLineCode: () => {},
})('DEF'),
).toEqual([]);
});
});
describe('shouldRenderParallelCommentRow', () => {
let line;
beforeEach(() => {
line = {};
line.left = {
lineCode: 'ABC',
};
line.right = {
lineCode: 'DEF',
};
});
it('returns true when discussion is expanded', () => {
discussionMock.expanded = true;
expect(
getters.shouldRenderParallelCommentRow(localState, {
singleDiscussionByLineCode: () => [discussionMock],
})(line),
).toEqual(true);
});
it('returns false when no discussion was found', () => {
localState.diffLineCommentForms.ABC = false;
localState.diffLineCommentForms.DEF = false;
expect(
getters.shouldRenderParallelCommentRow(localState, {
singleDiscussionByLineCode: () => [],
})(line),
).toEqual(false);
});
it('returns true when discussionForm was found', () => {
localState.diffLineCommentForms.ABC = {};
expect(
getters.shouldRenderParallelCommentRow(localState, {
singleDiscussionByLineCode: () => [discussionMock],
})(line),
).toEqual(true);
});
});
describe('shouldRenderInlineCommentRow', () => {
it('returns true when diffLineCommentForms has form', () => {
localState.diffLineCommentForms.ABC = {};
expect(
getters.shouldRenderInlineCommentRow(localState)({
lineCode: 'ABC',
}),
).toEqual(true);
});
it('returns false when no line discussions were found', () => {
expect(
getters.shouldRenderInlineCommentRow(localState, {
singleDiscussionByLineCode: () => [],
})('DEF'),
).toEqual(false);
});
it('returns true if all found discussions are expanded', () => {
discussionMock.expanded = true;
expect(
getters.shouldRenderInlineCommentRow(localState, {
singleDiscussionByLineCode: () => [discussionMock],
})('ABC'),
).toEqual(true);
});
});
describe('getDiffFileDiscussions', () => {
it('returns an array with discussions when fileHash matches and the discussion belongs to a diff', () => {
discussionMock.diff_file.file_hash = diffFileMock.fileHash;

2
spec/javascripts/vue_shared/components/diff_viewer/viewers/image_diff_viewer_spec.js
View File

@ -170,8 +170,6 @@ describe('ImageDiffViewer', () => {
vm.$el.querySelector('.view-modes-menu li:nth-child(3)').click();
vm.$nextTick(() => {
expect(vm.$el.querySelector('.dragger').style.left).toBe('100px');
dragSlider(vm.$el.querySelector('.dragger'));
vm.$nextTick(() => {