Skip to content

Commit aaaf238

Browse files
committed
docs+ci: codify the 'use client' server-import rule + add check:client-boundary
Document the Next.js rule that server code can only render a 'use client' export as a component, never call it (server imports resolve to client-reference stubs that throw — the tables-page crash). Add the rule to .claude/rules/sim-queries.md + a cross-ref in sim-architecture.md. Add scripts/check-client-boundary-imports.ts (wired into CI as check:client-boundary) that flags any value import from a 'use client' module in a server-evaluated, non-JSX surface (prefetch / route handler / trigger / block definition), so this class can't silently recur. Escape hatch: // client-boundary-allow: <reason>.
1 parent c90df6b commit aaaf238

5 files changed

Lines changed: 252 additions & 0 deletions

File tree

.claude/rules/sim-architecture.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ packages/ # @sim/* — audit, auth, db, logger, realtime-protocol
3838
- `apps/* → packages/*` only. Packages never import from `apps/*`.
3939
- `apps/realtime` avoids Next.js, React, the block/tool registry, provider SDKs, and the executor; never add `@/lib/webhooks/providers/*`, `@/executor/*`, `@/blocks/*`, or `@/tools/*` imports to any package it consumes. CI enforces this via `scripts/check-monorepo-boundaries.ts` and `scripts/check-realtime-prune-graph.ts`.
4040

41+
## The `'use client'` server boundary
42+
43+
Every export of a `'use client'` module becomes a *client reference* on the server — server-evaluated code (RSC pages/layouts, `prefetch.ts`, route handlers, block definitions, triggers) can only *render* it as a component or pass it as a prop, never *call* it (doing so throws at runtime, e.g. `tableKeys.list is not a function`; `next build` does not catch it). Keep server-importable query primitives (key factories, fetchers, mappers, constants) in non-`'use client'` modules — see `.claude/rules/sim-queries.md`. Enforced by `scripts/check-client-boundary-imports.ts`.
44+
4145
## Feature Organization
4246

4347
Features live under `app/workspace/[workspaceId]/`:

.claude/rules/sim-queries.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,17 @@ Never use inline query keys — always use the factory.
2727

2828
**Every identifier the `queryFn` forwards into the fetch MUST appear in the `queryKey`.** (Query-machinery identifiers — `signal`, `pageParam` — are exempt; they aren't fetch-scoping args.) If the fetch is scoped by `workspaceId`, `cursor`, `limit`, an org id, etc., those values must be part of the key — otherwise distinct fetch args share one cache entry (a cross-tenant / per-param cache collision). The lone exception is a globally-unique id used as the key while a second fetch arg is only an authz scope that cannot collide; annotate those with `// rq-lint-allow: <reason>`. Enforced by the `key-fetch-arg-drift` check in `scripts/check-react-query-patterns.ts`.
2929

30+
## Server-importable query primitives must NOT live in a `'use client'` module
31+
32+
Next.js rewrites **every** export of a `'use client'` module into a *client reference* in the server bundle. Server-evaluated code — RSC `page.tsx`/`layout.tsx`, `prefetch.ts`, route handlers, **block definitions**, triggers/workers — can only *render* such an export as a component or pass it as a prop; **calling** one throws at runtime (`Attempted to call X from the server but X is on the client` — for an object export it surfaces as `X.list is not a function`). `next build` does **not** catch this — only SSR/runtime does.
33+
34+
So any **query-key factory, standalone `requestJson` fetcher, mapper, or constant** that a server module imports must live in a **non-`'use client'`** module:
35+
36+
- 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`)
38+
39+
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.
40+
3041
## File Structure
3142

3243
```typescript

.github/workflows/test-build.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,9 @@ jobs:
122122
- name: React Query pattern audit
123123
run: bun run check:react-query
124124

125+
- name: Client boundary import audit
126+
run: bun run check:client-boundary
127+
125128
- name: Verify realtime prune graph
126129
run: bun run check:realtime-prune
127130

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
"check:realtime-prune": "bun run scripts/check-realtime-prune-graph.ts",
2929
"check:zustand-v5": "bun run scripts/check-zustand-v5-selectors.ts",
3030
"check:react-query": "bun run scripts/check-react-query-patterns.ts --check",
31+
"check:client-boundary": "bun run scripts/check-client-boundary-imports.ts --check",
3132
"check:utils": "bun run scripts/check-utils-enforcement.ts",
3233
"check:migrations": "bun run scripts/check-migrations-safety.ts",
3334
"mship-contracts:generate": "bun run scripts/sync-mothership-stream-contract.ts",
Lines changed: 233 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,233 @@
1+
#!/usr/bin/env bun
2+
/**
3+
* Guards against the Next.js `'use client'` server-import foot-gun.
4+
*
5+
* Next.js rewrites EVERY export of a `'use client'` module into a client
6+
* reference in the server bundle. Server-evaluated code can only *render* such
7+
* an export as a component or pass it as a prop — *calling* one throws at
8+
* runtime ("Attempted to call X from the server but X is on the client"). The
9+
* crash for an object export looks like `tableKeys.list is not a function`.
10+
* `next build` does NOT catch this; only SSR/runtime does.
11+
*
12+
* This script flags any **value** import (not `import type`) that resolves to a
13+
* `'use client'` module from a server-evaluated, non-JSX surface — the places
14+
* that never legitimately render a client component and so only ever import a
15+
* client module to (illegally) call its values:
16+
*
17+
* - `apps/sim/app/** /prefetch*.ts` (RSC server prefetch)
18+
* - `apps/sim/app/api/** /route.ts(x)` (route handlers)
19+
* - `apps/sim/triggers/**` (trigger.dev tasks/pollers/webhooks)
20+
* - `apps/sim/blocks/**` (block definitions — evaluated server-side)
21+
*
22+
* Fix: move the imported query-key factory / standalone fetcher / mapper /
23+
* constant into a non-`'use client'` module (e.g. `hooks/queries/utils/*-keys.ts`
24+
* or `hooks/queries/utils/fetch-*.ts`) and import it from there. See the rule in
25+
* `.claude/rules/sim-queries.md`.
26+
*
27+
* Escape hatch: `// client-boundary-allow: <reason>` on the line directly above
28+
* the import (reason required). Use only for a genuinely browser-only code path.
29+
*
30+
* Usage:
31+
* bun run scripts/check-client-boundary-imports.ts # report
32+
* bun run scripts/check-client-boundary-imports.ts --check # CI gate (fail on any)
33+
*/
34+
import { readdir, readFile } from 'node:fs/promises'
35+
import path from 'node:path'
36+
37+
const ROOT = path.resolve(import.meta.dir, '..')
38+
const APP_DIR = path.join(ROOT, 'apps/sim')
39+
40+
/** Server-evaluated, non-JSX surfaces. A file matches if its path passes one. */
41+
function isServerSurface(rel: string): boolean {
42+
if (/(^|\/)prefetch[^/]*\.ts$/.test(rel)) return true
43+
if (/^app\/api\/.+\/route\.tsx?$/.test(rel)) return true
44+
if (/^triggers\//.test(rel)) return true
45+
if (/^blocks\//.test(rel)) return true
46+
return false
47+
}
48+
49+
const SOURCE_EXTENSIONS = ['.ts', '.tsx']
50+
const ALLOW_DIRECTIVE = 'client-boundary-allow'
51+
52+
async function listFiles(dir: string): Promise<string[]> {
53+
const out: string[] = []
54+
let entries: Awaited<ReturnType<typeof readdir>>
55+
try {
56+
entries = await readdir(dir, { withFileTypes: true })
57+
} catch {
58+
return out
59+
}
60+
for (const entry of entries) {
61+
const full = path.join(dir, entry.name)
62+
if (entry.isDirectory()) {
63+
if (entry.name === 'node_modules' || entry.name === '.next') continue
64+
out.push(...(await listFiles(full)))
65+
} else if (SOURCE_EXTENSIONS.includes(path.extname(entry.name))) {
66+
out.push(full)
67+
}
68+
}
69+
return out
70+
}
71+
72+
const useClientCache = new Map<string, boolean>()
73+
74+
async function isUseClientModule(absFile: string): Promise<boolean> {
75+
const cached = useClientCache.get(absFile)
76+
if (cached !== undefined) return cached
77+
let content: string
78+
try {
79+
content = await readFile(absFile, 'utf8')
80+
} catch {
81+
useClientCache.set(absFile, false)
82+
return false
83+
}
84+
// The directive must be the first statement (comments/blank lines may precede it).
85+
let isClient = false
86+
for (const raw of content.split('\n')) {
87+
const line = raw.trim()
88+
if (line === '' || line.startsWith('//') || line.startsWith('/*') || line.startsWith('*')) {
89+
continue
90+
}
91+
isClient = line === "'use client'" || line === '"use client"'
92+
break
93+
}
94+
useClientCache.set(absFile, isClient)
95+
return isClient
96+
}
97+
98+
/** Resolve an import specifier to an absolute source file, or null if external/unresolved. */
99+
async function resolveSpecifier(spec: string, fromFile: string): Promise<string | null> {
100+
let base: string
101+
if (spec.startsWith('@/')) {
102+
base = path.join(APP_DIR, spec.slice(2))
103+
} else if (spec.startsWith('./') || spec.startsWith('../')) {
104+
base = path.resolve(path.dirname(fromFile), spec)
105+
} else {
106+
return null // external package
107+
}
108+
const candidates = [
109+
base,
110+
...SOURCE_EXTENSIONS.map((ext) => base + ext),
111+
...SOURCE_EXTENSIONS.map((ext) => path.join(base, `index${ext}`)),
112+
]
113+
for (const candidate of candidates) {
114+
if (!SOURCE_EXTENSIONS.includes(path.extname(candidate))) continue
115+
try {
116+
await readFile(candidate, 'utf8')
117+
return candidate
118+
} catch {}
119+
}
120+
return null
121+
}
122+
123+
interface ImportInfo {
124+
line: number
125+
specifier: string
126+
clause: string
127+
}
128+
129+
/** Parse `import ... from '...'` statements, skipping side-effect-only imports. */
130+
function parseImports(content: string): ImportInfo[] {
131+
const lines = content.split('\n')
132+
const imports: ImportInfo[] = []
133+
const re = /^\s*import\s+([\s\S]*?)\s+from\s+['"]([^'"]+)['"]/
134+
for (let i = 0; i < lines.length; i++) {
135+
if (!/^\s*import\b/.test(lines[i]) || !lines[i].includes('import')) continue
136+
// Join up to 12 following lines to capture multi-line import clauses.
137+
const block = lines.slice(i, i + 12).join('\n')
138+
const match = re.exec(block)
139+
if (!match) continue
140+
imports.push({ line: i + 1, clause: match[1], specifier: match[2] })
141+
}
142+
return imports
143+
}
144+
145+
/** True when the import brings in at least one runtime VALUE (not purely types). */
146+
function importsAValue(clause: string): boolean {
147+
const trimmed = clause.trim()
148+
if (trimmed.startsWith('type ')) return false // `import type { ... }` / `import type X`
149+
const braceStart = trimmed.indexOf('{')
150+
// A default or namespace binding outside the braces is always a value.
151+
const beforeBrace = braceStart === -1 ? trimmed : trimmed.slice(0, braceStart)
152+
if (beforeBrace.replace(/[,\s]/g, '').length > 0) return true
153+
if (braceStart === -1) return true
154+
const inner = trimmed.slice(braceStart + 1, trimmed.lastIndexOf('}'))
155+
// A named import is a value unless every member is `type`-prefixed.
156+
return inner
157+
.split(',')
158+
.map((s) => s.trim())
159+
.filter(Boolean)
160+
.some((member) => !member.startsWith('type '))
161+
}
162+
163+
function hasAllowDirective(content: string, importLine: number): boolean {
164+
const lines = content.split('\n')
165+
for (let i = importLine - 2; i >= 0 && i >= importLine - 5; i--) {
166+
const line = lines[i]?.trim() ?? ''
167+
if (line === '' || line.startsWith('//') || line.startsWith('*') || line.startsWith('/*')) {
168+
if (line.includes(ALLOW_DIRECTIVE)) {
169+
const reason =
170+
line
171+
.split(ALLOW_DIRECTIVE)[1]
172+
?.replace(/^[:\s]+/, '')
173+
.trim() ?? ''
174+
return reason.length > 0
175+
}
176+
continue
177+
}
178+
break
179+
}
180+
return false
181+
}
182+
183+
interface Violation {
184+
file: string
185+
line: number
186+
specifier: string
187+
}
188+
189+
async function main() {
190+
const checkMode = process.argv.includes('--check')
191+
const allFiles = await listFiles(APP_DIR)
192+
const violations: Violation[] = []
193+
194+
for (const absFile of allFiles) {
195+
const rel = path.relative(APP_DIR, absFile)
196+
if (!isServerSurface(rel)) continue
197+
// A server file that is itself `'use client'` is a client component — out of scope.
198+
if (await isUseClientModule(absFile)) continue
199+
200+
const content = await readFile(absFile, 'utf8')
201+
for (const imp of parseImports(content)) {
202+
if (!importsAValue(imp.clause)) continue
203+
const resolved = await resolveSpecifier(imp.specifier, absFile)
204+
if (!resolved) continue
205+
if (!(await isUseClientModule(resolved))) continue
206+
if (hasAllowDirective(content, imp.line)) continue
207+
violations.push({ file: rel, line: imp.line, specifier: imp.specifier })
208+
}
209+
}
210+
211+
if (violations.length === 0) {
212+
console.log(
213+
"✓ Client-boundary import check passed (no server file imports a value from a 'use client' module)."
214+
)
215+
return
216+
}
217+
218+
console.error(
219+
`\n✗ ${violations.length} server file(s) import a runtime value from a 'use client' module.\n` +
220+
` On the server these resolve to client-reference stubs and throw when called (e.g. 'X.list is not a function').\n` +
221+
` Move the imported factory/fetcher/constant into a non-'use client' module (hooks/queries/utils/*-keys.ts or fetch-*.ts).\n` +
222+
` See .claude/rules/sim-queries.md. Escape hatch: // ${ALLOW_DIRECTIVE}: <reason> above the import.\n`
223+
)
224+
for (const v of violations) {
225+
console.error(` ${v.file}:${v.line} imports from '${v.specifier}'`)
226+
}
227+
if (checkMode) process.exit(1)
228+
}
229+
230+
main().catch((error) => {
231+
console.error(error)
232+
process.exit(1)
233+
})

0 commit comments

Comments
 (0)