diff --git a/.claude/rules/sim-components.md b/.claude/rules/sim-components.md index 78b8f127a68..51090d2fb2a 100644 --- a/.claude/rules/sim-components.md +++ b/.claude/rules/sim-components.md @@ -12,3 +12,21 @@ Component authoring rules — structure order (refs → external hooks → store - `'use client'` only when using React hooks or browser-only APIs. - Prefer semantic HTML (`aside`, `nav`, `article`). - Optional-chain callbacks: `onAction?.(id)`. + +## List-render performance + +When rendering or sorting a list of rows against a lookup collection (members, folders, tags), keep the per-row work O(1): + +- **Precompute a lookup `Map` once**, never `array.find(...)` per row. Build `const byId = useMemo(() => { const m = new Map(); for (const x of items ?? []) m.set(x.id, x); return m }, [items])` and read `byId.get(id)` in the sort comparator, `.map(...)`, and cell builders. A `.find` inside a sort comparator is O(n²·log n) — the worst offender. Depend memos on the derived `Map`, not the raw array. +- **Sort with `[...array].sort(cmp)` in client code — NOT `array.toSorted(cmp)`.** SWC (Next's compiler) transforms syntax but does not polyfill prototype methods, and the repo sets no core-js/browserslist target, so `toSorted`/`toReversed`/`toSpliced`/`Array.prototype.with` ship as-is and throw `TypeError` on Safari <16 / iOS 15 (they landed in Safari 16). `tsconfig` `lib: ES2023` only affects type-checking, never the runtime. The `[...arr].sort()` spread copy is the intended cost — it keeps the sort non-mutating without an unpolyfilled builtin. These methods are only safe in server-only modules (no `'use client'`, executed on Node ≥20). react-doctor's `js-tosorted-immutable` is therefore a won't-fix in client components. +- **Partition in a single pass** — when splitting one collection into several (`fileIds`/`folderIds`), do one `for…of` pushing into each bucket and return `{ a, b }` from a single `useMemo`, not two memos that each `map→filter→map` the same source twice. + +## react-doctor (`npx react-doctor`) — apply the wins, skip the false positives + +react-doctor diagnostics are hypotheses, not verdicts — confirm against the code before acting, and preserve behavior. Known repo-specific false positives to NOT "fix": + +- `no-barrel-import` — barrel imports are the repo convention (see sim-imports.md, "Barrel Exports"). Keep them. +- `js-tosorted-immutable` — in `'use client'` code, keep `[...arr].sort(cmp)`; `toSorted` is unpolyfilled and crashes Safari <16 / iOS 15 (see "List-render performance" above). Only apply it in server-only modules. +- `rerender-state-only-in-handlers` / "state set but never rendered" — a false positive when the `useState` is consumed by a `useEffect`/`useLayoutEffect` dependency (the effect must re-run on change). Only convert to a ref when nothing reads the value reactively. +- `async-await-in-loop` on an upload/progress loop where sequential execution is intentional (per-item progress, server backpressure) — leave it. +- Broad refactors (`prefer-useReducer` for many `useState`, `no-giant-component` splits) — out of scope for a perf pass; note, don't churn. diff --git a/apps/sim/app/workspace/[workspaceId]/components/resource/components/owner-cell/owner-cell.tsx b/apps/sim/app/workspace/[workspaceId]/components/resource/components/owner-cell/owner-cell.tsx index 3df02cde797..23a845af1b6 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/resource/components/owner-cell/owner-cell.tsx +++ b/apps/sim/app/workspace/[workspaceId]/components/resource/components/owner-cell/owner-cell.tsx @@ -29,15 +29,20 @@ const OwnerAvatar = memo(function OwnerAvatar({ name, image }: OwnerAvatarProps) /** * Resolves a user ID into a ResourceCell with an avatar icon and display name. * Returns null label while members are still loading to avoid flashing raw IDs. + * + * Accepts either the raw member array or a precomputed `userId → member` map. + * Prefer the map form when resolving many rows so lookups stay O(1) instead of + * scanning the array per row. */ export function ownerCell( userId: string | null | undefined, - members?: WorkspaceMember[] + members?: WorkspaceMember[] | Map ): ResourceCell { if (!userId) return { label: null } if (!members) return { label: null } - const member = members.find((m) => m.userId === userId) + const member = + members instanceof Map ? members.get(userId) : members.find((m) => m.userId === userId) if (!member) return { label: null } return { diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index cbc9828cd53..e8be45d8fa6 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -78,7 +78,7 @@ import type { MoveOptionNode } from '@/app/workspace/[workspaceId]/files/move-op import { filesParsers, filesUrlKeys } from '@/app/workspace/[workspaceId]/files/search-params' import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' import { useContextMenu } from '@/app/workspace/[workspaceId]/w/components/sidebar/hooks' -import { useWorkspaceMembersQuery } from '@/hooks/queries/workspace' +import { useWorkspaceMembersQuery, type WorkspaceMember } from '@/hooks/queries/workspace' import { useBulkArchiveWorkspaceFileItems, useCreateWorkspaceFileFolder, @@ -197,6 +197,11 @@ export function Files() { const { data: files = EMPTY_WORKSPACE_FILES, isLoading, error } = useWorkspaceFiles(workspaceId) const { data: folders = EMPTY_WORKSPACE_FILE_FOLDERS } = useWorkspaceFileFolders(workspaceId) const { data: members } = useWorkspaceMembersQuery(workspaceId) + const membersById = useMemo(() => { + const map = new Map() + for (const member of members ?? []) map.set(member.userId, member) + return map + }, [members]) const uploadFile = useUploadWorkspaceFile() const notifyLimit = useLimitUpgradeToast() const deleteFile = useDeleteWorkspaceFile() @@ -416,8 +421,8 @@ export function Files() { cmp = new Date(a.updatedAt).getTime() - new Date(b.updatedAt).getTime() break case 'owner': - cmp = (members?.find((m) => m.userId === a.uploadedBy)?.name ?? '').localeCompare( - members?.find((m) => m.userId === b.uploadedBy)?.name ?? '' + cmp = (membersById.get(a.uploadedBy)?.name ?? '').localeCompare( + membersById.get(b.uploadedBy)?.name ?? '' ) break } @@ -431,7 +436,7 @@ export function Files() { sizeFilter, uploadedByFilter, activeSort, - members, + membersById, ]) const baseRows: ResourceRow[] = useMemo(() => { @@ -453,7 +458,7 @@ export function Files() { label: 'Folder', }, created: timeCell(folder.createdAt), - owner: ownerCell(folder.userId, members), + owner: ownerCell(folder.userId, membersById), updated: timeCell(folder.updatedAt), }, })) @@ -475,7 +480,7 @@ export function Files() { label: formatFileType(file.type, file.name), }, created: timeCell(file.uploadedAt), - owner: ownerCell(file.uploadedBy, members), + owner: ownerCell(file.uploadedBy, membersById), updated: timeCell(file.updatedAt), }, } @@ -483,7 +488,7 @@ export function Files() { }) return [...folderRows, ...fileRows] - }, [visibleFolders, filteredFiles, members, folderSizeMap]) + }, [visibleFolders, filteredFiles, membersById, folderSizeMap]) const rows: ResourceRow[] = useMemo(() => { if (!listRename.editingId) return baseRows @@ -525,22 +530,16 @@ export function Files() { const isAllSelected = visibleRowIds.length > 0 && visibleRowIds.every((id) => selectedRowIds.has(id)) - const selectedFileIds = useMemo( - () => - Array.from(selectedRowIds) - .map(parseRowId) - .filter((item) => item.kind === 'file') - .map((item) => item.id), - [selectedRowIds] - ) - const selectedFolderIds = useMemo( - () => - Array.from(selectedRowIds) - .map(parseRowId) - .filter((item) => item.kind === 'folder') - .map((item) => item.id), - [selectedRowIds] - ) + const { selectedFileIds, selectedFolderIds } = useMemo(() => { + const fileIds: string[] = [] + const folderIds: string[] = [] + for (const rowId of selectedRowIds) { + const item = parseRowId(rowId) + if (item.kind === 'file') fileIds.push(item.id) + else folderIds.push(item.id) + } + return { selectedFileIds: fileIds, selectedFolderIds: folderIds } + }, [selectedRowIds]) const selectableConfig = useMemo( () => ({ @@ -1749,7 +1748,7 @@ export function Files() { uploadedByFilter.length === 0 ? 'All' : uploadedByFilter.length === 1 - ? (members?.find((m) => m.userId === uploadedByFilter[0])?.name ?? '1 member') + ? (membersById.get(uploadedByFilter[0])?.name ?? '1 member') : `${uploadedByFilter.length} members` return ( @@ -1831,7 +1830,7 @@ export function Files() { )} ) - }, [typeFilter, sizeFilter, uploadedByFilter, memberOptions, members, hasActiveFilters]) + }, [typeFilter, sizeFilter, uploadedByFilter, memberOptions, membersById, hasActiveFilters]) const filterTags: FilterTag[] = useMemo(() => { const tags: FilterTag[] = [] @@ -1863,12 +1862,12 @@ export function Files() { if (uploadedByFilter.length > 0) { const label = uploadedByFilter.length === 1 - ? `Uploaded by: ${members?.find((m) => m.userId === uploadedByFilter[0])?.name ?? '1 member'}` + ? `Uploaded by: ${membersById.get(uploadedByFilter[0])?.name ?? '1 member'}` : `Uploaded by: ${uploadedByFilter.length} members` tags.push({ label, onRemove: () => setUploadedByFilter([]) }) } return tags - }, [typeFilter, sizeFilter, uploadedByFilter, members]) + }, [typeFilter, sizeFilter, uploadedByFilter, membersById]) if (fileIdFromRoute && !selectedFile && isLoading) { return (