Commit 422dcfde authored by Tim Zallmann's avatar Tim Zallmann Committed by Fatih Acet

Resolve "MR: Reduce the memory footprint of the component tree"

parent 98547133
...@@ -85,6 +85,9 @@ export default { ...@@ -85,6 +85,9 @@ export default {
} }
return __('Show latest version'); return __('Show latest version');
}, },
canCurrentUserFork() {
return this.currentUser.canFork === true && this.currentUser.canCreateMergeRequest;
},
}, },
watch: { watch: {
diffViewType() { diffViewType() {
...@@ -192,7 +195,7 @@ export default { ...@@ -192,7 +195,7 @@ export default {
v-for="file in diffFiles" v-for="file in diffFiles"
:key="file.newPath" :key="file.newPath"
:file="file" :file="file"
:current-user="currentUser" :can-current-user-fork="canCurrentUserFork"
/> />
</div> </div>
<no-changes v-else /> <no-changes v-else />
......
...@@ -18,8 +18,8 @@ export default { ...@@ -18,8 +18,8 @@ export default {
type: Object, type: Object,
required: true, required: true,
}, },
currentUser: { canCurrentUserFork: {
type: Object, type: Boolean,
required: true, required: true,
}, },
}, },
...@@ -87,7 +87,7 @@ export default { ...@@ -87,7 +87,7 @@ export default {
class="diff-file file-holder" class="diff-file file-holder"
> >
<diff-file-header <diff-file-header
:current-user="currentUser" :can-current-user-fork="canCurrentUserFork"
:diff-file="file" :diff-file="file"
:collapsible="true" :collapsible="true"
:expanded="!isCollapsed" :expanded="!isCollapsed"
......
...@@ -39,8 +39,8 @@ export default { ...@@ -39,8 +39,8 @@ export default {
required: false, required: false,
default: true, default: true,
}, },
currentUser: { canCurrentUserFork: {
type: Object, type: Boolean,
required: true, required: true,
}, },
}, },
...@@ -228,7 +228,7 @@ export default { ...@@ -228,7 +228,7 @@ export default {
<edit-button <edit-button
v-if="!diffFile.deletedFile" v-if="!diffFile.deletedFile"
:current-user="currentUser" :can-current-user-fork="canCurrentUserFork"
:edit-path="diffFile.editPath" :edit-path="diffFile.editPath"
:can-modify-blob="diffFile.canModifyBlob" :can-modify-blob="diffFile.canModifyBlob"
@showForkMessage="showForkMessage" @showForkMessage="showForkMessage"
......
...@@ -13,12 +13,8 @@ export default { ...@@ -13,12 +13,8 @@ export default {
noteForm, noteForm,
}, },
props: { props: {
diffFile: { diffFileHash: {
type: Object, type: String,
required: true,
},
diffLines: {
type: Array,
required: true, required: true,
}, },
line: { line: {
...@@ -40,6 +36,7 @@ export default { ...@@ -40,6 +36,7 @@ export default {
noteableData: state => state.notes.noteableData, noteableData: state => state.notes.noteableData,
diffViewType: state => state.diffs.diffViewType, diffViewType: state => state.diffs.diffViewType,
}), }),
...mapGetters('diffs', ['getDiffFileByHash']),
...mapGetters(['isLoggedIn', 'noteableType', 'getNoteableData', 'getNotesDataByProp']), ...mapGetters(['isLoggedIn', 'noteableType', 'getNoteableData', 'getNotesDataByProp']),
}, },
mounted() { mounted() {
...@@ -68,13 +65,14 @@ export default { ...@@ -68,13 +65,14 @@ export default {
}); });
}, },
handleSaveNote(note) { handleSaveNote(note) {
const selectedDiffFile = this.getDiffFileByHash(this.diffFileHash);
const postData = getNoteFormData({ const postData = getNoteFormData({
note, note,
noteableData: this.noteableData, noteableData: this.noteableData,
noteableType: this.noteableType, noteableType: this.noteableType,
noteTargetLine: this.noteTargetLine, noteTargetLine: this.noteTargetLine,
diffViewType: this.diffViewType, diffViewType: this.diffViewType,
diffFile: this.diffFile, diffFile: selectedDiffFile,
linePosition: this.position, linePosition: this.position,
}); });
......
...@@ -24,8 +24,12 @@ export default { ...@@ -24,8 +24,12 @@ export default {
type: Object, type: Object,
required: true, required: true,
}, },
diffFile: { fileHash: {
type: Object, type: String,
required: true,
},
contextLinesPath: {
type: String,
required: true, required: true,
}, },
diffViewType: { diffViewType: {
...@@ -120,14 +124,14 @@ export default { ...@@ -120,14 +124,14 @@ export default {
:class="classNameMap" :class="classNameMap"
> >
<diff-line-gutter-content <diff-line-gutter-content
:file-hash="diffFile.fileHash" :file-hash="fileHash"
:context-lines-path="contextLinesPath"
:line-type="normalizedLine.type" :line-type="normalizedLine.type"
:line-code="normalizedLine.lineCode" :line-code="normalizedLine.lineCode"
:line-position="linePosition" :line-position="linePosition"
:line-number="lineNumber" :line-number="lineNumber"
:meta-data="normalizedLine.metaData" :meta-data="normalizedLine.metaData"
:show-comment-button="showCommentButton" :show-comment-button="showCommentButton"
:context-lines-path="diffFile.contextLinesPath"
:is-bottom="isBottom" :is-bottom="isBottom"
:is-match-line="isMatchLine" :is-match-line="isMatchLine"
:is-context-line="isContentLine" :is-context-line="isContentLine"
......
...@@ -5,8 +5,8 @@ export default { ...@@ -5,8 +5,8 @@ export default {
type: String, type: String,
required: true, required: true,
}, },
currentUser: { canCurrentUserFork: {
type: Object, type: Boolean,
required: true, required: true,
}, },
canModifyBlob: { canModifyBlob: {
...@@ -17,12 +17,12 @@ export default { ...@@ -17,12 +17,12 @@ export default {
}, },
methods: { methods: {
handleEditClick(evt) { handleEditClick(evt) {
if (!this.currentUser || this.canModifyBlob) { if (!this.canCurrentUserFork || this.canModifyBlob) {
// if we can Edit, do default Edit button behavior // if we can Edit, do default Edit button behavior
return; return;
} }
if (this.currentUser.canFork && this.currentUser.canCreateMergeRequest) { if (this.canCurrentUserFork) {
evt.preventDefault(); evt.preventDefault();
this.$emit('showForkMessage'); this.$emit('showForkMessage');
} }
......
...@@ -13,12 +13,8 @@ export default { ...@@ -13,12 +13,8 @@ export default {
type: Object, type: Object,
required: true, required: true,
}, },
diffFile: { diffFileHash: {
type: Object, type: String,
required: true,
},
diffLines: {
type: Array,
required: true, required: true,
}, },
lineIndex: { lineIndex: {
...@@ -58,10 +54,9 @@ export default { ...@@ -58,10 +54,9 @@ export default {
/> />
<diff-line-note-form <diff-line-note-form
v-if="diffLineCommentForms[line.lineCode]" v-if="diffLineCommentForms[line.lineCode]"
:diff-file="diffFile" :diff-file-hash="diffFileHash"
:diff-lines="diffLines"
:line="line" :line="line"
:note-target-line="diffLines[lineIndex]" :note-target-line="line"
/> />
</div> </div>
</td> </td>
......
...@@ -16,8 +16,12 @@ export default { ...@@ -16,8 +16,12 @@ export default {
DiffTableCell, DiffTableCell,
}, },
props: { props: {
diffFile: { fileHash: {
type: Object, type: String,
required: true,
},
contextLinesPath: {
type: String,
required: true, required: true,
}, },
line: { line: {
...@@ -50,7 +54,7 @@ export default { ...@@ -50,7 +54,7 @@ export default {
inlineRowId() { inlineRowId() {
const { lineCode, oldLine, newLine } = this.line; const { lineCode, oldLine, newLine } = this.line;
return lineCode || `${this.diffFile.fileHash}_${oldLine}_${newLine}`; return lineCode || `${this.fileHash}_${oldLine}_${newLine}`;
}, },
}, },
created() { created() {
...@@ -78,7 +82,8 @@ export default { ...@@ -78,7 +82,8 @@ export default {
@mouseout="handleMouseMove" @mouseout="handleMouseMove"
> >
<diff-table-cell <diff-table-cell
:diff-file="diffFile" :file-hash="fileHash"
:context-lines-path="contextLinesPath"
:line="line" :line="line"
:line-type="oldLineType" :line-type="oldLineType"
:is-bottom="isBottom" :is-bottom="isBottom"
...@@ -87,7 +92,8 @@ export default { ...@@ -87,7 +92,8 @@ export default {
class="diff-line-num old_line" class="diff-line-num old_line"
/> />
<diff-table-cell <diff-table-cell
:diff-file="diffFile" :file-hash="fileHash"
:context-lines-path="contextLinesPath"
:line="line" :line="line"
:line-type="newLineType" :line-type="newLineType"
:is-bottom="isBottom" :is-bottom="isBottom"
......
...@@ -60,15 +60,15 @@ export default { ...@@ -60,15 +60,15 @@ export default {
v-for="(line, index) in normalizedDiffLines" v-for="(line, index) in normalizedDiffLines"
> >
<inline-diff-table-row <inline-diff-table-row
:diff-file="diffFile" :file-hash="diffFile.fileHash"
:context-lines-path="diffFile.contextLinesPath"
:line="line" :line="line"
:is-bottom="index + 1 === diffLinesLength" :is-bottom="index + 1 === diffLinesLength"
:key="line.lineCode" :key="line.lineCode"
/> />
<inline-diff-comment-row <inline-diff-comment-row
v-if="shouldRenderCommentRow(line)" v-if="shouldRenderCommentRow(line)"
:diff-file="diffFile" :diff-file-hash="diffFile.fileHash"
:diff-lines="normalizedDiffLines"
:line="line" :line="line"
:line-index="index" :line-index="index"
:key="index" :key="index"
......
...@@ -13,12 +13,8 @@ export default { ...@@ -13,12 +13,8 @@ export default {
type: Object, type: Object,
required: true, required: true,
}, },
diffFile: { diffFileHash: {
type: Object, type: String,
required: true,
},
diffLines: {
type: Array,
required: true, required: true,
}, },
lineIndex: { lineIndex: {
...@@ -91,10 +87,9 @@ export default { ...@@ -91,10 +87,9 @@ export default {
<diff-line-note-form <diff-line-note-form
v-if="diffLineCommentForms[leftLineCode] && v-if="diffLineCommentForms[leftLineCode] &&
diffLineCommentForms[leftLineCode]" diffLineCommentForms[leftLineCode]"
:diff-file="diffFile" :diff-file-hash="diffFileHash"
:diff-lines="diffLines"
:line="line.left" :line="line.left"
:note-target-line="diffLines[lineIndex].left" :note-target-line="line.left"
position="left" position="left"
/> />
</td> </td>
...@@ -112,10 +107,9 @@ export default { ...@@ -112,10 +107,9 @@ export default {
<diff-line-note-form <diff-line-note-form
v-if="diffLineCommentForms[rightLineCode] && v-if="diffLineCommentForms[rightLineCode] &&
diffLineCommentForms[rightLineCode] && line.right.type" diffLineCommentForms[rightLineCode] && line.right.type"
:diff-file="diffFile" :diff-file-hash="diffFileHash"
:diff-lines="diffLines"
:line="line.right" :line="line.right"
:note-target-line="diffLines[lineIndex].right" :note-target-line="line.right"
position="right" position="right"
/> />
</td> </td>
......
...@@ -19,8 +19,12 @@ export default { ...@@ -19,8 +19,12 @@ export default {
DiffTableCell, DiffTableCell,
}, },
props: { props: {
diffFile: { fileHash: {
type: Object, type: String,
required: true,
},
contextLinesPath: {
type: String,
required: true, required: true,
}, },
line: { line: {
...@@ -103,7 +107,8 @@ export default { ...@@ -103,7 +107,8 @@ export default {
@mouseout="handleMouseMove" @mouseout="handleMouseMove"
> >
<diff-table-cell <diff-table-cell
:diff-file="diffFile" :file-hash="fileHash"
:context-lines-path="contextLinesPath"
:line="line" :line="line"
:line-type="oldLineType" :line-type="oldLineType"
:line-position="linePositionLeft" :line-position="linePositionLeft"
...@@ -123,7 +128,8 @@ export default { ...@@ -123,7 +128,8 @@ export default {
> >
</td> </td>
<diff-table-cell <diff-table-cell
:diff-file="diffFile" :file-hash="fileHash"
:context-lines-path="contextLinesPath"
:line="line" :line="line"
:line-type="newLineType" :line-type="newLineType"
:line-position="linePositionRight" :line-position="linePositionRight"
......
...@@ -93,17 +93,17 @@ export default { ...@@ -93,17 +93,17 @@ export default {
v-for="(line, index) in parallelDiffLines" v-for="(line, index) in parallelDiffLines"
> >
<parallel-diff-table-row <parallel-diff-table-row
:diff-file="diffFile" :file-hash="diffFile.fileHash"
:context-lines-path="diffFile.contextLinesPath"
:line="line" :line="line"
:is-bottom="index + 1 === diffLinesLength" :is-bottom="index + 1 === diffLinesLength"
:key="index" :key="index"
/> />
<parallel-diff-comment-row <parallel-diff-comment-row
v-if="shouldRenderCommentRow(line)" v-if="shouldRenderCommentRow(line)"
:key="line.left.lineCode || line.right.lineCode" :key="`dcr-${index}`"
:line="line" :line="line"
:diff-file="diffFile" :diff-file-hash="diffFile.fileHash"
:diff-lines="parallelDiffLines"
:line-index="index" :line-index="index"
/> />
</template> </template>
......
...@@ -57,4 +57,8 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) = ...@@ -57,4 +57,8 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) =
) || []; ) || [];
// prevent babel-plugin-rewire from generating an invalid default during karma∂ tests // prevent babel-plugin-rewire from generating an invalid default during karma∂ tests
export const getDiffFileByHash = state => fileHash =>
state.diffFiles.find(file => file.fileHash === fileHash);
// prevent babel-plugin-rewire from generating an invalid default during karma tests
export default () => {}; export default () => {};
<script> <script>
import { mapState, mapActions } from 'vuex'; import { mapState, mapActions } from 'vuex';
import imageDiffHelper from '~/image_diff/helpers/index'; import imageDiffHelper from '~/image_diff/helpers/index';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import DiffFileHeader from '~/diffs/components/diff_file_header.vue'; import DiffFileHeader from '~/diffs/components/diff_file_header.vue';
import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue'; import SkeletonLoadingContainer from '~/vue_shared/components/skeleton_loading_container.vue';
import { trimFirstCharOfLineContent } from '~/diffs/store/utils'; import { trimFirstCharOfLineContent } from '~/diffs/store/utils';
export default { export default {
components: { components: {
DiffFileHeader, DiffFileHeader,
SkeletonLoadingContainer, SkeletonLoadingContainer,
},
props: {
discussion: {
type: Object,
required: true,
}, },
props: { },
discussion: { data() {
type: Object, return {
required: true, error: false,
}, };
},
computed: {
...mapState({
noteableData: state => state.notes.noteableData,
}),
hasTruncatedDiffLines() {
return this.discussion.truncatedDiffLines && this.discussion.truncatedDiffLines.length !== 0;
}, },
data() { isDiscussionsExpanded() {
return { return true; // TODO: @fatihacet - Fix this.
error: false,
};
}, },
computed: { isCollapsed() {
...mapState({ return this.diffFile.collapsed || false;
noteableData: state => state.notes.noteableData, },
}), isImageDiff() {
hasTruncatedDiffLines() { return !this.diffFile.text;
return this.discussion.truncatedDiffLines && },
this.discussion.truncatedDiffLines.length !== 0; diffFileClass() {
}, const { text } = this.diffFile;
isDiscussionsExpanded() { return text ? 'text-file' : 'js-image-file';
return true; // TODO: @fatihacet - Fix this. },
}, diffFile() {
isCollapsed() { return convertObjectPropsToCamelCase(this.discussion.diffFile, { deep: true });
return this.diffFile.collapsed || false;
},
isImageDiff() {
return !this.diffFile.text;
},
diffFileClass() {
const { text } = this.diffFile;
return text ? 'text-file' : 'js-image-file';
},
diffFile() {
return convertObjectPropsToCamelCase(this.discussion.diffFile, { deep: true });
},
imageDiffHtml() {
return this.discussion.imageDiffHtml;
},
currentUser() {
return this.noteableData.current_user;
},
userColorScheme() {
return window.gon.user_color_scheme;
},
normalizedDiffLines() {
if (this.discussion.truncatedDiffLines) {
return this.discussion.truncatedDiffLines.map(line =>
trimFirstCharOfLineContent(convertObjectPropsToCamelCase(line)),
);
}
return [];
},
}, },
mounted() { imageDiffHtml() {
if (this.isImageDiff) { return this.discussion.imageDiffHtml;
const canCreateNote = false; },
const renderCommentBadge = true; userColorScheme() {
imageDiffHelper.initImageDiff(this.$refs.fileHolder, canCreateNote, renderCommentBadge); return window.gon.user_color_scheme;
} else if (!this.hasTruncatedDiffLines) { },
this.fetchDiff(); normalizedDiffLines() {
if (this.discussion.truncatedDiffLines) {
return this.discussion.truncatedDiffLines.map(line =>
trimFirstCharOfLineContent(convertObjectPropsToCamelCase(line)),
);
} }
return [];
},
},
mounted() {
if (this.isImageDiff) {
const canCreateNote = false;
const renderCommentBadge = true;
imageDiffHelper.initImageDiff(this.$refs.fileHolder, canCreateNote, renderCommentBadge);
} else if (!this.hasTruncatedDiffLines) {
this.fetchDiff();
}
},
methods: {
...mapActions(['fetchDiscussionDiffLines']),
rowTag(html) {
return html.outerHTML ? 'tr' : 'template';
}, },
methods: { fetchDiff() {
...mapActions(['fetchDiscussionDiffLines']), this.error = false;
rowTag(html) { this.fetchDiscussionDiffLines(this.discussion)
return html.outerHTML ? 'tr' : 'template'; .then(this.highlight)
}, .catch(() => {
fetchDiff() { this.error = true;
this.error = false; });
this.fetchDiscussionDiffLines(this.discussion)
.then(this.highlight)
.catch(() => {
this.error = true;
});
},
}, },
}; },
};
</script> </script>
<template> <template>
...@@ -99,7 +95,7 @@ ...@@ -99,7 +95,7 @@
> >
<diff-file-header <diff-file-header
:diff-file="diffFile" :diff-file="diffFile"
:current-user="currentUser" :can-current-user-fork="false"
:discussions-expanded="isDiscussionsExpanded" :discussions-expanded="isDiscussionsExpanded"
:expanded="!isCollapsed" :expanded="!isCollapsed"
/> />
......
...@@ -24,7 +24,7 @@ describe('diff_file_header', () => { ...@@ -24,7 +24,7 @@ describe('diff_file_header', () => {
const diffFile = convertObjectPropsToCamelCase(diffDiscussionMock.diff_file, { deep: true }); const diffFile = convertObjectPropsToCamelCase(diffDiscussionMock.diff_file, { deep: true });
props = { props = {
diffFile, diffFile,
currentUser: {}, canCurrentUserFork: false,
}; };
}); });
......
...@@ -11,7 +11,7 @@ describe('DiffFile', () => { ...@@ -11,7 +11,7 @@ describe('DiffFile', () => {
beforeEach(() => { beforeEach(() => {
vm = createComponentWithStore(Vue.extend(DiffFileComponent), store, { vm = createComponentWithStore(Vue.extend(DiffFileComponent), store, {
file: getDiffFileMock(), file: getDiffFileMock(),
currentUser: {}, canCurrentUserFork: false,
}).$mount(); }).$mount();
}); });
......
...@@ -15,7 +15,7 @@ describe('DiffLineNoteForm', () => { ...@@ -15,7 +15,7 @@ describe('DiffLineNoteForm', () => {
diffLines = diffFile.highlightedDiffLines; diffLines = diffFile.highlightedDiffLines;
component = createComponentWithStore(Vue.extend(DiffLineNoteForm), store, { component = createComponentWithStore(Vue.extend(DiffLineNoteForm), store, {
diffFile, diffFileHash: diffFile.fileHash,
diffLines, diffLines,
line: diffLines[0], line: diffLines[0],
noteTargetLine: diffLines[0], noteTargetLine: diffLines[0],
......
...@@ -184,4 +184,23 @@ describe('Diffs Module Getters', () => { ...@@ -184,4 +184,23 @@ describe('Diffs Module Getters', () => {
).toEqual(0); ).toEqual(0);
}); });
}); });
describe('getDiffFileByHash', () => {
it('returns file by hash', () => {
const fileA = {
fileHash: '123',
};
const fileB = {
fileHash: '456',
};
localState.diffFiles = [fileA, fileB];
expect(getters.getDiffFileByHash(localState)('456')).toEqual(fileB);
});
it('returns null if no matching file is found', () => {
localState.diffFiles = [];
expect(getters.getDiffFileByHash(localState)('123')).toBeUndefined();
});
});
}); });
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment