diff --git a/.claude/rules/sim-react-performance.md b/.claude/rules/sim-react-performance.md new file mode 100644 index 00000000000..13e0d9a9d9c --- /dev/null +++ b/.claude/rules/sim-react-performance.md @@ -0,0 +1,80 @@ +# React & Render Performance + +Behavior-preserving performance idioms for components, hooks, and hot render paths. These are safe defaults — apply them freely. For the render-causing *effect/state* anti-patterns (derived state in effects, effect chains, state synced to a prop), use the dedicated skills: `/you-might-not-need-an-effect`, `/you-might-not-need-state`, `/you-might-not-need-a-memo`, `/you-might-not-need-a-callback`. Those refactors change render timing — verify them against the running UI, never mass-apply blind. + +## Lazy-init refs that hold objects + +`useRef(new Map())` / `useRef(new Set())` / `useRef({...})` allocates a fresh object on **every render** and throws it away — only the first is ever kept. Lazy-init instead so the allocation happens once. + +```typescript +// ✗ Bad — allocates a new Map each render, discards all but the first +const cacheRef = useRef>(new Map()) + +// ✓ Good — allocated once, stable identity thereafter +const cacheRef = useRef | null>(null) +cacheRef.current ??= new Map() +``` + +Read `cacheRef.current` directly inside effects/handlers — refs are stable and never belong in a dependency array. A cheap primitive (`useRef(0)`, `useRef('')`, `useRef(null)`) needs no lazy init. + +## Hoist static values and closure-free functions to module scope + +A value or function declared inside a component is rebuilt every render. If it captures **nothing** from component scope (no props/state/refs), move it above the component at module scope. This skips the per-render allocation and keeps a stable identity so memoized children don't re-render. + +```typescript +// ✗ Bad — rebuilt every render, new identity each time +function Toolbar({ mode }: ToolbarProps) { + const TITLES = { create: 'Add', edit: 'Configure' } as const + const handleWheel = (e: React.WheelEvent) => e.currentTarget.scrollBy(e.deltaX, e.deltaY) + // ... +} + +// ✓ Good — allocated once at module load +const TITLES = { create: 'Add', edit: 'Configure' } as const +function handleWheel(e: React.WheelEvent) { + e.currentTarget.scrollBy(e.deltaX, e.deltaY) +} +function Toolbar({ mode }: ToolbarProps) { /* ... */ } +``` + +A closure-free function that IS wired through a ref sink or intentionally kept for stable identity may stay inline — hoisting a one-line `preventDefault` handler is churn, not a win. Hoist when it removes a real per-render allocation or unblocks child memoization. + +## Pre-index with Map/Set for repeated lookups + +`array.find()` / `array.includes()` / `array.indexOf()` scan the whole list each call. Inside a loop or a hot render path over a non-trivial list, that is O(n·m). Build a `Map` (for lookup-by-key) or `Set` (for membership) **once before** the loop, then look up in O(1). + +```typescript +// ✗ Bad — find() re-scans outputs for every column +for (const child of columns) { + const output = group.outputs.find((o) => o.columnName === getColumnId(child)) +} + +// ✓ Good — index once, then O(1) lookups +const outputByName = new Map() +for (const o of group.outputs) { + if (!outputByName.has(o.columnName)) outputByName.set(o.columnName, o) // first wins, matches find() +} +for (const child of columns) { + const output = outputByName.get(getColumnId(child)) +} +``` + +Preserve `.find()`'s **first-match** semantics when duplicate keys are possible: `new Map(arr.map(...))` keeps the *last* entry, so guard with `if (!map.has(key))` when replacing a `.find()`. Skip this for tiny, cold arrays (a handful of items in an event handler) where the Map build costs more than it saves. + +## Never mutate a shared array in place + +The real bug to avoid is `array.sort()` / `array.reverse()` on an array you don't own — sorting a React Query cache array in place corrupts shared state. Always sort a copy: + +```typescript +// ✗ Bad — mutates the (possibly shared) source array in place +return items.sort(compare) + +// ✓ Good — sorts a throwaway copy, source untouched +return [...items].sort(compare) +``` + +**Do NOT reach for `toSorted()` / `toReversed()` / `with()` / `toSpliced()` on client render paths.** They are ES2023 *runtime* methods — and a tsconfig `"lib": ["ES2023"]` only makes them **type-check**, it does not make them **run**. Next/SWC compiles syntax but does **not** polyfill prototype methods, and the default browserslist still includes browsers without them (`toSorted` landed in Safari 16 / iOS 16, so any device capped at iOS 15 throws `TypeError: x.toSorted is not a function` and crashes the page). The perf difference vs `[...arr].sort()` is negligible (both allocate one array), so the copy-then-sort form is the correct default everywhere client code runs. Only consider the immutable methods in Node-only code (server routes, scripts) on Node ≥20, where the runtime is known. + +## Local feature barrels are the convention — do not "fix" them + +Tooling (e.g. react-doctor's `no-barrel-import`) will flag imports from local `index.ts` barrels as a bundle cost. In this repo that is a **false positive**: barrel imports for 3+ export folders are mandated by `.claude/rules/sim-imports.md`. Leave them. diff --git a/CLAUDE.md b/CLAUDE.md index 704a4446179..df1f7fb9826 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -127,6 +127,8 @@ export function Component({ requiredProp, optionalProp = false }: ComponentProps Extract when: 50+ lines, used in 2+ files, or has own state/logic. Keep inline when: < 10 lines, single use, purely presentational. +Behavior-preserving render-performance idioms — lazy-init object refs, hoist closure-free values/functions to module scope, pre-index repeated lookups with `Map`/`Set`, and never mutating a shared array in place — are in `.claude/rules/sim-react-performance.md` (which also explains why `toSorted`/`toReversed` are unsafe on client render paths despite the ES2023 tsconfig lib — SWC does not polyfill prototype methods, so use `[...arr].sort()`). For the render-timing effect/state anti-patterns use the `/you-might-not-need-*` skills and verify against the running UI. + ## API Contracts Boundary HTTP request and response shapes for all routes under `apps/sim/app/api/**` live in `apps/sim/lib/api/contracts/**` (one file per resource family — `folders.ts`, `chats.ts`, `knowledge.ts`, etc.). Routes never define route-local boundary Zod schemas, and clients never define ad-hoc wire types — both sides consume the same contract. diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx index 77f5521a8b7..d937a772d33 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/cells/inline-editors.tsx @@ -19,6 +19,15 @@ interface InlineEditorProps { onCancel: () => void } +/** Redirect wheel gestures over an inline editor to the surrounding table scroll container. */ +function handleEditorWheel(e: React.WheelEvent) { + e.preventDefault() + const container = e.currentTarget.closest('[data-table-scroll]') as HTMLElement | null + if (container) { + container.scrollBy(e.deltaX, e.deltaY) + } +} + /** Inline editor for `date` columns — text input + popover calendar. */ function InlineDateEditor({ value, @@ -152,14 +161,6 @@ function InlineTextEditor({ } }, []) - const handleWheel = (e: React.WheelEvent) => { - e.preventDefault() - const container = e.currentTarget.closest('[data-table-scroll]') as HTMLElement | null - if (container) { - container.scrollBy(e.deltaX, e.deltaY) - } - } - const doSave = (reason: SaveReason) => { if (doneRef.current) return doneRef.current = true @@ -194,7 +195,7 @@ function InlineTextEditor({ value={draft ?? ''} onChange={(e) => setDraft(e.target.value)} onKeyDown={handleKeyDown} - onWheel={handleWheel} + onWheel={handleEditorWheel} onBlur={() => doSave('blur')} className='w-full min-w-0 select-text border-none bg-transparent p-0 text-[var(--text-primary)] text-small outline-none' /> diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts index 674f00865e0..da0e5674751 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts @@ -116,10 +116,18 @@ export function expandToDisplayColumns( size++ } const group = groupById.get(gid) + // Pre-index outputs by column name for O(1) lookup. First output wins on a + // duplicate columnName, exactly matching the previous `Array.find()` behavior. + const outputByColumnName = new Map() + if (group) { + for (const o of group.outputs) { + if (!outputByColumnName.has(o.columnName)) outputByColumnName.set(o.columnName, o) + } + } const startIdx = out.length for (let k = 0; k < size; k++) { const child = columns[i + k] - const output = group?.outputs.find((o) => o.columnName === getColumnId(child)) + const output = outputByColumnName.get(getColumnId(child)) out.push({ ...child, key: getColumnId(child), diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/workflow-sidebar/workflow-sidebar.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/workflow-sidebar/workflow-sidebar.tsx index f6fe1149990..8392000e51a 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/workflow-sidebar/workflow-sidebar.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/workflow-sidebar/workflow-sidebar.tsx @@ -121,6 +121,12 @@ interface WorkflowSidebarProps { const OUTPUT_VALUE_SEPARATOR = '::' +const TITLE_BY_MODE = { + create: 'Add workflow', + 'edit-group': 'Configure workflow', + 'edit-output': 'Configure output column', +} as const + const encodeOutputValue = (blockId: string, path: string) => `${blockId}${OUTPUT_VALUE_SEPARATOR}${path}` @@ -772,15 +778,10 @@ export function WorkflowSidebarBody({ updateWorkflowGroup.isPending || updateColumn.isPending || !depsValid - const titleByMode = { - create: 'Add workflow', - 'edit-group': 'Configure workflow', - 'edit-output': 'Configure output column', - } as const const title = config.mode === 'create' && config.kind === 'enrichment' && config.enrichmentName ? config.enrichmentName - : titleByMode[config.mode] + : TITLE_BY_MODE[config.mode] const showBackButton = isEnrichment && Boolean(onBack) // edit-output mode is single-select on the output picker; everywhere else diff --git a/apps/sim/app/workspace/[workspaceId]/tables/components/import-progress-menu/use-workspace-imports.ts b/apps/sim/app/workspace/[workspaceId]/tables/components/import-progress-menu/use-workspace-imports.ts index a89483fc9e7..d72ed8b6d20 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/components/import-progress-menu/use-workspace-imports.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/components/import-progress-menu/use-workspace-imports.ts @@ -51,12 +51,13 @@ export function useWorkspaceImports( // jobs), so the tray reads them from their dedicated workspace listing. const { data: exportJobs } = useWorkspaceExportJobs(workspaceId) - const prevStatus = useRef>(new Map()) + const prevStatusRef = useRef | null>(null) useEffect(() => { if (!tables) return + const prevStatus = (prevStatusRef.current ??= new Map()) const store = useImportTrayStore.getState() for (const table of tables) { - const before = prevStatus.current.get(table.id) + const before = prevStatus.get(table.id) const now = table.jobStatus ?? 'none' if (before === 'running' && now === 'ready') { // Success toast only for imports — deletes reflect instantly in the grid and backfills @@ -80,7 +81,7 @@ export function useWorkspaceImports( if (table.jobType === 'import') store.notify(table.id) } if (now !== 'running' && store.isCanceled(table.id)) store.consumeCanceled(table.id) - prevStatus.current.set(table.id, now) + prevStatus.set(table.id, now) } }, [tables])