Merge pull request #3830 from nocodb/fix/mem-leak-avoid-rerender

Fix : Avoid re-rendering smartsheet

packages/nc-gui/components/smartsheet/Form.vue

@@ -1,6 +1,6 @@
 <script setup lang="ts">
 import Draggable from 'vuedraggable'
-import { RelationTypes, UITypes, getSystemColumns, isVirtualCol } from 'nocodb-sdk'
+import { RelationTypes, UITypes, ViewTypes, getSystemColumns, isVirtualCol } from 'nocodb-sdk'
 import {
   ActiveViewInj,
   IsFormInj,
@@ -375,6 +375,12 @@ onMounted(async () => {
   await loadFormView()
   setFormData()
 })
+
+watch(view, (nextView) => {
+  if (nextView?.type === ViewTypes.FORM) {
+    reloadEventHook.trigger()
+  }
+})
 </script>
 
 <template>

packages/nc-gui/components/smartsheet/Gallery.vue

@@ -1,5 +1,5 @@
 <script lang="ts" setup>
-import { isVirtualCol } from 'nocodb-sdk'
+import { ViewTypes, isVirtualCol } from 'nocodb-sdk'
 import {
   ActiveViewInj,
   ChangePageInj,
@@ -159,6 +159,13 @@ onMounted(async () => {
 
 // provide view data reload hook as fallback to row data reload
 provide(ReloadRowDataHookInj, reloadViewDataHook)
+
+watch(view, async (nextView) => {
+  if (nextView?.type === ViewTypes.GALLERY) {
+    await loadData()
+    await loadGalleryData()
+  }
+})
 </script>
 
 <template>

packages/nc-gui/components/smartsheet/Grid.vue

@@ -161,16 +161,6 @@ openNewRecordFormHook?.on(async () => {
   expandForm(newRow, undefined, true)
 })
 
-watch(
-  () => view.value?.id,
-  async (next, old) => {
-    if (next && old && next !== old) {
-      await loadData()
-    }
-  },
-  { immediate: true },
-)
-
 const onresize = (colID: string, event: any) => {
   updateWidth(colID, event.detail)
 }
@@ -239,7 +229,6 @@ useEventListener(document, 'keyup', async (e: KeyboardEvent) => {
 
 /** On clicking outside of table reset active cell  */
 const smartTable = ref(null)
-
 onClickOutside(smartTable, () => {
   clearRangeRows()
   if (selected.col === null) return
@@ -247,6 +236,7 @@ onClickOutside(smartTable, () => {
   const activeCol = fields.value[selected.col]
 
   if (editEnabled && (isVirtualCol(activeCol) || activeCol.uidt === UITypes.JSON)) return
+
   selected.row = null
   selected.col = null
 })
@@ -326,7 +316,17 @@ const expandedFormOnRowIdDlg = computed({
 provide(ReloadRowDataHookInj, reloadViewDataHook)
 
 // trigger initial data load in grid
-reloadViewDataHook.trigger()
+// reloadViewDataHook.trigger()
+
+watch(
+  () => view.value?.id,
+  async (next, old) => {
+    if (next && next !== old) {
+      await loadData()
+    }
+  },
+  { immediate: true },
+)
 </script>
 
 <template>
@@ -489,12 +489,7 @@ reloadViewDataHook.trigger()
                         v-else
                         v-model="row.row[columnObj.title]"
                         :column="columnObj"
-                        :edit-enabled="
+                        :edit-enabled="hasEditPermission && editEnabled && selected.col === colIndex && selected.row === rowIndex"
-                          isUIAllowed('xcDatatableEditable') &&
-                          editEnabled &&
-                          selected.col === colIndex &&
-                          selected.row === rowIndex
-                        "
                         :row-index="rowIndex"
                         :active="selected.col === colIndex && selected.row === rowIndex"
                         @update:edit-enabled="editEnabled = false"
@@ -508,7 +503,7 @@ reloadViewDataHook.trigger()
               </template>
             </LazySmartsheetRow>
 
-            <tr v-if="!isView && !isLocked && isUIAllowed('xcDatatableEditable') && !isSqlView">
+            <tr v-if="!isView && !isLocked && hasEditPermission && !isSqlView">
               <td
                 v-e="['c:row:add:grid-bottom']"
                 :colspan="visibleColLength + 1"
@@ -527,7 +522,7 @@ reloadViewDataHook.trigger()
           </tbody>
         </table>
 
-        <template v-if="!isLocked && isUIAllowed('xcDatatableEditable')" #overlay>
+        <template v-if="!isLocked && hasEditPermission" #overlay>
           <a-menu class="shadow !rounded !py-0" @click="contextMenu = false">
             <a-menu-item v-if="contextMenuTarget" @click="deleteRow(contextMenuTarget.row)">
               <div v-e="['a:row:delete']" class="nc-project-menu-item">

packages/nc-gui/components/smartsheet/sidebar/MenuTop.vue

@@ -43,8 +43,6 @@ const { api } = useApi()
 
 const router = useRouter()
 
-const route = useRoute()
-
 /** Selected view(s) for menu */
 const selected = ref<string[]>([])
 
@@ -214,12 +212,18 @@ function openDeleteDialog(view: Record<string, any>) {
     close(1000)
   }
 }
+
+watch(views, (nextViews) => {
+  if (nextViews?.length && (!activeView.value || !nextViews.includes(activeView.value))) {
+    activeView.value = nextViews[0]
+  }
+})
 </script>
 
 <template>
   <a-menu ref="menuRef" :class="{ dragging }" class="nc-views-menu flex-1" :selected-keys="selected">
     <LazySmartsheetSidebarRenameableMenuItem
-      v-for="(view, index) of views"
+      v-for="view of views"
       :id="view.id"
       :key="view.id"
       :view="view"
@@ -227,8 +231,7 @@ function openDeleteDialog(view: Record<string, any>) {
       class="transition-all ease-in duration-300"
       :class="{
         'bg-gray-100': isMarked === view.id,
-        'active':
+        'active': activeView.id === view.id,
-          (route.params.viewTitle && route.params.viewTitle === view.title) || (route.params.viewTitle === '' && index === 0),
         [`nc-view-item nc-${viewTypeAlias[view.type] || view.type}-view-item`]: true,
       }"
       @change-view="changeView"

packages/nc-gui/components/smartsheet/sidebar/index.vue

@@ -72,6 +72,10 @@ watch(
           })
         }
       }
+    } else {
+      if (nextViews?.length && activeView.value !== nextViews[0]) {
+        activeView.value = nextViews[0]
+      }
     }
     /** if active view is not found, set it to first view */
     if (!activeView.value && nextViews.length) {
@@ -90,8 +94,8 @@ function openModal({ type, title = '', copyViewId }: { type: ViewTypes; title: s
 }
 
 /** Handle view creation */
-function onCreate(view: ViewType) {
+async function onCreate(view: ViewType) {
-  views.value.push(view)
+  await loadViews()
   router.push({ params: { viewTitle: view.title || '' } })
   modalOpen = false
   $e('a:view:create', { view: view.type })

packages/nc-gui/components/smartsheet/toolbar/FieldsMenu.vue

@@ -86,14 +86,16 @@ const onMove = (_event: { moved: { newIndex: number } }) => {
 
 const coverImageColumnId = computed({
   get: () =>
-    activeView.value?.type === ViewTypes.GALLERY ? (activeView.value?.view as GalleryType).fk_cover_image_col_id : undefined,
+    activeView.value?.type === ViewTypes.GALLERY && activeView.value?.view
+      ? (activeView.value?.view as GalleryType).fk_cover_image_col_id
+      : undefined,
   set: async (val) => {
     if (val && activeView.value?.type === ViewTypes.GALLERY && activeView.value?.id && activeView.value?.view) {
       await $api.dbView.galleryUpdate(activeView.value?.id, {
         ...activeView.value?.view,
         fk_cover_image_col_id: val,
       })
-      ;(activeView.value?.view as GalleryType).fk_cover_image_col_id = val
+      ;(activeView.value.view as GalleryType).fk_cover_image_col_id = val
       reloadViewMetaHook.trigger()
     }
   },
@@ -203,6 +205,7 @@ const getIcon = (c: ColumnType) =>
 :deep(.ant-checkbox-inner) {
   @apply transform scale-60;
 }
+
 :deep(.ant-checkbox) {
   @apply top-auto;
 }

packages/nc-gui/composables/useViews.ts

@@ -18,7 +18,7 @@ export function useViews(meta: MaybeRef<TableType | undefined>) {
     }
   }
 
-  watch(() => meta, loadViews, { immediate: true })
+  watch(meta, loadViews, { immediate: true })
 
   return { views: $$(views), loadViews }
 }

packages/nc-gui/pages/[projectType]/[projectId]/index.vue

@@ -491,10 +491,10 @@ onBeforeUnmount(reset)
       </a-layout-sider>
     </template>
 
-    <div :key="$route.fullPath.split('?')[0]">
+    <div>
       <LazyDashboardSettingsModal v-model="dialogOpen" :open-key="openDialogKey" />
 
-      <NuxtPage />
+      <NuxtPage :page-key="$route.params.projectId" />
 
       <LazyGeneralPreviewAs float />
     </div>

packages/nc-gui/pages/[projectType]/[projectId]/index/index.vue

@@ -79,7 +79,7 @@ function onEdit(targetKey: number, action: 'add' | 'remove' | string) {
       </div>
 
       <div class="w-full min-h-[300px] flex-auto">
-        <NuxtPage />
+        <NuxtPage :page-key="$route.name" />
       </div>
     </div>
   </div>

packages/nc-gui/pages/[projectType]/[projectId]/index/index/[type]/[title]/[[viewTitle]].vue

@@ -15,20 +15,22 @@ const activeTab = inject(
   computed(() => ({} as TabItem)),
 )
 
+watch(
+  () => route.params.title,
+  (tableTitle) => {
     /** wait until table list loads since meta load requires table list **/
     until(tables)
       .toMatch((tables) => tables.length > 0)
       .then(() => {
-    getMeta(route.params.title as string, true).finally(() => (loading.value = false))
+        getMeta(tableTitle as string, true).finally(() => (loading.value = false))
       })
+  },
+  { immediate: true },
+)
 </script>
 
 <template>
   <div class="w-full h-full">
-    <div v-if="loading" class="flex items-center justify-center h-full w-full">
+    <LazyTabsSmartsheet :active-tab="activeTab" />
-      <a-spin size="large" />
-    </div>
-
-    <LazyTabsSmartsheet v-else :key="route.params.title" :active-tab="activeTab" />
   </div>
 </template>

packages/nc-gui/pages/[projectType]/form/[viewId]/index.vue

@@ -14,7 +14,6 @@ const {
   passwordDlg,
   password,
   loadSharedView,
-  isLoading,
 } = useSharedFormStoreOrThrow()
 
 function isRequired(_columnObj: Record<string, any>, required = false) {

scripts/cypress/integration/common/00_pre_configurations.js

@@ -248,6 +248,7 @@ export const genTest = (apiType, dbType) => {
           .should("be.visible")
           .click();
         cy.get("button.ant-tabs-tab-remove").should("not.exist");
+        cy.wait(2000);
 
         // first instance of updating local storage information
         cy.saveLocalStorage();

scripts/cypress/integration/common/4c_form_view_detailed.js

@@ -214,9 +214,9 @@ export const genTest = (apiType, dbType) => {
         // submit button & validate
         cy.get(".nc-form").find("button").contains("Submit").click();
 
-        cy.get(".ant-alert-message")
+        cy.get(
-          .contains("Successfully submitted form data")
+          ".ant-alert-message :contains('Successfully submitted form data')"
-          .should("exist");
+        ).should("exist");
 
         // end of test removes newly added rows from table. that step validates if row was successfully added.
       });
@@ -238,9 +238,9 @@ export const genTest = (apiType, dbType) => {
 
         // submit button & validate
         cy.get(".nc-form").find("button").contains("Submit").click();
-        cy.get(".ant-alert-message")
+        cy.get(".ant-alert-message :contains('Congratulations!')").should(
-          .contains("Congratulations!")
+          "exist"
-          .should("exist");
+        );
 
         // end of test removes newly added rows from table. that step validates if row was successfully added.
       });
@@ -262,9 +262,9 @@ export const genTest = (apiType, dbType) => {
 
         // submit button & validate
         cy.get(".nc-form").find("button").contains("Submit").click();
-        cy.get(".ant-alert-message")
+        cy.get(".ant-alert-message :contains('Congratulations!')").should(
-          .contains("Congratulations")
+          "exist"
-          .should("exist");
+        );
         cy.get("button")
           .contains("Submit Another Form")
           .should("exist")
@@ -288,8 +288,7 @@ export const genTest = (apiType, dbType) => {
 
         // submit button & validate
         cy.get(".nc-form").find("button").contains("Submit").click();
-        cy.get(".ant-alert-message")
+        cy.get(".ant-alert-message :contains('Congratulations!')")
-          .contains("Congratulations")
           .should("exist")
           .then(() => {
             // validate if form has appeared again

scripts/cypress/integration/common/5a_user_role.js

@@ -152,7 +152,7 @@ export const genTest = (apiType, dbType) => {
           cy.getSettled("button.ant-tabs-tab-remove")
             .should("be.visible")
             .click();
-          cy.wait(500);
+          cy.wait(2000);
         }
 
         cy.saveLocalStorage();

scripts/cypress/integration/common/9a_QuickTest.js

@@ -115,7 +115,7 @@ export const genTest = (apiType, dbType, testMode) => {
         cy.wait(2000);
         // close team & auth tab
         cy.get("button.ant-tabs-tab-remove").should("exist").click();
-        cy.wait(1000);
+        cy.wait(2000);
       } else {
         cy.restoreLocalStorage();
       }

scripts/cypress/support/commands.js

@@ -179,7 +179,7 @@ Cypress.Commands.add("openTableTab", (tn, rc) => {
   // for some tables, linked records are not available immediately
   cy.wait(1000);
 
-  cy.get(".xc-row-table.nc-grid").should("exist");
+  cy.get(".xc-row-table.nc-grid", { timeout: 30000 }).should("exist");
 
   // wait for page rendering to complete
   if (rc != 0) {
@@ -199,7 +199,7 @@ Cypress.Commands.add("closeTableTab", (tn) => {
     .click();
 
   // subsequent tab open commands will fail if tab is not closed completely
-  cy.wait(100);
+  cy.wait(2000)
 });
 
 Cypress.Commands.add("openOrCreateGqlProject", (_args) => {

scripts/cypress/support/page_objects/navigation.js

@@ -110,6 +110,7 @@ export class _projectsPage {
     // close team & auth tab
     cy.getSettled("button.ant-tabs-tab-remove").should("be.visible").click();
     cy.get("button.ant-tabs-tab-remove").should("not.exist");
+    cy.wait(2000);
   }
 
   // Open existing project