Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .claude/rules/sim-components.md
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, T>(); 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.
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, WorkspaceMember>
): 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 {
Expand Down
53 changes: 26 additions & 27 deletions apps/sim/app/workspace/[workspaceId]/files/files.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<string, WorkspaceMember>()
for (const member of members ?? []) map.set(member.userId, member)
return map
}, [members])
const uploadFile = useUploadWorkspaceFile()
const notifyLimit = useLimitUpgradeToast()
const deleteFile = useDeleteWorkspaceFile()
Expand Down Expand Up @@ -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
}
Expand All @@ -431,7 +436,7 @@ export function Files() {
sizeFilter,
uploadedByFilter,
activeSort,
members,
membersById,
])

const baseRows: ResourceRow[] = useMemo(() => {
Expand All @@ -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),
},
}))
Expand All @@ -475,15 +480,15 @@ 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),
},
}
return row
})

return [...folderRows, ...fileRows]
}, [visibleFolders, filteredFiles, members, folderSizeMap])
}, [visibleFolders, filteredFiles, membersById, folderSizeMap])

const rows: ResourceRow[] = useMemo(() => {
if (!listRename.editingId) return baseRows
Expand Down Expand Up @@ -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(
() => ({
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -1831,7 +1830,7 @@ export function Files() {
)}
</div>
)
}, [typeFilter, sizeFilter, uploadedByFilter, memberOptions, members, hasActiveFilters])
}, [typeFilter, sizeFilter, uploadedByFilter, memberOptions, membersById, hasActiveFilters])

const filterTags: FilterTag[] = useMemo(() => {
const tags: FilterTag[] = []
Expand Down Expand Up @@ -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 (
Expand Down
Loading