mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-21 11:46:37 +00:00
Moved .claude to internal repo for development
This commit is contained in:
Ben Duncan
@@ -1,189 +0,0 @@
|
||||
# CLAUDE.md
|
||||
|
||||
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
|
||||
|
||||
## Project Overview
|
||||
|
||||
This is the Hive platform - an AWS-compatible infrastructure stack for bare-metal, edge, and on-premise deployments. Hive orchestrates multiple independent components that work together to provide AWS-compatible services.
|
||||
|
||||
## Component Repositories
|
||||
|
||||
The Hive platform consists of these independent repositories:
|
||||
|
||||
- **[viperblock](../viperblock/)** - High-performance block storage service (EBS alternative) with WAL-backed storage
|
||||
- **[predastore](../predastore/)** - S3-compatible object storage service with Reed-Solomon erasure coding
|
||||
- **nbdkit/** - Network Block Device (NBD) server for exporting block devices (upstream project)
|
||||
- **hive/** (this repo) - VM orchestration service and platform coordinator
|
||||
|
||||
Each component can be used independently or as part of the integrated Hive stack. See component-specific `CLAUDE.md` files for detailed development guidance:
|
||||
- `../viperblock/CLAUDE.md`
|
||||
- `../predastore/CLAUDE.md`
|
||||
|
||||
## Commands
|
||||
|
||||
### Go (backend)
|
||||
|
||||
```bash
|
||||
make preflight # Run ALL CI checks locally (format + vet + security + tests)
|
||||
make build # Build hive binary to ./bin/hive
|
||||
make test # Run all unit tests (sets LOG_IGNORE=1)
|
||||
make check-format # Check gofmt (fails on diff, same as CI)
|
||||
make format # Fix gofmt in place
|
||||
make vet # Run go vet (fails on issues)
|
||||
make security # Run go vet + govulncheck, gosec, staticcheck (fails on issues)
|
||||
make bench # Run benchmarks
|
||||
make clean # Remove build artifacts
|
||||
make test-docker # Run single + multi-node E2E in Docker (requires /dev/kvm)
|
||||
make test-docker-single # Single-node E2E only
|
||||
make test-docker-multi # Multi-node E2E only
|
||||
```
|
||||
|
||||
### Frontend (hive-ui)
|
||||
|
||||
```bash
|
||||
make build-ui # Production build
|
||||
cd hive/services/hiveui/frontend && pnpm dev # Dev server on port 3000
|
||||
cd hive/services/hiveui/frontend && pnpm fix # Biome lint/format fix
|
||||
cd hive/services/hiveui/frontend && pnpm build # Build + typecheck
|
||||
```
|
||||
|
||||
Frontend stack: React 19, TanStack Router/Query, Tailwind CSS v4, Biome (lint/format), Vite, pnpm. See `.claude/rules/frontend-standards.md` for coding standards.
|
||||
|
||||
### Dev Environment
|
||||
|
||||
```bash
|
||||
scripts/dev-setup.sh # One-time setup: creates directories, generates config/certs
|
||||
scripts/start-dev.sh # Start all services (NATS → Predastore → Viperblock → NBDkit → Gateway → Daemon → UI)
|
||||
scripts/stop-dev.sh # Stop all services
|
||||
```
|
||||
|
||||
Data directory: `~/hive/` (logs in `~/hive/logs/`, config in `~/hive/config/`)
|
||||
|
||||
## Development Process
|
||||
|
||||
All non-trivial work (features, bug fixes, improvements) follows a plan-first, document-as-you-go process. Plans are committed to git so they are visible to the whole team, reviewable in PRs, and serve as permanent documentation.
|
||||
|
||||
### When to write a plan
|
||||
|
||||
- **Required**: New features, multi-file changes, architectural changes, performance work, non-obvious bug fixes
|
||||
- **Not required**: Typo fixes, single-line bug fixes, config tweaks, small refactors contained to one function
|
||||
|
||||
### Plan location
|
||||
|
||||
Plans live in `docs/development/` under the appropriate category:
|
||||
|
||||
```
|
||||
docs/development/
|
||||
├── bugs/ # Bug fixes
|
||||
├── feature/ # New features
|
||||
└── improvements/ # Performance, refactoring, tech debt
|
||||
```
|
||||
|
||||
File naming: lowercase kebab-case describing the work, e.g. `nbd-performance.md`, `attach-volume.md`, `instance-state.md`.
|
||||
|
||||
### Plan lifecycle
|
||||
|
||||
#### 1. Planning phase
|
||||
|
||||
When entering plan mode for non-trivial work, **create the plan document first** and write it to the appropriate `docs/development/{category}/` path. The plan must include:
|
||||
|
||||
- **Summary** — What and why, in 1-3 sentences
|
||||
- **Context / Problem Statement** — Current behavior, what's wrong or missing
|
||||
- **Proposed Changes** — What will change, which files, key design decisions
|
||||
- **Files to Modify** — List of files with brief description of changes
|
||||
- **Testing** — How the changes will be verified
|
||||
|
||||
The plan document is committed to git before implementation begins. This makes the plan reviewable and visible to the team.
|
||||
|
||||
#### 2. Implementation phase
|
||||
|
||||
During implementation, **update the plan document as you go**:
|
||||
|
||||
- Mark completed steps (use `[x]` checkboxes or note "Done" inline)
|
||||
- Record findings, corrections, or deviations from the original plan
|
||||
- Add code references (file paths, line numbers, struct/function names) for what was actually implemented
|
||||
- Note any follow-up work discovered during implementation
|
||||
|
||||
#### 3. Completion
|
||||
|
||||
When development is complete, update the document to serve as **permanent reference documentation**:
|
||||
|
||||
- Add a status line near the top: `**Status: Complete**` (or `**Status: In Progress**`, `**Status: Planned**`)
|
||||
- Replace future-tense plan language with past-tense description of what was done
|
||||
- Add a **"Files Modified"** summary table
|
||||
- Add a **"Future Work"** section if there are known follow-ups
|
||||
- Keep the original context (benchmarks, research, root cause analysis) — this is valuable reference material
|
||||
- The document should read as a complete technical reference, not just a plan
|
||||
|
||||
#### Example status markers
|
||||
|
||||
```markdown
|
||||
**Status: Planned** — Awaiting review/approval before implementation
|
||||
**Status: In Progress** — Implementation underway
|
||||
**Status: Complete** — All changes implemented and tests passing
|
||||
```
|
||||
|
||||
### Git history as context
|
||||
|
||||
When starting work on a feature or investigating an issue, review the last 10-20 git commits (`git log --oneline -20`) to understand the current development context. Recent commits contain valuable context about in-progress features, architectural decisions, and related changes.
|
||||
|
||||
### Existing examples
|
||||
|
||||
- `docs/development/improvements/nbd-performance.md` — Complete improvement with benchmarks, root cause analysis, implementation details, and future work
|
||||
- `docs/development/feature/attach-volume.md` — Complete feature with request flow, files changed, and design decisions
|
||||
- `docs/development/bugs/instance-state.md` — Bug fix with problem statement, proposed architecture, and state transitions
|
||||
|
||||
## Project Standards
|
||||
|
||||
- Use `log/slog` instead of `log`. Use appropriate log level, eg `slog.Debug`
|
||||
- All new features must have comprehensive unit tests
|
||||
- For returning AWS errors, use `awserrors` package instead of manually typed strings
|
||||
|
||||
**Preflight Policy:**
|
||||
- Run `make preflight` before pushing any major changes — it runs the same checks as GitHub Actions (gofmt, go vet, gosec, staticcheck, govulncheck, and all unit tests)
|
||||
- All developers must install the pre-push hook to enforce this automatically:
|
||||
|
||||
```bash
|
||||
cat > .git/hooks/pre-push << 'EOF'
|
||||
#!/bin/sh
|
||||
make preflight
|
||||
make test
|
||||
EOF
|
||||
chmod +x .git/hooks/pre-push
|
||||
```
|
||||
|
||||
**Testing Policy:**
|
||||
- **All unit tests MUST pass** before committing changes
|
||||
- **No exceptions** - failing tests block commits
|
||||
- Tests must complete without errors or panics
|
||||
|
||||
## Architecture
|
||||
|
||||
Read `DESIGN.md` to understand the full architecture.
|
||||
|
||||
### Message-Driven Architecture
|
||||
|
||||
```
|
||||
AWS SDK (custom endpoint) → AWS Gateway (port 9999) → NATS → Specialized Daemons → Response
|
||||
```
|
||||
|
||||
- **AWS Gateway** (`hive/services/awsgw/`): TLS endpoint, SigV4 auth, routes to service handlers via NATS
|
||||
- **Daemons** (`hive/daemon/`): Subscribe to NATS topics, process requests, manage QEMU/KVM VMs
|
||||
- **NATS Topics**: `ec2.*`, `ebs.*`, `s3.*`, `vpc.*` — queue groups (`hive-workers`) for load balancing, no queue group for fan-out (e.g., DescribeInstances)
|
||||
- **Storage**: Viperblock for EBS volumes (`ebs.mount` topic), Predastore for S3/AMI/key storage
|
||||
|
||||
### Go Workspace
|
||||
|
||||
Uses `go.work` with local module replacements — all repos must be cloned to the same parent directory:
|
||||
```
|
||||
mulga/
|
||||
├── hive/ # This repo
|
||||
├── predastore/ # S3-compatible storage
|
||||
└── viperblock/ # Block storage
|
||||
```
|
||||
|
||||
### Testing with AWS CLI
|
||||
|
||||
```bash
|
||||
AWS_PROFILE=hive aws ec2 describe-instances
|
||||
```
|
||||
@@ -1,90 +0,0 @@
|
||||
# Quick Reference
|
||||
|
||||
Most formatting and common issues are automatically fixed by Biome. Run `pnpm fix` before committing to ensure compliance and run `pnpm build` for typechecking and route tree generation.
|
||||
|
||||
## Core Principles
|
||||
|
||||
Write code that is **accessible, performant, type-safe, and maintainable**. Focus on clarity and explicit intent over brevity.
|
||||
|
||||
### Type Safety & Explicitness
|
||||
|
||||
- Use explicit types for function parameters and return values when they enhance clarity
|
||||
- Prefer `unknown` over `any` when the type is genuinely unknown
|
||||
- Use const assertions (`as const`) for immutable values and literal types
|
||||
- Leverage TypeScript's type narrowing instead of type assertions
|
||||
- Use meaningful variable names instead of magic numbers - extract constants with descriptive names
|
||||
|
||||
### Modern JavaScript/TypeScript
|
||||
|
||||
- Use arrow functions for callbacks and short functions
|
||||
- Prefer `for...of` loops over `.forEach()` and indexed `for` loops
|
||||
- Use optional chaining (`?.`) and nullish coalescing (`??`) for safer property access
|
||||
- Prefer template literals over string concatenation
|
||||
- Use destructuring for object and array assignments
|
||||
- Use `const` by default, `let` only when reassignment is needed, never `var`
|
||||
- Use interfaces for objects, otherwise use type aliases
|
||||
- Prefer using function declarations over arrow functions for named functions
|
||||
|
||||
### Async & Promises
|
||||
|
||||
- Always `await` promises in async functions - don't forget to use the return value
|
||||
- Use `async/await` syntax instead of promise chains for better readability
|
||||
- Handle errors appropriately in async code with try-catch blocks
|
||||
- Don't use async functions as Promise executors
|
||||
|
||||
### React & JSX
|
||||
|
||||
- Use function components over class components
|
||||
- Call hooks at the top level only, never conditionally
|
||||
- Specify all dependencies in hook dependency arrays correctly
|
||||
- Use the `key` prop for elements in iterables (prefer unique IDs over array indices)
|
||||
- Nest children between opening and closing tags instead of passing as props
|
||||
- Don't define components inside other components
|
||||
- Use semantic HTML and ARIA attributes for accessibility:
|
||||
- Provide meaningful alt text for images
|
||||
- Use proper heading hierarchy
|
||||
- Add labels for form inputs
|
||||
- Include keyboard event handlers alongside mouse events
|
||||
- Use semantic elements (`<button>`, `<nav>`, etc.) instead of divs with roles
|
||||
- Don't use `useMemo` or `useCallback`
|
||||
- Use ref as a prop instead of `React.forwardRef`
|
||||
|
||||
### Error Handling & Debugging
|
||||
|
||||
- Remove `console.log`, `debugger`, and `alert` statements from production code
|
||||
- Throw `Error` objects with descriptive messages, not strings or other values
|
||||
- Use `try-catch` blocks meaningfully - don't catch errors just to rethrow them
|
||||
- Prefer early returns over nested conditionals for error cases
|
||||
|
||||
### Code Organization
|
||||
|
||||
- Keep functions focused and under reasonable cognitive complexity limits
|
||||
- Extract complex conditions into well-named boolean variables
|
||||
- Use early returns to reduce nesting
|
||||
- Prefer simple conditionals over nested ternary operators
|
||||
- Group related code together and separate concerns
|
||||
|
||||
### Security
|
||||
|
||||
- Add `rel="noopener"` when using `target="_blank"` on links
|
||||
- Avoid `dangerouslySetInnerHTML` unless absolutely necessary
|
||||
- Don't use `eval()` or assign directly to `document.cookie`
|
||||
- Validate and sanitize user input
|
||||
|
||||
### Performance
|
||||
|
||||
- Avoid spread syntax in accumulators within loops
|
||||
- Use top-level regex literals instead of creating them in loops
|
||||
- Prefer specific imports over namespace imports
|
||||
- Avoid barrel files (index files that re-export everything)
|
||||
|
||||
## When Biome Can't Help
|
||||
|
||||
Biome's linter will catch most issues automatically. Focus your attention on:
|
||||
|
||||
1. **Business logic correctness** - Biome can't validate your algorithms
|
||||
2. **Meaningful naming** - Use descriptive names for functions, variables, and types
|
||||
3. **Architecture decisions** - Component structure, data flow, and API design
|
||||
4. **Edge cases** - Handle boundary conditions and error states
|
||||
5. **User experience** - Accessibility, performance, and usability considerations
|
||||
6. **Documentation** - Add comments for complex logic, but prefer self-documenting code
|
||||
@@ -1,80 +0,0 @@
|
||||
{
|
||||
"permissions": {
|
||||
"allow": [
|
||||
"Skill(feature-dev:feature-dev)",
|
||||
"Skill(pr-review-toolkit:review-pr)",
|
||||
|
||||
"Skill(frontend-design:frontend-design)",
|
||||
"Skill(react-best-practices)",
|
||||
"Skill(react-composition-patterns)",
|
||||
"Skill(web-design-guidelines)",
|
||||
|
||||
"Bash(make build)",
|
||||
"Bash(make build-ui)",
|
||||
"Bash(LOG_IGNORE=1 make test:*)",
|
||||
"Bash(make test)",
|
||||
"Bash(make bench)",
|
||||
"Bash(make clean)",
|
||||
"Bash(make security)",
|
||||
|
||||
"Bash(go build*)",
|
||||
"Bash(go doc:*)",
|
||||
"Bash(LOG_IGNORE=1 go test:*)",
|
||||
"Bash(go test:*)",
|
||||
|
||||
"Bash(ls*)",
|
||||
"Bash(ls -la*)",
|
||||
"Bash(ls -l*)",
|
||||
"Bash(ll*)",
|
||||
"Bash(ls -a*)",
|
||||
"Bash(ls -lh*)",
|
||||
"Bash(ls -la */)",
|
||||
"Bash(cat *)",
|
||||
"Bash(cat -n *)",
|
||||
"Bash(less *)",
|
||||
"Bash(more *)",
|
||||
"Bash(head *)",
|
||||
"Bash(head -n *)",
|
||||
"Bash(head -*)",
|
||||
"Bash(tail *)",
|
||||
"Bash(tail -n *)",
|
||||
"Bash(tail -*)",
|
||||
"Bash(tail -f *)",
|
||||
"Bash(grep *)",
|
||||
"Bash(grep -r *)",
|
||||
"Bash(grep -i *)",
|
||||
"Bash(grep -n *)",
|
||||
"Bash(grep -E *)",
|
||||
"Bash(grep -v *)",
|
||||
"Bash(grep -A* -B* *)",
|
||||
"Bash(find *)",
|
||||
"Bash(find . -name *)",
|
||||
"Bash(find . -type *)",
|
||||
"Bash(find . -path *)",
|
||||
"Bash(diff *)",
|
||||
"Bash(tree *)",
|
||||
"Bash(pwd*)",
|
||||
"Bash(cd *)",
|
||||
"Bash(cd * && *)",
|
||||
"Bash(cd ../viperblock && *)",
|
||||
"Bash(cd ../predastore && *)",
|
||||
"Bash(wc:*)",
|
||||
|
||||
"Bash(pnpm fix:*)",
|
||||
"Bash(pnpm check:*)",
|
||||
"Bash(pnpm build:*)",
|
||||
"Bash(pnpm tsc *)",
|
||||
|
||||
"Bash(git diff*)",
|
||||
"Bash(git diff --staged*)",
|
||||
"Bash(git diff HEAD*)",
|
||||
"Bash(git log*)",
|
||||
"Bash(git log --oneline*)",
|
||||
"Bash(git log --graph*)",
|
||||
"Bash(git status*)",
|
||||
"Bash(git stash*)",
|
||||
"Bash(git stash pop*)",
|
||||
"Bash(git stash list*)"
|
||||
]
|
||||
}
|
||||
}
|
||||
@@ -1,75 +0,0 @@
|
||||
---
|
||||
name: go-best-practices
|
||||
description: Go best practices and security guidelines. This skill should be used when writing, reviewing, or refactoring Go code to ensure correct error handling, secure HTTP servers, proper concurrency patterns, and idiomatic Go. Triggers on tasks involving Go backend code, HTTP handlers, goroutines, or system-level programming.
|
||||
---
|
||||
|
||||
# Go Best Practices
|
||||
|
||||
Comprehensive best practices for writing secure, correct, and idiomatic Go code. Contains rules prioritized by impact, covering security (gosec compliance), error handling, HTTP servers, concurrency, and common pitfalls. All rules are enforced by `make security` (go vet, gosec, govulncheck, staticcheck).
|
||||
|
||||
## When to Apply
|
||||
|
||||
Reference these guidelines when:
|
||||
- Writing new Go code (handlers, services, packages)
|
||||
- Creating or modifying HTTP servers
|
||||
- Working with goroutines, channels, or mutexes
|
||||
- Writing functions that return errors
|
||||
- Reviewing Go code for correctness and security
|
||||
- Implementing AWS-compatible service handlers
|
||||
|
||||
## Rule Categories by Priority
|
||||
|
||||
| Priority | Category | Impact | Prefix |
|
||||
|----------|----------|--------|--------|
|
||||
| 1 | HTTP Server Security | CRITICAL | `http-` |
|
||||
| 2 | Error Handling | HIGH | `errors-` |
|
||||
| 3 | Concurrency | HIGH | `concurrency-` |
|
||||
| 4 | Resource Management | MEDIUM | `resources-` |
|
||||
| 5 | Code Style & Idioms | LOW-MEDIUM | `style-` |
|
||||
|
||||
## Quick Reference
|
||||
|
||||
### 1. HTTP Server Security (CRITICAL)
|
||||
|
||||
- `http-server-timeouts` - Always configure all timeouts on http.Server
|
||||
- `http-handle-write-errors` - Always handle errors from ResponseWriter.Write and JSON encoding
|
||||
- `http-input-validation` - Validate and limit request bodies, never trust client input
|
||||
|
||||
### 2. Error Handling (HIGH)
|
||||
|
||||
- `errors-always-check` - Never ignore returned errors (gosec G104)
|
||||
- `errors-wrap-context` - Wrap errors with fmt.Errorf and %w for context
|
||||
- `errors-sentinel` - Use sentinel errors and errors.Is/errors.As for comparison
|
||||
|
||||
### 3. Concurrency (HIGH)
|
||||
|
||||
- `concurrency-goroutine-leaks` - Ensure goroutines can always exit
|
||||
- `concurrency-mutex-patterns` - Correct mutex usage and defer unlock
|
||||
- `concurrency-channel-ownership` - Clear channel ownership and closing conventions
|
||||
|
||||
### 4. Resource Management (MEDIUM)
|
||||
|
||||
- `resources-close-bodies` - Always close http response bodies and file handles
|
||||
- `resources-context-propagation` - Pass and respect context.Context for cancellation
|
||||
- `resources-defer-cleanup` - Use defer for cleanup to ensure execution on all paths
|
||||
|
||||
### 5. Code Style & Idioms (LOW-MEDIUM)
|
||||
|
||||
- `style-slog-logging` - Use log/slog instead of log, with appropriate levels
|
||||
- `style-error-types` - Use awserrors package for AWS error responses
|
||||
- `style-table-driven-tests` - Use table-driven tests with subtests
|
||||
|
||||
## How to Use
|
||||
|
||||
Read individual rule files for detailed explanations and code examples:
|
||||
|
||||
```
|
||||
rules/http-server-timeouts.md
|
||||
rules/errors-always-check.md
|
||||
```
|
||||
|
||||
Each rule file contains:
|
||||
- Brief explanation of why it matters
|
||||
- Incorrect code example with explanation
|
||||
- Correct code example with explanation
|
||||
- gosec/vet rule ID where applicable
|
||||
@@ -1,54 +0,0 @@
|
||||
---
|
||||
title: Clear Channel Ownership and Closing
|
||||
impact: HIGH
|
||||
impactDescription: prevents panics from double-close and sends on closed channels
|
||||
tags: go, concurrency, channels, ownership
|
||||
---
|
||||
|
||||
## Clear Channel Ownership and Closing
|
||||
|
||||
Follow clear ownership rules for channels: only the sender (producer) should close a channel, never the receiver. Closing a channel from the wrong goroutine causes panics.
|
||||
|
||||
**Incorrect (receiver closes, or double close):**
|
||||
|
||||
```go
|
||||
func producer(ch chan<- int) {
|
||||
for i := 0; i < 10; i++ {
|
||||
ch <- i
|
||||
}
|
||||
// forgot to close — receiver blocks forever
|
||||
}
|
||||
|
||||
func consumer(ch <-chan int, done chan struct{}) {
|
||||
for val := range ch {
|
||||
process(val)
|
||||
}
|
||||
close(done)
|
||||
close(done) // panic: close of closed channel
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (sender closes, clear ownership):**
|
||||
|
||||
```go
|
||||
func producer(ch chan<- int) {
|
||||
defer close(ch) // sender closes when done
|
||||
for i := 0; i < 10; i++ {
|
||||
ch <- i
|
||||
}
|
||||
}
|
||||
|
||||
func consumer(ch <-chan int) {
|
||||
for val := range ch {
|
||||
process(val)
|
||||
}
|
||||
// channel was closed by producer, range exits naturally
|
||||
}
|
||||
```
|
||||
|
||||
**Key points:**
|
||||
- The goroutine that creates/sends on a channel should close it
|
||||
- Use `defer close(ch)` at the top of the sending function
|
||||
- Use directional channel types (`chan<-`, `<-chan`) in function signatures to enforce ownership
|
||||
- For signaling (done channels), use `chan struct{}` and close it once — don't send values
|
||||
- Use `sync.Once` if multiple goroutines might try to close the same channel
|
||||
@@ -1,77 +0,0 @@
|
||||
---
|
||||
title: Prevent Goroutine Leaks
|
||||
impact: HIGH
|
||||
impactDescription: prevents memory leaks and resource exhaustion from orphaned goroutines
|
||||
tags: go, concurrency, goroutines, context, leaks
|
||||
---
|
||||
|
||||
## Prevent Goroutine Leaks
|
||||
|
||||
Every goroutine must have a clear exit path. Goroutines that block forever on channels or I/O without a cancellation mechanism leak memory and resources.
|
||||
|
||||
**Incorrect (goroutine can never exit):**
|
||||
|
||||
```go
|
||||
func startWorker(jobs <-chan Job) {
|
||||
go func() {
|
||||
for job := range jobs {
|
||||
process(job)
|
||||
}
|
||||
}()
|
||||
// If no one closes jobs channel and no context, this goroutine lives forever
|
||||
}
|
||||
|
||||
func pollStatus(url string, interval time.Duration) {
|
||||
go func() {
|
||||
for {
|
||||
resp, _ := http.Get(url)
|
||||
// runs forever, no way to stop
|
||||
time.Sleep(interval)
|
||||
}
|
||||
}()
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (goroutine exits via context or done channel):**
|
||||
|
||||
```go
|
||||
func startWorker(ctx context.Context, jobs <-chan Job) {
|
||||
go func() {
|
||||
for {
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return
|
||||
case job, ok := <-jobs:
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
process(job)
|
||||
}
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
||||
func pollStatus(ctx context.Context, url string, interval time.Duration) {
|
||||
go func() {
|
||||
ticker := time.NewTicker(interval)
|
||||
defer ticker.Stop()
|
||||
for {
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return
|
||||
case <-ticker.C:
|
||||
// use context-aware HTTP request
|
||||
req, _ := http.NewRequestWithContext(ctx, "GET", url, nil)
|
||||
http.DefaultClient.Do(req)
|
||||
}
|
||||
}
|
||||
}()
|
||||
}
|
||||
```
|
||||
|
||||
**Key points:**
|
||||
- Always pass `context.Context` to functions that spawn goroutines
|
||||
- Use `select` with `ctx.Done()` to enable cancellation
|
||||
- Use `time.NewTicker` (and defer `Stop()`) instead of `time.Sleep` in loops
|
||||
- Close channels from the sender side to signal completion
|
||||
- In server shutdown, use `server.Shutdown(ctx)` to drain connections gracefully
|
||||
@@ -1,82 +0,0 @@
|
||||
---
|
||||
title: Correct Mutex Usage Patterns
|
||||
impact: HIGH
|
||||
impactDescription: prevents data races, deadlocks, and race condition bugs
|
||||
tags: go, concurrency, mutex, sync, data-race
|
||||
---
|
||||
|
||||
## Correct Mutex Usage Patterns
|
||||
|
||||
Use mutexes correctly to protect shared state. Common mistakes include forgetting to unlock, holding locks across blocking operations, and using the wrong lock type.
|
||||
|
||||
**Incorrect (common mutex mistakes):**
|
||||
|
||||
```go
|
||||
// Forgetting to unlock on error paths
|
||||
func (s *Store) Get(key string) (string, error) {
|
||||
s.mu.Lock()
|
||||
val, ok := s.data[key]
|
||||
if !ok {
|
||||
return "", ErrNotFound // mutex never unlocked!
|
||||
}
|
||||
s.mu.Unlock()
|
||||
return val, nil
|
||||
}
|
||||
|
||||
// Holding lock across blocking operation
|
||||
func (s *Store) Refresh() {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
data, _ := http.Get(s.url) // blocks while holding lock!
|
||||
s.data = parse(data)
|
||||
}
|
||||
|
||||
// Using RWMutex wrong — write lock where read lock suffices
|
||||
func (s *Store) Count() int {
|
||||
s.mu.Lock() // should be RLock for read-only access
|
||||
defer s.mu.Unlock()
|
||||
return len(s.data)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct:**
|
||||
|
||||
```go
|
||||
// Always use defer Unlock
|
||||
func (s *Store) Get(key string) (string, error) {
|
||||
s.mu.RLock()
|
||||
defer s.mu.RUnlock()
|
||||
val, ok := s.data[key]
|
||||
if !ok {
|
||||
return "", ErrNotFound
|
||||
}
|
||||
return val, nil
|
||||
}
|
||||
|
||||
// Don't hold lock across blocking ops
|
||||
func (s *Store) Refresh() {
|
||||
data, err := http.Get(s.url) // fetch without lock
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
parsed := parse(data)
|
||||
|
||||
s.mu.Lock()
|
||||
s.data = parsed // only hold lock for assignment
|
||||
s.mu.Unlock()
|
||||
}
|
||||
|
||||
// Use RLock for read-only access
|
||||
func (s *Store) Count() int {
|
||||
s.mu.RLock()
|
||||
defer s.mu.RUnlock()
|
||||
return len(s.data)
|
||||
}
|
||||
```
|
||||
|
||||
**Key points:**
|
||||
- Always use `defer mu.Unlock()` immediately after `mu.Lock()` to prevent missed unlocks
|
||||
- Use `sync.RWMutex` when reads vastly outnumber writes — `RLock()` for reads, `Lock()` for writes
|
||||
- Never hold a lock while doing I/O, network calls, or channel operations
|
||||
- Keep critical sections as small as possible
|
||||
- Document which fields a mutex protects with a comment above the mutex field
|
||||
@@ -1,61 +0,0 @@
|
||||
---
|
||||
title: Always Check Returned Errors
|
||||
impact: HIGH
|
||||
impactDescription: prevents silent failures and data corruption (gosec G104)
|
||||
tags: go, errors, gosec, reliability
|
||||
---
|
||||
|
||||
## Always Check Returned Errors
|
||||
|
||||
Never ignore error return values. Every function that returns an error must have that error checked. This is the single most common source of bugs in Go code.
|
||||
|
||||
**Incorrect (errors discarded):**
|
||||
|
||||
```go
|
||||
file, _ := os.Open(path) // crash if file doesn't exist
|
||||
json.Unmarshal(data, &config) // silent corruption if data is invalid
|
||||
db.Exec("DELETE FROM users WHERE id = ?", id) // silent failure
|
||||
conn.Close() // resource leak on error
|
||||
```
|
||||
|
||||
**Correct (errors handled):**
|
||||
|
||||
```go
|
||||
file, err := os.Open(path)
|
||||
if err != nil {
|
||||
return fmt.Errorf("open config: %w", err)
|
||||
}
|
||||
|
||||
if err := json.Unmarshal(data, &config); err != nil {
|
||||
return fmt.Errorf("parse config: %w", err)
|
||||
}
|
||||
|
||||
if _, err := db.Exec("DELETE FROM users WHERE id = ?", id); err != nil {
|
||||
return fmt.Errorf("delete user %s: %w", id, err)
|
||||
}
|
||||
|
||||
if err := conn.Close(); err != nil {
|
||||
slog.Error("Failed to close connection", "error", err)
|
||||
}
|
||||
```
|
||||
|
||||
**Acceptable exceptions:**
|
||||
|
||||
```go
|
||||
// In deferred cleanup where you can't return the error
|
||||
defer func() {
|
||||
if err := resp.Body.Close(); err != nil {
|
||||
slog.Debug("Failed to close response body", "error", err)
|
||||
}
|
||||
}()
|
||||
|
||||
// fmt.Fprintf to a bytes.Buffer never fails — but document why
|
||||
_, _ = fmt.Fprintf(&buf, "value: %d", n) // bytes.Buffer.Write never returns error
|
||||
```
|
||||
|
||||
**Key points:**
|
||||
- Use `_ =` explicitly when you intentionally discard an error, with a comment explaining why
|
||||
- Log errors in cleanup paths (defer) where you can't propagate them
|
||||
- Wrap errors with `%w` to preserve the error chain for callers
|
||||
|
||||
**gosec rule:** G104 (CWE-703) — Errors unhandled
|
||||
@@ -1,57 +0,0 @@
|
||||
---
|
||||
title: Use Sentinel Errors and errors.Is/As
|
||||
impact: HIGH
|
||||
impactDescription: enables correct error matching across wrapped error chains
|
||||
tags: go, errors, sentinel, comparison
|
||||
---
|
||||
|
||||
## Use Sentinel Errors and errors.Is/As
|
||||
|
||||
Never compare errors with `==`. Use `errors.Is()` for sentinel error comparison and `errors.As()` for type assertion. Direct comparison breaks when errors are wrapped.
|
||||
|
||||
**Incorrect (direct comparison):**
|
||||
|
||||
```go
|
||||
if err == sql.ErrNoRows {
|
||||
return nil, nil // breaks if err is wrapped
|
||||
}
|
||||
|
||||
if err.Error() == "not found" {
|
||||
// fragile string comparison
|
||||
}
|
||||
|
||||
switch err.(type) {
|
||||
case *os.PathError:
|
||||
// breaks with wrapped errors
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (errors.Is and errors.As):**
|
||||
|
||||
```go
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return nil, nil // works even if wrapped
|
||||
}
|
||||
|
||||
var pathErr *os.PathError
|
||||
if errors.As(err, &pathErr) {
|
||||
slog.Error("Path error", "path", pathErr.Path, "op", pathErr.Op)
|
||||
}
|
||||
```
|
||||
|
||||
**Defining sentinel errors:**
|
||||
|
||||
```go
|
||||
var (
|
||||
ErrNotFound = errors.New("not found")
|
||||
ErrUnauthorized = errors.New("unauthorized")
|
||||
)
|
||||
|
||||
// Usage
|
||||
return fmt.Errorf("get volume %s: %w", id, ErrNotFound)
|
||||
|
||||
// Checking
|
||||
if errors.Is(err, ErrNotFound) {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
}
|
||||
```
|
||||
@@ -1,52 +0,0 @@
|
||||
---
|
||||
title: Wrap Errors with Context
|
||||
impact: HIGH
|
||||
impactDescription: enables debugging by preserving error chains and adding call context
|
||||
tags: go, errors, debugging, fmt
|
||||
---
|
||||
|
||||
## Wrap Errors with Context
|
||||
|
||||
When returning an error from a called function, wrap it with `fmt.Errorf` and `%w` to add context about what operation failed. Bare error returns make debugging difficult because you lose the call chain.
|
||||
|
||||
**Incorrect (bare error returns):**
|
||||
|
||||
```go
|
||||
func LoadConfig(path string) (*Config, error) {
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
return nil, err // caller gets "open /etc/config.yaml: no such file or directory"
|
||||
}
|
||||
|
||||
var cfg Config
|
||||
if err := json.Unmarshal(data, &cfg); err != nil {
|
||||
return nil, err // caller gets "invalid character '}'" — no idea what file
|
||||
}
|
||||
|
||||
return &cfg, nil
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (wrapped with context):**
|
||||
|
||||
```go
|
||||
func LoadConfig(path string) (*Config, error) {
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("read config %s: %w", path, err)
|
||||
}
|
||||
|
||||
var cfg Config
|
||||
if err := json.Unmarshal(data, &cfg); err != nil {
|
||||
return nil, fmt.Errorf("parse config %s: %w", path, err)
|
||||
}
|
||||
|
||||
return &cfg, nil
|
||||
}
|
||||
```
|
||||
|
||||
**Key points:**
|
||||
- Use `%w` (not `%v`) to wrap errors — this preserves `errors.Is()` and `errors.As()` checks
|
||||
- Add the operation name and key parameters (file path, ID, etc.)
|
||||
- Keep wrap messages lowercase, no trailing punctuation (Go convention)
|
||||
- Don't prefix with "failed to" or "error" — the error itself communicates failure
|
||||
@@ -1,52 +0,0 @@
|
||||
---
|
||||
title: Handle HTTP Response Write Errors
|
||||
impact: CRITICAL
|
||||
impactDescription: prevents silent failures and satisfies gosec G104
|
||||
tags: go, http, error-handling, gosec, handlers
|
||||
---
|
||||
|
||||
## Handle HTTP Response Write Errors
|
||||
|
||||
Always check errors returned by `http.ResponseWriter.Write()`, `io.Writer.Write()`, `json.Encoder.Encode()`, and `fmt.Fprintf(w, ...)` in HTTP handlers. Ignoring these errors means you won't know when clients disconnect or responses fail to send.
|
||||
|
||||
**Incorrect (errors ignored — gosec G104):**
|
||||
|
||||
```go
|
||||
func handleHealth(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
w.Write([]byte("ok")) // error ignored
|
||||
}
|
||||
|
||||
func writeJSON(w http.ResponseWriter, status int, v any) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(status)
|
||||
json.NewEncoder(w).Encode(v) // error ignored
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (errors handled):**
|
||||
|
||||
```go
|
||||
func handleHealth(w http.ResponseWriter, r *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
if _, err := w.Write([]byte("ok")); err != nil {
|
||||
slog.Error("Failed to write health response", "error", err)
|
||||
}
|
||||
}
|
||||
|
||||
func writeJSON(w http.ResponseWriter, status int, v any) {
|
||||
w.Header().Set("Content-Type", "application/json")
|
||||
w.WriteHeader(status)
|
||||
if err := json.NewEncoder(w).Encode(v); err != nil {
|
||||
slog.Error("Failed to encode JSON response", "error", err)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Key points:**
|
||||
- Log write errors at error level — they indicate client disconnects or network issues
|
||||
- Don't try to write an HTTP error response after a write failure (the connection is likely broken)
|
||||
- This applies to all `io.Writer` methods, not just `http.ResponseWriter`
|
||||
- `fmt.Fprintf(w, ...)` also returns an error that must be checked
|
||||
|
||||
**gosec rule:** G104 (CWE-703) — Errors unhandled
|
||||
@@ -1,52 +0,0 @@
|
||||
---
|
||||
title: Validate and Limit HTTP Request Input
|
||||
impact: CRITICAL
|
||||
impactDescription: prevents denial of service via oversized payloads and injection attacks
|
||||
tags: go, http, security, validation, input
|
||||
---
|
||||
|
||||
## Validate and Limit HTTP Request Input
|
||||
|
||||
Always limit request body size and validate input in HTTP handlers. An unbounded `json.NewDecoder(r.Body).Decode()` allows attackers to send arbitrarily large payloads, exhausting server memory.
|
||||
|
||||
**Incorrect (unbounded request body):**
|
||||
|
||||
```go
|
||||
func handleCreate(w http.ResponseWriter, r *http.Request) {
|
||||
var req CreateRequest
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
http.Error(w, "bad request", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
// process req...
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (limited and validated):**
|
||||
|
||||
```go
|
||||
const maxRequestBodySize = 1 << 20 // 1 MB
|
||||
|
||||
func handleCreate(w http.ResponseWriter, r *http.Request) {
|
||||
r.Body = http.MaxBytesReader(w, r.Body, maxRequestBodySize)
|
||||
|
||||
var req CreateRequest
|
||||
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
|
||||
http.Error(w, "bad request", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
// Validate required fields
|
||||
if req.Name == "" {
|
||||
http.Error(w, "name is required", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
// process req...
|
||||
}
|
||||
```
|
||||
|
||||
**Key points:**
|
||||
- Use `http.MaxBytesReader` to limit body size — it returns a 413 error if exceeded
|
||||
- Validate all required fields after decoding
|
||||
- Sanitize string inputs if they'll be used in commands, queries, or file paths
|
||||
- Use early returns for validation failures
|
||||
@@ -1,50 +0,0 @@
|
||||
---
|
||||
title: Configure HTTP Server Timeouts
|
||||
impact: CRITICAL
|
||||
impactDescription: prevents Slowloris attacks and resource exhaustion (gosec G112)
|
||||
tags: go, http, security, gosec, server, timeouts
|
||||
---
|
||||
|
||||
## Configure HTTP Server Timeouts
|
||||
|
||||
Every `http.Server` must have `ReadHeaderTimeout` configured at minimum. Omitting timeouts allows Slowloris-style attacks where an attacker opens connections and sends headers slowly, exhausting server resources.
|
||||
|
||||
**Incorrect (no timeouts — gosec G112):**
|
||||
|
||||
```go
|
||||
server := &http.Server{
|
||||
Addr: ":8080",
|
||||
Handler: mux,
|
||||
}
|
||||
```
|
||||
|
||||
This server has no timeout protection. A malicious client can hold connections open indefinitely.
|
||||
|
||||
**Correct (all timeouts configured):**
|
||||
|
||||
```go
|
||||
server := &http.Server{
|
||||
Addr: ":8080",
|
||||
Handler: mux,
|
||||
ReadTimeout: 10 * time.Second,
|
||||
WriteTimeout: 10 * time.Second,
|
||||
IdleTimeout: 30 * time.Second,
|
||||
ReadHeaderTimeout: 5 * time.Second,
|
||||
}
|
||||
```
|
||||
|
||||
**Timeout guidelines:**
|
||||
|
||||
| Timeout | Purpose | Typical Value |
|
||||
|---------|---------|---------------|
|
||||
| `ReadHeaderTimeout` | Time to read request headers (Slowloris protection) | 5s |
|
||||
| `ReadTimeout` | Time to read entire request including body | 10-30s |
|
||||
| `WriteTimeout` | Time to write the response | 10-30s |
|
||||
| `IdleTimeout` | Time to keep idle keep-alive connections open | 30-120s |
|
||||
|
||||
**Adjust for your use case:**
|
||||
- Short-lived internal APIs (formation, health): 10s read/write, 30s idle
|
||||
- User-facing APIs with uploads: 30s+ read, 15s write
|
||||
- Long-polling endpoints: increase WriteTimeout accordingly
|
||||
|
||||
**gosec rule:** G112 (CWE-400) — Potential Slowloris Attack
|
||||
@@ -1,47 +0,0 @@
|
||||
---
|
||||
title: Close Response Bodies and File Handles
|
||||
impact: MEDIUM
|
||||
impactDescription: prevents resource leaks that exhaust file descriptors and connections
|
||||
tags: go, resources, http, files, defer, leak
|
||||
---
|
||||
|
||||
## Close Response Bodies and File Handles
|
||||
|
||||
Always close `http.Response.Body`, files, and other `io.Closer` resources. Failing to close HTTP response bodies leaks TCP connections; failing to close files leaks file descriptors.
|
||||
|
||||
**Incorrect (response body not closed):**
|
||||
|
||||
```go
|
||||
func fetchData(url string) ([]byte, error) {
|
||||
resp, err := http.Get(url)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// resp.Body is never closed — leaks TCP connection
|
||||
return io.ReadAll(resp.Body)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (defer close immediately after error check):**
|
||||
|
||||
```go
|
||||
func fetchData(url string) ([]byte, error) {
|
||||
resp, err := http.Get(url)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("fetch %s: %w", url, err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
if resp.StatusCode != http.StatusOK {
|
||||
return nil, fmt.Errorf("fetch %s: status %d", url, resp.StatusCode)
|
||||
}
|
||||
|
||||
return io.ReadAll(resp.Body)
|
||||
}
|
||||
```
|
||||
|
||||
**Key points:**
|
||||
- Place `defer Close()` immediately after the nil-error check, before any other logic
|
||||
- For files: `defer f.Close()` right after `os.Open`/`os.Create`
|
||||
- The body must be fully read and closed even for non-2xx responses — otherwise the connection can't be reused
|
||||
- If you need the close error (e.g., for writes): use a named return and check in defer
|
||||
@@ -1,49 +0,0 @@
|
||||
---
|
||||
title: Propagate and Respect context.Context
|
||||
impact: MEDIUM
|
||||
impactDescription: enables cancellation, timeouts, and graceful shutdown across the call chain
|
||||
tags: go, context, cancellation, timeout, graceful-shutdown
|
||||
---
|
||||
|
||||
## Propagate and Respect context.Context
|
||||
|
||||
Pass `context.Context` as the first parameter to functions that do I/O, run goroutines, or may need cancellation. Never store contexts in structs — they represent a single request lifecycle.
|
||||
|
||||
**Incorrect (context ignored or stored):**
|
||||
|
||||
```go
|
||||
type Service struct {
|
||||
ctx context.Context // don't store context in structs
|
||||
}
|
||||
|
||||
func (s *Service) FetchData() ([]byte, error) {
|
||||
resp, err := http.Get(s.url) // no context — can't cancel
|
||||
return io.ReadAll(resp.Body)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (context flows through the call chain):**
|
||||
|
||||
```go
|
||||
func (s *Service) FetchData(ctx context.Context) ([]byte, error) {
|
||||
req, err := http.NewRequestWithContext(ctx, "GET", s.url, nil)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("create request: %w", err)
|
||||
}
|
||||
|
||||
resp, err := s.client.Do(req)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("fetch data: %w", err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
|
||||
return io.ReadAll(resp.Body)
|
||||
}
|
||||
```
|
||||
|
||||
**Key points:**
|
||||
- `context.Context` is always the first parameter, named `ctx`
|
||||
- Use `context.WithTimeout` or `context.WithCancel` to set deadlines
|
||||
- Check `ctx.Err()` or `select` on `ctx.Done()` in long-running loops
|
||||
- Use `http.NewRequestWithContext` instead of `http.Get`/`http.Post`
|
||||
- In tests, use `context.Background()` or `t.Context()` (Go 1.24+)
|
||||
@@ -1,54 +0,0 @@
|
||||
---
|
||||
title: Use defer for Cleanup
|
||||
impact: MEDIUM
|
||||
impactDescription: ensures resources are released on all exit paths including panics
|
||||
tags: go, defer, cleanup, resources
|
||||
---
|
||||
|
||||
## Use defer for Cleanup
|
||||
|
||||
Use `defer` to ensure cleanup code runs on all function exit paths, including early returns and panics. Place the defer immediately after acquiring the resource.
|
||||
|
||||
**Incorrect (cleanup missed on error paths):**
|
||||
|
||||
```go
|
||||
func processFile(path string) error {
|
||||
f, err := os.Open(path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
data, err := io.ReadAll(f)
|
||||
if err != nil {
|
||||
return err // f is never closed!
|
||||
}
|
||||
|
||||
f.Close()
|
||||
return process(data)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (defer immediately after acquire):**
|
||||
|
||||
```go
|
||||
func processFile(path string) error {
|
||||
f, err := os.Open(path)
|
||||
if err != nil {
|
||||
return fmt.Errorf("open %s: %w", path, err)
|
||||
}
|
||||
defer f.Close()
|
||||
|
||||
data, err := io.ReadAll(f)
|
||||
if err != nil {
|
||||
return fmt.Errorf("read %s: %w", path, err)
|
||||
}
|
||||
|
||||
return process(data)
|
||||
}
|
||||
```
|
||||
|
||||
**Key points:**
|
||||
- Place `defer` on the line immediately after the error check for the acquire operation
|
||||
- Defers execute in LIFO order — last deferred runs first
|
||||
- For mutexes: `mu.Lock()` then `defer mu.Unlock()` on the next line
|
||||
- For write operations where you need the close error, use a named return
|
||||
@@ -1,40 +0,0 @@
|
||||
---
|
||||
title: Use awserrors Package for AWS Error Responses
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: ensures consistent AWS-compatible error responses across all handlers
|
||||
tags: go, aws, errors, handlers, compatibility
|
||||
---
|
||||
|
||||
## Use awserrors Package for AWS Error Responses
|
||||
|
||||
When returning AWS-compatible error responses, use the `awserrors` package instead of manually constructing error strings or XML. This ensures consistent error formatting across all service handlers.
|
||||
|
||||
**Incorrect (manual error strings):**
|
||||
|
||||
```go
|
||||
func handleDescribeInstances(w http.ResponseWriter, r *http.Request) {
|
||||
// ...
|
||||
if err != nil {
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
fmt.Fprintf(w, `<Error><Code>InternalError</Code><Message>%s</Message></Error>`, err)
|
||||
return
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (awserrors package):**
|
||||
|
||||
```go
|
||||
func handleDescribeInstances(w http.ResponseWriter, r *http.Request) {
|
||||
// ...
|
||||
if err != nil {
|
||||
awserrors.WriteError(w, awserrors.InternalError(err.Error()))
|
||||
return
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Key points:**
|
||||
- The `awserrors` package handles XML formatting, status codes, and AWS error code conventions
|
||||
- Use predefined error constructors where available
|
||||
- This ensures AWS SDK clients can parse error responses correctly
|
||||
@@ -1,42 +0,0 @@
|
||||
---
|
||||
title: Use log/slog for Structured Logging
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: enables consistent, queryable structured logs across the codebase
|
||||
tags: go, logging, slog, structured
|
||||
---
|
||||
|
||||
## Use log/slog for Structured Logging
|
||||
|
||||
Use `log/slog` instead of `log` or `fmt.Println` for all logging. Use appropriate log levels and structured key-value pairs.
|
||||
|
||||
**Incorrect:**
|
||||
|
||||
```go
|
||||
import "log"
|
||||
|
||||
log.Printf("failed to connect to %s: %v", addr, err)
|
||||
fmt.Println("starting server on", addr)
|
||||
log.Fatal("database connection failed")
|
||||
```
|
||||
|
||||
**Correct:**
|
||||
|
||||
```go
|
||||
import "log/slog"
|
||||
|
||||
slog.Error("Failed to connect", "addr", addr, "error", err)
|
||||
slog.Info("Server starting", "addr", addr)
|
||||
slog.Debug("Cache hit", "key", key, "ttl", ttl)
|
||||
```
|
||||
|
||||
**Log levels:**
|
||||
- `slog.Debug` — verbose diagnostic info, disabled in production
|
||||
- `slog.Info` — normal operational events (startup, shutdown, config loaded)
|
||||
- `slog.Warn` — unexpected but recoverable situations
|
||||
- `slog.Error` — failures that need attention (lost connections, failed operations)
|
||||
|
||||
**Key points:**
|
||||
- Key-value pairs must alternate: key (string), value (any)
|
||||
- Keep keys lowercase, snake_case: `"node_id"`, `"bind_ip"`, `"error"`
|
||||
- Always include `"error", err` when logging an error
|
||||
- Don't log and return the same error — pick one (usually return)
|
||||
@@ -1,67 +0,0 @@
|
||||
---
|
||||
title: Use Table-Driven Tests
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: improves test coverage, readability, and makes it easy to add new cases
|
||||
tags: go, testing, table-driven, subtests
|
||||
---
|
||||
|
||||
## Use Table-Driven Tests
|
||||
|
||||
Use table-driven tests with `t.Run` subtests for functions with multiple input/output cases. This makes tests easy to extend and provides clear failure messages.
|
||||
|
||||
**Incorrect (repetitive test functions):**
|
||||
|
||||
```go
|
||||
func TestValidatePort_Valid(t *testing.T) {
|
||||
if err := ValidatePort(8080); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidatePort_Zero(t *testing.T) {
|
||||
if err := ValidatePort(0); err == nil {
|
||||
t.Fatal("expected error for port 0")
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidatePort_Negative(t *testing.T) {
|
||||
if err := ValidatePort(-1); err == nil {
|
||||
t.Fatal("expected error for port -1")
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (table-driven):**
|
||||
|
||||
```go
|
||||
func TestValidatePort(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
port int
|
||||
wantErr bool
|
||||
}{
|
||||
{"valid port", 8080, false},
|
||||
{"min valid", 1, false},
|
||||
{"max valid", 65535, false},
|
||||
{"zero", 0, true},
|
||||
{"negative", -1, true},
|
||||
{"too high", 65536, true},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
err := ValidatePort(tt.port)
|
||||
if (err != nil) != tt.wantErr {
|
||||
t.Errorf("ValidatePort(%d) error = %v, wantErr %v", tt.port, err, tt.wantErr)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Key points:**
|
||||
- Name each test case descriptively — it appears in test output on failure
|
||||
- Use `t.Run` for subtests so individual cases can be run with `-run`
|
||||
- Keep test struct fields minimal — only what varies between cases
|
||||
- For complex expected outputs, add a `want` field with the expected value
|
||||
- Use `t.Parallel()` in subtests when cases are independent
|
||||
@@ -1,99 +0,0 @@
|
||||
---
|
||||
name: react-best-practices
|
||||
description: React performance optimization guidelines. This skill should be used when writing, reviewing, or refactoring React code to ensure optimal performance patterns. Triggers on tasks involving React components, data fetching, bundle optimization, or performance improvements.
|
||||
---
|
||||
|
||||
# React Best Practices
|
||||
|
||||
Comprehensive performance optimization guide for React applications. Contains 36 rules across 6 categories, prioritized by impact to guide automated refactoring and code generation.
|
||||
|
||||
## When to Apply
|
||||
|
||||
Reference these guidelines when:
|
||||
- Writing new React components
|
||||
- Implementing data fetching
|
||||
- Reviewing code for performance issues
|
||||
- Refactoring existing React code
|
||||
- Optimizing bundle size or load times
|
||||
|
||||
## Rule Categories by Priority
|
||||
|
||||
| Priority | Category | Impact | Prefix |
|
||||
|----------|----------|--------|--------|
|
||||
| 1 | Eliminating Waterfalls | CRITICAL | `async-` |
|
||||
| 2 | Client-Side Data Fetching | MEDIUM-HIGH | `client-` |
|
||||
| 3 | Re-render Optimization | MEDIUM | `rerender-` |
|
||||
| 4 | Rendering Performance | MEDIUM | `rendering-` |
|
||||
| 5 | JavaScript Performance | LOW-MEDIUM | `js-` |
|
||||
| 6 | Advanced Patterns | LOW | `advanced-` |
|
||||
|
||||
## Quick Reference
|
||||
|
||||
### 1. Eliminating Waterfalls (CRITICAL)
|
||||
|
||||
- `async-defer-await` - Move await into branches where actually used
|
||||
- `async-parallel` - Use Promise.all() for independent operations
|
||||
- `async-dependencies` - Use better-all for partial dependencies
|
||||
|
||||
### 2. Client-Side Data Fetching (MEDIUM-HIGH)
|
||||
|
||||
- `client-passive-event-listeners` - Use passive listeners for scroll
|
||||
- `client-localstorage-schema` - Version and minimize localStorage data
|
||||
|
||||
### 3. Re-render Optimization (MEDIUM)
|
||||
|
||||
- `rerender-defer-reads` - Don't subscribe to state only used in callbacks
|
||||
- `rerender-dependencies` - Use primitive dependencies in effects
|
||||
- `rerender-derived-state` - Subscribe to derived booleans, not raw values
|
||||
- `rerender-derived-state-no-effect` - Derive state during render, not effects
|
||||
- `rerender-functional-setstate` - Use functional setState for stable callbacks
|
||||
- `rerender-lazy-state-init` - Pass function to useState for expensive values
|
||||
- `rerender-move-effect-to-event` - Put interaction logic in event handlers
|
||||
- `rerender-transitions` - Use startTransition for non-urgent updates
|
||||
- `rerender-use-ref-transient-values` - Use refs for transient frequent values
|
||||
|
||||
### 4. Rendering Performance (MEDIUM)
|
||||
|
||||
- `rendering-animate-svg-wrapper` - Animate div wrapper, not SVG element
|
||||
- `rendering-content-visibility` - Use content-visibility for long lists
|
||||
- `rendering-hoist-jsx` - Extract static JSX outside components
|
||||
- `rendering-svg-precision` - Reduce SVG coordinate precision
|
||||
- `rendering-activity` - Use Activity component for show/hide
|
||||
- `rendering-conditional-render` - Use ternary, not && for conditionals
|
||||
- `rendering-usetransition-loading` - Prefer useTransition for loading state
|
||||
|
||||
### 5. JavaScript Performance (LOW-MEDIUM)
|
||||
|
||||
- `js-batch-dom-css` - Group CSS changes via classes or cssText
|
||||
- `js-index-maps` - Build Map for repeated lookups
|
||||
- `js-cache-property-access` - Cache object properties in loops
|
||||
- `js-cache-function-results` - Cache function results in module-level Map
|
||||
- `js-cache-storage` - Cache localStorage/sessionStorage reads
|
||||
- `js-combine-iterations` - Combine multiple filter/map into one loop
|
||||
- `js-length-check-first` - Check array length before expensive comparison
|
||||
- `js-early-exit` - Return early from functions
|
||||
- `js-hoist-regexp` - Hoist RegExp creation outside loops
|
||||
- `js-min-max-loop` - Use loop for min/max instead of sort
|
||||
- `js-set-map-lookups` - Use Set/Map for O(1) lookups
|
||||
- `js-tosorted-immutable` - Use toSorted() for immutability
|
||||
|
||||
### 6. Advanced Patterns (LOW)
|
||||
|
||||
- `advanced-event-handler-refs` - Store event handlers in refs
|
||||
- `advanced-init-once` - Initialize app once per app load
|
||||
- `advanced-use-latest` - useLatest for stable callback refs
|
||||
|
||||
## How to Use
|
||||
|
||||
Read individual rule files for detailed explanations and code examples:
|
||||
|
||||
```
|
||||
rules/async-parallel.md
|
||||
rules/bundle-barrel-imports.md
|
||||
```
|
||||
|
||||
Each rule file contains:
|
||||
- Brief explanation of why it matters
|
||||
- Incorrect code example with explanation
|
||||
- Correct code example with explanation
|
||||
- Additional context and references
|
||||
@@ -1,55 +0,0 @@
|
||||
---
|
||||
title: Store Event Handlers in Refs
|
||||
impact: LOW
|
||||
impactDescription: stable subscriptions
|
||||
tags: advanced, hooks, refs, event-handlers, optimization
|
||||
---
|
||||
|
||||
## Store Event Handlers in Refs
|
||||
|
||||
Store callbacks in refs when used in effects that shouldn't re-subscribe on callback changes.
|
||||
|
||||
**Incorrect (re-subscribes on every render):**
|
||||
|
||||
```tsx
|
||||
function useWindowEvent(event: string, handler: (e) => void) {
|
||||
useEffect(() => {
|
||||
window.addEventListener(event, handler)
|
||||
return () => window.removeEventListener(event, handler)
|
||||
}, [event, handler])
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (stable subscription):**
|
||||
|
||||
```tsx
|
||||
function useWindowEvent(event: string, handler: (e) => void) {
|
||||
const handlerRef = useRef(handler)
|
||||
useEffect(() => {
|
||||
handlerRef.current = handler
|
||||
}, [handler])
|
||||
|
||||
useEffect(() => {
|
||||
const listener = (e) => handlerRef.current(e)
|
||||
window.addEventListener(event, listener)
|
||||
return () => window.removeEventListener(event, listener)
|
||||
}, [event])
|
||||
}
|
||||
```
|
||||
|
||||
**Alternative: use `useEffectEvent` if you're on latest React:**
|
||||
|
||||
```tsx
|
||||
import { useEffectEvent } from 'react'
|
||||
|
||||
function useWindowEvent(event: string, handler: (e) => void) {
|
||||
const onEvent = useEffectEvent(handler)
|
||||
|
||||
useEffect(() => {
|
||||
window.addEventListener(event, onEvent)
|
||||
return () => window.removeEventListener(event, onEvent)
|
||||
}, [event])
|
||||
}
|
||||
```
|
||||
|
||||
`useEffectEvent` provides a cleaner API for the same pattern: it creates a stable function reference that always calls the latest version of the handler.
|
||||
@@ -1,42 +0,0 @@
|
||||
---
|
||||
title: Initialize App Once, Not Per Mount
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: avoids duplicate init in development
|
||||
tags: initialization, useEffect, app-startup, side-effects
|
||||
---
|
||||
|
||||
## Initialize App Once, Not Per Mount
|
||||
|
||||
Do not put app-wide initialization that must run once per app load inside `useEffect([])` of a component. Components can remount and effects will re-run. Use a module-level guard or top-level init in the entry module instead.
|
||||
|
||||
**Incorrect (runs twice in dev, re-runs on remount):**
|
||||
|
||||
```tsx
|
||||
function Comp() {
|
||||
useEffect(() => {
|
||||
loadFromStorage()
|
||||
checkAuthToken()
|
||||
}, [])
|
||||
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (once per app load):**
|
||||
|
||||
```tsx
|
||||
let didInit = false
|
||||
|
||||
function Comp() {
|
||||
useEffect(() => {
|
||||
if (didInit) return
|
||||
didInit = true
|
||||
loadFromStorage()
|
||||
checkAuthToken()
|
||||
}, [])
|
||||
|
||||
// ...
|
||||
}
|
||||
```
|
||||
|
||||
Reference: [Initializing the application](https://react.dev/learn/you-might-not-need-an-effect#initializing-the-application)
|
||||
@@ -1,39 +0,0 @@
|
||||
---
|
||||
title: useEffectEvent for Stable Callback Refs
|
||||
impact: LOW
|
||||
impactDescription: prevents effect re-runs
|
||||
tags: advanced, hooks, useEffectEvent, refs, optimization
|
||||
---
|
||||
|
||||
## useEffectEvent for Stable Callback Refs
|
||||
|
||||
Access latest values in callbacks without adding them to dependency arrays. Prevents effect re-runs while avoiding stale closures.
|
||||
|
||||
**Incorrect (effect re-runs on every callback change):**
|
||||
|
||||
```tsx
|
||||
function SearchInput({ onSearch }: { onSearch: (q: string) => void }) {
|
||||
const [query, setQuery] = useState('')
|
||||
|
||||
useEffect(() => {
|
||||
const timeout = setTimeout(() => onSearch(query), 300)
|
||||
return () => clearTimeout(timeout)
|
||||
}, [query, onSearch])
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (using React's useEffectEvent):**
|
||||
|
||||
```tsx
|
||||
import { useEffectEvent } from 'react';
|
||||
|
||||
function SearchInput({ onSearch }: { onSearch: (q: string) => void }) {
|
||||
const [query, setQuery] = useState('')
|
||||
const onSearchEvent = useEffectEvent(onSearch)
|
||||
|
||||
useEffect(() => {
|
||||
const timeout = setTimeout(() => onSearchEvent(query), 300)
|
||||
return () => clearTimeout(timeout)
|
||||
}, [query])
|
||||
}
|
||||
```
|
||||
@@ -1,80 +0,0 @@
|
||||
---
|
||||
title: Defer Await Until Needed
|
||||
impact: HIGH
|
||||
impactDescription: avoids blocking unused code paths
|
||||
tags: async, await, conditional, optimization
|
||||
---
|
||||
|
||||
## Defer Await Until Needed
|
||||
|
||||
Move `await` operations into the branches where they're actually used to avoid blocking code paths that don't need them.
|
||||
|
||||
**Incorrect (blocks both branches):**
|
||||
|
||||
```typescript
|
||||
async function handleRequest(userId: string, skipProcessing: boolean) {
|
||||
const userData = await fetchUserData(userId)
|
||||
|
||||
if (skipProcessing) {
|
||||
// Returns immediately but still waited for userData
|
||||
return { skipped: true }
|
||||
}
|
||||
|
||||
// Only this branch uses userData
|
||||
return processUserData(userData)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (only blocks when needed):**
|
||||
|
||||
```typescript
|
||||
async function handleRequest(userId: string, skipProcessing: boolean) {
|
||||
if (skipProcessing) {
|
||||
// Returns immediately without waiting
|
||||
return { skipped: true }
|
||||
}
|
||||
|
||||
// Fetch only when needed
|
||||
const userData = await fetchUserData(userId)
|
||||
return processUserData(userData)
|
||||
}
|
||||
```
|
||||
|
||||
**Another example (early return optimization):**
|
||||
|
||||
```typescript
|
||||
// Incorrect: always fetches permissions
|
||||
async function updateResource(resourceId: string, userId: string) {
|
||||
const permissions = await fetchPermissions(userId)
|
||||
const resource = await getResource(resourceId)
|
||||
|
||||
if (!resource) {
|
||||
return { error: 'Not found' }
|
||||
}
|
||||
|
||||
if (!permissions.canEdit) {
|
||||
return { error: 'Forbidden' }
|
||||
}
|
||||
|
||||
return await updateResourceData(resource, permissions)
|
||||
}
|
||||
|
||||
// Correct: fetches only when needed
|
||||
async function updateResource(resourceId: string, userId: string) {
|
||||
const resource = await getResource(resourceId)
|
||||
|
||||
if (!resource) {
|
||||
return { error: 'Not found' }
|
||||
}
|
||||
|
||||
const permissions = await fetchPermissions(userId)
|
||||
|
||||
if (!permissions.canEdit) {
|
||||
return { error: 'Forbidden' }
|
||||
}
|
||||
|
||||
return await updateResourceData(resource, permissions)
|
||||
}
|
||||
```
|
||||
|
||||
This optimization is especially valuable when the skipped branch is frequently taken, or when the deferred operation is expensive.
|
||||
@@ -1,51 +0,0 @@
|
||||
---
|
||||
title: Dependency-Based Parallelization
|
||||
impact: CRITICAL
|
||||
impactDescription: 2-10× improvement
|
||||
tags: async, parallelization, dependencies, better-all
|
||||
---
|
||||
|
||||
## Dependency-Based Parallelization
|
||||
|
||||
For operations with partial dependencies, use `better-all` to maximize parallelism. It automatically starts each task at the earliest possible moment.
|
||||
|
||||
**Incorrect (profile waits for config unnecessarily):**
|
||||
|
||||
```typescript
|
||||
const [user, config] = await Promise.all([
|
||||
fetchUser(),
|
||||
fetchConfig()
|
||||
])
|
||||
const profile = await fetchProfile(user.id)
|
||||
```
|
||||
|
||||
**Correct (config and profile run in parallel):**
|
||||
|
||||
```typescript
|
||||
import { all } from 'better-all'
|
||||
|
||||
const { user, config, profile } = await all({
|
||||
async user() { return fetchUser() },
|
||||
async config() { return fetchConfig() },
|
||||
async profile() {
|
||||
return fetchProfile((await this.$.user).id)
|
||||
}
|
||||
})
|
||||
```
|
||||
|
||||
**Alternative without extra dependencies:**
|
||||
|
||||
We can also create all the promises first, and do `Promise.all()` at the end.
|
||||
|
||||
```typescript
|
||||
const userPromise = fetchUser()
|
||||
const profilePromise = userPromise.then(user => fetchProfile(user.id))
|
||||
|
||||
const [user, config, profile] = await Promise.all([
|
||||
userPromise,
|
||||
fetchConfig(),
|
||||
profilePromise
|
||||
])
|
||||
```
|
||||
|
||||
Reference: [https://github.com/shuding/better-all](https://github.com/shuding/better-all)
|
||||
@@ -1,28 +0,0 @@
|
||||
---
|
||||
title: Promise.all() for Independent Operations
|
||||
impact: CRITICAL
|
||||
impactDescription: 2-10× improvement
|
||||
tags: async, parallelization, promises, waterfalls
|
||||
---
|
||||
|
||||
## Promise.all() for Independent Operations
|
||||
|
||||
When async operations have no interdependencies, execute them concurrently using `Promise.all()`.
|
||||
|
||||
**Incorrect (sequential execution, 3 round trips):**
|
||||
|
||||
```typescript
|
||||
const user = await fetchUser()
|
||||
const posts = await fetchPosts()
|
||||
const comments = await fetchComments()
|
||||
```
|
||||
|
||||
**Correct (parallel execution, 1 round trip):**
|
||||
|
||||
```typescript
|
||||
const [user, posts, comments] = await Promise.all([
|
||||
fetchUser(),
|
||||
fetchPosts(),
|
||||
fetchComments()
|
||||
])
|
||||
```
|
||||
@@ -1,71 +0,0 @@
|
||||
---
|
||||
title: Version and Minimize localStorage Data
|
||||
impact: MEDIUM
|
||||
impactDescription: prevents schema conflicts, reduces storage size
|
||||
tags: client, localStorage, storage, versioning, data-minimization
|
||||
---
|
||||
|
||||
## Version and Minimize localStorage Data
|
||||
|
||||
Add version prefix to keys and store only needed fields. Prevents schema conflicts and accidental storage of sensitive data.
|
||||
|
||||
**Incorrect:**
|
||||
|
||||
```typescript
|
||||
// No version, stores everything, no error handling
|
||||
localStorage.setItem('userConfig', JSON.stringify(fullUserObject))
|
||||
const data = localStorage.getItem('userConfig')
|
||||
```
|
||||
|
||||
**Correct:**
|
||||
|
||||
```typescript
|
||||
const VERSION = 'v2'
|
||||
|
||||
function saveConfig(config: { theme: string; language: string }) {
|
||||
try {
|
||||
localStorage.setItem(`userConfig:${VERSION}`, JSON.stringify(config))
|
||||
} catch {
|
||||
// Throws in incognito/private browsing, quota exceeded, or disabled
|
||||
}
|
||||
}
|
||||
|
||||
function loadConfig() {
|
||||
try {
|
||||
const data = localStorage.getItem(`userConfig:${VERSION}`)
|
||||
return data ? JSON.parse(data) : null
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
}
|
||||
|
||||
// Migration from v1 to v2
|
||||
function migrate() {
|
||||
try {
|
||||
const v1 = localStorage.getItem('userConfig:v1')
|
||||
if (v1) {
|
||||
const old = JSON.parse(v1)
|
||||
saveConfig({ theme: old.darkMode ? 'dark' : 'light', language: old.lang })
|
||||
localStorage.removeItem('userConfig:v1')
|
||||
}
|
||||
} catch {}
|
||||
}
|
||||
```
|
||||
|
||||
**Store minimal fields from server responses:**
|
||||
|
||||
```typescript
|
||||
// User object has 20+ fields, only store what UI needs
|
||||
function cachePrefs(user: FullUser) {
|
||||
try {
|
||||
localStorage.setItem('prefs:v1', JSON.stringify({
|
||||
theme: user.preferences.theme,
|
||||
notifications: user.preferences.notifications
|
||||
}))
|
||||
} catch {}
|
||||
}
|
||||
```
|
||||
|
||||
**Always wrap in try-catch:** `getItem()` and `setItem()` throw in incognito/private browsing (Safari, Firefox), when quota exceeded, or when disabled.
|
||||
|
||||
**Benefits:** Schema evolution via versioning, reduced storage size, prevents storing tokens/PII/internal flags.
|
||||
@@ -1,48 +0,0 @@
|
||||
---
|
||||
title: Use Passive Event Listeners for Scrolling Performance
|
||||
impact: MEDIUM
|
||||
impactDescription: eliminates scroll delay caused by event listeners
|
||||
tags: client, event-listeners, scrolling, performance, touch, wheel
|
||||
---
|
||||
|
||||
## Use Passive Event Listeners for Scrolling Performance
|
||||
|
||||
Add `{ passive: true }` to touch and wheel event listeners to enable immediate scrolling. Browsers normally wait for listeners to finish to check if `preventDefault()` is called, causing scroll delay.
|
||||
|
||||
**Incorrect:**
|
||||
|
||||
```typescript
|
||||
useEffect(() => {
|
||||
const handleTouch = (e: TouchEvent) => console.log(e.touches[0].clientX)
|
||||
const handleWheel = (e: WheelEvent) => console.log(e.deltaY)
|
||||
|
||||
document.addEventListener('touchstart', handleTouch)
|
||||
document.addEventListener('wheel', handleWheel)
|
||||
|
||||
return () => {
|
||||
document.removeEventListener('touchstart', handleTouch)
|
||||
document.removeEventListener('wheel', handleWheel)
|
||||
}
|
||||
}, [])
|
||||
```
|
||||
|
||||
**Correct:**
|
||||
|
||||
```typescript
|
||||
useEffect(() => {
|
||||
const handleTouch = (e: TouchEvent) => console.log(e.touches[0].clientX)
|
||||
const handleWheel = (e: WheelEvent) => console.log(e.deltaY)
|
||||
|
||||
document.addEventListener('touchstart', handleTouch, { passive: true })
|
||||
document.addEventListener('wheel', handleWheel, { passive: true })
|
||||
|
||||
return () => {
|
||||
document.removeEventListener('touchstart', handleTouch)
|
||||
document.removeEventListener('wheel', handleWheel)
|
||||
}
|
||||
}, [])
|
||||
```
|
||||
|
||||
**Use passive when:** tracking/analytics, logging, any listener that doesn't call `preventDefault()`.
|
||||
|
||||
**Don't use passive when:** implementing custom swipe gestures, custom zoom controls, or any listener that needs `preventDefault()`.
|
||||
@@ -1,107 +0,0 @@
|
||||
---
|
||||
title: Avoid Layout Thrashing
|
||||
impact: MEDIUM
|
||||
impactDescription: prevents forced synchronous layouts and reduces performance bottlenecks
|
||||
tags: javascript, dom, css, performance, reflow, layout-thrashing
|
||||
---
|
||||
|
||||
## Avoid Layout Thrashing
|
||||
|
||||
Avoid interleaving style writes with layout reads. When you read a layout property (like `offsetWidth`, `getBoundingClientRect()`, or `getComputedStyle()`) between style changes, the browser is forced to trigger a synchronous reflow.
|
||||
|
||||
**This is OK (browser batches style changes):**
|
||||
```typescript
|
||||
function updateElementStyles(element: HTMLElement) {
|
||||
// Each line invalidates style, but browser batches the recalculation
|
||||
element.style.width = '100px'
|
||||
element.style.height = '200px'
|
||||
element.style.backgroundColor = 'blue'
|
||||
element.style.border = '1px solid black'
|
||||
}
|
||||
```
|
||||
|
||||
**Incorrect (interleaved reads and writes force reflows):**
|
||||
```typescript
|
||||
function layoutThrashing(element: HTMLElement) {
|
||||
element.style.width = '100px'
|
||||
const width = element.offsetWidth // Forces reflow
|
||||
element.style.height = '200px'
|
||||
const height = element.offsetHeight // Forces another reflow
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (batch writes, then read once):**
|
||||
```typescript
|
||||
function updateElementStyles(element: HTMLElement) {
|
||||
// Batch all writes together
|
||||
element.style.width = '100px'
|
||||
element.style.height = '200px'
|
||||
element.style.backgroundColor = 'blue'
|
||||
element.style.border = '1px solid black'
|
||||
|
||||
// Read after all writes are done (single reflow)
|
||||
const { width, height } = element.getBoundingClientRect()
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (batch reads, then writes):**
|
||||
```typescript
|
||||
function avoidThrashing(element: HTMLElement) {
|
||||
// Read phase - all layout queries first
|
||||
const rect1 = element.getBoundingClientRect()
|
||||
const offsetWidth = element.offsetWidth
|
||||
const offsetHeight = element.offsetHeight
|
||||
|
||||
// Write phase - all style changes after
|
||||
element.style.width = '100px'
|
||||
element.style.height = '200px'
|
||||
}
|
||||
```
|
||||
|
||||
**Better: use CSS classes**
|
||||
```css
|
||||
.highlighted-box {
|
||||
width: 100px;
|
||||
height: 200px;
|
||||
background-color: blue;
|
||||
border: 1px solid black;
|
||||
}
|
||||
```
|
||||
```typescript
|
||||
function updateElementStyles(element: HTMLElement) {
|
||||
element.classList.add('highlighted-box')
|
||||
|
||||
const { width, height } = element.getBoundingClientRect()
|
||||
}
|
||||
```
|
||||
|
||||
**React example:**
|
||||
```tsx
|
||||
// Incorrect: interleaving style changes with layout queries
|
||||
function Box({ isHighlighted }: { isHighlighted: boolean }) {
|
||||
const ref = useRef<HTMLDivElement>(null)
|
||||
|
||||
useEffect(() => {
|
||||
if (ref.current && isHighlighted) {
|
||||
ref.current.style.width = '100px'
|
||||
const width = ref.current.offsetWidth // Forces layout
|
||||
ref.current.style.height = '200px'
|
||||
}
|
||||
}, [isHighlighted])
|
||||
|
||||
return <div ref={ref}>Content</div>
|
||||
}
|
||||
|
||||
// Correct: toggle class
|
||||
function Box({ isHighlighted }: { isHighlighted: boolean }) {
|
||||
return (
|
||||
<div className={isHighlighted ? 'highlighted-box' : ''}>
|
||||
Content
|
||||
</div>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
Prefer CSS classes over inline styles when possible. CSS files are cached by the browser, and classes provide better separation of concerns and are easier to maintain.
|
||||
|
||||
See [this gist](https://gist.github.com/paulirish/5d52fb081b3570c81e3a) and [CSS Triggers](https://csstriggers.com/) for more information on layout-forcing operations.
|
||||
@@ -1,80 +0,0 @@
|
||||
---
|
||||
title: Cache Repeated Function Calls
|
||||
impact: MEDIUM
|
||||
impactDescription: avoid redundant computation
|
||||
tags: javascript, cache, memoization, performance
|
||||
---
|
||||
|
||||
## Cache Repeated Function Calls
|
||||
|
||||
Use a module-level Map to cache function results when the same function is called repeatedly with the same inputs during render.
|
||||
|
||||
**Incorrect (redundant computation):**
|
||||
|
||||
```typescript
|
||||
function ProjectList({ projects }: { projects: Project[] }) {
|
||||
return (
|
||||
<div>
|
||||
{projects.map(project => {
|
||||
// slugify() called 100+ times for same project names
|
||||
const slug = slugify(project.name)
|
||||
|
||||
return <ProjectCard key={project.id} slug={slug} />
|
||||
})}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (cached results):**
|
||||
|
||||
```typescript
|
||||
// Module-level cache
|
||||
const slugifyCache = new Map<string, string>()
|
||||
|
||||
function cachedSlugify(text: string): string {
|
||||
if (slugifyCache.has(text)) {
|
||||
return slugifyCache.get(text)!
|
||||
}
|
||||
const result = slugify(text)
|
||||
slugifyCache.set(text, result)
|
||||
return result
|
||||
}
|
||||
|
||||
function ProjectList({ projects }: { projects: Project[] }) {
|
||||
return (
|
||||
<div>
|
||||
{projects.map(project => {
|
||||
// Computed only once per unique project name
|
||||
const slug = cachedSlugify(project.name)
|
||||
|
||||
return <ProjectCard key={project.id} slug={slug} />
|
||||
})}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Simpler pattern for single-value functions:**
|
||||
|
||||
```typescript
|
||||
let isLoggedInCache: boolean | null = null
|
||||
|
||||
function isLoggedIn(): boolean {
|
||||
if (isLoggedInCache !== null) {
|
||||
return isLoggedInCache
|
||||
}
|
||||
|
||||
isLoggedInCache = document.cookie.includes('auth=')
|
||||
return isLoggedInCache
|
||||
}
|
||||
|
||||
// Clear cache when auth changes
|
||||
function onAuthChange() {
|
||||
isLoggedInCache = null
|
||||
}
|
||||
```
|
||||
|
||||
Use a Map (not a hook) so it works everywhere: utilities, event handlers, not just React components.
|
||||
|
||||
Reference: [How we made the Vercel Dashboard twice as fast](https://vercel.com/blog/how-we-made-the-vercel-dashboard-twice-as-fast)
|
||||
@@ -1,28 +0,0 @@
|
||||
---
|
||||
title: Cache Property Access in Loops
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: reduces lookups
|
||||
tags: javascript, loops, optimization, caching
|
||||
---
|
||||
|
||||
## Cache Property Access in Loops
|
||||
|
||||
Cache object property lookups in hot paths.
|
||||
|
||||
**Incorrect (3 lookups × N iterations):**
|
||||
|
||||
```typescript
|
||||
for (let i = 0; i < arr.length; i++) {
|
||||
process(obj.config.settings.value)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (1 lookup total):**
|
||||
|
||||
```typescript
|
||||
const value = obj.config.settings.value
|
||||
const len = arr.length
|
||||
for (let i = 0; i < len; i++) {
|
||||
process(value)
|
||||
}
|
||||
```
|
||||
@@ -1,70 +0,0 @@
|
||||
---
|
||||
title: Cache Storage API Calls
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: reduces expensive I/O
|
||||
tags: javascript, localStorage, storage, caching, performance
|
||||
---
|
||||
|
||||
## Cache Storage API Calls
|
||||
|
||||
`localStorage`, `sessionStorage`, and `document.cookie` are synchronous and expensive. Cache reads in memory.
|
||||
|
||||
**Incorrect (reads storage on every call):**
|
||||
|
||||
```typescript
|
||||
function getTheme() {
|
||||
return localStorage.getItem('theme') ?? 'light'
|
||||
}
|
||||
// Called 10 times = 10 storage reads
|
||||
```
|
||||
|
||||
**Correct (Map cache):**
|
||||
|
||||
```typescript
|
||||
const storageCache = new Map<string, string | null>()
|
||||
|
||||
function getLocalStorage(key: string) {
|
||||
if (!storageCache.has(key)) {
|
||||
storageCache.set(key, localStorage.getItem(key))
|
||||
}
|
||||
return storageCache.get(key)
|
||||
}
|
||||
|
||||
function setLocalStorage(key: string, value: string) {
|
||||
localStorage.setItem(key, value)
|
||||
storageCache.set(key, value) // keep cache in sync
|
||||
}
|
||||
```
|
||||
|
||||
Use a Map (not a hook) so it works everywhere: utilities, event handlers, not just React components.
|
||||
|
||||
**Cookie caching:**
|
||||
|
||||
```typescript
|
||||
let cookieCache: Record<string, string> | null = null
|
||||
|
||||
function getCookie(name: string) {
|
||||
if (!cookieCache) {
|
||||
cookieCache = Object.fromEntries(
|
||||
document.cookie.split('; ').map(c => c.split('='))
|
||||
)
|
||||
}
|
||||
return cookieCache[name]
|
||||
}
|
||||
```
|
||||
|
||||
**Important (invalidate on external changes):**
|
||||
|
||||
If storage can change externally (another tab, server-set cookies), invalidate cache:
|
||||
|
||||
```typescript
|
||||
window.addEventListener('storage', (e) => {
|
||||
if (e.key) storageCache.delete(e.key)
|
||||
})
|
||||
|
||||
document.addEventListener('visibilitychange', () => {
|
||||
if (document.visibilityState === 'visible') {
|
||||
storageCache.clear()
|
||||
}
|
||||
})
|
||||
```
|
||||
@@ -1,32 +0,0 @@
|
||||
---
|
||||
title: Combine Multiple Array Iterations
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: reduces iterations
|
||||
tags: javascript, arrays, loops, performance
|
||||
---
|
||||
|
||||
## Combine Multiple Array Iterations
|
||||
|
||||
Multiple `.filter()` or `.map()` calls iterate the array multiple times. Combine into one loop.
|
||||
|
||||
**Incorrect (3 iterations):**
|
||||
|
||||
```typescript
|
||||
const admins = users.filter(u => u.isAdmin)
|
||||
const testers = users.filter(u => u.isTester)
|
||||
const inactive = users.filter(u => !u.isActive)
|
||||
```
|
||||
|
||||
**Correct (1 iteration):**
|
||||
|
||||
```typescript
|
||||
const admins: User[] = []
|
||||
const testers: User[] = []
|
||||
const inactive: User[] = []
|
||||
|
||||
for (const user of users) {
|
||||
if (user.isAdmin) admins.push(user)
|
||||
if (user.isTester) testers.push(user)
|
||||
if (!user.isActive) inactive.push(user)
|
||||
}
|
||||
```
|
||||
@@ -1,50 +0,0 @@
|
||||
---
|
||||
title: Early Return from Functions
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: avoids unnecessary computation
|
||||
tags: javascript, functions, optimization, early-return
|
||||
---
|
||||
|
||||
## Early Return from Functions
|
||||
|
||||
Return early when result is determined to skip unnecessary processing.
|
||||
|
||||
**Incorrect (processes all items even after finding answer):**
|
||||
|
||||
```typescript
|
||||
function validateUsers(users: User[]) {
|
||||
let hasError = false
|
||||
let errorMessage = ''
|
||||
|
||||
for (const user of users) {
|
||||
if (!user.email) {
|
||||
hasError = true
|
||||
errorMessage = 'Email required'
|
||||
}
|
||||
if (!user.name) {
|
||||
hasError = true
|
||||
errorMessage = 'Name required'
|
||||
}
|
||||
// Continues checking all users even after error found
|
||||
}
|
||||
|
||||
return hasError ? { valid: false, error: errorMessage } : { valid: true }
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (returns immediately on first error):**
|
||||
|
||||
```typescript
|
||||
function validateUsers(users: User[]) {
|
||||
for (const user of users) {
|
||||
if (!user.email) {
|
||||
return { valid: false, error: 'Email required' }
|
||||
}
|
||||
if (!user.name) {
|
||||
return { valid: false, error: 'Name required' }
|
||||
}
|
||||
}
|
||||
|
||||
return { valid: true }
|
||||
}
|
||||
```
|
||||
@@ -1,45 +0,0 @@
|
||||
---
|
||||
title: Hoist RegExp Creation
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: avoids recreation
|
||||
tags: javascript, regexp, optimization, memoization
|
||||
---
|
||||
|
||||
## Hoist RegExp Creation
|
||||
|
||||
Don't create RegExp inside render. Hoist to module scope or memoize with `useMemo()`.
|
||||
|
||||
**Incorrect (new RegExp every render):**
|
||||
|
||||
```tsx
|
||||
function Highlighter({ text, query }: Props) {
|
||||
const regex = new RegExp(`(${query})`, 'gi')
|
||||
const parts = text.split(regex)
|
||||
return <>{parts.map((part, i) => ...)}</>
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (memoize or hoist):**
|
||||
|
||||
```tsx
|
||||
const EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[^\s@]+$/
|
||||
|
||||
function Highlighter({ text, query }: Props) {
|
||||
const regex = useMemo(
|
||||
() => new RegExp(`(${escapeRegex(query)})`, 'gi'),
|
||||
[query]
|
||||
)
|
||||
const parts = text.split(regex)
|
||||
return <>{parts.map((part, i) => ...)}</>
|
||||
}
|
||||
```
|
||||
|
||||
**Warning (global regex has mutable state):**
|
||||
|
||||
Global regex (`/g`) has mutable `lastIndex` state:
|
||||
|
||||
```typescript
|
||||
const regex = /foo/g
|
||||
regex.test('foo') // true, lastIndex = 3
|
||||
regex.test('foo') // false, lastIndex = 0
|
||||
```
|
||||
@@ -1,37 +0,0 @@
|
||||
---
|
||||
title: Build Index Maps for Repeated Lookups
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: 1M ops to 2K ops
|
||||
tags: javascript, map, indexing, optimization, performance
|
||||
---
|
||||
|
||||
## Build Index Maps for Repeated Lookups
|
||||
|
||||
Multiple `.find()` calls by the same key should use a Map.
|
||||
|
||||
**Incorrect (O(n) per lookup):**
|
||||
|
||||
```typescript
|
||||
function processOrders(orders: Order[], users: User[]) {
|
||||
return orders.map(order => ({
|
||||
...order,
|
||||
user: users.find(u => u.id === order.userId)
|
||||
}))
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (O(1) per lookup):**
|
||||
|
||||
```typescript
|
||||
function processOrders(orders: Order[], users: User[]) {
|
||||
const userById = new Map(users.map(u => [u.id, u]))
|
||||
|
||||
return orders.map(order => ({
|
||||
...order,
|
||||
user: userById.get(order.userId)
|
||||
}))
|
||||
}
|
||||
```
|
||||
|
||||
Build map once (O(n)), then all lookups are O(1).
|
||||
For 1000 orders × 1000 users: 1M ops → 2K ops.
|
||||
@@ -1,49 +0,0 @@
|
||||
---
|
||||
title: Early Length Check for Array Comparisons
|
||||
impact: MEDIUM-HIGH
|
||||
impactDescription: avoids expensive operations when lengths differ
|
||||
tags: javascript, arrays, performance, optimization, comparison
|
||||
---
|
||||
|
||||
## Early Length Check for Array Comparisons
|
||||
|
||||
When comparing arrays with expensive operations (sorting, deep equality, serialization), check lengths first. If lengths differ, the arrays cannot be equal.
|
||||
|
||||
In real-world applications, this optimization is especially valuable when the comparison runs in hot paths (event handlers, render loops).
|
||||
|
||||
**Incorrect (always runs expensive comparison):**
|
||||
|
||||
```typescript
|
||||
function hasChanges(current: string[], original: string[]) {
|
||||
// Always sorts and joins, even when lengths differ
|
||||
return current.sort().join() !== original.sort().join()
|
||||
}
|
||||
```
|
||||
|
||||
Two O(n log n) sorts run even when `current.length` is 5 and `original.length` is 100. There is also overhead of joining the arrays and comparing the strings.
|
||||
|
||||
**Correct (O(1) length check first):**
|
||||
|
||||
```typescript
|
||||
function hasChanges(current: string[], original: string[]) {
|
||||
// Early return if lengths differ
|
||||
if (current.length !== original.length) {
|
||||
return true
|
||||
}
|
||||
// Only sort when lengths match
|
||||
const currentSorted = current.toSorted()
|
||||
const originalSorted = original.toSorted()
|
||||
for (let i = 0; i < currentSorted.length; i++) {
|
||||
if (currentSorted[i] !== originalSorted[i]) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
```
|
||||
|
||||
This new approach is more efficient because:
|
||||
- It avoids the overhead of sorting and joining the arrays when lengths differ
|
||||
- It avoids consuming memory for the joined strings (especially important for large arrays)
|
||||
- It avoids mutating the original arrays
|
||||
- It returns early when a difference is found
|
||||
@@ -1,82 +0,0 @@
|
||||
---
|
||||
title: Use Loop for Min/Max Instead of Sort
|
||||
impact: LOW
|
||||
impactDescription: O(n) instead of O(n log n)
|
||||
tags: javascript, arrays, performance, sorting, algorithms
|
||||
---
|
||||
|
||||
## Use Loop for Min/Max Instead of Sort
|
||||
|
||||
Finding the smallest or largest element only requires a single pass through the array. Sorting is wasteful and slower.
|
||||
|
||||
**Incorrect (O(n log n) - sort to find latest):**
|
||||
|
||||
```typescript
|
||||
interface Project {
|
||||
id: string
|
||||
name: string
|
||||
updatedAt: number
|
||||
}
|
||||
|
||||
function getLatestProject(projects: Project[]) {
|
||||
const sorted = [...projects].sort((a, b) => b.updatedAt - a.updatedAt)
|
||||
return sorted[0]
|
||||
}
|
||||
```
|
||||
|
||||
Sorts the entire array just to find the maximum value.
|
||||
|
||||
**Incorrect (O(n log n) - sort for oldest and newest):**
|
||||
|
||||
```typescript
|
||||
function getOldestAndNewest(projects: Project[]) {
|
||||
const sorted = [...projects].sort((a, b) => a.updatedAt - b.updatedAt)
|
||||
return { oldest: sorted[0], newest: sorted[sorted.length - 1] }
|
||||
}
|
||||
```
|
||||
|
||||
Still sorts unnecessarily when only min/max are needed.
|
||||
|
||||
**Correct (O(n) - single loop):**
|
||||
|
||||
```typescript
|
||||
function getLatestProject(projects: Project[]) {
|
||||
if (projects.length === 0) return null
|
||||
|
||||
let latest = projects[0]
|
||||
|
||||
for (let i = 1; i < projects.length; i++) {
|
||||
if (projects[i].updatedAt > latest.updatedAt) {
|
||||
latest = projects[i]
|
||||
}
|
||||
}
|
||||
|
||||
return latest
|
||||
}
|
||||
|
||||
function getOldestAndNewest(projects: Project[]) {
|
||||
if (projects.length === 0) return { oldest: null, newest: null }
|
||||
|
||||
let oldest = projects[0]
|
||||
let newest = projects[0]
|
||||
|
||||
for (let i = 1; i < projects.length; i++) {
|
||||
if (projects[i].updatedAt < oldest.updatedAt) oldest = projects[i]
|
||||
if (projects[i].updatedAt > newest.updatedAt) newest = projects[i]
|
||||
}
|
||||
|
||||
return { oldest, newest }
|
||||
}
|
||||
```
|
||||
|
||||
Single pass through the array, no copying, no sorting.
|
||||
|
||||
**Alternative (Math.min/Math.max for small arrays):**
|
||||
|
||||
```typescript
|
||||
const numbers = [5, 2, 8, 1, 9]
|
||||
const min = Math.min(...numbers)
|
||||
const max = Math.max(...numbers)
|
||||
```
|
||||
|
||||
This works for small arrays, but can be slower or just throw an error for very large arrays due to spread operator limitations. Maximal array length is approximately 124000 in Chrome 143 and 638000 in Safari 18; exact numbers may vary - see [the fiddle](https://jsfiddle.net/qw1jabsx/4/). Use the loop approach for reliability.
|
||||
@@ -1,24 +0,0 @@
|
||||
---
|
||||
title: Use Set/Map for O(1) Lookups
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: O(n) to O(1)
|
||||
tags: javascript, set, map, data-structures, performance
|
||||
---
|
||||
|
||||
## Use Set/Map for O(1) Lookups
|
||||
|
||||
Convert arrays to Set/Map for repeated membership checks.
|
||||
|
||||
**Incorrect (O(n) per check):**
|
||||
|
||||
```typescript
|
||||
const allowedIds = ['a', 'b', 'c', ...]
|
||||
items.filter(item => allowedIds.includes(item.id))
|
||||
```
|
||||
|
||||
**Correct (O(1) per check):**
|
||||
|
||||
```typescript
|
||||
const allowedIds = new Set(['a', 'b', 'c', ...])
|
||||
items.filter(item => allowedIds.has(item.id))
|
||||
```
|
||||
@@ -1,57 +0,0 @@
|
||||
---
|
||||
title: Use toSorted() Instead of sort() for Immutability
|
||||
impact: MEDIUM-HIGH
|
||||
impactDescription: prevents mutation bugs in React state
|
||||
tags: javascript, arrays, immutability, react, state, mutation
|
||||
---
|
||||
|
||||
## Use toSorted() Instead of sort() for Immutability
|
||||
|
||||
`.sort()` mutates the array in place, which can cause bugs with React state and props. Use `.toSorted()` to create a new sorted array without mutation.
|
||||
|
||||
**Incorrect (mutates original array):**
|
||||
|
||||
```typescript
|
||||
function UserList({ users }: { users: User[] }) {
|
||||
// Mutates the users prop array!
|
||||
const sorted = useMemo(
|
||||
() => users.sort((a, b) => a.name.localeCompare(b.name)),
|
||||
[users]
|
||||
)
|
||||
return <div>{sorted.map(renderUser)}</div>
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (creates new array):**
|
||||
|
||||
```typescript
|
||||
function UserList({ users }: { users: User[] }) {
|
||||
// Creates new sorted array, original unchanged
|
||||
const sorted = useMemo(
|
||||
() => users.toSorted((a, b) => a.name.localeCompare(b.name)),
|
||||
[users]
|
||||
)
|
||||
return <div>{sorted.map(renderUser)}</div>
|
||||
}
|
||||
```
|
||||
|
||||
**Why this matters in React:**
|
||||
|
||||
1. Props/state mutations break React's immutability model - React expects props and state to be treated as read-only
|
||||
2. Causes stale closure bugs - Mutating arrays inside closures (callbacks, effects) can lead to unexpected behavior
|
||||
|
||||
**Browser support (fallback for older browsers):**
|
||||
|
||||
`.toSorted()` is available in all modern browsers (Chrome 110+, Safari 16+, Firefox 115+, Node.js 20+). For older environments, use spread operator:
|
||||
|
||||
```typescript
|
||||
// Fallback for older browsers
|
||||
const sorted = [...items].sort((a, b) => a.value - b.value)
|
||||
```
|
||||
|
||||
**Other immutable array methods:**
|
||||
|
||||
- `.toSorted()` - immutable sort
|
||||
- `.toReversed()` - immutable reverse
|
||||
- `.toSpliced()` - immutable splice
|
||||
- `.with()` - immutable element replacement
|
||||
@@ -1,26 +0,0 @@
|
||||
---
|
||||
title: Use Activity Component for Show/Hide
|
||||
impact: MEDIUM
|
||||
impactDescription: preserves state/DOM
|
||||
tags: rendering, activity, visibility, state-preservation
|
||||
---
|
||||
|
||||
## Use Activity Component for Show/Hide
|
||||
|
||||
Use React's `<Activity>` to preserve state/DOM for expensive components that frequently toggle visibility.
|
||||
|
||||
**Usage:**
|
||||
|
||||
```tsx
|
||||
import { Activity } from 'react'
|
||||
|
||||
function Dropdown({ isOpen }: Props) {
|
||||
return (
|
||||
<Activity mode={isOpen ? 'visible' : 'hidden'}>
|
||||
<ExpensiveMenu />
|
||||
</Activity>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
Avoids expensive re-renders and state loss.
|
||||
@@ -1,47 +0,0 @@
|
||||
---
|
||||
title: Animate SVG Wrapper Instead of SVG Element
|
||||
impact: LOW
|
||||
impactDescription: enables hardware acceleration
|
||||
tags: rendering, svg, css, animation, performance
|
||||
---
|
||||
|
||||
## Animate SVG Wrapper Instead of SVG Element
|
||||
|
||||
Many browsers don't have hardware acceleration for CSS3 animations on SVG elements. Wrap SVG in a `<div>` and animate the wrapper instead.
|
||||
|
||||
**Incorrect (animating SVG directly - no hardware acceleration):**
|
||||
|
||||
```tsx
|
||||
function LoadingSpinner() {
|
||||
return (
|
||||
<svg
|
||||
className="animate-spin"
|
||||
width="24"
|
||||
height="24"
|
||||
viewBox="0 0 24 24"
|
||||
>
|
||||
<circle cx="12" cy="12" r="10" stroke="currentColor" />
|
||||
</svg>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (animating wrapper div - hardware accelerated):**
|
||||
|
||||
```tsx
|
||||
function LoadingSpinner() {
|
||||
return (
|
||||
<div className="animate-spin">
|
||||
<svg
|
||||
width="24"
|
||||
height="24"
|
||||
viewBox="0 0 24 24"
|
||||
>
|
||||
<circle cx="12" cy="12" r="10" stroke="currentColor" />
|
||||
</svg>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
This applies to all CSS transforms and transitions (`transform`, `opacity`, `translate`, `scale`, `rotate`). The wrapper div allows browsers to use GPU acceleration for smoother animations.
|
||||
@@ -1,40 +0,0 @@
|
||||
---
|
||||
title: Use Explicit Conditional Rendering
|
||||
impact: LOW
|
||||
impactDescription: prevents rendering 0 or NaN
|
||||
tags: rendering, conditional, jsx, falsy-values
|
||||
---
|
||||
|
||||
## Use Explicit Conditional Rendering
|
||||
|
||||
Use explicit ternary operators (`? :`) instead of `&&` for conditional rendering when the condition can be `0`, `NaN`, or other falsy values that render.
|
||||
|
||||
**Incorrect (renders "0" when count is 0):**
|
||||
|
||||
```tsx
|
||||
function Badge({ count }: { count: number }) {
|
||||
return (
|
||||
<div>
|
||||
{count && <span className="badge">{count}</span>}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
// When count = 0, renders: <div>0</div>
|
||||
// When count = 5, renders: <div><span class="badge">5</span></div>
|
||||
```
|
||||
|
||||
**Correct (renders nothing when count is 0):**
|
||||
|
||||
```tsx
|
||||
function Badge({ count }: { count: number }) {
|
||||
return (
|
||||
<div>
|
||||
{count > 0 ? <span className="badge">{count}</span> : null}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
// When count = 0, renders: <div></div>
|
||||
// When count = 5, renders: <div><span class="badge">5</span></div>
|
||||
```
|
||||
@@ -1,38 +0,0 @@
|
||||
---
|
||||
title: CSS content-visibility for Long Lists
|
||||
impact: HIGH
|
||||
impactDescription: faster initial render
|
||||
tags: rendering, css, content-visibility, long-lists
|
||||
---
|
||||
|
||||
## CSS content-visibility for Long Lists
|
||||
|
||||
Apply `content-visibility: auto` to defer off-screen rendering.
|
||||
|
||||
**CSS:**
|
||||
|
||||
```css
|
||||
.message-item {
|
||||
content-visibility: auto;
|
||||
contain-intrinsic-size: 0 80px;
|
||||
}
|
||||
```
|
||||
|
||||
**Example:**
|
||||
|
||||
```tsx
|
||||
function MessageList({ messages }: { messages: Message[] }) {
|
||||
return (
|
||||
<div className="overflow-y-auto h-screen">
|
||||
{messages.map(msg => (
|
||||
<div key={msg.id} className="message-item">
|
||||
<Avatar user={msg.author} />
|
||||
<div>{msg.content}</div>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
For 1000 messages, browser skips layout/paint for ~990 off-screen items (10× faster initial render).
|
||||
@@ -1,46 +0,0 @@
|
||||
---
|
||||
title: Hoist Static JSX Elements
|
||||
impact: LOW
|
||||
impactDescription: avoids re-creation
|
||||
tags: rendering, jsx, static, optimization
|
||||
---
|
||||
|
||||
## Hoist Static JSX Elements
|
||||
|
||||
Extract static JSX outside components to avoid re-creation.
|
||||
|
||||
**Incorrect (recreates element every render):**
|
||||
|
||||
```tsx
|
||||
function LoadingSkeleton() {
|
||||
return <div className="animate-pulse h-20 bg-gray-200" />
|
||||
}
|
||||
|
||||
function Container() {
|
||||
return (
|
||||
<div>
|
||||
{loading && <LoadingSkeleton />}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (reuses same element):**
|
||||
|
||||
```tsx
|
||||
const loadingSkeleton = (
|
||||
<div className="animate-pulse h-20 bg-gray-200" />
|
||||
)
|
||||
|
||||
function Container() {
|
||||
return (
|
||||
<div>
|
||||
{loading && loadingSkeleton}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
This is especially helpful for large and static SVG nodes, which can be expensive to recreate on every render.
|
||||
|
||||
**Note:** If your project has [React Compiler](https://react.dev/learn/react-compiler) enabled, the compiler automatically hoists static JSX elements and optimizes component re-renders, making manual hoisting unnecessary.
|
||||
@@ -1,28 +0,0 @@
|
||||
---
|
||||
title: Optimize SVG Precision
|
||||
impact: LOW
|
||||
impactDescription: reduces file size
|
||||
tags: rendering, svg, optimization, svgo
|
||||
---
|
||||
|
||||
## Optimize SVG Precision
|
||||
|
||||
Reduce SVG coordinate precision to decrease file size. The optimal precision depends on the viewBox size, but in general reducing precision should be considered.
|
||||
|
||||
**Incorrect (excessive precision):**
|
||||
|
||||
```svg
|
||||
<path d="M 10.293847 20.847362 L 30.938472 40.192837" />
|
||||
```
|
||||
|
||||
**Correct (1 decimal place):**
|
||||
|
||||
```svg
|
||||
<path d="M 10.3 20.8 L 30.9 40.2" />
|
||||
```
|
||||
|
||||
**Automate with SVGO:**
|
||||
|
||||
```bash
|
||||
npx svgo --precision=1 --multipass icon.svg
|
||||
```
|
||||
@@ -1,75 +0,0 @@
|
||||
---
|
||||
title: Use useTransition Over Manual Loading States
|
||||
impact: LOW
|
||||
impactDescription: reduces re-renders and improves code clarity
|
||||
tags: rendering, transitions, useTransition, loading, state
|
||||
---
|
||||
|
||||
## Use useTransition Over Manual Loading States
|
||||
|
||||
Use `useTransition` instead of manual `useState` for loading states. This provides built-in `isPending` state and automatically manages transitions.
|
||||
|
||||
**Incorrect (manual loading state):**
|
||||
|
||||
```tsx
|
||||
function SearchResults() {
|
||||
const [query, setQuery] = useState('')
|
||||
const [results, setResults] = useState([])
|
||||
const [isLoading, setIsLoading] = useState(false)
|
||||
|
||||
const handleSearch = async (value: string) => {
|
||||
setIsLoading(true)
|
||||
setQuery(value)
|
||||
const data = await fetchResults(value)
|
||||
setResults(data)
|
||||
setIsLoading(false)
|
||||
}
|
||||
|
||||
return (
|
||||
<>
|
||||
<input onChange={(e) => handleSearch(e.target.value)} />
|
||||
{isLoading && <Spinner />}
|
||||
<ResultsList results={results} />
|
||||
</>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (useTransition with built-in pending state):**
|
||||
|
||||
```tsx
|
||||
import { useTransition, useState } from 'react'
|
||||
|
||||
function SearchResults() {
|
||||
const [query, setQuery] = useState('')
|
||||
const [results, setResults] = useState([])
|
||||
const [isPending, startTransition] = useTransition()
|
||||
|
||||
const handleSearch = (value: string) => {
|
||||
setQuery(value) // Update input immediately
|
||||
|
||||
startTransition(async () => {
|
||||
// Fetch and update results
|
||||
const data = await fetchResults(value)
|
||||
setResults(data)
|
||||
})
|
||||
}
|
||||
|
||||
return (
|
||||
<>
|
||||
<input onChange={(e) => handleSearch(e.target.value)} />
|
||||
{isPending && <Spinner />}
|
||||
<ResultsList results={results} />
|
||||
</>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
|
||||
- **Automatic pending state**: No need to manually manage `setIsLoading(true/false)`
|
||||
- **Error resilience**: Pending state correctly resets even if the transition throws
|
||||
- **Better responsiveness**: Keeps the UI responsive during updates
|
||||
- **Interrupt handling**: New transitions automatically cancel pending ones
|
||||
|
||||
Reference: [useTransition](https://react.dev/reference/react/useTransition)
|
||||
@@ -1,39 +0,0 @@
|
||||
---
|
||||
title: Defer State Reads to Usage Point
|
||||
impact: MEDIUM
|
||||
impactDescription: avoids unnecessary subscriptions
|
||||
tags: rerender, searchParams, localStorage, optimization
|
||||
---
|
||||
|
||||
## Defer State Reads to Usage Point
|
||||
|
||||
Don't subscribe to dynamic state (searchParams, localStorage) if you only read it inside callbacks.
|
||||
|
||||
**Incorrect (subscribes to all searchParams changes):**
|
||||
|
||||
```tsx
|
||||
function ShareButton({ chatId }: { chatId: string }) {
|
||||
const searchParams = useSearchParams()
|
||||
|
||||
const handleShare = () => {
|
||||
const ref = searchParams.get('ref')
|
||||
shareChat(chatId, { ref })
|
||||
}
|
||||
|
||||
return <button onClick={handleShare}>Share</button>
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (reads on demand, no subscription):**
|
||||
|
||||
```tsx
|
||||
function ShareButton({ chatId }: { chatId: string }) {
|
||||
const handleShare = () => {
|
||||
const params = new URLSearchParams(window.location.search)
|
||||
const ref = params.get('ref')
|
||||
shareChat(chatId, { ref })
|
||||
}
|
||||
|
||||
return <button onClick={handleShare}>Share</button>
|
||||
}
|
||||
```
|
||||
@@ -1,45 +0,0 @@
|
||||
---
|
||||
title: Narrow Effect Dependencies
|
||||
impact: LOW
|
||||
impactDescription: minimizes effect re-runs
|
||||
tags: rerender, useEffect, dependencies, optimization
|
||||
---
|
||||
|
||||
## Narrow Effect Dependencies
|
||||
|
||||
Specify primitive dependencies instead of objects to minimize effect re-runs.
|
||||
|
||||
**Incorrect (re-runs on any user field change):**
|
||||
|
||||
```tsx
|
||||
useEffect(() => {
|
||||
console.log(user.id)
|
||||
}, [user])
|
||||
```
|
||||
|
||||
**Correct (re-runs only when id changes):**
|
||||
|
||||
```tsx
|
||||
useEffect(() => {
|
||||
console.log(user.id)
|
||||
}, [user.id])
|
||||
```
|
||||
|
||||
**For derived state, compute outside effect:**
|
||||
|
||||
```tsx
|
||||
// Incorrect: runs on width=767, 766, 765...
|
||||
useEffect(() => {
|
||||
if (width < 768) {
|
||||
enableMobileMode()
|
||||
}
|
||||
}, [width])
|
||||
|
||||
// Correct: runs only on boolean transition
|
||||
const isMobile = width < 768
|
||||
useEffect(() => {
|
||||
if (isMobile) {
|
||||
enableMobileMode()
|
||||
}
|
||||
}, [isMobile])
|
||||
```
|
||||
@@ -1,40 +0,0 @@
|
||||
---
|
||||
title: Calculate Derived State During Rendering
|
||||
impact: MEDIUM
|
||||
impactDescription: avoids redundant renders and state drift
|
||||
tags: rerender, derived-state, useEffect, state
|
||||
---
|
||||
|
||||
## Calculate Derived State During Rendering
|
||||
|
||||
If a value can be computed from current props/state, do not store it in state or update it in an effect. Derive it during render to avoid extra renders and state drift. Do not set state in effects solely in response to prop changes; prefer derived values or keyed resets instead.
|
||||
|
||||
**Incorrect (redundant state and effect):**
|
||||
|
||||
```tsx
|
||||
function Form() {
|
||||
const [firstName, setFirstName] = useState('First')
|
||||
const [lastName, setLastName] = useState('Last')
|
||||
const [fullName, setFullName] = useState('')
|
||||
|
||||
useEffect(() => {
|
||||
setFullName(firstName + ' ' + lastName)
|
||||
}, [firstName, lastName])
|
||||
|
||||
return <p>{fullName}</p>
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (derive during render):**
|
||||
|
||||
```tsx
|
||||
function Form() {
|
||||
const [firstName, setFirstName] = useState('First')
|
||||
const [lastName, setLastName] = useState('Last')
|
||||
const fullName = firstName + ' ' + lastName
|
||||
|
||||
return <p>{fullName}</p>
|
||||
}
|
||||
```
|
||||
|
||||
References: [You Might Not Need an Effect](https://react.dev/learn/you-might-not-need-an-effect)
|
||||
@@ -1,29 +0,0 @@
|
||||
---
|
||||
title: Subscribe to Derived State
|
||||
impact: MEDIUM
|
||||
impactDescription: reduces re-render frequency
|
||||
tags: rerender, derived-state, media-query, optimization
|
||||
---
|
||||
|
||||
## Subscribe to Derived State
|
||||
|
||||
Subscribe to derived boolean state instead of continuous values to reduce re-render frequency.
|
||||
|
||||
**Incorrect (re-renders on every pixel change):**
|
||||
|
||||
```tsx
|
||||
function Sidebar() {
|
||||
const width = useWindowWidth() // updates continuously
|
||||
const isMobile = width < 768
|
||||
return <nav className={isMobile ? 'mobile' : 'desktop'} />
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (re-renders only when boolean changes):**
|
||||
|
||||
```tsx
|
||||
function Sidebar() {
|
||||
const isMobile = useMediaQuery('(max-width: 767px)')
|
||||
return <nav className={isMobile ? 'mobile' : 'desktop'} />
|
||||
}
|
||||
```
|
||||
@@ -1,74 +0,0 @@
|
||||
---
|
||||
title: Use Functional setState Updates
|
||||
impact: MEDIUM
|
||||
impactDescription: prevents stale closures and unnecessary callback recreations
|
||||
tags: react, hooks, useState, useCallback, callbacks, closures
|
||||
---
|
||||
|
||||
## Use Functional setState Updates
|
||||
|
||||
When updating state based on the current state value, use the functional update form of setState instead of directly referencing the state variable. This prevents stale closures, eliminates unnecessary dependencies, and creates stable callback references.
|
||||
|
||||
**Incorrect (requires state as dependency):**
|
||||
|
||||
```tsx
|
||||
function TodoList() {
|
||||
const [items, setItems] = useState(initialItems)
|
||||
|
||||
// Callback must depend on items, recreated on every items change
|
||||
const addItems = useCallback((newItems: Item[]) => {
|
||||
setItems([...items, ...newItems])
|
||||
}, [items]) // ❌ items dependency causes recreations
|
||||
|
||||
// Risk of stale closure if dependency is forgotten
|
||||
const removeItem = useCallback((id: string) => {
|
||||
setItems(items.filter(item => item.id !== id))
|
||||
}, []) // ❌ Missing items dependency - will use stale items!
|
||||
|
||||
return <ItemsEditor items={items} onAdd={addItems} onRemove={removeItem} />
|
||||
}
|
||||
```
|
||||
|
||||
The first callback is recreated every time `items` changes, which can cause child components to re-render unnecessarily. The second callback has a stale closure bug—it will always reference the initial `items` value.
|
||||
|
||||
**Correct (stable callbacks, no stale closures):**
|
||||
|
||||
```tsx
|
||||
function TodoList() {
|
||||
const [items, setItems] = useState(initialItems)
|
||||
|
||||
// Stable callback, never recreated
|
||||
const addItems = useCallback((newItems: Item[]) => {
|
||||
setItems(curr => [...curr, ...newItems])
|
||||
}, []) // ✅ No dependencies needed
|
||||
|
||||
// Always uses latest state, no stale closure risk
|
||||
const removeItem = useCallback((id: string) => {
|
||||
setItems(curr => curr.filter(item => item.id !== id))
|
||||
}, []) // ✅ Safe and stable
|
||||
|
||||
return <ItemsEditor items={items} onAdd={addItems} onRemove={removeItem} />
|
||||
}
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
|
||||
1. **Stable callback references** - Callbacks don't need to be recreated when state changes
|
||||
2. **No stale closures** - Always operates on the latest state value
|
||||
3. **Fewer dependencies** - Simplifies dependency arrays and reduces memory leaks
|
||||
4. **Prevents bugs** - Eliminates the most common source of React closure bugs
|
||||
|
||||
**When to use functional updates:**
|
||||
|
||||
- Any setState that depends on the current state value
|
||||
- Inside useCallback/useMemo when state is needed
|
||||
- Event handlers that reference state
|
||||
- Async operations that update state
|
||||
|
||||
**When direct updates are fine:**
|
||||
|
||||
- Setting state to a static value: `setCount(0)`
|
||||
- Setting state from props/arguments only: `setName(newName)`
|
||||
- State doesn't depend on previous value
|
||||
|
||||
**Note:** If your project has [React Compiler](https://react.dev/learn/react-compiler) enabled, the compiler can automatically optimize some cases, but functional updates are still recommended for correctness and to prevent stale closure bugs.
|
||||
@@ -1,58 +0,0 @@
|
||||
---
|
||||
title: Use Lazy State Initialization
|
||||
impact: MEDIUM
|
||||
impactDescription: wasted computation on every render
|
||||
tags: react, hooks, useState, performance, initialization
|
||||
---
|
||||
|
||||
## Use Lazy State Initialization
|
||||
|
||||
Pass a function to `useState` for expensive initial values. Without the function form, the initializer runs on every render even though the value is only used once.
|
||||
|
||||
**Incorrect (runs on every render):**
|
||||
|
||||
```tsx
|
||||
function FilteredList({ items }: { items: Item[] }) {
|
||||
// buildSearchIndex() runs on EVERY render, even after initialization
|
||||
const [searchIndex, setSearchIndex] = useState(buildSearchIndex(items))
|
||||
const [query, setQuery] = useState('')
|
||||
|
||||
// When query changes, buildSearchIndex runs again unnecessarily
|
||||
return <SearchResults index={searchIndex} query={query} />
|
||||
}
|
||||
|
||||
function UserProfile() {
|
||||
// JSON.parse runs on every render
|
||||
const [settings, setSettings] = useState(
|
||||
JSON.parse(localStorage.getItem('settings') || '{}')
|
||||
)
|
||||
|
||||
return <SettingsForm settings={settings} onChange={setSettings} />
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (runs only once):**
|
||||
|
||||
```tsx
|
||||
function FilteredList({ items }: { items: Item[] }) {
|
||||
// buildSearchIndex() runs ONLY on initial render
|
||||
const [searchIndex, setSearchIndex] = useState(() => buildSearchIndex(items))
|
||||
const [query, setQuery] = useState('')
|
||||
|
||||
return <SearchResults index={searchIndex} query={query} />
|
||||
}
|
||||
|
||||
function UserProfile() {
|
||||
// JSON.parse runs only on initial render
|
||||
const [settings, setSettings] = useState(() => {
|
||||
const stored = localStorage.getItem('settings')
|
||||
return stored ? JSON.parse(stored) : {}
|
||||
})
|
||||
|
||||
return <SettingsForm settings={settings} onChange={setSettings} />
|
||||
}
|
||||
```
|
||||
|
||||
Use lazy initialization when computing initial values from localStorage/sessionStorage, building data structures (indexes, maps), reading from the DOM, or performing heavy transformations.
|
||||
|
||||
For simple primitives (`useState(0)`), direct references (`useState(props.value)`), or cheap literals (`useState({})`), the function form is unnecessary.
|
||||
@@ -1,45 +0,0 @@
|
||||
---
|
||||
title: Put Interaction Logic in Event Handlers
|
||||
impact: MEDIUM
|
||||
impactDescription: avoids effect re-runs and duplicate side effects
|
||||
tags: rerender, useEffect, events, side-effects, dependencies
|
||||
---
|
||||
|
||||
## Put Interaction Logic in Event Handlers
|
||||
|
||||
If a side effect is triggered by a specific user action (submit, click, drag), run it in that event handler. Do not model the action as state + effect; it makes effects re-run on unrelated changes and can duplicate the action.
|
||||
|
||||
**Incorrect (event modeled as state + effect):**
|
||||
|
||||
```tsx
|
||||
function Form() {
|
||||
const [submitted, setSubmitted] = useState(false)
|
||||
const theme = useContext(ThemeContext)
|
||||
|
||||
useEffect(() => {
|
||||
if (submitted) {
|
||||
post('/api/register')
|
||||
showToast('Registered', theme)
|
||||
}
|
||||
}, [submitted, theme])
|
||||
|
||||
return <button onClick={() => setSubmitted(true)}>Submit</button>
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (do it in the handler):**
|
||||
|
||||
```tsx
|
||||
function Form() {
|
||||
const theme = useContext(ThemeContext)
|
||||
|
||||
function handleSubmit() {
|
||||
post('/api/register')
|
||||
showToast('Registered', theme)
|
||||
}
|
||||
|
||||
return <button onClick={handleSubmit}>Submit</button>
|
||||
}
|
||||
```
|
||||
|
||||
Reference: [Should this code move to an event handler?](https://react.dev/learn/removing-effect-dependencies#should-this-code-move-to-an-event-handler)
|
||||
@@ -1,40 +0,0 @@
|
||||
---
|
||||
title: Use Transitions for Non-Urgent Updates
|
||||
impact: MEDIUM
|
||||
impactDescription: maintains UI responsiveness
|
||||
tags: rerender, transitions, startTransition, performance
|
||||
---
|
||||
|
||||
## Use Transitions for Non-Urgent Updates
|
||||
|
||||
Mark frequent, non-urgent state updates as transitions to maintain UI responsiveness.
|
||||
|
||||
**Incorrect (blocks UI on every scroll):**
|
||||
|
||||
```tsx
|
||||
function ScrollTracker() {
|
||||
const [scrollY, setScrollY] = useState(0)
|
||||
useEffect(() => {
|
||||
const handler = () => setScrollY(window.scrollY)
|
||||
window.addEventListener('scroll', handler, { passive: true })
|
||||
return () => window.removeEventListener('scroll', handler)
|
||||
}, [])
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (non-blocking updates):**
|
||||
|
||||
```tsx
|
||||
import { startTransition } from 'react'
|
||||
|
||||
function ScrollTracker() {
|
||||
const [scrollY, setScrollY] = useState(0)
|
||||
useEffect(() => {
|
||||
const handler = () => {
|
||||
startTransition(() => setScrollY(window.scrollY))
|
||||
}
|
||||
window.addEventListener('scroll', handler, { passive: true })
|
||||
return () => window.removeEventListener('scroll', handler)
|
||||
}, [])
|
||||
}
|
||||
```
|
||||
@@ -1,73 +0,0 @@
|
||||
---
|
||||
title: Use useRef for Transient Values
|
||||
impact: MEDIUM
|
||||
impactDescription: avoids unnecessary re-renders on frequent updates
|
||||
tags: rerender, useref, state, performance
|
||||
---
|
||||
|
||||
## Use useRef for Transient Values
|
||||
|
||||
When a value changes frequently and you don't want a re-render on every update (e.g., mouse trackers, intervals, transient flags), store it in `useRef` instead of `useState`. Keep component state for UI; use refs for temporary DOM-adjacent values. Updating a ref does not trigger a re-render.
|
||||
|
||||
**Incorrect (renders every update):**
|
||||
|
||||
```tsx
|
||||
function Tracker() {
|
||||
const [lastX, setLastX] = useState(0)
|
||||
|
||||
useEffect(() => {
|
||||
const onMove = (e: MouseEvent) => setLastX(e.clientX)
|
||||
window.addEventListener('mousemove', onMove)
|
||||
return () => window.removeEventListener('mousemove', onMove)
|
||||
}, [])
|
||||
|
||||
return (
|
||||
<div
|
||||
style={{
|
||||
position: 'fixed',
|
||||
top: 0,
|
||||
left: lastX,
|
||||
width: 8,
|
||||
height: 8,
|
||||
background: 'black',
|
||||
}}
|
||||
/>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (no re-render for tracking):**
|
||||
|
||||
```tsx
|
||||
function Tracker() {
|
||||
const lastXRef = useRef(0)
|
||||
const dotRef = useRef<HTMLDivElement>(null)
|
||||
|
||||
useEffect(() => {
|
||||
const onMove = (e: MouseEvent) => {
|
||||
lastXRef.current = e.clientX
|
||||
const node = dotRef.current
|
||||
if (node) {
|
||||
node.style.transform = `translateX(${e.clientX}px)`
|
||||
}
|
||||
}
|
||||
window.addEventListener('mousemove', onMove)
|
||||
return () => window.removeEventListener('mousemove', onMove)
|
||||
}, [])
|
||||
|
||||
return (
|
||||
<div
|
||||
ref={dotRef}
|
||||
style={{
|
||||
position: 'fixed',
|
||||
top: 0,
|
||||
left: 0,
|
||||
width: 8,
|
||||
height: 8,
|
||||
background: 'black',
|
||||
transform: 'translateX(0px)',
|
||||
}}
|
||||
/>
|
||||
)
|
||||
}
|
||||
```
|
||||
@@ -1,73 +0,0 @@
|
||||
---
|
||||
name: react-composition-patterns
|
||||
description:
|
||||
React composition patterns that scale. Use when refactoring components with
|
||||
boolean prop proliferation, building flexible component libraries, or
|
||||
designing reusable APIs. Triggers on tasks involving compound components,
|
||||
render props, context providers, or component architecture.
|
||||
---
|
||||
|
||||
# React Composition Patterns
|
||||
|
||||
Composition patterns for building flexible, maintainable React components. Avoid
|
||||
boolean prop proliferation by using compound components, lifting state, and
|
||||
composing internals. These patterns make codebases easier for both humans and AI
|
||||
agents to work with as they scale.
|
||||
|
||||
## When to Apply
|
||||
|
||||
Reference these guidelines when:
|
||||
|
||||
- Refactoring components with many boolean props
|
||||
- Building reusable component libraries
|
||||
- Designing flexible component APIs
|
||||
- Reviewing component architecture
|
||||
- Working with compound components or context providers
|
||||
|
||||
## Rule Categories by Priority
|
||||
|
||||
| Priority | Category | Impact | Prefix |
|
||||
| -------- | ----------------------- | ------ | --------------- |
|
||||
| 1 | Component Architecture | HIGH | `architecture-` |
|
||||
| 2 | State Management | MEDIUM | `state-` |
|
||||
| 3 | Implementation Patterns | MEDIUM | `patterns-` |
|
||||
|
||||
## Quick Reference
|
||||
|
||||
### 1. Component Architecture (HIGH)
|
||||
|
||||
- `architecture-avoid-boolean-props` - Don't add boolean props to customize
|
||||
behavior; use composition
|
||||
- `architecture-compound-components` - Structure complex components with shared
|
||||
context
|
||||
|
||||
### 2. State Management (MEDIUM)
|
||||
|
||||
- `state-decouple-implementation` - Provider is the only place that knows how
|
||||
state is managed
|
||||
- `state-context-interface` - Define generic interface with state, actions, meta
|
||||
for dependency injection
|
||||
- `state-lift-state` - Move state into provider components for sibling access
|
||||
|
||||
### 3. Implementation Patterns (MEDIUM)
|
||||
|
||||
- `patterns-explicit-variants` - Create explicit variant components instead of
|
||||
boolean modes
|
||||
- `patterns-children-over-render-props` - Use children for composition instead
|
||||
of renderX props
|
||||
|
||||
## How to Use
|
||||
|
||||
Read individual rule files for detailed explanations and code examples:
|
||||
|
||||
```
|
||||
rules/architecture-avoid-boolean-props.md
|
||||
rules/state-context-interface.md
|
||||
```
|
||||
|
||||
Each rule file contains:
|
||||
|
||||
- Brief explanation of why it matters
|
||||
- Incorrect code example with explanation
|
||||
- Correct code example with explanation
|
||||
- Additional context and references
|
||||
@@ -1,100 +0,0 @@
|
||||
---
|
||||
title: Avoid Boolean Prop Proliferation
|
||||
impact: CRITICAL
|
||||
impactDescription: prevents unmaintainable component variants
|
||||
tags: composition, props, architecture
|
||||
---
|
||||
|
||||
## Avoid Boolean Prop Proliferation
|
||||
|
||||
Don't add boolean props like `isThread`, `isEditing`, `isDMThread` to customize
|
||||
component behavior. Each boolean doubles possible states and creates
|
||||
unmaintainable conditional logic. Use composition instead.
|
||||
|
||||
**Incorrect (boolean props create exponential complexity):**
|
||||
|
||||
```tsx
|
||||
function Composer({
|
||||
onSubmit,
|
||||
isThread,
|
||||
channelId,
|
||||
isDMThread,
|
||||
dmId,
|
||||
isEditing,
|
||||
isForwarding,
|
||||
}: Props) {
|
||||
return (
|
||||
<form>
|
||||
<Header />
|
||||
<Input />
|
||||
{isDMThread ? (
|
||||
<AlsoSendToDMField id={dmId} />
|
||||
) : isThread ? (
|
||||
<AlsoSendToChannelField id={channelId} />
|
||||
) : null}
|
||||
{isEditing ? (
|
||||
<EditActions />
|
||||
) : isForwarding ? (
|
||||
<ForwardActions />
|
||||
) : (
|
||||
<DefaultActions />
|
||||
)}
|
||||
<Footer onSubmit={onSubmit} />
|
||||
</form>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (composition eliminates conditionals):**
|
||||
|
||||
```tsx
|
||||
// Channel composer
|
||||
function ChannelComposer() {
|
||||
return (
|
||||
<Composer.Frame>
|
||||
<Composer.Header />
|
||||
<Composer.Input />
|
||||
<Composer.Footer>
|
||||
<Composer.Attachments />
|
||||
<Composer.Formatting />
|
||||
<Composer.Emojis />
|
||||
<Composer.Submit />
|
||||
</Composer.Footer>
|
||||
</Composer.Frame>
|
||||
)
|
||||
}
|
||||
|
||||
// Thread composer - adds "also send to channel" field
|
||||
function ThreadComposer({ channelId }: { channelId: string }) {
|
||||
return (
|
||||
<Composer.Frame>
|
||||
<Composer.Header />
|
||||
<Composer.Input />
|
||||
<AlsoSendToChannelField id={channelId} />
|
||||
<Composer.Footer>
|
||||
<Composer.Formatting />
|
||||
<Composer.Emojis />
|
||||
<Composer.Submit />
|
||||
</Composer.Footer>
|
||||
</Composer.Frame>
|
||||
)
|
||||
}
|
||||
|
||||
// Edit composer - different footer actions
|
||||
function EditComposer() {
|
||||
return (
|
||||
<Composer.Frame>
|
||||
<Composer.Input />
|
||||
<Composer.Footer>
|
||||
<Composer.Formatting />
|
||||
<Composer.Emojis />
|
||||
<Composer.CancelEdit />
|
||||
<Composer.SaveEdit />
|
||||
</Composer.Footer>
|
||||
</Composer.Frame>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
Each variant is explicit about what it renders. We can share internals without
|
||||
sharing a single monolithic parent.
|
||||
@@ -1,112 +0,0 @@
|
||||
---
|
||||
title: Use Compound Components
|
||||
impact: HIGH
|
||||
impactDescription: enables flexible composition without prop drilling
|
||||
tags: composition, compound-components, architecture
|
||||
---
|
||||
|
||||
## Use Compound Components
|
||||
|
||||
Structure complex components as compound components with a shared context. Each
|
||||
subcomponent accesses shared state via context, not props. Consumers compose the
|
||||
pieces they need.
|
||||
|
||||
**Incorrect (monolithic component with render props):**
|
||||
|
||||
```tsx
|
||||
function Composer({
|
||||
renderHeader,
|
||||
renderFooter,
|
||||
renderActions,
|
||||
showAttachments,
|
||||
showFormatting,
|
||||
showEmojis,
|
||||
}: Props) {
|
||||
return (
|
||||
<form>
|
||||
{renderHeader?.()}
|
||||
<Input />
|
||||
{showAttachments && <Attachments />}
|
||||
{renderFooter ? (
|
||||
renderFooter()
|
||||
) : (
|
||||
<Footer>
|
||||
{showFormatting && <Formatting />}
|
||||
{showEmojis && <Emojis />}
|
||||
{renderActions?.()}
|
||||
</Footer>
|
||||
)}
|
||||
</form>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (compound components with shared context):**
|
||||
|
||||
```tsx
|
||||
const ComposerContext = createContext<ComposerContextValue | null>(null)
|
||||
|
||||
function ComposerProvider({ children, state, actions, meta }: ProviderProps) {
|
||||
return (
|
||||
<ComposerContext value={{ state, actions, meta }}>
|
||||
{children}
|
||||
</ComposerContext>
|
||||
)
|
||||
}
|
||||
|
||||
function ComposerFrame({ children }: { children: React.ReactNode }) {
|
||||
return <form>{children}</form>
|
||||
}
|
||||
|
||||
function ComposerInput() {
|
||||
const {
|
||||
state,
|
||||
actions: { update },
|
||||
meta: { inputRef },
|
||||
} = use(ComposerContext)
|
||||
return (
|
||||
<TextInput
|
||||
ref={inputRef}
|
||||
value={state.input}
|
||||
onChangeText={(text) => update((s) => ({ ...s, input: text }))}
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
function ComposerSubmit() {
|
||||
const {
|
||||
actions: { submit },
|
||||
} = use(ComposerContext)
|
||||
return <Button onPress={submit}>Send</Button>
|
||||
}
|
||||
|
||||
// Export as compound component
|
||||
const Composer = {
|
||||
Provider: ComposerProvider,
|
||||
Frame: ComposerFrame,
|
||||
Input: ComposerInput,
|
||||
Submit: ComposerSubmit,
|
||||
Header: ComposerHeader,
|
||||
Footer: ComposerFooter,
|
||||
Attachments: ComposerAttachments,
|
||||
Formatting: ComposerFormatting,
|
||||
Emojis: ComposerEmojis,
|
||||
}
|
||||
```
|
||||
|
||||
**Usage:**
|
||||
|
||||
```tsx
|
||||
<Composer.Provider state={state} actions={actions} meta={meta}>
|
||||
<Composer.Frame>
|
||||
<Composer.Header />
|
||||
<Composer.Input />
|
||||
<Composer.Footer>
|
||||
<Composer.Formatting />
|
||||
<Composer.Submit />
|
||||
</Composer.Footer>
|
||||
</Composer.Frame>
|
||||
</Composer.Provider>
|
||||
```
|
||||
|
||||
Consumers explicitly compose exactly what they need. No hidden conditionals. And the state, actions and meta are dependency-injected by a parent provider, allowing multiple usages of the same component structure.
|
||||
-87
@@ -1,87 +0,0 @@
|
||||
---
|
||||
title: Prefer Composing Children Over Render Props
|
||||
impact: MEDIUM
|
||||
impactDescription: cleaner composition, better readability
|
||||
tags: composition, children, render-props
|
||||
---
|
||||
|
||||
## Prefer Children Over Render Props
|
||||
|
||||
Use `children` for composition instead of `renderX` props. Children are more
|
||||
readable, compose naturally, and don't require understanding callback
|
||||
signatures.
|
||||
|
||||
**Incorrect (render props):**
|
||||
|
||||
```tsx
|
||||
function Composer({
|
||||
renderHeader,
|
||||
renderFooter,
|
||||
renderActions,
|
||||
}: {
|
||||
renderHeader?: () => React.ReactNode
|
||||
renderFooter?: () => React.ReactNode
|
||||
renderActions?: () => React.ReactNode
|
||||
}) {
|
||||
return (
|
||||
<form>
|
||||
{renderHeader?.()}
|
||||
<Input />
|
||||
{renderFooter ? renderFooter() : <DefaultFooter />}
|
||||
{renderActions?.()}
|
||||
</form>
|
||||
)
|
||||
}
|
||||
|
||||
// Usage is awkward and inflexible
|
||||
return (
|
||||
<Composer
|
||||
renderHeader={() => <CustomHeader />}
|
||||
renderFooter={() => (
|
||||
<>
|
||||
<Formatting />
|
||||
<Emojis />
|
||||
</>
|
||||
)}
|
||||
renderActions={() => <SubmitButton />}
|
||||
/>
|
||||
)
|
||||
```
|
||||
|
||||
**Correct (compound components with children):**
|
||||
|
||||
```tsx
|
||||
function ComposerFrame({ children }: { children: React.ReactNode }) {
|
||||
return <form>{children}</form>
|
||||
}
|
||||
|
||||
function ComposerFooter({ children }: { children: React.ReactNode }) {
|
||||
return <footer className='flex'>{children}</footer>
|
||||
}
|
||||
|
||||
// Usage is flexible
|
||||
return (
|
||||
<Composer.Frame>
|
||||
<CustomHeader />
|
||||
<Composer.Input />
|
||||
<Composer.Footer>
|
||||
<Composer.Formatting />
|
||||
<Composer.Emojis />
|
||||
<SubmitButton />
|
||||
</Composer.Footer>
|
||||
</Composer.Frame>
|
||||
)
|
||||
```
|
||||
|
||||
**When render props are appropriate:**
|
||||
|
||||
```tsx
|
||||
// Render props work well when you need to pass data back
|
||||
<List
|
||||
data={items}
|
||||
renderItem={({ item, index }) => <Item item={item} index={index} />}
|
||||
/>
|
||||
```
|
||||
|
||||
Use render props when the parent needs to provide data or state to the child.
|
||||
Use children when composing static structure.
|
||||
@@ -1,100 +0,0 @@
|
||||
---
|
||||
title: Create Explicit Component Variants
|
||||
impact: MEDIUM
|
||||
impactDescription: self-documenting code, no hidden conditionals
|
||||
tags: composition, variants, architecture
|
||||
---
|
||||
|
||||
## Create Explicit Component Variants
|
||||
|
||||
Instead of one component with many boolean props, create explicit variant
|
||||
components. Each variant composes the pieces it needs. The code documents
|
||||
itself.
|
||||
|
||||
**Incorrect (one component, many modes):**
|
||||
|
||||
```tsx
|
||||
// What does this component actually render?
|
||||
<Composer
|
||||
isThread
|
||||
isEditing={false}
|
||||
channelId='abc'
|
||||
showAttachments
|
||||
showFormatting={false}
|
||||
/>
|
||||
```
|
||||
|
||||
**Correct (explicit variants):**
|
||||
|
||||
```tsx
|
||||
// Immediately clear what this renders
|
||||
<ThreadComposer channelId="abc" />
|
||||
|
||||
// Or
|
||||
<EditMessageComposer messageId="xyz" />
|
||||
|
||||
// Or
|
||||
<ForwardMessageComposer messageId="123" />
|
||||
```
|
||||
|
||||
Each implementation is unique, explicit and self-contained. Yet they can each
|
||||
use shared parts.
|
||||
|
||||
**Implementation:**
|
||||
|
||||
```tsx
|
||||
function ThreadComposer({ channelId }: { channelId: string }) {
|
||||
return (
|
||||
<ThreadProvider channelId={channelId}>
|
||||
<Composer.Frame>
|
||||
<Composer.Input />
|
||||
<AlsoSendToChannelField channelId={channelId} />
|
||||
<Composer.Footer>
|
||||
<Composer.Formatting />
|
||||
<Composer.Emojis />
|
||||
<Composer.Submit />
|
||||
</Composer.Footer>
|
||||
</Composer.Frame>
|
||||
</ThreadProvider>
|
||||
)
|
||||
}
|
||||
|
||||
function EditMessageComposer({ messageId }: { messageId: string }) {
|
||||
return (
|
||||
<EditMessageProvider messageId={messageId}>
|
||||
<Composer.Frame>
|
||||
<Composer.Input />
|
||||
<Composer.Footer>
|
||||
<Composer.Formatting />
|
||||
<Composer.Emojis />
|
||||
<Composer.CancelEdit />
|
||||
<Composer.SaveEdit />
|
||||
</Composer.Footer>
|
||||
</Composer.Frame>
|
||||
</EditMessageProvider>
|
||||
)
|
||||
}
|
||||
|
||||
function ForwardMessageComposer({ messageId }: { messageId: string }) {
|
||||
return (
|
||||
<ForwardMessageProvider messageId={messageId}>
|
||||
<Composer.Frame>
|
||||
<Composer.Input placeholder="Add a message, if you'd like." />
|
||||
<Composer.Footer>
|
||||
<Composer.Formatting />
|
||||
<Composer.Emojis />
|
||||
<Composer.Mentions />
|
||||
</Composer.Footer>
|
||||
</Composer.Frame>
|
||||
</ForwardMessageProvider>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
Each variant is explicit about:
|
||||
|
||||
- What provider/state it uses
|
||||
- What UI elements it includes
|
||||
- What actions are available
|
||||
|
||||
No boolean prop combinations to reason about. No impossible states.
|
||||
@@ -1,191 +0,0 @@
|
||||
---
|
||||
title: Define Generic Context Interfaces for Dependency Injection
|
||||
impact: HIGH
|
||||
impactDescription: enables dependency-injectable state across use-cases
|
||||
tags: composition, context, state, typescript, dependency-injection
|
||||
---
|
||||
|
||||
## Define Generic Context Interfaces for Dependency Injection
|
||||
|
||||
Define a **generic interface** for your component context with three parts:
|
||||
`state`, `actions`, and `meta`. This interface is a contract that any provider
|
||||
can implement—enabling the same UI components to work with completely different
|
||||
state implementations.
|
||||
|
||||
**Core principle:** Lift state, compose internals, make state
|
||||
dependency-injectable.
|
||||
|
||||
**Incorrect (UI coupled to specific state implementation):**
|
||||
|
||||
```tsx
|
||||
function ComposerInput() {
|
||||
// Tightly coupled to a specific hook
|
||||
const { input, setInput } = useChannelComposerState()
|
||||
return <TextInput value={input} onChangeText={setInput} />
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (generic interface enables dependency injection):**
|
||||
|
||||
```tsx
|
||||
// Define a GENERIC interface that any provider can implement
|
||||
interface ComposerState {
|
||||
input: string
|
||||
attachments: Attachment[]
|
||||
isSubmitting: boolean
|
||||
}
|
||||
|
||||
interface ComposerActions {
|
||||
update: (updater: (state: ComposerState) => ComposerState) => void
|
||||
submit: () => void
|
||||
}
|
||||
|
||||
interface ComposerMeta {
|
||||
inputRef: React.RefObject<TextInput>
|
||||
}
|
||||
|
||||
interface ComposerContextValue {
|
||||
state: ComposerState
|
||||
actions: ComposerActions
|
||||
meta: ComposerMeta
|
||||
}
|
||||
|
||||
const ComposerContext = createContext<ComposerContextValue | null>(null)
|
||||
```
|
||||
|
||||
**UI components consume the interface, not the implementation:**
|
||||
|
||||
```tsx
|
||||
function ComposerInput() {
|
||||
const {
|
||||
state,
|
||||
actions: { update },
|
||||
meta,
|
||||
} = use(ComposerContext)
|
||||
|
||||
// This component works with ANY provider that implements the interface
|
||||
return (
|
||||
<TextInput
|
||||
ref={meta.inputRef}
|
||||
value={state.input}
|
||||
onChangeText={(text) => update((s) => ({ ...s, input: text }))}
|
||||
/>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Different providers implement the same interface:**
|
||||
|
||||
```tsx
|
||||
// Provider A: Local state for ephemeral forms
|
||||
function ForwardMessageProvider({ children }: { children: React.ReactNode }) {
|
||||
const [state, setState] = useState(initialState)
|
||||
const inputRef = useRef(null)
|
||||
const submit = useForwardMessage()
|
||||
|
||||
return (
|
||||
<ComposerContext
|
||||
value={{
|
||||
state,
|
||||
actions: { update: setState, submit },
|
||||
meta: { inputRef },
|
||||
}}
|
||||
>
|
||||
{children}
|
||||
</ComposerContext>
|
||||
)
|
||||
}
|
||||
|
||||
// Provider B: Global synced state for channels
|
||||
function ChannelProvider({ channelId, children }: Props) {
|
||||
const { state, update, submit } = useGlobalChannel(channelId)
|
||||
const inputRef = useRef(null)
|
||||
|
||||
return (
|
||||
<ComposerContext
|
||||
value={{
|
||||
state,
|
||||
actions: { update, submit },
|
||||
meta: { inputRef },
|
||||
}}
|
||||
>
|
||||
{children}
|
||||
</ComposerContext>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**The same composed UI works with both:**
|
||||
|
||||
```tsx
|
||||
// Works with ForwardMessageProvider (local state)
|
||||
<ForwardMessageProvider>
|
||||
<Composer.Frame>
|
||||
<Composer.Input />
|
||||
<Composer.Submit />
|
||||
</Composer.Frame>
|
||||
</ForwardMessageProvider>
|
||||
|
||||
// Works with ChannelProvider (global synced state)
|
||||
<ChannelProvider channelId="abc">
|
||||
<Composer.Frame>
|
||||
<Composer.Input />
|
||||
<Composer.Submit />
|
||||
</Composer.Frame>
|
||||
</ChannelProvider>
|
||||
```
|
||||
|
||||
**Custom UI outside the component can access state and actions:**
|
||||
|
||||
The provider boundary is what matters—not the visual nesting. Components that
|
||||
need shared state don't have to be inside the `Composer.Frame`. They just need
|
||||
to be within the provider.
|
||||
|
||||
```tsx
|
||||
function ForwardMessageDialog() {
|
||||
return (
|
||||
<ForwardMessageProvider>
|
||||
<Dialog>
|
||||
{/* The composer UI */}
|
||||
<Composer.Frame>
|
||||
<Composer.Input placeholder="Add a message, if you'd like." />
|
||||
<Composer.Footer>
|
||||
<Composer.Formatting />
|
||||
<Composer.Emojis />
|
||||
</Composer.Footer>
|
||||
</Composer.Frame>
|
||||
|
||||
{/* Custom UI OUTSIDE the composer, but INSIDE the provider */}
|
||||
<MessagePreview />
|
||||
|
||||
{/* Actions at the bottom of the dialog */}
|
||||
<DialogActions>
|
||||
<CancelButton />
|
||||
<ForwardButton />
|
||||
</DialogActions>
|
||||
</Dialog>
|
||||
</ForwardMessageProvider>
|
||||
)
|
||||
}
|
||||
|
||||
// This button lives OUTSIDE Composer.Frame but can still submit based on its context!
|
||||
function ForwardButton() {
|
||||
const {
|
||||
actions: { submit },
|
||||
} = use(ComposerContext)
|
||||
return <Button onPress={submit}>Forward</Button>
|
||||
}
|
||||
|
||||
// This preview lives OUTSIDE Composer.Frame but can read composer's state!
|
||||
function MessagePreview() {
|
||||
const { state } = use(ComposerContext)
|
||||
return <Preview message={state.input} attachments={state.attachments} />
|
||||
}
|
||||
```
|
||||
|
||||
The `ForwardButton` and `MessagePreview` are not visually inside the composer
|
||||
box, but they can still access its state and actions. This is the power of
|
||||
lifting state into providers.
|
||||
|
||||
The UI is reusable bits you compose together. The state is dependency-injected
|
||||
by the provider. Swap the provider, keep the UI.
|
||||
@@ -1,113 +0,0 @@
|
||||
---
|
||||
title: Decouple State Management from UI
|
||||
impact: MEDIUM
|
||||
impactDescription: enables swapping state implementations without changing UI
|
||||
tags: composition, state, architecture
|
||||
---
|
||||
|
||||
## Decouple State Management from UI
|
||||
|
||||
The provider component should be the only place that knows how state is managed.
|
||||
UI components consume the context interface—they don't know if state comes from
|
||||
useState, Zustand, or a server sync.
|
||||
|
||||
**Incorrect (UI coupled to state implementation):**
|
||||
|
||||
```tsx
|
||||
function ChannelComposer({ channelId }: { channelId: string }) {
|
||||
// UI component knows about global state implementation
|
||||
const state = useGlobalChannelState(channelId)
|
||||
const { submit, updateInput } = useChannelSync(channelId)
|
||||
|
||||
return (
|
||||
<Composer.Frame>
|
||||
<Composer.Input
|
||||
value={state.input}
|
||||
onChange={(text) => sync.updateInput(text)}
|
||||
/>
|
||||
<Composer.Submit onPress={() => sync.submit()} />
|
||||
</Composer.Frame>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (state management isolated in provider):**
|
||||
|
||||
```tsx
|
||||
// Provider handles all state management details
|
||||
function ChannelProvider({
|
||||
channelId,
|
||||
children,
|
||||
}: {
|
||||
channelId: string
|
||||
children: React.ReactNode
|
||||
}) {
|
||||
const { state, update, submit } = useGlobalChannel(channelId)
|
||||
const inputRef = useRef(null)
|
||||
|
||||
return (
|
||||
<Composer.Provider
|
||||
state={state}
|
||||
actions={{ update, submit }}
|
||||
meta={{ inputRef }}
|
||||
>
|
||||
{children}
|
||||
</Composer.Provider>
|
||||
)
|
||||
}
|
||||
|
||||
// UI component only knows about the context interface
|
||||
function ChannelComposer() {
|
||||
return (
|
||||
<Composer.Frame>
|
||||
<Composer.Header />
|
||||
<Composer.Input />
|
||||
<Composer.Footer>
|
||||
<Composer.Submit />
|
||||
</Composer.Footer>
|
||||
</Composer.Frame>
|
||||
)
|
||||
}
|
||||
|
||||
// Usage
|
||||
function Channel({ channelId }: { channelId: string }) {
|
||||
return (
|
||||
<ChannelProvider channelId={channelId}>
|
||||
<ChannelComposer />
|
||||
</ChannelProvider>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Different providers, same UI:**
|
||||
|
||||
```tsx
|
||||
// Local state for ephemeral forms
|
||||
function ForwardMessageProvider({ children }) {
|
||||
const [state, setState] = useState(initialState)
|
||||
const forwardMessage = useForwardMessage()
|
||||
|
||||
return (
|
||||
<Composer.Provider
|
||||
state={state}
|
||||
actions={{ update: setState, submit: forwardMessage }}
|
||||
>
|
||||
{children}
|
||||
</Composer.Provider>
|
||||
)
|
||||
}
|
||||
|
||||
// Global synced state for channels
|
||||
function ChannelProvider({ channelId, children }) {
|
||||
const { state, update, submit } = useGlobalChannel(channelId)
|
||||
|
||||
return (
|
||||
<Composer.Provider state={state} actions={{ update, submit }}>
|
||||
{children}
|
||||
</Composer.Provider>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
The same `Composer.Input` component works with both providers because it only
|
||||
depends on the context interface, not the implementation.
|
||||
@@ -1,125 +0,0 @@
|
||||
---
|
||||
title: Lift State into Provider Components
|
||||
impact: HIGH
|
||||
impactDescription: enables state sharing outside component boundaries
|
||||
tags: composition, state, context, providers
|
||||
---
|
||||
|
||||
## Lift State into Provider Components
|
||||
|
||||
Move state management into dedicated provider components. This allows sibling
|
||||
components outside the main UI to access and modify state without prop drilling
|
||||
or awkward refs.
|
||||
|
||||
**Incorrect (state trapped inside component):**
|
||||
|
||||
```tsx
|
||||
function ForwardMessageComposer() {
|
||||
const [state, setState] = useState(initialState)
|
||||
const forwardMessage = useForwardMessage()
|
||||
|
||||
return (
|
||||
<Composer.Frame>
|
||||
<Composer.Input />
|
||||
<Composer.Footer />
|
||||
</Composer.Frame>
|
||||
)
|
||||
}
|
||||
|
||||
// Problem: How does this button access composer state?
|
||||
function ForwardMessageDialog() {
|
||||
return (
|
||||
<Dialog>
|
||||
<ForwardMessageComposer />
|
||||
<MessagePreview /> {/* Needs composer state */}
|
||||
<DialogActions>
|
||||
<CancelButton />
|
||||
<ForwardButton /> {/* Needs to call submit */}
|
||||
</DialogActions>
|
||||
</Dialog>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Incorrect (useEffect to sync state up):**
|
||||
|
||||
```tsx
|
||||
function ForwardMessageDialog() {
|
||||
const [input, setInput] = useState('')
|
||||
return (
|
||||
<Dialog>
|
||||
<ForwardMessageComposer onInputChange={setInput} />
|
||||
<MessagePreview input={input} />
|
||||
</Dialog>
|
||||
)
|
||||
}
|
||||
|
||||
function ForwardMessageComposer({ onInputChange }) {
|
||||
const [state, setState] = useState(initialState)
|
||||
useEffect(() => {
|
||||
onInputChange(state.input) // Sync on every change 😬
|
||||
}, [state.input])
|
||||
}
|
||||
```
|
||||
|
||||
**Incorrect (reading state from ref on submit):**
|
||||
|
||||
```tsx
|
||||
function ForwardMessageDialog() {
|
||||
const stateRef = useRef(null)
|
||||
return (
|
||||
<Dialog>
|
||||
<ForwardMessageComposer stateRef={stateRef} />
|
||||
<ForwardButton onPress={() => submit(stateRef.current)} />
|
||||
</Dialog>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (state lifted to provider):**
|
||||
|
||||
```tsx
|
||||
function ForwardMessageProvider({ children }: { children: React.ReactNode }) {
|
||||
const [state, setState] = useState(initialState)
|
||||
const forwardMessage = useForwardMessage()
|
||||
const inputRef = useRef(null)
|
||||
|
||||
return (
|
||||
<Composer.Provider
|
||||
state={state}
|
||||
actions={{ update: setState, submit: forwardMessage }}
|
||||
meta={{ inputRef }}
|
||||
>
|
||||
{children}
|
||||
</Composer.Provider>
|
||||
)
|
||||
}
|
||||
|
||||
function ForwardMessageDialog() {
|
||||
return (
|
||||
<ForwardMessageProvider>
|
||||
<Dialog>
|
||||
<ForwardMessageComposer />
|
||||
<MessagePreview /> {/* Custom components can access state and actions */}
|
||||
<DialogActions>
|
||||
<CancelButton />
|
||||
<ForwardButton /> {/* Custom components can access state and actions */}
|
||||
</DialogActions>
|
||||
</Dialog>
|
||||
</ForwardMessageProvider>
|
||||
)
|
||||
}
|
||||
|
||||
function ForwardButton() {
|
||||
const { actions } = use(Composer.Context)
|
||||
return <Button onPress={actions.submit}>Forward</Button>
|
||||
}
|
||||
```
|
||||
|
||||
The ForwardButton lives outside the Composer.Frame but still has access to the
|
||||
submit action because it's within the provider. Even though it's a one-off
|
||||
component, it can still access the composer's state and actions from outside the
|
||||
UI itself.
|
||||
|
||||
**Key insight:** Components that need shared state don't have to be visually
|
||||
nested inside each other—they just need to be within the same provider.
|
||||
@@ -1,19 +0,0 @@
|
||||
---
|
||||
name: web-design-guidelines
|
||||
description: Review UI code for Web Interface Guidelines compliance. Use when asked to "review my UI", "check accessibility", "audit design", "review UX", or "check my site against best practices".
|
||||
argument-hint: <file-or-pattern>
|
||||
---
|
||||
|
||||
# Web Interface Guidelines
|
||||
|
||||
Review files for compliance with Web Interface Guidelines.
|
||||
|
||||
## Usage
|
||||
|
||||
When a user provides a file or pattern argument:
|
||||
1. Read guidelines from rules/accessibility.md
|
||||
2. Read the specified files
|
||||
3. Apply all rules from the fetched guidelines
|
||||
4. Output findings using the format specified in the guidelines
|
||||
|
||||
If no files specified, ask the user which files to review.
|
||||
@@ -1,179 +0,0 @@
|
||||
---
|
||||
description: Review UI code for Web Interface Guidelines compliance
|
||||
argument-hint: <file-or-pattern>
|
||||
---
|
||||
|
||||
# Web Interface Guidelines
|
||||
|
||||
Review these files for compliance: $ARGUMENTS
|
||||
|
||||
Read files, check against rules below. Output concise but comprehensive—sacrifice grammar for brevity. High signal-to-noise.
|
||||
|
||||
## Rules
|
||||
|
||||
### Accessibility
|
||||
|
||||
- Icon-only buttons need `aria-label`
|
||||
- Form controls need `<label>` or `aria-label`
|
||||
- Interactive elements need keyboard handlers (`onKeyDown`/`onKeyUp`)
|
||||
- `<button>` for actions, `<a>`/`<Link>` for navigation (not `<div onClick>`)
|
||||
- Images need `alt` (or `alt=""` if decorative)
|
||||
- Decorative icons need `aria-hidden="true"`
|
||||
- Async updates (toasts, validation) need `aria-live="polite"`
|
||||
- Use semantic HTML (`<button>`, `<a>`, `<label>`, `<table>`) before ARIA
|
||||
- Headings hierarchical `<h1>`–`<h6>`; include skip link for main content
|
||||
- `scroll-margin-top` on heading anchors
|
||||
|
||||
### Focus States
|
||||
|
||||
- Interactive elements need visible focus: `focus-visible:ring-*` or equivalent
|
||||
- Never `outline-none` / `outline: none` without focus replacement
|
||||
- Use `:focus-visible` over `:focus` (avoid focus ring on click)
|
||||
- Group focus with `:focus-within` for compound controls
|
||||
|
||||
### Forms
|
||||
|
||||
- Inputs need `autocomplete` and meaningful `name`
|
||||
- Use correct `type` (`email`, `tel`, `url`, `number`) and `inputmode`
|
||||
- Never block paste (`onPaste` + `preventDefault`)
|
||||
- Labels clickable (`htmlFor` or wrapping control)
|
||||
- Disable spellcheck on emails, codes, usernames (`spellCheck={false}`)
|
||||
- Checkboxes/radios: label + control share single hit target (no dead zones)
|
||||
- Submit button stays enabled until request starts; spinner during request
|
||||
- Errors inline next to fields; focus first error on submit
|
||||
- Placeholders end with `…` and show example pattern
|
||||
- `autocomplete="off"` on non-auth fields to avoid password manager triggers
|
||||
- Warn before navigation with unsaved changes (`beforeunload` or router guard)
|
||||
|
||||
### Animation
|
||||
|
||||
- Honor `prefers-reduced-motion` (provide reduced variant or disable)
|
||||
- Animate `transform`/`opacity` only (compositor-friendly)
|
||||
- Never `transition: all`—list properties explicitly
|
||||
- Set correct `transform-origin`
|
||||
- SVG: transforms on `<g>` wrapper with `transform-box: fill-box; transform-origin: center`
|
||||
- Animations interruptible—respond to user input mid-animation
|
||||
|
||||
### Typography
|
||||
|
||||
- `…` not `...`
|
||||
- Curly quotes `"` `"` not straight `"`
|
||||
- Non-breaking spaces: `10 MB`, `⌘ K`, brand names
|
||||
- Loading states end with `…`: `"Loading…"`, `"Saving…"`
|
||||
- `font-variant-numeric: tabular-nums` for number columns/comparisons
|
||||
- Use `text-wrap: balance` or `text-pretty` on headings (prevents widows)
|
||||
|
||||
### Content Handling
|
||||
|
||||
- Text containers handle long content: `truncate`, `line-clamp-*`, or `break-words`
|
||||
- Flex children need `min-w-0` to allow text truncation
|
||||
- Handle empty states—don't render broken UI for empty strings/arrays
|
||||
- User-generated content: anticipate short, average, and very long inputs
|
||||
|
||||
### Images
|
||||
|
||||
- `<img>` needs explicit `width` and `height` (prevents CLS)
|
||||
- Below-fold images: `loading="lazy"`
|
||||
- Above-fold critical images: `priority` or `fetchpriority="high"`
|
||||
|
||||
### Performance
|
||||
|
||||
- Large lists (>50 items): virtualize (`virtua`, `content-visibility: auto`)
|
||||
- No layout reads in render (`getBoundingClientRect`, `offsetHeight`, `offsetWidth`, `scrollTop`)
|
||||
- Batch DOM reads/writes; avoid interleaving
|
||||
- Prefer uncontrolled inputs; controlled inputs must be cheap per keystroke
|
||||
- Add `<link rel="preconnect">` for CDN/asset domains
|
||||
- Critical fonts: `<link rel="preload" as="font">` with `font-display: swap`
|
||||
|
||||
### Navigation & State
|
||||
|
||||
- URL reflects state—filters, tabs, pagination, expanded panels in query params
|
||||
- Links use `<a>`/`<Link>` (Cmd/Ctrl+click, middle-click support)
|
||||
- Deep-link all stateful UI (if uses `useState`, consider URL sync via nuqs or similar)
|
||||
- Destructive actions need confirmation modal or undo window—never immediate
|
||||
|
||||
### Touch & Interaction
|
||||
|
||||
- `touch-action: manipulation` (prevents double-tap zoom delay)
|
||||
- `-webkit-tap-highlight-color` set intentionally
|
||||
- `overscroll-behavior: contain` in modals/drawers/sheets
|
||||
- During drag: disable text selection, `inert` on dragged elements
|
||||
- `autoFocus` sparingly—desktop only, single primary input; avoid on mobile
|
||||
|
||||
### Safe Areas & Layout
|
||||
|
||||
- Full-bleed layouts need `env(safe-area-inset-*)` for notches
|
||||
- Avoid unwanted scrollbars: `overflow-x-hidden` on containers, fix content overflow
|
||||
- Flex/grid over JS measurement for layout
|
||||
|
||||
### Dark Mode & Theming
|
||||
|
||||
- `color-scheme: dark` on `<html>` for dark themes (fixes scrollbar, inputs)
|
||||
- `<meta name="theme-color">` matches page background
|
||||
- Native `<select>`: explicit `background-color` and `color` (Windows dark mode)
|
||||
|
||||
### Locale & i18n
|
||||
|
||||
- Dates/times: use `Intl.DateTimeFormat` not hardcoded formats
|
||||
- Numbers/currency: use `Intl.NumberFormat` not hardcoded formats
|
||||
- Detect language via `Accept-Language` / `navigator.languages`, not IP
|
||||
|
||||
### Hydration Safety
|
||||
|
||||
- Inputs with `value` need `onChange` (or use `defaultValue` for uncontrolled)
|
||||
- Date/time rendering: guard against hydration mismatch (server vs client)
|
||||
- `suppressHydrationWarning` only where truly needed
|
||||
|
||||
### Hover & Interactive States
|
||||
|
||||
- Buttons/links need `hover:` state (visual feedback)
|
||||
- Interactive states increase contrast: hover/active/focus more prominent than rest
|
||||
|
||||
### Content & Copy
|
||||
|
||||
- Active voice: "Install the CLI" not "The CLI will be installed"
|
||||
- Title Case for headings/buttons (Chicago style)
|
||||
- Numerals for counts: "8 deployments" not "eight"
|
||||
- Specific button labels: "Save API Key" not "Continue"
|
||||
- Error messages include fix/next step, not just problem
|
||||
- Second person; avoid first person
|
||||
- `&` over "and" where space-constrained
|
||||
|
||||
### Anti-patterns (flag these)
|
||||
|
||||
- `user-scalable=no` or `maximum-scale=1` disabling zoom
|
||||
- `onPaste` with `preventDefault`
|
||||
- `transition: all`
|
||||
- `outline-none` without focus-visible replacement
|
||||
- Inline `onClick` navigation without `<a>`
|
||||
- `<div>` or `<span>` with click handlers (should be `<button>`)
|
||||
- Images without dimensions
|
||||
- Large arrays `.map()` without virtualization
|
||||
- Form inputs without labels
|
||||
- Icon buttons without `aria-label`
|
||||
- Hardcoded date/number formats (use `Intl.*`)
|
||||
- `autoFocus` without clear justification
|
||||
|
||||
## Output Format
|
||||
|
||||
Group by file. Use `file:line` format (VS Code clickable). Terse findings.
|
||||
|
||||
```text
|
||||
## src/Button.tsx
|
||||
|
||||
src/Button.tsx:42 - icon button missing aria-label
|
||||
src/Button.tsx:18 - input lacks label
|
||||
src/Button.tsx:55 - animation missing prefers-reduced-motion
|
||||
src/Button.tsx:67 - transition: all → list properties
|
||||
|
||||
## src/Modal.tsx
|
||||
|
||||
src/Modal.tsx:12 - missing overscroll-behavior: contain
|
||||
src/Modal.tsx:34 - "..." → "…"
|
||||
|
||||
## src/Card.tsx
|
||||
|
||||
✓ pass
|
||||
```
|
||||
|
||||
State issue + location. Skip explanation unless fix non-obvious. No preamble.
|
||||
Reference in New Issue
Block a user