From 8a32b0b865b31cbcbb4c19ec96dd83758d0320bc Mon Sep 17 00:00:00 2001 From: somebody1234 Date: Thu, 9 Jan 2025 22:45:50 +1000 Subject: [PATCH 1/2] Fix asset rows not being memoized --- app/gui/src/dashboard/layouts/AssetsTable.tsx | 148 +++++------------- .../layouts/Drive/assetTreeHooks.tsx | 10 +- .../layouts/Drive/assetsTableItemsHooks.tsx | 19 +-- .../layouts/Drive/suggestionsConstants.ts | 88 +++++++++++ 4 files changed, 143 insertions(+), 122 deletions(-) create mode 100644 app/gui/src/dashboard/layouts/Drive/suggestionsConstants.ts diff --git a/app/gui/src/dashboard/layouts/AssetsTable.tsx b/app/gui/src/dashboard/layouts/AssetsTable.tsx index d99417aefb9d..4f7d702b6a7c 100644 --- 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 = ( - {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 index ae78547764c3..9a43af00895b 100644 --- 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 index 7dad7f7826b9..19cafea8de10 100644 --- 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 = {} 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() 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 new file mode 100644 index 000000000000..be9790bdcc21 --- /dev/null +++ b/app/gui/src/dashboard/layouts/Drive/suggestionsConstants.ts @@ -0,0 +1,88 @@ +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'] }), + }, +] From e361f08c9f699287c72d46f1ecf4ec77f97ab7d1 Mon Sep 17 00:00:00 2001 From: somebody1234 Date: Thu, 9 Jan 2025 22:50:53 +1000 Subject: [PATCH 2/2] Fix lints --- app/gui/src/dashboard/layouts/Drive/suggestionsConstants.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/app/gui/src/dashboard/layouts/Drive/suggestionsConstants.ts b/app/gui/src/dashboard/layouts/Drive/suggestionsConstants.ts index be9790bdcc21..5401f1bcc745 100644 --- a/app/gui/src/dashboard/layouts/Drive/suggestionsConstants.ts +++ b/app/gui/src/dashboard/layouts/Drive/suggestionsConstants.ts @@ -1,3 +1,4 @@ +/** @file Constants related to suggestions for the asset search bar. */ import type * as assetSearchBar from '#/layouts/AssetSearchBar' export const SUGGESTIONS_FOR_NO: assetSearchBar.Suggestion[] = [