Fix asset rows not being memoized (#12029)

- Fix asset rows no longer being memoized - Regression *likely* introduced by #11937 - The fix (`[...columns]` in `AssetsTable.tsx`) is unrelated though which is weird # Important Notes None
enso-org · Jan 9, 2025 · 0970a2a · 0970a2a
1 parent f5e330d
commit 0970a2a
Show file tree

Hide file tree

Showing 4 changed files with 144 additions and 122 deletions.
diff --git a/app/gui/src/dashboard/layouts/AssetsTable.tsx b/app/gui/src/dashboard/layouts/AssetsTable.tsx
@@ -146,6 +146,12 @@ import type { SortInfo } from '#/utilities/sorting'
 import { twJoin, twMerge } from '#/utilities/tailwindMerge'
 import Visibility from '#/utilities/Visibility'
 import invariant from 'tiny-invariant'
+import {
+  SUGGESTIONS_FOR_HAS,
+  SUGGESTIONS_FOR_NEGATIVE_TYPE,
+  SUGGESTIONS_FOR_NO,
+  SUGGESTIONS_FOR_TYPE,
+} from './Drive/suggestionsConstants'
 
 declare module '#/utilities/LocalStorage' {
   /** */
@@ -171,93 +177,6 @@ const ROW_HEIGHT_PX = 36
 /** The size of the loading spinner. */
 const LOADING_SPINNER_SIZE_PX = 36
 
-const SUGGESTIONS_FOR_NO: assetSearchBar.Suggestion[] = [
-  {
-    key: 'no:label',
-    render: () => 'no:label',
-    addToQuery: (query) => query.addToLastTerm({ nos: ['label'] }),
-    deleteFromQuery: (query) => query.deleteFromLastTerm({ nos: ['label'] }),
-  },
-  {
-    key: 'no:description',
-    render: () => 'no:description',
-    addToQuery: (query) => query.addToLastTerm({ nos: ['description'] }),
-    deleteFromQuery: (query) => query.deleteFromLastTerm({ nos: ['description'] }),
-  },
-]
-const SUGGESTIONS_FOR_HAS: assetSearchBar.Suggestion[] = [
-  {
-    key: 'has:label',
-    render: () => 'has:label',
-    addToQuery: (query) => query.addToLastTerm({ negativeNos: ['label'] }),
-    deleteFromQuery: (query) => query.deleteFromLastTerm({ negativeNos: ['label'] }),
-  },
-  {
-    key: 'has:description',
-    render: () => 'has:description',
-    addToQuery: (query) => query.addToLastTerm({ negativeNos: ['description'] }),
-    deleteFromQuery: (query) => query.deleteFromLastTerm({ negativeNos: ['description'] }),
-  },
-]
-const SUGGESTIONS_FOR_TYPE: assetSearchBar.Suggestion[] = [
-  {
-    key: 'type:project',
-    render: () => 'type:project',
-    addToQuery: (query) => query.addToLastTerm({ types: ['project'] }),
-    deleteFromQuery: (query) => query.deleteFromLastTerm({ types: ['project'] }),
-  },
-  {
-    key: 'type:folder',
-    render: () => 'type:folder',
-    addToQuery: (query) => query.addToLastTerm({ types: ['folder'] }),
-    deleteFromQuery: (query) => query.deleteFromLastTerm({ types: ['folder'] }),
-  },
-  {
-    key: 'type:file',
-    render: () => 'type:file',
-    addToQuery: (query) => query.addToLastTerm({ types: ['file'] }),
-    deleteFromQuery: (query) => query.deleteFromLastTerm({ types: ['file'] }),
-  },
-  {
-    key: 'type:secret',
-    render: () => 'type:secret',
-    addToQuery: (query) => query.addToLastTerm({ types: ['secret'] }),
-    deleteFromQuery: (query) => query.deleteFromLastTerm({ types: ['secret'] }),
-  },
-  {
-    key: 'type:datalink',
-    render: () => 'type:datalink',
-    addToQuery: (query) => query.addToLastTerm({ types: ['datalink'] }),
-    deleteFromQuery: (query) => query.deleteFromLastTerm({ types: ['datalink'] }),
-  },
-]
-const SUGGESTIONS_FOR_NEGATIVE_TYPE: assetSearchBar.Suggestion[] = [
-  {
-    key: 'type:project',
-    render: () => 'type:project',
-    addToQuery: (query) => query.addToLastTerm({ negativeTypes: ['project'] }),
-    deleteFromQuery: (query) => query.deleteFromLastTerm({ negativeTypes: ['project'] }),
-  },
-  {
-    key: 'type:folder',
-    render: () => 'type:folder',
-    addToQuery: (query) => query.addToLastTerm({ negativeTypes: ['folder'] }),
-    deleteFromQuery: (query) => query.deleteFromLastTerm({ negativeTypes: ['folder'] }),
-  },
-  {
-    key: 'type:file',
-    render: () => 'type:file',
-    addToQuery: (query) => query.addToLastTerm({ negativeTypes: ['file'] }),
-    deleteFromQuery: (query) => query.deleteFromLastTerm({ negativeTypes: ['file'] }),
-  },
-  {
-    key: 'type:datalink',
-    render: () => 'type:datalink',
-    addToQuery: (query) => query.addToLastTerm({ negativeTypes: ['datalink'] }),
-    deleteFromQuery: (query) => query.deleteFromLastTerm({ negativeTypes: ['datalink'] }),
-  },
-]
-
 /** Information related to a drag selection. */
 interface DragSelectionInfo {
   readonly initialIndex: number
@@ -803,7 +722,7 @@ function AssetsTable(props: AssetsTableProps) {
     }
   }, [navigator2D, setMostRecentlySelectedIndex])
 
-  const onKeyDown = (event: KeyboardEvent) => {
+  const onKeyDown = useEventCallback((event: KeyboardEvent) => {
     const { selectedAssets } = driveStore.getState()
     const prevIndex = mostRecentlySelectedIndexRef.current
     const item = prevIndex == null ? null : visibleItems[prevIndex]
@@ -983,7 +902,7 @@ function AssetsTable(props: AssetsTableProps) {
         break
       }
     }
-  }
+  })
 
   useEffect(() => {
     const onClick = () => {
@@ -1087,22 +1006,38 @@ function AssetsTable(props: AssetsTableProps) {
     setEnabledColumns((currentColumns) => withPresence(currentColumns, column, false))
   })
 
-  const state: AssetsTableState = {
-    backend,
-    rootDirectoryId,
-    scrollContainerRef: rootRef,
-    category,
-    sortInfo,
-    setSortInfo,
-    query,
-    setQuery,
-    nodeMap: nodeMapRef,
-    hideColumn,
-    doCopy,
-    doCut,
-    doPaste,
-    getAssetNodeById,
-  }
+  const state: AssetsTableState = useMemo(
+    () => ({
+      backend,
+      rootDirectoryId,
+      scrollContainerRef: rootRef,
+      category,
+      sortInfo,
+      setSortInfo,
+      query,
+      setQuery,
+      nodeMap: nodeMapRef,
+      hideColumn,
+      doCopy,
+      doCut,
+      doPaste,
+      getAssetNodeById,
+    }),
+    [
+      backend,
+      category,
+      doCopy,
+      doCut,
+      doPaste,
+      getAssetNodeById,
+      hideColumn,
+      nodeMapRef,
+      query,
+      rootDirectoryId,
+      setQuery,
+      sortInfo,
+    ],
+  )
 
   useEffect(() => {
     // In some browsers, at least in Chrome 126,
@@ -1412,7 +1347,8 @@ function AssetsTable(props: AssetsTableProps) {
 
   const headerRow = (
     <tr ref={headerRowRef} className="rounded-none text-sm font-semibold">
-      {columns.map((column) => {
+      {[...columns].map((column) => {
+        // The spread on the line above is required for React Compiler to compile this component.
         // This is a React component, even though it does not contain JSX.
         const Heading = COLUMN_HEADING[column]
 

diff --git a/app/gui/src/dashboard/layouts/Drive/assetTreeHooks.tsx b/app/gui/src/dashboard/layouts/Drive/assetTreeHooks.tsx
@@ -125,7 +125,7 @@ export function useAssetTree(options: UseAssetTreeOptions) {
        * in the loaded data. If it is loaded, we append that data to the asset node
        * and do the same for the children.
        */
-      const withChildren = (node: AnyAssetTreeNode, depth: number) => {
+      const withChildren = (node: AnyAssetTreeNode, depth: number): AnyAssetTreeNode => {
         const { item } = node
 
         if (assetIsDirectory(item)) {
@@ -136,19 +136,19 @@ export function useAssetTree(options: UseAssetTreeOptions) {
           )
 
           if (childrenAssetsQuery == null || childrenAssetsQuery.isLoading) {
-            node = node.with({
+            return node.with({
               children: [AssetTreeNode.fromAsset(createSpecialLoadingAsset(item.id), depth, '')],
             })
           } else if (childrenAssetsQuery.isError) {
-            node = node.with({
+            return node.with({
               children: [AssetTreeNode.fromAsset(createSpecialErrorAsset(item.id), depth, '')],
             })
           } else if (nestedChildren?.length === 0) {
-            node = node.with({
+            return node.with({
               children: [AssetTreeNode.fromAsset(createSpecialEmptyAsset(item.id), depth, '')],
             })
           } else if (nestedChildren != null) {
-            node = node.with({
+            return node.with({
               children: nestedChildren.map((child) => withChildren(child, depth + 1)),
             })
           }

diff --git a/app/gui/src/dashboard/layouts/Drive/assetsTableItemsHooks.tsx b/app/gui/src/dashboard/layouts/Drive/assetsTableItemsHooks.tsx
@@ -1,6 +1,4 @@
 /** @file A hook to return the items in the assets table. */
-import { useMemo } from 'react'
-
 import type { AnyAsset, AssetId } from 'enso-common/src/services/Backend'
 import { AssetType, getAssetPermissionName } from 'enso-common/src/services/Backend'
 import { PermissionAction } from 'enso-common/src/utilities/permissions'
@@ -65,7 +63,7 @@ export function useAssetsTableItems(options: UseAssetsTableOptions) {
 
   const setAssetItems = useStore(ASSET_ITEMS_STORE, (store) => store.setItems)
 
-  const filter = useMemo(() => {
+  const filter = (() => {
     const globCache: Record<string, RegExp> = {}
     if (/^\s*$/.test(query.query)) {
       return null
@@ -171,9 +169,9 @@ export function useAssetsTableItems(options: UseAssetsTableOptions) {
         )
       }
     }
-  }, [query])
+  })()
 
-  const visibilities = useMemo(() => {
+  const visibilities = (() => {
     const map = new Map<AssetId, Visibility>()
 
     const processNode = (node: AnyAssetTreeNode) => {
@@ -204,9 +202,9 @@ export function useAssetsTableItems(options: UseAssetsTableOptions) {
     processNode(assetTree)
 
     return map
-  }, [assetTree, filter])
+  })()
 
-  const displayItems = useMemo(() => {
+  const displayItems = (() => {
     if (sortInfo == null) {
       const flatTree = assetTree.preorderTraversal((children) =>
         children.filter((child) => expandedDirectoryIds.includes(child.item.parentId)),
@@ -238,13 +236,12 @@ export function useAssetsTableItems(options: UseAssetsTableOptions) {
 
       return flatTree
     }
-  }, [sortInfo, assetTree, expandedDirectoryIds])
+  })()
 
   setAssetItems(displayItems.map((item) => item.item))
 
-  const visibleItems = useMemo(
-    () => displayItems.filter((item) => visibilities.get(item.item.id) !== Visibility.hidden),
-    [displayItems, visibilities],
+  const visibleItems = displayItems.filter(
+    (item) => visibilities.get(item.item.id) !== Visibility.hidden,
   )
 
   return { visibilities, displayItems, visibleItems } as const

diff --git a/app/gui/src/dashboard/layouts/Drive/suggestionsConstants.ts b/app/gui/src/dashboard/layouts/Drive/suggestionsConstants.ts
@@ -0,0 +1,89 @@
+/** @file Constants related to suggestions for the asset search bar. */
+import type * as assetSearchBar from '#/layouts/AssetSearchBar'
+
+export const SUGGESTIONS_FOR_NO: assetSearchBar.Suggestion[] = [
+  {
+    key: 'no:label',
+    render: () => 'no:label',
+    addToQuery: (query) => query.addToLastTerm({ nos: ['label'] }),
+    deleteFromQuery: (query) => query.deleteFromLastTerm({ nos: ['label'] }),
+  },
+  {
+    key: 'no:description',
+    render: () => 'no:description',
+    addToQuery: (query) => query.addToLastTerm({ nos: ['description'] }),
+    deleteFromQuery: (query) => query.deleteFromLastTerm({ nos: ['description'] }),
+  },
+]
+export const SUGGESTIONS_FOR_HAS: assetSearchBar.Suggestion[] = [
+  {
+    key: 'has:label',
+    render: () => 'has:label',
+    addToQuery: (query) => query.addToLastTerm({ negativeNos: ['label'] }),
+    deleteFromQuery: (query) => query.deleteFromLastTerm({ negativeNos: ['label'] }),
+  },
+  {
+    key: 'has:description',
+    render: () => 'has:description',
+    addToQuery: (query) => query.addToLastTerm({ negativeNos: ['description'] }),
+    deleteFromQuery: (query) => query.deleteFromLastTerm({ negativeNos: ['description'] }),
+  },
+]
+export const SUGGESTIONS_FOR_TYPE: assetSearchBar.Suggestion[] = [
+  {
+    key: 'type:project',
+    render: () => 'type:project',
+    addToQuery: (query) => query.addToLastTerm({ types: ['project'] }),
+    deleteFromQuery: (query) => query.deleteFromLastTerm({ types: ['project'] }),
+  },
+  {
+    key: 'type:folder',
+    render: () => 'type:folder',
+    addToQuery: (query) => query.addToLastTerm({ types: ['folder'] }),
+    deleteFromQuery: (query) => query.deleteFromLastTerm({ types: ['folder'] }),
+  },
+  {
+    key: 'type:file',
+    render: () => 'type:file',
+    addToQuery: (query) => query.addToLastTerm({ types: ['file'] }),
+    deleteFromQuery: (query) => query.deleteFromLastTerm({ types: ['file'] }),
+  },
+  {
+    key: 'type:secret',
+    render: () => 'type:secret',
+    addToQuery: (query) => query.addToLastTerm({ types: ['secret'] }),
+    deleteFromQuery: (query) => query.deleteFromLastTerm({ types: ['secret'] }),
+  },
+  {
+    key: 'type:datalink',
+    render: () => 'type:datalink',
+    addToQuery: (query) => query.addToLastTerm({ types: ['datalink'] }),
+    deleteFromQuery: (query) => query.deleteFromLastTerm({ types: ['datalink'] }),
+  },
+]
+export const SUGGESTIONS_FOR_NEGATIVE_TYPE: assetSearchBar.Suggestion[] = [
+  {
+    key: 'type:project',
+    render: () => 'type:project',
+    addToQuery: (query) => query.addToLastTerm({ negativeTypes: ['project'] }),
+    deleteFromQuery: (query) => query.deleteFromLastTerm({ negativeTypes: ['project'] }),
+  },
+  {
+    key: 'type:folder',
+    render: () => 'type:folder',
+    addToQuery: (query) => query.addToLastTerm({ negativeTypes: ['folder'] }),
+    deleteFromQuery: (query) => query.deleteFromLastTerm({ negativeTypes: ['folder'] }),
+  },
+  {
+    key: 'type:file',
+    render: () => 'type:file',
+    addToQuery: (query) => query.addToLastTerm({ negativeTypes: ['file'] }),
+    deleteFromQuery: (query) => query.deleteFromLastTerm({ negativeTypes: ['file'] }),
+  },
+  {
+    key: 'type:datalink',
+    render: () => 'type:datalink',
+    addToQuery: (query) => query.addToLastTerm({ negativeTypes: ['datalink'] }),
+    deleteFromQuery: (query) => query.deleteFromLastTerm({ negativeTypes: ['datalink'] }),
+  },
+]