Skip to content

Commit cd25b34

Browse files
committed
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.
1 parent 1da718f commit cd25b34

3 files changed

Lines changed: 91 additions & 4 deletions

File tree

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
# React & Render Performance
2+
3+
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.
4+
5+
## Lazy-init refs that hold objects
6+
7+
`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.
8+
9+
```typescript
10+
// ✗ Bad — allocates a new Map each render, discards all but the first
11+
const cacheRef = useRef<Map<string, string>>(new Map())
12+
13+
// ✓ Good — allocated once, stable identity thereafter
14+
const cacheRef = useRef<Map<string, string> | null>(null)
15+
cacheRef.current ??= new Map()
16+
```
17+
18+
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.
19+
20+
## Hoist static values and closure-free functions to module scope
21+
22+
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.
23+
24+
```typescript
25+
// ✗ Bad — rebuilt every render, new identity each time
26+
function Toolbar({ mode }: ToolbarProps) {
27+
const TITLES = { create: 'Add', edit: 'Configure' } as const
28+
const handleWheel = (e: React.WheelEvent) => e.currentTarget.scrollBy(e.deltaX, e.deltaY)
29+
// ...
30+
}
31+
32+
// ✓ Good — allocated once at module load
33+
const TITLES = { create: 'Add', edit: 'Configure' } as const
34+
function handleWheel(e: React.WheelEvent) {
35+
e.currentTarget.scrollBy(e.deltaX, e.deltaY)
36+
}
37+
function Toolbar({ mode }: ToolbarProps) { /* ... */ }
38+
```
39+
40+
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.
41+
42+
## Pre-index with Map/Set for repeated lookups
43+
44+
`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).
45+
46+
```typescript
47+
// ✗ Bad — find() re-scans outputs for every column
48+
for (const child of columns) {
49+
const output = group.outputs.find((o) => o.columnName === getColumnId(child))
50+
}
51+
52+
// ✓ Good — index once, then O(1) lookups
53+
const outputByName = new Map<string, Output>()
54+
for (const o of group.outputs) {
55+
if (!outputByName.has(o.columnName)) outputByName.set(o.columnName, o) // first wins, matches find()
56+
}
57+
for (const child of columns) {
58+
const output = outputByName.get(getColumnId(child))
59+
}
60+
```
61+
62+
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.
63+
64+
## Immutable array methods over spread-then-mutate
65+
66+
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).
67+
68+
```typescript
69+
// ✗ Bad — copies just to sort the copy
70+
return [...items].sort(compare)
71+
72+
// ✓ Good — sorts without the throwaway copy, still non-mutating
73+
return items.toSorted(compare)
74+
```
75+
76+
**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.
77+
78+
## Local feature barrels are the convention — do not "fix" them
79+
80+
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.

CLAUDE.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ export function Component({ requiredProp, optionalProp = false }: ComponentProps
127127

128128
Extract when: 50+ lines, used in 2+ files, or has own state/logic. Keep inline when: < 10 lines, single use, purely presentational.
129129

130+
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.
131+
130132
## API Contracts
131133

132134
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.

apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,13 +116,18 @@ export function expandToDisplayColumns(
116116
size++
117117
}
118118
const group = groupById.get(gid)
119-
const outputByColumnName = group
120-
? new Map(group.outputs.map((o) => [o.columnName, o]))
121-
: undefined
119+
// Pre-index outputs by column name for O(1) lookup. First output wins on a
120+
// duplicate columnName, exactly matching the previous `Array.find()` behavior.
121+
const outputByColumnName = new Map<string, WorkflowGroup['outputs'][number]>()
122+
if (group) {
123+
for (const o of group.outputs) {
124+
if (!outputByColumnName.has(o.columnName)) outputByColumnName.set(o.columnName, o)
125+
}
126+
}
122127
const startIdx = out.length
123128
for (let k = 0; k < size; k++) {
124129
const child = columns[i + k]
125-
const output = outputByColumnName?.get(getColumnId(child))
130+
const output = outputByColumnName.get(getColumnId(child))
126131
out.push({
127132
...child,
128133
key: getColumnId(child),

0 commit comments

Comments
 (0)