From 1da718f1a82492fce55f1e54ab6124afbc308c4e Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 1 Jul 2026 12:21:52 -0700 Subject: [PATCH 1/4] improvement(tables): react-doctor safe performance pass Apply high-confidence, behavior-preserving perf fixes surfaced by react-doctor on the tables module: - Lazy-init the import-status ref so it stops allocating and discarding a fresh Map on every render (rerender-lazy-ref-init). - expandToDisplayColumns: build an outputs-by-column-name Map once per workflow group instead of Array.find() per column (js-index-maps). - Sort the tables list with toSorted() instead of [...list].sort() (js-tosorted-immutable; ES2023 lib is enabled for apps/sim). - Hoist the static TITLE_BY_MODE map and the pure editor wheel handler to module scope so they are not rebuilt each render. Left the effect/selection refactors (state-synced-to-prop, effect chains) untouched: those effects react to async-loaded rows and are not the copy-a-prop-into-state anti-pattern; refactoring them needs live grid verification. Barrel-import findings are false positives against the repo's mandated barrel-import convention. --- .../table-grid/cells/inline-editors.tsx | 19 ++++++++++--------- .../[tableId]/components/table-grid/utils.ts | 5 ++++- .../workflow-sidebar/workflow-sidebar.tsx | 13 +++++++------ .../use-workspace-imports.ts | 8 +++++--- .../workspace/[workspaceId]/tables/tables.tsx | 2 +- 5 files changed, 27 insertions(+), 20 deletions(-) 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..b7c7c13ac1e 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,13 @@ export function expandToDisplayColumns( size++ } const group = groupById.get(gid) + const outputByColumnName = group + ? new Map(group.outputs.map((o) => [o.columnName, o])) + : undefined 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..49dd19645d7 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,14 @@ 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) + prevStatusRef.current ??= new Map() 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 +82,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]) diff --git a/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx b/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx index c88ae332b2b..a33ae1da8f4 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx @@ -185,7 +185,7 @@ export function Tables() { } const col = activeSort?.column ?? 'updated' const dir = activeSort?.direction ?? 'desc' - return [...result].sort((a, b) => { + return result.toSorted((a, b) => { let cmp = 0 switch (col) { case 'name': From cd25b344a09f5ae8a38df2e8ce695027f6573775 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 1 Jul 2026 12:37:39 -0700 Subject: [PATCH 2/4] improvement(tables): preserve find() first-match semantics + document perf idioms - expandToDisplayColumns: build the outputs-by-column-name index with an explicit first-write-wins loop so it is provably identical to the prior Array.find() on the (schema-precluded) duplicate-columnName case, not last-wins as new Map(entries) would be. - Add .claude/rules/sim-react-performance.md capturing the behavior- preserving render-perf idioms applied here (lazy object refs, module- scope hoisting, Map/Set pre-indexing, immutable array methods with the ES2022/ES2023 lib caveat, local-barrel false-positive note), and point to it from CLAUDE.md. --- .claude/rules/sim-react-performance.md | 80 +++++++++++++++++++ CLAUDE.md | 2 + .../[tableId]/components/table-grid/utils.ts | 13 ++- 3 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 .claude/rules/sim-react-performance.md diff --git a/.claude/rules/sim-react-performance.md b/.claude/rules/sim-react-performance.md new file mode 100644 index 00000000000..5447c268466 --- /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. + +## Immutable array methods over spread-then-mutate + +Use `toSorted()` / `toReversed()` / `with()` / `toSpliced()` instead of copying an array only to mutate the copy. One pass instead of copy-then-mutate, and non-mutating by construction (so it never risks mutating a React Query cache array in place — which a bare `.sort()` would). + +```typescript +// ✗ Bad — copies just to sort the copy +return [...items].sort(compare) + +// ✓ Good — sorts without the throwaway copy, still non-mutating +return items.toSorted(compare) +``` + +**Lib caveat:** these are ES2023. `apps/sim` sets `"lib": ["ES2023", ...]` in its `tsconfig.json`, so they type-check there. Packages under `packages/*` inherit the **ES2022** base tsconfig — in those, `toSorted` does not resolve; keep `[...arr].sort()`. Check the nearest `tsconfig` `lib` before reaching for these. + +## 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..2fbd8c817df 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 immutable array methods (`toSorted`) with the ES2022-vs-ES2023 lib caveat — are in `.claude/rules/sim-react-performance.md`. 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/utils.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts index b7c7c13ac1e..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,13 +116,18 @@ export function expandToDisplayColumns( size++ } const group = groupById.get(gid) - const outputByColumnName = group - ? new Map(group.outputs.map((o) => [o.columnName, o])) - : undefined + // 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 = outputByColumnName?.get(getColumnId(child)) + const output = outputByColumnName.get(getColumnId(child)) out.push({ ...child, key: getColumnId(child), From dc4510c771a73124b5fcf133bc100579e2e77bce Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 1 Jul 2026 12:40:17 -0700 Subject: [PATCH 3/4] fix(tables): revert toSorted to spread-sort for universal browser support Array.prototype.toSorted is ES2023 runtime; SWC does not polyfill it and the default browserslist still includes browsers without it (iOS 15 Safari), so the tables page would crash there. [...result].sort() is non-mutating (sorts a copy, never touches the React Query cache array) and works everywhere; the perf delta is negligible. Correct the sim-react-performance rule doc to reflect that tsconfig lib only affects type-checking, not runtime availability. --- .claude/rules/sim-react-performance.md | 14 +++++++------- .../app/workspace/[workspaceId]/tables/tables.tsx | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.claude/rules/sim-react-performance.md b/.claude/rules/sim-react-performance.md index 5447c268466..13e0d9a9d9c 100644 --- a/.claude/rules/sim-react-performance.md +++ b/.claude/rules/sim-react-performance.md @@ -61,19 +61,19 @@ for (const child of columns) { 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. -## Immutable array methods over spread-then-mutate +## Never mutate a shared array in place -Use `toSorted()` / `toReversed()` / `with()` / `toSpliced()` instead of copying an array only to mutate the copy. One pass instead of copy-then-mutate, and non-mutating by construction (so it never risks mutating a React Query cache array in place — which a bare `.sort()` would). +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 — copies just to sort the copy -return [...items].sort(compare) +// ✗ Bad — mutates the (possibly shared) source array in place +return items.sort(compare) -// ✓ Good — sorts without the throwaway copy, still non-mutating -return items.toSorted(compare) +// ✓ Good — sorts a throwaway copy, source untouched +return [...items].sort(compare) ``` -**Lib caveat:** these are ES2023. `apps/sim` sets `"lib": ["ES2023", ...]` in its `tsconfig.json`, so they type-check there. Packages under `packages/*` inherit the **ES2022** base tsconfig — in those, `toSorted` does not resolve; keep `[...arr].sort()`. Check the nearest `tsconfig` `lib` before reaching for these. +**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 diff --git a/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx b/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx index a33ae1da8f4..c88ae332b2b 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/tables.tsx @@ -185,7 +185,7 @@ export function Tables() { } const col = activeSort?.column ?? 'updated' const dir = activeSort?.direction ?? 'desc' - return result.toSorted((a, b) => { + return [...result].sort((a, b) => { let cmp = 0 switch (col) { case 'name': From ba2f6f1fa77fdc1682d949c2dc6eaae9e291cb32 Mon Sep 17 00:00:00 2001 From: waleed Date: Wed, 1 Jul 2026 13:06:48 -0700 Subject: [PATCH 4/4] refactor(tables): tidy lazy ref init and align perf-rule cross-reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - use-workspace-imports: lazy-init the status Map at its point of use in the effect ((ref.current ??= new Map())) instead of mutating the ref during render with a separate in-effect fallback. The ref is only read in the effect, so this removes the render-phase write and the dead '?? new Map()' branch while keeping the identical persistent Map. - CLAUDE.md: fix the render-performance pointer so it matches the rule doc — toSorted/toReversed are unsafe on client render paths (SWC does not polyfill prototype methods), not merely an ES2022/ES2023 lib note. --- CLAUDE.md | 2 +- .../components/import-progress-menu/use-workspace-imports.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2fbd8c817df..df1f7fb9826 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -127,7 +127,7 @@ 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 immutable array methods (`toSorted`) with the ES2022-vs-ES2023 lib caveat — are in `.claude/rules/sim-react-performance.md`. For the render-timing effect/state anti-patterns use the `/you-might-not-need-*` skills and verify against the running UI. +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 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 49dd19645d7..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 @@ -52,10 +52,9 @@ export function useWorkspaceImports( const { data: exportJobs } = useWorkspaceExportJobs(workspaceId) const prevStatusRef = useRef | null>(null) - prevStatusRef.current ??= new Map() useEffect(() => { if (!tables) return - const prevStatus = prevStatusRef.current ?? new Map() + const prevStatus = (prevStatusRef.current ??= new Map()) const store = useImportTrayStore.getState() for (const table of tables) { const before = prevStatus.get(table.id)