[#1382] Avoid rerendering tabs unnecessarily (#1390)

Tabs are destroyed and rerendered based on a key which can lead to mysterious bugs. Furthermore, tab information is passed indirectly from store to `main.js` before being passed to the tab itself. Let's - clean up the code such that tabs do not need to completely rerender when information changes. - pass the tab information directly.
reposense · Feb 24, 2021 · 7e4e050 · 7e4e050
1 parent aa1ba08
commit 7e4e050
Show file tree

Hide file tree

Showing 4 changed files with 87 additions and 71 deletions.
diff --git a/frontend/src/index.pug b/frontend/src/index.pug
@@ -78,14 +78,8 @@ html
           #tabs-wrapper(ref="tabWrapper", slot="right")
             .tab-content.panel-padding
               .tab-pane
-                v-authorship#tab-authorship(
-                  v-if="tabType === 'authorship'",
-                  v-bind:key="generateKey(tabInfo.tabAuthorship, ['author', 'repo', 'isMergeGroup'])",
-                  v-bind:info="tabInfo.tabAuthorship")
-                v-zoom#tab-zoom(
-                  v-else-if="tabType === 'zoom'",
-                  v-bind:key="generateKey(tabInfo.tabZoom, ['zRepo', 'zAuthor', 'zFilterGroup', 'zTimeFrame'])",
-                  v-bind:info="tabInfo.tabZoom")
+                v-authorship#tab-authorship(v-if="tabType === 'authorship'")
+                v-zoom#tab-zoom(v-else-if="tabType === 'zoom'")
                 #tab-empty(v-else)
                   .title
                     h2 Welcome to this RepoSense report!

diff --git a/frontend/src/static/js/main.js b/frontend/src/static/js/main.js
@@ -28,22 +28,18 @@ window.app = new window.Vue({
     isLoadingOverlayEnabled: false,
     loadingOverlayOpacity: 1,
 
-    isCollapsed: false,
     isTabActive: true, // to force tab wrapper to load
 
     tabType: 'empty',
-    tabInfo: {},
     creationDate: '',
 
     errorMessages: {},
   },
   watch: {
     '$store.state.tabZoomInfo': function () {
-      this.tabInfo.tabZoom = Object.assign({}, this.$store.state.tabZoomInfo);
       this.activateTab('zoom');
     },
     '$store.state.tabAuthorshipInfo': function () {
-      this.tabInfo.tabAuthorship = Object.assign({}, this.$store.state.tabAuthorshipInfo);
       this.activateTab('authorship');
     },
     '$store.state.loadingOverlayCount': function () {
@@ -107,14 +103,11 @@ window.app = new window.Vue({
 
     // handle opening of sidebar //
     activateTab(tabName) {
-      // changing isTabActive to trigger redrawing of component
-      this.isTabActive = false;
       if (this.$refs.tabWrapper) {
         this.$refs.tabWrapper.scrollTop = 0;
       }
 
       this.isTabActive = true;
-      this.isCollapsed = false;
       this.tabType = tabName;
 
       window.addHash('tabOpen', this.isTabActive);
@@ -135,6 +128,7 @@ window.app = new window.Vue({
         author: hash.tabAuthor,
         repo: hash.tabRepo,
         isMergeGroup: hash.authorshipIsMergeGroup === 'true',
+        isRefresh: true,
         minDate,
         maxDate,
       };
@@ -189,11 +183,6 @@ window.app = new window.Vue({
       }
     },
 
-    generateKey(dataObj, keysToUse) {
-      const picked = keysToUse.map((key) => dataObj[key]);
-      return JSON.stringify(picked);
-    },
-
     getRepoSenseHomeLink() {
       const version = window.app.repoSenseVersion;
       if (!version) {

diff --git a/frontend/src/static/js/v_authorship.js b/frontend/src/static/js/v_authorship.js
@@ -6,26 +6,29 @@ const filesSortDict = {
   fileType: (file) => file.fileType,
 };
 
+function initialState() {
+  return {
+    isLoaded: false,
+    files: [],
+    selectedFiles: [],
+    filterType: 'checkboxes',
+    selectedFileTypes: [],
+    fileTypes: [],
+    filesLinesObj: {},
+    fileTypeBlankLinesObj: {},
+    filesSortType: 'lineOfCode',
+    toReverseSortFiles: true,
+    searchBarValue: '',
+  };
+}
+
 const repoCache = [];
 const minimatch = require('minimatch');
 
 window.vAuthorship = {
-  props: ['info'],
   template: window.$('v_authorship').innerHTML,
   data() {
-    return {
-      isLoaded: false,
-      files: [],
-      selectedFiles: [],
-      filterType: 'checkboxes',
-      selectedFileTypes: [],
-      fileTypes: [],
-      filesLinesObj: {},
-      fileTypeBlankLinesObj: {},
-      filesSortType: 'lineOfCode',
-      toReverseSortFiles: true,
-      searchBarValue: '',
-    };
+    return initialState();
   },
 
   watch: {
@@ -49,11 +52,9 @@ window.vAuthorship = {
       this.updateSelectedFiles();
     },
 
-    isLoaded() {
-      if (this.isLoaded) {
-        this.retrieveHashes();
-        this.setInfoHash();
-      }
+    authorshipOwnerWatchable() {
+      Object.assign(this.$data, initialState());
+      this.initiate();
     },
   },
 
@@ -73,13 +74,12 @@ window.vAuthorship = {
 
       this.toReverseSortFiles = hash.reverseAuthorshipOrder !== 'false';
 
-      this.selectedFileTypes = this.info.checkedFileTypes
-        ? this.info.checkedFileTypes.filter((fileType) => this.fileTypes.includes(fileType))
-        : [];
       if (hash.authorshipFileTypes) {
         this.selectedFileTypes = hash.authorshipFileTypes
             .split(window.HASH_DELIMITER)
             .filter((fileType) => this.fileTypes.includes(fileType));
+      } else {
+        this.resetSelectedFileTypes();
       }
 
       if ('authorshipFilesGlob' in hash) {
@@ -88,6 +88,12 @@ window.vAuthorship = {
       }
     },
 
+    resetSelectedFileTypes() {
+      this.selectedFileTypes = this.info.checkedFileTypes
+        ? this.info.checkedFileTypes.filter((fileType) => this.fileTypes.includes(fileType))
+        : [];
+    },
+
     setInfoHash() {
       const { addHash } = window;
       // We only set these hashes as they are propagated from summary_charts
@@ -108,7 +114,7 @@ window.vAuthorship = {
       window.encodeHash();
     },
 
-    initiate() {
+    async initiate() {
       const repo = window.REPOS[this.info.repo];
 
       this.getRepoProps(repo);
@@ -124,12 +130,19 @@ window.vAuthorship = {
       }
       repoCache.push(this.info.repo);
 
-      if (repo.files) {
-        this.processFiles(repo.files);
+      let { files } = repo;
+      if (!files) {
+        files = await window.api.loadAuthorship(this.info.repo);
+      }
+      this.processFiles(files);
+
+      if (this.info.isRefresh) {
+        this.retrieveHashes();
       } else {
-        window.api.loadAuthorship(this.info.repo)
-            .then((files) => this.processFiles(files));
+        this.resetSelectedFileTypes();
       }
+
+      this.setInfoHash();
     },
 
     getRepoProps(repo) {
@@ -273,8 +286,7 @@ window.vAuthorship = {
 
       this.fileTypeBlankLinesObj = fileTypeBlanksInfoObj;
       this.files = res;
-      this.isLoaded = true;
-      this.updateSelectedFiles();
+      this.updateSelectedFiles(true);
     },
 
     getContributionFromAllAuthors(contributionMap) {
@@ -309,14 +321,17 @@ window.vAuthorship = {
       window.encodeHash();
     },
 
-    updateSelectedFiles() {
+    updateSelectedFiles(setIsLoaded = false) {
       this.$store.commit('incrementLoadingOverlayCount', 1);
       setTimeout(() => {
         this.selectedFiles = this.files.filter(
             (file) => this.selectedFileTypes.includes(file.fileType)
             && minimatch(file.path, this.searchBarValue || '*', { matchBase: true, dot: true }),
         )
             .sort(this.sortingFunction);
+        if (setIsLoaded) {
+          this.isLoaded = true;
+        }
         this.$store.commit('incrementLoadingOverlayCount', -1);
       });
     },
@@ -352,6 +367,10 @@ window.vAuthorship = {
   },
 
   computed: {
+    authorshipOwnerWatchable() {
+      return `${this.info.author}|${this.info.repo}|${this.info.isMergeGroup}`;
+    },
+
     sortingFunction() {
       return (a, b) => (this.toReverseSortFiles ? -1 : 1)
         * window.comparator(filesSortDict[this.filesSortType])(a, b);
@@ -394,7 +413,10 @@ window.vAuthorship = {
       return numLinesModified;
     },
 
-    ...Vuex.mapState(['fileTypeColors']),
+    ...Vuex.mapState({
+      fileTypeColors: 'fileTypeColors',
+      info: 'tabAuthorshipInfo',
+    }),
   },
 
   created() {

diff --git a/frontend/src/static/js/v_zoom.js b/frontend/src/static/js/v_zoom.js
@@ -1,21 +1,28 @@
 /* global Vuex */
 
+function initialState() {
+  return {
+    showAllCommitMessageBody: true,
+    expandedCommitMessagesCount: this.totalCommitMessageBodyCount,
+    commitsSortType: 'time',
+    toReverseSortedCommits: true,
+    isCommitsFinalized: false,
+    selectedFileTypes: [],
+    fileTypes: [],
+  };
+}
+
 window.vZoom = {
-  props: ['info'],
   template: window.$('v_zoom').innerHTML,
   data() {
-    return {
-      showAllCommitMessageBody: true,
-      expandedCommitMessagesCount: this.totalCommitMessageBodyCount,
-      commitsSortType: 'time',
-      toReverseSortedCommits: true,
-      isCommitsFinalized: false,
-      selectedFileTypes: [],
-      fileTypes: [],
-    };
+    return initialState();
   },
 
   computed: {
+    zoomOwnerWatchable() {
+      return `${this.info.zRepo}|${this.info.zAuthor}|${this.info.zFilterGroup}|${this.info.zTimeFrame}`;
+    },
+
     sortingFunction() {
       const commitSortFunction = this.commitsSortType === 'time'
         ? (commit) => commit.date
@@ -34,7 +41,6 @@ window.vZoom = {
       filteredUser.commits = zUser.commits.filter(
           (commit) => commit[date] >= zSince && commit[date] <= zUntil,
       ).sort(this.sortingFunction);
-      this.isCommitsFinalized = true;
 
       return filteredUser;
     },
@@ -81,17 +87,20 @@ window.vZoom = {
         this.updateSelectedFileTypesHash();
       },
     },
-    ...Vuex.mapState(['fileTypeColors']),
+
+    ...Vuex.mapState({
+      fileTypeColors: 'fileTypeColors',
+      info: 'tabZoomInfo',
+    }),
   },
 
   watch: {
-    isCommitsFinalized() {
-      if (this.isCommitsFinalized) {
-        this.updateFileTypes();
-        this.selectedFileTypes = this.fileTypes.slice();
-        this.retrieveHashes();
-      }
+    zoomOwnerWatchable() {
+      Object.assign(this.$data, initialState());
+      this.initiate();
+      this.setInfoHash();
     },
+
     selectedFileTypes() {
       this.$nextTick(() => {
         this.updateExpandedCommitMessagesCount();
@@ -112,6 +121,9 @@ window.vZoom = {
       if (!this.info.zUser) { // restoring zoom tab from reloaded page
         this.restoreZoomTab();
       }
+
+      this.updateFileTypes();
+      this.selectedFileTypes = this.fileTypes.slice();
     },
 
     openSummary() {
@@ -246,8 +258,7 @@ window.vZoom = {
   },
   created() {
     this.initiate();
-  },
-  mounted() {
+    this.retrieveHashes();
     this.setInfoHash();
   },
   beforeDestroy() {