anti-patterns
Skill from vincentbollaert/turborepo-monorepo
- Category: discovered
- Tags: github, community
- Version: 1.0.0
- Author: walid
- Downloads: 0
- GitHub stars: 2
- Updated: 2025-11-21
Download the latest version (v1.0.0) from this page.
SKILL.md
# Code Review & Anti-Patterns
**Auto-detection:** Code review, PR review, anti-patterns, code quality checks, best practices enforcement, refactoring guidance
**When to use:**
- Reviewing pull requests for common anti-patterns
- Identifying state management mistakes (server data in Zustand, Context for state)
- Catching performance anti-patterns (memoize everything, no lazy loading)
- Ensuring accessibility and security best practices
**Key patterns covered:**
- State management anti-patterns (server data in Zustand/Context, Context for state management)
- Component anti-patterns (God components >300 lines, missing ref forwarding, no className prop)
- Performance anti-patterns (premature memoization, importing entire libraries, no code splitting)
- Security anti-patterns (committed secrets, dangerouslySetInnerHTML with user input, no validation)
---
# Anti-Patterns
> **Quick Guide:** Common mistakes compiled from all frontend patterns. Review these to avoid pitfalls.
This document compiles all anti-patterns identified across the frontend standards documentation.
---
## State Management Anti-Patterns
**❌ NEVER:**
- Store server data in Zustand (use React Query instead)
- Store client UI state in React Query (use Zustand instead)
- Create unnecessary object references in Zustand selectors (causes re-renders)
- Use prop drilling instead of state management for global state
- Mutate state directly (use immutable updates)
- Create separate stores for related data (use one store with slices)
---
## TypeScript Anti-Patterns
**❌ NEVER:**
- Use `any` without explicit justification comment
- Use `@ts-ignore` or `@ts-expect-error` without explaining why
- Skip type definitions for exported functions
- Use implicit `any` (enable `strict` mode)
- Define types far from their usage
- Use `I` prefix for interfaces (e.g., `IProduct`)
- Use `interface` for component props (use `type` instead)
---
## Component Anti-Patterns
**❌ NEVER:**
- Create God components (> 300 lines, > 10 props)
- Skip ref forwarding on interactive elements
- Skip className exposure (prevents customization)
- Use prop spreading without type safety
- Use inline styles instead of design tokens
- Use cva for components with no variants (over-engineering)
- Use default exports in library components (use named exports)
- Mix casing (Button.tsx vs button.module.scss)
---
## Performance Anti-Patterns
**❌ NEVER:**
- Memoize everything (premature optimization has overhead)
- Skip memoization for expensive operations
- Use inline function definitions in JSX props (causes re-renders)
- Import entire libraries (`import _ from 'lodash'` instead of `import debounce from 'lodash/debounce'`)
- Skip lazy loading for routes
- Skip code splitting
- Not optimize images (missing WebP, lazy loading, responsive sizes)
- Block main thread with heavy computation (use web workers)
- Not measure performance before optimizing
- Set bundle size budgets (sizes grow unnoticed)
---
## Testing Anti-Patterns
**❌ NEVER:**
- Test implementation details (test behavior, not implementation)
- Use brittle selectors (prefer `getByRole`, `getByLabelText`)
- Skip MSW setup for API tests
- Skip integration tests (only unit tests)
- Have incomplete test coverage (< 80%)
- Test private methods directly
- Mock too much (integration tests should use real dependencies)
- Skip accessibility testing
---
## API & Data Fetching Anti-Patterns
**❌ NEVER:**
- Hardcode API URLs (use environment variables)
- Skip error handling for API calls
- Skip loading states
- Skip retry logic for transient failures
- Mutate cache directly (use React Query mutation helpers)
- Fetch data on every render (use caching)
- Not handle race conditions
- Skip request deduplication
---
## Styling Anti-Patterns
**❌ NEVER:**
- Use CSS-in-JS (styled-components, Emotion) - use SCSS Modules
- Use inline styles for anything other than dynamic values
- Hardcode colors/spacing (use design tokens)
- Create component variables unnecessarily
- Use className toggling for state (use data-attributes)
- Skip Ladle stories for design system components
- Use Tailwind classes directly in components (use design tokens)
---
## Accessibility Anti-Patterns
**❌ NEVER:**
- Remove focus outlines without replacement
- Use `div` or `span` for buttons/links
- Add click handlers on non-interactive elements without role/keyboard support
- Use color-only error indicators
- Use placeholder text as label replacement
- Disable form submit buttons (show validation errors instead)
- Auto-play audio/video without controls
- Open new windows without warning
- Create form inputs without labels
- Skip keyboard navigation support
---
## Build & Tooling Anti-Patterns
**❌ NEVER:**
- Skip linting configuration
- Use multiple icon libraries (use lucide-react)
- Import entire lucide-react package
- Use magic numbers anywhere (use named constants)
- Skip TypeScript strict mode
- Modify generated files manually (regenerate from source)
- Skip pre-commit hooks
- Not use Turborepo caching (wastes build time)
---
## Environment & Security Anti-Patterns
**❌ NEVER:**
- Commit secrets to repository
- Use `process.env.VARIABLE` directly without validation
- Hardcode environment values in code
- Share `.env` files via Slack/email
- Use production secrets in development
- Skip `.env.example` documentation
- Skip validation (missing variables cause runtime errors)
- Use different variable names across environments
- Not rotate secrets
- Expose secrets in client-side code (must use framework prefixes)
- Use `dangerouslySetInnerHTML` with user input
---
## File & Directory Anti-Patterns
**❌ NEVER:**
- Use PascalCase for file names (use kebab-case)
- Mix casing (Button.tsx and button.module.scss)
- Use default exports in libraries
- Create barrel files in components (use package.json exports)
- Import from internal paths instead of package exports
- Use relative imports for cross-package imports
---
## Monorepo Anti-Patterns
**❌ NEVER:**
- Have version mismatches across packages (use syncpack)
- Skip dependency synchronization
- Not declare environment variables in turbo.json
- Modify turbo cache manually
- Run full test suite on every PR (use affected detection)
- Not use remote caching (wastes CI time)
- Not use affected builds
---
## Quick Anti-Pattern Checklist
Review this checklist before submitting code:
**Code Quality:**
- [ ] No `any` without justification
- [ ] No magic numbers
- [ ] No hardcoded values
- [ ] Named exports only
- [ ] kebab-case file names
**Components:**
- [ ] Ref forwarding on interactive elements
- [ ] className prop exposed
- [ ] No God components (< 300 lines)
- [ ] Data-attributes for state styling
- [ ] Design tokens (no hardcoded colors/spacing)
**State & Data:**
- [ ] Server data in React Query
- [ ] UI state in Zustand
- [ ] Separate selectors (or `shallow` when destructuring)
- [ ] No prop drilling for global state
**Performance:**
- [ ] Lazy load routes
- [ ] No unnecessary memoization
- [ ] Optimized images
- [ ] No large libraries imported whole
**Testing:**
- [ ] MSW for API mocking
- [ ] Testing Library queries (getByRole)
- [ ] Integration tests
- [ ] Accessibility tests
**Security:**
- [ ] No committed secrets
- [ ] Validated environment variables
- [ ] No exposed API keys
- [ ] Input sanitization
**Accessibility:**
- [ ] Semantic HTML
- [ ] Keyboard navigation
- [ ] ARIA labels
- [ ] Focus management
- [ ] Screen reader support
---
# Anti-Patterns Observed - Examples
---
## State Management Anti-Patterns
### Example: Mixing Server State with Client State
```typescript
// ❌ BAD: Managing server data in useState
function ProductList() {
const [products, setProducts] = useState<Product[]>([]);
const [loading, setLoading] = useState(false);
const [error, setError] = useState<Error | null>(null);
useEffect(() => {
setLoading(true);
fetchProducts()
.then(setProducts)
.catch(setError)
.finally(() => setLoading(false));
}, []);
const handleUpdate = async (id: string, updates: Partial<Product>) => {
const updated = await updateProduct(id, updates);
setProducts(prev =>
prev.map(p => p.id === id ? updated : p)
); // Manual cache update!
};
return (
<div>
{loading && <Spinner />}
{error && <ErrorMessage error={error} />}
{products.map(product => (
<ProductCard
key={product.id}
product={product}
onUpdate={handleUpdate}
/>
))}
</div>
);
}
// ✅ GOOD: TanStack Query for server state
function ProductList() {
const { data: products, isLoading, error } = useQuery({
queryKey: ['products'],
queryFn: fetchProducts,
});
const { mutate: updateProduct } = useMutation({
mutationFn: (vars: { id: string; updates: Partial<Product> }) =>
apiClient.updateProduct(vars.id, vars.updates),
onSuccess: () => {
queryClient.invalidateQueries({ queryKey: ['products'] });
},
});
if (isLoading) return <Spinner />;
if (error) return <ErrorMessage error={error} />;
return (
<div>
{products?.map(product => (
<ProductCard
key={product.id}
product={product}
onUpdate={(updates) => updateProduct({ id: product.id, updates })}
/>
))}
</div>
);
}
```
**Why:** TanStack Query handles caching, revalidation, loading states, error handling automatically. Prevents stale data. Better UX.
**Edge Cases:**
- Use optimistic updates for instant feedback
- Configure stale time based on data volatility
- Prefetch for better perceived performance
---
## TypeScript Anti-Patterns
### Example: Avoiding `any` Abuse
```typescript
// ❌ BAD: Using any everywhere
function processData(data: any) {
return data.map((item: any) => ({
id: item.id,
name: item.name.toUpperCase(), // No type safety!
}));
}
// ✅ GOOD: Proper types
interface RawItem {
id: string;
name: string;
}
interface ProcessedItem {
id: string;
name: string;
}
function processData(data: RawItem[]): ProcessedItem[] {
return data.map(item => ({
id: item.id,
name: item.name.toUpperCase(), // Type-safe!
}));
}
// ✅ ACCEPTABLE: any with justification
function parseUnknownJSON(json: string): any {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
return JSON.parse(json); // any is justified - unknown JSON structure
}
// ✅ BETTER: Use unknown instead of any when possible
function parseUnknownJSON(json: string): unknown {
return JSON.parse(json); // Forces type checking before use
}
```
**Why:** Type safety prevents bugs. TypeScript's value is in catching errors. `any` disables all type checking.
**Edge Cases:**
- Use `unknown` when type is truly unknown
- Use type guards to narrow unknown types
- Document why `any` is necessary if used
- Consider generic types for flexible yet safe code
---
### Example: Interface vs Type for Props
```typescript
// ❌ BAD: Using interface for component props
interface ButtonProps {
variant: "primary" | "secondary";
onClick: () => void;
}
// ✅ GOOD: Use type for component props
type ButtonProps = {
variant: "primary" | "secondary";
onClick: () => void;
};
// ✅ GOOD: Use interface for API contracts and extensible types
interface User {
id: string;
name: string;
email: string;
}
interface AdminUser extends User {
role: "admin";
permissions: string[];
}
// ✅ GOOD: Use type for unions and complex types
type Status = "idle" | "loading" | "success" | "error";
type ApiResponse<T> =
| { status: "success"; data: T }
| { status: "error"; error: string };
```
**Why:** Types are more flexible for component props. Interfaces are better for object contracts. Consistent pattern across codebase.
**Edge Cases:**
- Use interface when you need declaration merging
- Use type for union types and mapped types
- Be consistent within a file/module
- Both work for most cases - consistency matters most
---
### Example: Proper Type Inference
```typescript
// ❌ BAD: Redundant type annotations
const users: User[] = getUsers();
const count: number = users.length;
const first: User | undefined = users[0];
// ✅ GOOD: Let TypeScript infer
const users = getUsers(); // Type inferred from return type
const count = users.length; // Inferred as number
const first = users[0]; // Inferred as User | undefined
// ❌ BAD: Not annotating function returns
function calculateTotal(items) {
return items.reduce((sum, item) => sum + item.price, 0);
}
// ✅ GOOD: Always annotate function return types
function calculateTotal(items: Item[]): number {
return items.reduce((sum, item) => sum + item.price, 0);
}
```
**Why:** Less noise in code. TypeScript is smart. Return type annotations catch errors and document intent.
**Edge Cases:**
- Annotate when inference is complex or wrong
- Always annotate public API function returns
- Let TypeScript infer local variables
- Use explicit types for complex generic inference
---
## Component Anti-Patterns
1: Prop Drilling Hell
```typescript
// ❌ BAD: Passing props through many layers
function App() {
const [user, setUser] = useState<User>();
return <Dashboard user={user} setUser={setUser} />;
}
function Dashboard({ user, setUser }: Props) {
return <Sidebar user={user} setUser={setUser} />;
}
function Sidebar({ user, setUser }: Props) {
return <UserMenu user={user} setUser={setUser} />;
}
function UserMenu({ user, setUser }: Props) {
return <ProfileButton user={user} onUpdate={setUser} />;
}
// ✅ GOOD: Use Context for widely-used data
const UserContext = createContext<UserContextType | null>(null);
function App() {
const [user, setUser] = useState<User>();
return (
<UserContext.Provider value={{ user, setUser }}>
<Dashboard />
</UserContext.Provider>
);
}
function ProfileButton() {
const { user, setUser } = useContext(UserContext)!;
return <button onClick={() => setUser(newUser)}>Update</button>;
}
// ✅ BETTER: Zustand for complex global state
import create from 'zustand';
const useUserStore = create<UserStore>(set => ({
user: null,
setUser: (user) => set({ user }),
logout: () => set({ user: null }),
}));
function ProfileButton() {
const { user, setUser } = useUserStore();
return <button onClick={() => setUser(newUser)}>Update</button>;
}
```
**Why:** Prop drilling creates coupling. Makes refactoring hard. Context or state management scales better.
**Edge Cases:**
- Use context for truly global data only
- Consider composition for component-specific props
- State managers add bundle size - use when needed
---
### Example: Massive Uselessly Complex Components
```typescript
// ❌ BAD: 500-line component doing everything
function UserDashboard() {
const [user, setUser] = useState<User>();
const [posts, setPosts] = useState<Post[]>([]);
const [comments, setComments] = useState<Comment[]>([]);
const [loading, setLoading] = useState(false);
useEffect(() => {
fetchUser().then(setUser);
fetchPosts().then(setPosts);
fetchComments().then(setComments);
}, []);
const handlePostSubmit = async (content: string) => {
// 50 lines of logic
};
const handleCommentSubmit = async (postId: string, content: string) => {
// 50 lines of logic
};
const filteredPosts = posts.filter(/* complex logic */);
const sortedComments = comments.sort(/* complex logic */);
return (
<div>
{/* 300 lines of JSX */}
<UserProfile user={user} />
<PostList posts={filteredPosts} />
<CommentSection comments={sortedComments} />
<PostForm onSubmit={handlePostSubmit} />
</div>
);
}
// ✅ GOOD: Compose smaller, focused components
function UserDashboard() {
return (
<div className="dashboard">
<UserProfileSection />
<PostsSection />
<CommentsSection />
</div>
);
}
function PostsSection() {
const { posts, isLoading } = usePostsQuery();
const { mutate: createPost } = useCreatePostMutation();
if (isLoading) return <Spinner />;
return (
<section>
<PostList posts={posts} />
<PostForm onSubmit={createPost} />
</section>
);
}
```
**Why:** Single responsibility. Easier testing. Better reusability. Clearer code organization.
**Edge Cases:**
- Break down when component exceeds ~200 lines
- Extract custom hooks for complex logic
- Co-locate related components in feature folders
---
### Example: Overusing useEffect
```typescript
// ❌ BAD: useEffect for derived state
function ProductFilter({ products }: { products: Product[] }) {
const [filter, setFilter] = useState('');
const [filteredProducts, setFilteredProducts] = useState(products);
useEffect(() => {
setFilteredProducts(
products.filter(p => p.name.includes(filter))
);
}, [products, filter]); // Unnecessary effect!
return (
<>
<input value={filter} onChange={e => setFilter(e.target.value)} />
<ProductList products={filteredProducts} />
</>
);
}
// ✅ GOOD: Calculate during render
function ProductFilter({ products }: { products: Product[] }) {
const [filter, setFilter] = useState('');
const filteredProducts = useMemo(
() => products.filter(p => p.name.includes(filter)),
[products, filter]
);
return (
<>
<input value={filter} onChange={e => setFilter(e.target.value)} />
<ProductList products={filteredProducts} />
</>
);
}
// ✅ EVEN BETTER: Don't memoize unless proven slow
function ProductFilter({ products }: { products: Product[] }) {
const [filter, setFilter] = useState('');
const filteredProducts = products.filter(p =>
p.name.includes(filter)
); // Fast enough for most cases!
return (
<>
<input value={filter} onChange={e => setFilter(e.target.value)} />
<ProductList products={filteredProducts} />
</>
);
}
```
**Why:** useEffect is for synchronization with external systems. Derived state should compute during render. Simpler code. Fewer bugs.
**Edge Cases:**
- Use useMemo only for expensive computations
- Profile before optimizing
- Keep logic synchronous when possible
---
### Example: Index as Key in Lists
```typescript
// ❌ BAD: Using array index as key
function TodoList({ todos }: { todos: Todo[] }) {
return (
<ul>
{todos.map((todo, index) => (
<li key={index}>
<TodoItem todo={todo} />
</li>
))}
</ul>
);
}
// Problem: Reordering or deleting items causes bugs!
// ✅ GOOD: Use stable unique identifiers
function TodoList({ todos }: { todos: Todo[] }) {
return (
<ul>
{todos.map(todo => (
<li key={todo.id}>
<TodoItem todo={todo} />
</li>
))}
</ul>
);
}
// ✅ FALLBACK: Generate stable IDs if needed
import { nanoid } from 'nanoid';
function TodoList({ todos }: { todos: TodoWithoutId[] }) {
const [todosWithIds] = useState(() =>
todos.map(todo => ({ ...todo, id: nanoid() }))
);
return (
<ul>
{todosWithIds.map(todo => (
<li key={todo.id}>
<TodoItem todo={todo} />
</li>
))}
</ul>
);
}
```
**Why:** Index keys break when order changes. React can't track components correctly. Causes state bugs and performance issues.
**Edge Cases:**
- Index is OK if list never reorders/filters/deletes
- Use crypto.randomUUID() in browsers, nanoid elsewhere
- Backend should provide IDs ideally
---
## Performance Anti-Patterns
### Example: Over-Memoization
```typescript
// ❌ BAD: Memoizing everything unnecessarily
function ProductCard({ product }: { product: Product }) {
const name = useMemo(() => product.name, [product.name]);
const price = useMemo(() => `${product.price}`, [product.price]);
const isExpensive = useMemo(() => product.price > 100, [product.price]);
const handleClick = useCallback(() => {
console.log(product.id);
}, [product.id]);
return (
<div onClick={handleClick}>
<h3>{name}</h3>
<p>{price}</p>