Skip to content

Commit f6c27c8

Browse files
committed
improvement(audits): ban ES2023 array methods repo-wide, catch whole-state partialize, strip inline directive comments
1 parent ca7d92a commit f6c27c8

8 files changed

Lines changed: 21 additions & 40 deletions

File tree

‎.claude/rules/sim-components.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,15 @@ export function Component({ requiredProp, optionalProp = false }: ComponentProps
3333
When rendering or sorting a list of rows against a lookup collection (members, folders, tags), keep the per-row work O(1):
3434

3535
- **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.
36-
- **Sort a copy with `[...array].sort(cmp)`**, never `toSorted` in client code — see `sim-react-performance.md` → "Never mutate a shared array in place".
36+
- **Sort a copy with `[...array].sort(cmp)`**, never `toSorted` (`check:utils` bans it repo-wide) — see `sim-react-performance.md` → "Never mutate a shared array in place".
3737
- **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.
3838

3939
## react-doctor (`bunx react-doctor`) — apply the wins, skip the false positives
4040

4141
react-doctor diagnostics are hypotheses, not verdicts — confirm against the code before acting, and preserve behavior. Known repo-specific false positives to NOT "fix":
4242

4343
- `no-barrel-import` — barrel imports are the repo convention (see sim-imports.md, "Barrel Exports"). Keep them.
44-
- `js-tosorted-immutable` — won't-fix in `'use client'` code (see "List-render performance" above); apply it only in server-only modules.
44+
- `js-tosorted-immutable` — won't-fix anywhere; `check:utils` bans the ES2023 array methods repo-wide.
4545
- `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.
4646
- `no-render-in-render` — a helper *called inline* (`{renderRow()}`) is reconciled by position and does **not** remount, so extracting it to a component is usually pure churn and can regress behavior (prop-drilling many closures, focus/scroll loss on the inner `<input>`). Apply it only when the helper is genuinely a *component defined during render*, or when the move is mechanical (a stateless, ref-free helper whose closures become a small, explicit prop set).
4747
- `async-await-in-loop` on an upload/progress loop where sequential execution is intentional (per-item progress, server backpressure) — leave it.

‎.claude/rules/sim-react-performance.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ return items.sort(compare)
7777
return [...items].sort(compare)
7878
```
7979

80-
**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.
80+
**Do NOT use `toSorted()` / `toReversed()` / `with()` / `toSpliced()`.** 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 used everywhere: whether a module reaches the browser is not visible from its path, and `check:utils` bans the methods repo-wide.
8181

8282
## Run independent awaits in parallel
8383

‎.cursor/rules/sim-components.mdc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@ export function Component({ requiredProp, optionalProp = false }: ComponentProps
3434
When rendering or sorting a list of rows against a lookup collection (members, folders, tags), keep the per-row work O(1):
3535

3636
- **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.
37-
- **Sort a copy with `[...array].sort(cmp)`**, never `toSorted` in client code — see `sim-react-performance.md` → "Never mutate a shared array in place".
37+
- **Sort a copy with `[...array].sort(cmp)`**, never `toSorted` (`check:utils` bans it repo-wide) — see `sim-react-performance.md` → "Never mutate a shared array in place".
3838
- **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.
3939

4040
## react-doctor (`bunx react-doctor`) — apply the wins, skip the false positives
4141

4242
react-doctor diagnostics are hypotheses, not verdicts — confirm against the code before acting, and preserve behavior. Known repo-specific false positives to NOT "fix":
4343

4444
- `no-barrel-import` — barrel imports are the repo convention (see sim-imports.md, "Barrel Exports"). Keep them.
45-
- `js-tosorted-immutable` — won't-fix in `'use client'` code (see "List-render performance" above); apply it only in server-only modules.
45+
- `js-tosorted-immutable` — won't-fix anywhere; `check:utils` bans the ES2023 array methods repo-wide.
4646
- `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.
4747
- `no-render-in-render` — a helper *called inline* (`{renderRow()}`) is reconciled by position and does **not** remount, so extracting it to a component is usually pure churn and can regress behavior (prop-drilling many closures, focus/scroll loss on the inner `<input>`). Apply it only when the helper is genuinely a *component defined during render*, or when the move is mechanical (a stateless, ref-free helper whose closures become a small, explicit prop set).
4848
- `async-await-in-loop` on an upload/progress loop where sequential execution is intentional (per-item progress, server backpressure) — leave it.

‎.cursor/rules/sim-react-performance.mdc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ return items.sort(compare)
8080
return [...items].sort(compare)
8181
```
8282

83-
**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.
83+
**Do NOT use `toSorted()` / `toReversed()` / `with()` / `toSpliced()`.** 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 used everywhere: whether a module reaches the browser is not visible from its path, and `check:utils` bans the methods repo-wide.
8484

8585
## Run independent awaits in parallel
8686

‎CLAUDE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ The `'use client'` server boundary, the app/worker runtime env split, and featur
8989
- **Imports**: absolute (`@/...`) only, never relative. A folder with 3+ exports gets an `index.ts` barrel; never re-export from a non-barrel file. `import type` for type-only imports. Order and lazy-loading through barrels: `.claude/rules/sim-imports.md`.
9090
- **TypeScript**: no `any` and no non-null `!` (use precise types or `unknown` with guards; `check:explicit-any` ratchets both); no export nothing imports (`check:unused-exports`); a props interface for every component; `as const` for constant objects/arrays; explicit ref types (`useRef<HTMLDivElement>(null)`).
9191
- **Unused bindings** fail lint (biome `noUnusedVariables`, `noUnusedFunctionParameters`): delete the dead variable, import, or parameter and update callers; write `catch {}` when the error is unused. Prefix `_` only for a parameter that must hold its position because a later one is used. `const { a, ...rest } = obj` to omit keys is allowed. The rules carry no autofix, so `bun run lint` will not rename anything for you.
92-
- **Components**: `'use client'` only for hooks or browser APIs. Structure order, extraction thresholds, and list-render rules: `.claude/rules/sim-components.md`. Render-performance idioms (lazy-init refs, hoisting, `Map` pre-indexing, `[...arr].sort()` never `toSorted()` on client paths): `.claude/rules/sim-react-performance.md`. For effect/state/memo/callback anti-patterns use the `/you-might-not-need-*` skills and verify against the running UI.
92+
- **Components**: `'use client'` only for hooks or browser APIs. Structure order, extraction thresholds, and list-render rules: `.claude/rules/sim-components.md`. Render-performance idioms (lazy-init refs, hoisting, `Map` pre-indexing, `[...arr].sort()`, never `toSorted()`): `.claude/rules/sim-react-performance.md`. For effect/state/memo/callback anti-patterns use the `/you-might-not-need-*` skills and verify against the running UI.
9393
- **State ownership**: React Query owns server data — never `useState` + `fetch`; shareable client view-state (tabs, filters, search, pagination, selected id) lives in the URL via `nuqs`; Zustand owns global client state; `useState` owns UI-only state. Hooks: `.claude/rules/sim-hooks.md`. Stores (`devtools`, `persist` only with an explicit `partialize` whitelist, workflow value invariants): `.claude/rules/sim-stores.md`. URL state: `.claude/rules/sim-url-state.md`.
9494
- **Utils**: inline a helper with one consumer; create `utils.ts` when 2+ files share it — in `lib/` (app-wide) or `feature/utils/` (feature-scoped). Check `lib/` before writing a new one.
9595
- **Lists and menus** mirror the order the user already reads elsewhere (toolbar, settings nav), encoded in one exported order constant (resource menus share `RESOURCE_MENU_ORDER`, a product order that does not mirror the sidebar); a separator marks only a change in what the action acts on (typically one, before the destructive action): `.claude/rules/sim-list-ordering.md`.

‎scripts/check-utils-enforcement.ts‎

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
*
55
* Most patterns point at an `@sim/utils` helper (CLAUDE.md "Common utilities"). A few encode
66
* render-path rules from `.claude/rules/sim-react-performance.md` and `sim-styling.md` that no
7-
* linter covers: ES2023 array methods that Safari 15 lacks, `useRef(new X())` allocating on
7+
* linter covers: ES2023 array methods that Safari 15 lacks (banned everywhere, since whether a
8+
* module reaches the browser is not visible from its path and a copy-then-sort costs the same), `useRef(new X())` allocating on
89
* every render, and `h-N w-N` where `size-N` is the convention.
910
*
1011
* Biome's noRestrictedImports covers the import-based bans it lists — today `nanoid` and
@@ -18,7 +19,6 @@
1819
*/
1920
import { readdir, readFile } from 'node:fs/promises'
2021
import path from 'node:path'
21-
import { leadingDirective } from './source-kind'
2222

2323
const ROOT = path.resolve(import.meta.dir, '..')
2424

@@ -43,25 +43,6 @@ const ALLOWLISTED_FILES = new Set([
4343
'packages/testing/src/factories/id.ts',
4444
])
4545

46-
/** App Router files Next only ever evaluates on the server, plus `*.server.ts` modules. */
47-
const SERVER_ONLY_FILE =
48-
/\/(?:route|sitemap|robots|manifest)\.[jt]sx?$|\/(?:opengraph-image|twitter-image|icon|apple-icon)\.[^/]+$|\.server\.[jt]sx?$/
49-
50-
/**
51-
* Files that ship to the browser: client directories plus any `'use client'` module. Server-only
52-
* code (route handlers, metadata routes, tests) may use Node 20+ runtime methods freely.
53-
*/
54-
function isClientRenderPath(rel: string, content: string): boolean {
55-
if (/\.(test|spec)\.[cm]?[jt]sx?$/.test(rel) || /\/app\/api\//.test(rel)) return false
56-
if (SERVER_ONLY_FILE.test(rel)) return false
57-
return (
58-
/^apps\/sim\/(app|components|hooks|stores)\//.test(rel) ||
59-
/^apps\/docs\/(app|components)\//.test(rel) ||
60-
/^packages\/(emcn|workflow-renderer)\/src\//.test(rel) ||
61-
leadingDirective(content) === 'use client'
62-
)
63-
}
64-
6546
/** `s.slice(0, n)` plus a suffix, by `+` or in a template literal; `\1` is `s` and `\2` is `n`. */
6647
const TRUNCATED = String.raw`(?:\`\$\{\1\.(?:slice|substring)\(\s*0\s*,\s*\2\s*\)\}[^\`$]*\`|\1\.(?:slice|substring)\(\s*0\s*,\s*\2\s*\)\s*\+\s*(?:'[^']*'|"[^"]*"|\w+))`
6748
/** Literal gate for both truncate patterns, whose backreferences are slow over every file. */
@@ -75,7 +56,6 @@ const BANNED_PATTERNS: Array<{
7556
description: string
7657
suggestion: string
7758
/** Restricts the pattern to matching files; unrestricted patterns apply everywhere. */
78-
appliesTo?: (rel: string, content: string) => boolean
7959
/** Cheap literal test that skips the pattern on files that cannot match; memoized per file. */
8060
prefilter?: RegExp
8161
}> = [
@@ -167,9 +147,9 @@ const BANNED_PATTERNS: Array<{
167147
// Render-path rules (.claude/rules/sim-react-performance.md, sim-styling.md)
168148
{
169149
pattern: /\.(?:toSorted|toReversed|toSpliced)\s*\(|\.with\(\s*-?\d+\s*,/g,
170-
description: 'ES2023 array method in browser code (throws on Safari/iOS 15)',
150+
description:
151+
'ES2023 array method (throws on Safari/iOS 15 wherever the module reaches the browser)',
171152
suggestion: 'a copy you then mutate: [...arr].sort(), [...arr].reverse(), [...arr].splice()',
172-
appliesTo: isClientRenderPath,
173153
},
174154
{
175155
pattern: /\buseRef(?:<(?:[^<>]|<[^<>]*>)*>)?\(\s*new\s+[A-Z]\w*/g,
@@ -288,8 +268,7 @@ async function main() {
288268
}> = []
289269

290270
const prefilterHits = new Map<RegExp, boolean>()
291-
for (const { pattern, description, suggestion, appliesTo, prefilter } of BANNED_PATTERNS) {
292-
if (appliesTo && !appliesTo(rel, content)) continue
271+
for (const { pattern, description, suggestion, prefilter } of BANNED_PATTERNS) {
293272
if (prefilter) {
294273
let hit = prefilterHits.get(prefilter)
295274
if (hit === undefined) {

‎scripts/check-zustand-v5-selectors.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -317,9 +317,11 @@ function auditPersist(file: string, source: string): Violation[] {
317317
if (closeParenIndex === -1) continue
318318
const call = source.slice(openParenIndex + 1, closeParenIndex)
319319
const hasPartialize = /\bpartialize\b/.test(call)
320-
const spreadsState = /\bpartialize\s*:\s*\(?\s*(\w+)[^)]*\)?\s*=>\s*\(\s*\{\s*\.\.\.\1\b/.test(
321-
call
322-
)
320+
/** `(s) => s`, `(s) => ({ ...s })`, or a block body returning either: the whole state persists. */
321+
const spreadsState =
322+
/\bpartialize\s*:\s*\(?\s*(\w+)[^)]*\)?\s*=>\s*(?:\1\b(?!\s*[.[])|\(\s*\{\s*\.\.\.\1\b|\{[^}]*\breturn\s+(?:\1\b(?!\s*[.[])|\{\s*\.\.\.\1\b))/.test(
323+
call
324+
)
323325
if (hasPartialize && !spreadsState) continue
324326
violations.push({
325327
file,

‎scripts/source-kind.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,13 @@
22
const DIRECTIVE_STATEMENT = /^(['"])(use [a-z-]+)\1\s*;?$/
33

44
/**
5-
* The directive a single source line states, e.g. `use client`, or null. A note may follow the
6-
* directive on the same line, so a trailing `//` or `/* *\/` comment comes off before matching.
5+
* The directive a single source line states, e.g. `use client`, or null. Notes may sit on the same
6+
* line, so `//` and inline `/* *\/` comments come off before matching.
77
*/
88
export function directiveOn(line: string): string | null {
99
const statement = line
10-
.trim()
11-
.replace(/(?:\/\/.*|\/\*.*?\*\/)\s*$/, '')
10+
.replace(/\/\*.*?\*\//g, '')
11+
.replace(/\/\/.*$/, '')
1212
.trim()
1313
return DIRECTIVE_STATEMENT.exec(statement)?.[2] ?? null
1414
}

0 commit comments

Comments
 (0)