Skip to content

Commit 4692708

Browse files
Merge remote-tracking branch 'origin/staging' into improve/slack
# Conflicts: # apps/sim/lib/webhooks/deploy.ts # apps/sim/lib/webhooks/utils.server.ts # packages/db/migrations/meta/0254_snapshot.json # packages/db/migrations/meta/_journal.json # packages/db/schema.ts # scripts/check-api-validation-contracts.ts
2 parents 5ac0932 + b686111 commit 4692708

1,299 files changed

Lines changed: 124126 additions & 16979 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.agents/skills/react-query-best-practices/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ Read these before analyzing:
3131

3232
### Query hooks
3333
- Every `queryFn` must forward `signal` for request cancellation
34-
- Every query must have an explicit `staleTime` (default 0 is almost never correct)
34+
- Every query must have an explicit `staleTime` (default 0 is almost never correct), assigned from a named exported constant — never an inline numeric literal. A server-side prefetch hydrating the same query key must import and reuse that constant instead of restating the number
3535
- `keepPreviousData` / `placeholderData` only on variable-key queries (where params change), never on static keys
3636
- Use `enabled` to prevent queries from running without required params
3737

.claude/commands/add-trigger.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,7 @@ export const {service}PollingHandler: PollingProviderHandler = {
374374

375375
try {
376376
// For OAuth services:
377-
const accessToken = await resolveOAuthCredential(webhookData, '{service}', requestId, logger)
377+
const accessToken = await resolveOAuthCredential(webhookData, '{service}', requestId)
378378
const config = webhookData.providerConfig as unknown as {Service}WebhookConfig
379379

380380
// First poll: seed state, emit nothing
@@ -421,7 +421,7 @@ export const {service}PollingTrigger: TriggerConfig = {
421421
polling: true, // REQUIRED — routes to polling infrastructure
422422

423423
subBlocks: [
424-
{ id: 'triggerCredentials', type: 'oauth-input', title: 'Credentials', serviceId: '{service}', requiredScopes: [], required: true, mode: 'trigger', supportsCredentialSets: true },
424+
{ id: 'triggerCredentials', type: 'oauth-input', title: 'Credentials', serviceId: '{service}', requiredScopes: [], required: true, mode: 'trigger' },
425425
// ... service-specific config fields (dropdowns, inputs, switches) ...
426426
{ id: 'triggerInstructions', type: 'text', title: 'Setup Instructions', hideFromPreview: true, mode: 'trigger', defaultValue: '...' },
427427
],

.claude/commands/react-query-best-practices.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ Read these before analyzing:
3131

3232
### Query hooks
3333
- Every `queryFn` must forward `signal` for request cancellation
34-
- Every query must have an explicit `staleTime` (default 0 is almost never correct)
34+
- Every query must have an explicit `staleTime` (default 0 is almost never correct), assigned from a named exported constant — never an inline numeric literal. A server-side prefetch hydrating the same query key must import and reuse that constant instead of restating the number
3535
- `keepPreviousData` / `placeholderData` only on variable-key queries (where params change), never on static keys
3636
- Use `enabled` to prevent queries from running without required params
3737

.claude/rules/sim-components.md

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,22 @@ Component authoring rules — structure order (refs → external hooks → store
1212
- `'use client'` only when using React hooks or browser-only APIs.
1313
- Prefer semantic HTML (`aside`, `nav`, `article`).
1414
- Optional-chain callbacks: `onAction?.(id)`.
15+
16+
## List-render performance
17+
18+
When rendering or sorting a list of rows against a lookup collection (members, folders, tags), keep the per-row work O(1):
19+
20+
- **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.
21+
- **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.
22+
- **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.
23+
24+
## react-doctor (`npx react-doctor`) — apply the wins, skip the false positives
25+
26+
react-doctor diagnostics are hypotheses, not verdicts — confirm against the code before acting, and preserve behavior. Known repo-specific false positives to NOT "fix":
27+
28+
- `no-barrel-import` — barrel imports are the repo convention (see sim-imports.md, "Barrel Exports"). Keep them.
29+
- `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.
30+
- `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.
31+
- `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).
32+
- `async-await-in-loop` on an upload/progress loop where sequential execution is intentional (per-item progress, server backpressure) — leave it.
33+
- Broad refactors (`prefer-useReducer` for many `useState`, `no-giant-component` splits) — out of scope for a perf pass; note, don't churn.

.claude/rules/sim-hooks.md

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,24 @@ export function useFeature({ id, onSelect }: UseFeatureProps) {
5151

5252
## State shape
5353

54-
Never mirror a prop into state with `useState(prop)` + a syncing `useEffect` — a prop change clobbers in-progress local edits. Use the prop directly, reset via a remount `key`, or — when you must seed local state from a prop only on a transition (e.g. a modal opening) — reset during render with the `prevX` ref idiom:
54+
Never mirror a prop into state with `useState(prop)` + a syncing `useEffect` — a prop change clobbers in-progress local edits. Use the prop directly, reset via a remount `key`, or — when you must seed local state from a prop only on a transition (e.g. a modal opening) — adjust it **during render** with a `prev`-value tracker held in `useState`:
5555

5656
```typescript
57-
const prevOpenRef = useRef(open)
58-
if (prevOpenRef.current !== open) {
59-
prevOpenRef.current = open
57+
const [prevOpen, setPrevOpen] = useState(open)
58+
if (prevOpen !== open) {
59+
setPrevOpen(open)
6060
if (open) setName(initialName) // closed → open only
6161
}
6262
```
6363

64+
React re-renders immediately with the corrected state without committing the stale value. Rules: the `if (prev !== current)` guard is mandatory (an unconditional `setState` in render loops forever), the tracker is set **inside** the guard, and you may only set the currently-rendering component's state this way. Hold the tracker in `useState`, **not a `useRef`** — React forbids reading/writing `ref.current` during render (react.dev, useRef → "Do not write _or read_ `ref.current` during rendering"; the `react-hooks` `refs` lint flags it), and a `useState` tracker is concurrent-safe where a mutated ref is not (a discarded render rolls state back, not a ref).
65+
66+
**The tracker's initial value decides mount behavior — choose it deliberately.** The example seeds `useState(open)` because the modal mounts closed, so the first render's guard is `false` and nothing resets on mount (correct — `name` is already at its initial value). When the effect you're replacing did real work **on mount** — opening a panel because a prop already matches, seeding editable state from an already-present value, or a component that can mount in the active state — seed a **sentinel** the live value can't equal (e.g. `useState<T | null>(null)`), otherwise the guard is `false` on the first render and that mount action is silently dropped. Place the block before any early `return`.
67+
68+
> Some existing components use a `useRef` prev-tracker (`if (prevRef.current !== x) { prevRef.current = x; … }`). It works but reads/writes a ref during render — prefer the `useState` form above for new code.
69+
70+
Only convert a `useState` to a `useRef` when the value is **never read during render/JSX and is never a hook dependency** — a value in a `useEffect`/`useMemo`/`useCallback` dep array must re-run the hook on change, so it stays state (see also `rerender-state-only-in-handlers` in `sim-components.md`). Convert only set-only values read solely inside handlers or effect bodies (e.g. a prompt-history index, a pending-upload URL). If a ref feeds render, mutating it won't re-render and the UI goes stale.
71+
6472
Model mutually-exclusive flags as ONE `status` enum, not several contradictory booleans. `isLoading`/`isVerified`/`isInvalidOtp` describing one machine collapse to `status: 'idle' | 'verifying' | 'verified' | 'error'` (+ `errorMessage`); derive any boolean a consumer still needs (`status === 'error'`).
6573

6674
Derive busy/success from the mutation object — never duplicate `mutation.isPending`/`mutation.isSuccess` into local `useState`. Read them directly (`mutation.isSuccess`) and reset with `mutation.reset()`. A distinct phase the mutation doesn't cover — e.g. a pre-submit captcha/Turnstile gate that runs before `mutate()` — is not a duplicate; keep that flag.

.claude/rules/sim-imports.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,20 @@ import { Dashboard, Sidebar } from '@/app/workspace/[workspaceId]/logs/component
3131
import { Dashboard } from '@/app/workspace/[workspaceId]/logs/components/dashboard/dashboard'
3232
```
3333

34+
## Code-splitting through barrels
35+
36+
When you `lazy(() => import(...))` a component to keep it out of a route's initial bundle, import the **deep module path** (`./components/foo/foo`), never the barrel — and **delete the now-dead barrel re-export** of that component. This app has no `"sideEffects": false` in `apps/sim/package.json`, so when any sibling still imports that barrel, webpack can conservatively keep the barrel's re-export edge to the heavy module. A leftover `export { Foo } from './foo'` line can therefore drag `Foo` (and its transitive deps) back into the initial chunk and silently defeat the split. Removing the dead re-export is the guaranteed fix; verify with a production bundle diff, not by eyeballing the `lazy()` call.
37+
38+
```typescript
39+
// ✓ Good — deep lazy import + no barrel edge left behind
40+
const MothershipView = lazy(() =>
41+
import('./components/mothership-view/mothership-view').then((m) => ({ default: m.MothershipView }))
42+
)
43+
// (and remove `export { MothershipView } from './mothership-view'` from components/index.ts)
44+
```
45+
46+
Wrap the lazy component in a **local `<Suspense>`** so its suspension resolves at the nearest boundary instead of bubbling to the page-level fallback (which would flash the whole route). `React.lazy(memo(forwardRef(...)))` forwards a DOM `ref` correctly in React 19 — but during the fallback window `ref.current` is `null`, so every consumer must guard it (`if (!el) return` / `el?.`).
47+
3448
## No Re-exports
3549

3650
Do not re-export from non-barrel files. Import directly from the source.

.claude/rules/sim-queries.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ Next.js rewrites **every** export of a `'use client'` module into a *client refe
3434
So any **query-key factory, standalone `requestJson` fetcher, mapper, or constant** that a server module imports must live in a **non-`'use client'`** module:
3535

3636
- key factories → `hooks/queries/utils/<entity>-keys.ts` (see `folder-keys.ts`, `table-keys.ts`, `credential-keys.ts`)
37-
- standalone fetchers/mappers → `hooks/queries/utils/fetch-*.ts` / `*-list-query.ts` (see `fetch-workflow-envelope.ts`, `fetch-credential-set.ts`)
37+
- standalone fetchers/mappers → `hooks/queries/utils/fetch-*.ts` / `*-list-query.ts` (see `fetch-workflow-envelope.ts`, `fetch-workspace-credentials.ts`)
3838

3939
The `'use client'` hook module then imports these back for its hooks. **Never** define a server-imported factory/fetcher directly in a `'use client'` hooks file — it crashes SSR (this caused the tables-page crash). Enforced for prefetch/route/trigger/block files by `scripts/check-client-boundary-imports.ts` (`bun run check:client-boundary`, run in CI). Escape hatch for a genuinely browser-only path: `// client-boundary-allow: <reason>` on the line above the import.
4040

@@ -50,14 +50,16 @@ The `'use client'` hook module then imports these back for its hooks. **Never**
5050
## Query Hook
5151

5252
- Every `queryFn` must destructure and forward `signal` for request cancellation
53-
- Every query must have an explicit `staleTime`
53+
- Every query must have an explicit `staleTime`, assigned from a named exported constant (`ENTITY_LIST_STALE_TIME`), never an inline numeric literal. A server-side prefetch (`prefetch.ts`) hydrating the same query key must import and reuse that constant, not restate the number — this is what keeps a prefetched cache entry from going stale out of sync with the client hook that reads it
5454
- Use `keepPreviousData` only on variable-key queries (where params change), never on static keys
5555
- Same-origin JSON calls must go through `requestJson(contract, ...)` from `@/lib/api/client/request` against the contract in `@/lib/api/contracts/**`
5656

5757
```typescript
5858
import { requestJson } from '@/lib/api/client/request'
5959
import { listEntitiesContract, type EntityList } from '@/lib/api/contracts/entities'
6060

61+
export const ENTITY_LIST_STALE_TIME = 60 * 1000
62+
6163
async function fetchEntities(workspaceId: string, signal?: AbortSignal): Promise<EntityList> {
6264
const data = await requestJson(listEntitiesContract, {
6365
query: { workspaceId },
@@ -71,7 +73,7 @@ export function useEntityList(workspaceId?: string, options?: { enabled?: boolea
7173
queryKey: entityKeys.list(workspaceId),
7274
queryFn: ({ signal }) => fetchEntities(workspaceId as string, signal),
7375
enabled: Boolean(workspaceId) && (options?.enabled ?? true),
74-
staleTime: 60 * 1000,
76+
staleTime: ENTITY_LIST_STALE_TIME,
7577
placeholderData: keepPreviousData, // OK: workspaceId varies
7678
})
7779
}

0 commit comments

Comments
 (0)