docs: add Coding Standards & Banned Patterns to AGENTS.md - #706
Conversation
- Document Choose Your Bug framing for useEffect misuse - Add Rules 1–6: useMountEffect-only, ban as any, derive state, data-fetching libs, event handlers over effect flags, keys for reset - Cross-reference from ANTI-PATTERNS Co-authored-by: Leo <leoisadev1@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
🚀 Preview Deployment ReadyVercel is rebuilding the frontend with the new Convex backend URL. Vercel will post the preview URL automatically. Convex Preview Backend
🤖 Deployed automatically by GitHub Actions |
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
Greptile SummaryThis PR adds a Coding Standards & Banned Patterns section to the root
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Need a side effect or async op?] --> B{What kind?}
B --> C[SDK / third-party init\non mount only]
B --> D[Fetch / load server data]
B --> E[Sync / transform\nexisting state]
B --> F[React to user action]
B --> G[Reset component state\nwhen prop changes]
C --> C1["useMountEffect()\n⚠️ Rule 1 — must be justified\n& documented\n\nNote: useMountEffect not yet\ndefined in codebase"]
D --> D1["useQuery — TanStack Query\nor Convex useQuery\n✅ Rule 4"]
E --> E1["Derive inline or useMemo\n✅ Rule 3\n❌ No useEffect + useState"]
F --> F1["Handle in event handler\n✅ Rule 5\n❌ No effect flag watching"]
G --> G1["Pass value as key prop\nReact will remount\n✅ Rule 6\n❌ No dep array choreography"]
style C1 fill:#fff3cd,stroke:#ffc107
style D1 fill:#d4edda,stroke:#28a745
style E1 fill:#d4edda,stroke:#28a745
style F1 fill:#d4edda,stroke:#28a745
style G1 fill:#d4edda,stroke:#28a745
Last reviewed commit: "docs(AGENTS): add Co..." |
|
|
||
| **Rule 1 — BAN direct use of `useEffect`** | ||
|
|
||
| Do not use `useEffect` directly. Use `useMountEffect()` only for rare, justified external side-effect syncs (e.g. third-party SDK initialization). Any other use must be approved and documented with a comment explaining why no alternative works. |
There was a problem hiding this comment.
useMountEffect has no defined source or import path
Rule 1 instructs contributors to "Use useMountEffect()", but this hook does not exist in React's core API, and a project-wide search confirms it appears nowhere else in the codebase — no custom hook file, no react-use import, no @react-hookz/web dependency. An AI agent or human contributor following this rule literally has no way to know where to import it from, making the rule unenforceable as written.
You should either:
- Create and export a
useMountEffectutility hook (e.g.apps/web/src/hooks/useMountEffect.ts) and reference its import path here, or - Replace the reference with an existing, well-known alternative like
useEffectwith[](alongside a clear explanation of the constraints), or reference a specific library (e.g.useEffectOncefromreact-use) that is already inpackage.json.
Confidence this is correct: 5/5 — confirmed by grep across the entire repository.
|
|
||
| **Rule 4 — Use data-fetching libraries instead of fetch-in-effect** | ||
|
|
||
| Do not fetch data inside `useEffect`. Use `useQuery` (TanStack Query) or an equivalent data-fetching library. These libraries handle caching, deduplication, background refetching, loading states, and error states correctly. Effect-based fetching is a manual reimplementation of these features, done worse. |
There was a problem hiding this comment.
Rule 4 ambiguously excludes Convex's
useQuery
The rule currently reads: "Use useQuery (TanStack Query) or an equivalent data-fetching library." However, this project's backend is Convex, and practically all server-state queries are performed via Convex's useQuery (imported from convex/react) — not TanStack Query. An AI agent reading this strictly might flag or avoid Convex's useQuery as non-compliant.
The PR description clarifies this intent, but the actual rule text in AGENTS.md does not. Consider making this explicit in the rule itself:
| Do not fetch data inside `useEffect`. Use `useQuery` (TanStack Query) or an equivalent data-fetching library. These libraries handle caching, deduplication, background refetching, loading states, and error states correctly. Effect-based fetching is a manual reimplementation of these features, done worse. | |
| Do not fetch data inside `useEffect`. Use `useQuery` (TanStack Query), Convex's `useQuery` (`convex/react`), or an equivalent data-fetching library. These libraries handle caching, deduplication, background refetching, loading states, and error states correctly. Effect-based fetching is a manual reimplementation of these features, done worse. |
Confidence this is correct: 4/5 — the current wording creates genuine ambiguity for a codebase that relies heavily on Convex's reactive queries.
| ## ANTI-PATTERNS (THIS PROJECT) | ||
| - Follow **Coding Standards & Banned Patterns** above for enforced React and TypeScript rules (`useEffect` misuse, `as any`, and related patterns). | ||
| - Do not use `NEXT_PUBLIC_*` env vars in web code. |
There was a problem hiding this comment.
apps/web/AGENTS.md ANTI-PATTERNS not cross-referenced
The cross-reference to Coding Standards & Banned Patterns was added to the root AGENTS.md's ANTI-PATTERNS section, but apps/web/AGENTS.md — the most relevant child file since it governs all React/UI code — has its own ANTI-PATTERNS section that was not updated. AI agents working exclusively in apps/web/ will load the child AGENTS.md first, and may miss the new coding standards entirely.
Consider adding a pointer in apps/web/AGENTS.md's ANTI-PATTERNS section, e.g.:
- Follow **Coding Standards & Banned Patterns** in root `AGENTS.md` for all React and TypeScript rules (`useEffect` misuse, `as any`, etc.).
Confidence this is correct: 3/5 — the root policy technically applies globally, but discoverability from the child file is meaningfully lower without an explicit reference.
Summary
Adds a prominent Coding Standards & Banned Patterns section to the root
AGENTS.mdfor all AI agents and contributors.Contents
useEffectmisuse trades one class of bugs for anotheruseEffect(useuseMountEffect()only where justified), banas any, derive state without effect sync, use data-fetching libraries instead of fetch-in-effect, use event handlers instead of effect-driven “action flags”, reset viakeyinstead of dependency choreographyNotes
useQuery; Convex and other vetteduseQuery-style APIs remain valid under “equivalent data-fetching library” as used elsewhere in the repo.Note
Add Coding Standards & Banned Patterns section to AGENTS.md
Adds a new
Coding Standards & Banned Patternssection to AGENTS.md with six explicit rules for AI agents and contributors working in this codebase.useEffectin favor ofuseMountEffectfor rare external side-effects with required justificationas anyTypeScript casts in favor of proper typing orunknownwith narrowinguseMemorather than syncing with effectsuseQuery) instead of fetch-in-effect patternsANTI-PATTERNSsection to reference and enforce these new standardsMacroscope summarized 45dc8ca.