chore: move agent skills to internal delivery (#1254)
Private skill files are now stored and versioned in a separate internal repo; a setup hook populates .claude/skills and .agents/skills with machine-local symlinks instead. This keeps contributor-facing patterns (agent skills in the tree) while allowing some skills to be developed privately. - Remove tracked skill files (62 files across 7 skill dirs). - Gitignore the now machine-local skill directories. - Add an env-gated hook to orca.yaml scripts.setup. Runs a setup script whose path lives in $ORCA_INTERNAL_DEV_SETUP when present; silently no-ops for public contributors. No new contributor-facing requirements: the hook is optional, the env var is only set by internal tooling, and the setup itself runs in the worktree that Orca just created. Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
GitHub
parent
3a138a2f07
commit
c85f487ebf
|
|
@ -1,321 +0,0 @@
|
|||
---
|
||||
name: auto-pr-merge
|
||||
description: Create PR, wait for checks, fix issues iteratively, and merge with --admin
|
||||
---
|
||||
|
||||
# auto-pr-merge
|
||||
|
||||
End-to-end autonomous PR workflow: create PR, wait for CI checks to pass, fix any issues (typecheck, code review), update PR, and merge with admin override. Runs fully autonomously without user confirmation.
|
||||
|
||||
**IMPORTANT**: Execute this entire process autonomously without asking the user for confirmation at any step. Just do it.
|
||||
|
||||
---
|
||||
|
||||
## Process Overview
|
||||
|
||||
```
|
||||
CREATE PR → WAIT FOR CHECKS → FIX LOOP (max 3) → MERGE --admin
|
||||
│ │ │ │
|
||||
/create-pr 1min + 1min polls diagnose CI gh pr merge
|
||||
(up to ~6min) fix issues --admin
|
||||
pn typecheck --squash
|
||||
/review-code --delete-branch
|
||||
commit & push
|
||||
wait for checks
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Step 1: Create PR
|
||||
|
||||
Run the `/create-pr` skill to create or update the pull request.
|
||||
|
||||
```
|
||||
Use the Skill tool: skill: "create-pr"
|
||||
```
|
||||
|
||||
After `/create-pr` completes, extract the PR number and URL:
|
||||
|
||||
```bash
|
||||
gh pr view --json number,url --jq '.number, .url'
|
||||
```
|
||||
|
||||
Store the PR number for later use.
|
||||
|
||||
**CRITICAL: DO NOT STOP after /create-pr completes. Continue to Step 2.**
|
||||
|
||||
---
|
||||
|
||||
## Step 2: Wait for PR Checks
|
||||
|
||||
Use the **Check Polling Procedure** (defined below) to wait for CI checks. Based on the result:
|
||||
|
||||
- `PASSED` → Skip to Step 4 (MERGE)
|
||||
- `FAILED` → Proceed to Step 3 (FIX LOOP)
|
||||
- `NO_CHECKS` → Skip to Step 4 (MERGE) — no CI configured, nothing to wait for
|
||||
|
||||
---
|
||||
|
||||
## Step 3: Fix Loop (Max 3 Iterations)
|
||||
|
||||
Run up to **3 iterations** of the diagnose-fix-review-push cycle.
|
||||
|
||||
### 3a. Diagnose Failures
|
||||
|
||||
Identify which checks failed and get their logs:
|
||||
|
||||
```bash
|
||||
# Get the most recent failed run ID for this branch
|
||||
FAILED_RUN=$(gh run list --branch $(git branch --show-current) --limit 5 --json databaseId,conclusion,name --jq '[.[] | select(.conclusion == "failure")] | .[0].databaseId')
|
||||
echo "Failed run: $FAILED_RUN"
|
||||
|
||||
# View failed step logs (truncated to last 200 lines to avoid context bloat)
|
||||
if [ -n "$FAILED_RUN" ] && [ "$FAILED_RUN" != "null" ]; then
|
||||
gh run view "$FAILED_RUN" --log-failed 2>&1 | tail -200
|
||||
fi
|
||||
```
|
||||
|
||||
If `gh run view --log-failed` returns too much output or doesn't work, fall back to:
|
||||
|
||||
```bash
|
||||
gh run view "$FAILED_RUN" --json jobs --jq '.jobs[] | select(.conclusion == "failure") | {name, steps: [.steps[] | select(.conclusion == "failure") | {name, conclusion}]}'
|
||||
```
|
||||
|
||||
### 3b. Fix Issues
|
||||
|
||||
Based on the failure diagnosis:
|
||||
|
||||
1. **Read the error output** from the failed checks
|
||||
2. **Identify the root cause** (build error, lint error, test failure, type error, etc.)
|
||||
3. **Apply fixes** using the Edit tool
|
||||
4. **Delegate to specialized skills when appropriate**:
|
||||
- **Lint errors**: Run `/fix-lint` skill
|
||||
- **Build errors**: Run `/fix-build` skill
|
||||
- **Test failures**: Read failing test, fix the code or test manually
|
||||
|
||||
### 3c. Run Typecheck
|
||||
|
||||
After applying fixes, verify types are clean:
|
||||
|
||||
```bash
|
||||
pn typecheck 2>&1
|
||||
```
|
||||
|
||||
- If typecheck passes: continue to 3d
|
||||
- If typecheck fails: fix type errors and re-run (up to 3 attempts within this step)
|
||||
- Read the error output
|
||||
- Fix the type errors
|
||||
- Re-run `pn typecheck`
|
||||
- If still failing after 3 attempts: move on to 3d anyway (CI will catch remaining issues)
|
||||
|
||||
### 3d. Run Code Review (Quick Round)
|
||||
|
||||
Run **only** `/review-code` to catch any issues introduced by the fixes. Do NOT run `/review-correctness`, `/review-via-codex`, `/review-algorithm-architecture`, or any other review skill — only `/review-code`:
|
||||
|
||||
```
|
||||
Use the Skill tool: skill: "review-code"
|
||||
```
|
||||
|
||||
After review completes:
|
||||
|
||||
- If **Critical or High** issues found: fix them using the Edit tool, then re-run `pn typecheck` to verify the review fixes don't introduce type errors
|
||||
- If only **Medium or Low** issues: acceptable, continue
|
||||
- If **no issues**: continue
|
||||
|
||||
### 3e. Update PR
|
||||
|
||||
Commit and push only if there are actual changes:
|
||||
|
||||
```bash
|
||||
# Check if there are changes to commit
|
||||
if [ -n "$(git status --porcelain)" ]; then
|
||||
git add -A && git commit -m "fix: address CI failures and review feedback"
|
||||
git push --force-with-lease
|
||||
else
|
||||
echo "NO_CHANGES"
|
||||
fi
|
||||
```
|
||||
|
||||
- If `NO_CHANGES`: No fixes were needed/possible. Exit the fix loop and proceed to Step 4.
|
||||
- If changes were pushed: continue to 3f.
|
||||
|
||||
### 3f. Wait for PR Checks Again
|
||||
|
||||
Use the **Check Polling Procedure** (defined below) to wait for CI checks. Based on the result:
|
||||
|
||||
- `PASSED` → Exit fix loop, proceed to Step 4
|
||||
- `FAILED` → Next iteration of fix loop (back to 3a)
|
||||
- `NO_CHECKS` → Exit fix loop, proceed to Step 4
|
||||
|
||||
If max 3 fix iterations reached, proceed to Step 4 anyway.
|
||||
|
||||
---
|
||||
|
||||
## Step 4: Merge with --admin
|
||||
|
||||
Merge the PR using admin override with squash merge.
|
||||
|
||||
**IMPORTANT: Worktree detection.** When running from a git worktree, `--delete-branch` will fail because `gh` tries to checkout the default branch locally, but it's already checked out in the main worktree. Detect this and handle accordingly:
|
||||
|
||||
```bash
|
||||
# Check if we're in a worktree (not the main working tree)
|
||||
IS_WORKTREE=false
|
||||
if [ "$(git rev-parse --git-dir)" != "$(git rev-parse --git-common-dir)" ]; then
|
||||
IS_WORKTREE=true
|
||||
fi
|
||||
|
||||
BRANCH=$(git branch --show-current)
|
||||
|
||||
if [ "$IS_WORKTREE" = "true" ]; then
|
||||
# In a worktree: merge WITHOUT --delete-branch, then delete remote branch separately
|
||||
gh pr merge --admin --squash
|
||||
# Delete the remote branch manually (local cleanup happens when worktree is removed)
|
||||
git push origin --delete "$BRANCH" 2>/dev/null || true
|
||||
else
|
||||
# Normal repo: use --delete-branch as usual
|
||||
gh pr merge --admin --squash --delete-branch
|
||||
fi
|
||||
```
|
||||
|
||||
- `--admin` bypasses branch protection rules (required reviews, status checks)
|
||||
- `--squash` squashes all commits into one clean commit
|
||||
- `--delete-branch` cleans up the feature branch after merge (skipped in worktrees to avoid checkout conflict)
|
||||
|
||||
If merge fails:
|
||||
|
||||
1. Check the error message
|
||||
2. If the error contains `'master' is already used by worktree` or similar: retry without `--delete-branch` and delete the remote branch manually with `git push origin --delete <branch>`
|
||||
3. If merge conflicts: use Skill tool with `skill: "resolve-conflicts"`, push, then retry merge once
|
||||
4. If other error: report the full error to the user
|
||||
|
||||
---
|
||||
|
||||
## Check Polling Procedure
|
||||
|
||||
This is the shared polling logic used by Step 2 and Step 3f. **Do NOT run this as a single long bash command** — the Bash tool will timeout. Instead, run each poll as a **separate Bash call**.
|
||||
|
||||
**IMPORTANT: Minimize token waste.** Do NOT add verbose commentary between polls. Just run the wait, run the check, and act on the result. No cheerful status messages, no "patience is key", no filler text.
|
||||
|
||||
### Initial wait
|
||||
|
||||
After a push, CI takes time to run. Wait **1 minute** before the first poll:
|
||||
|
||||
```bash
|
||||
echo "Waiting 1 minute for CI checks to run..." && sleep 60
|
||||
```
|
||||
|
||||
**IMPORTANT**: You MUST actually execute the `sleep 60` command and wait for it to complete. Do NOT skip the sleep or claim you waited without running the command. The sleep ensures CI has time to finish.
|
||||
|
||||
**CRITICAL**: When calling the Bash tool for `sleep 60`, you MUST set `timeout: 120000` (2 minutes) on the Bash tool call. Similarly, for `sleep 60` poll interval calls, set `timeout: 120000` (2 minutes).
|
||||
|
||||
### Poll loop (run each poll as a separate Bash call)
|
||||
|
||||
For each poll attempt (1 through 5):
|
||||
|
||||
```bash
|
||||
CHECKS=$(gh pr checks --json name,state,bucket 2>&1)
|
||||
echo "$CHECKS"
|
||||
|
||||
# Parse results using 'bucket' field (pass, fail, pending, skipping, cancel)
|
||||
if echo "$CHECKS" | jq -e 'length == 0' >/dev/null 2>&1; then
|
||||
echo "RESULT:NO_CHECKS"
|
||||
elif echo "$CHECKS" | jq -e '[.[].bucket] | all(. == "pass" or . == "skipping")' >/dev/null 2>&1; then
|
||||
echo "RESULT:PASSED"
|
||||
elif echo "$CHECKS" | jq -e '[.[].bucket] | any(. == "fail" or . == "cancel")' >/dev/null 2>&1; then
|
||||
echo "RESULT:FAILED"
|
||||
else
|
||||
echo "RESULT:PENDING"
|
||||
fi
|
||||
```
|
||||
|
||||
### Decision logic after each poll
|
||||
|
||||
- `RESULT:PASSED` → Return `PASSED`. Stop polling.
|
||||
- `RESULT:FAILED` → Return `FAILED`. Stop polling.
|
||||
- `RESULT:PENDING` → Wait **1 minute**, then poll again (use `timeout: 120000` on the Bash tool call):
|
||||
```bash
|
||||
sleep 60
|
||||
```
|
||||
- `RESULT:NO_CHECKS` → Return `NO_CHECKS` (no CI configured).
|
||||
|
||||
### After 5 polls (timeout)
|
||||
|
||||
If checks are still pending after 5 polls (~6 min total), return `FAILED` to enter the fix loop for investigation.
|
||||
|
||||
### Handling stale checks after re-push (Step 3f only)
|
||||
|
||||
After pushing new commits in Step 3e, old check results may linger briefly. To avoid reading stale results:
|
||||
|
||||
1. Record the latest commit SHA **before pushing**: `OLD_SHA=$(git rev-parse HEAD)`
|
||||
2. After pushing, the first 1-2 polls should verify the check suite is for the **new** commit. If `gh pr checks` still shows the old results (check names match but they completed instantly), wait an extra 30s.
|
||||
3. Alternatively, look for checks with `state: "IN_PROGRESS"` or `"QUEUED"` as a signal that fresh checks have started.
|
||||
|
||||
---
|
||||
|
||||
## Exit Conditions
|
||||
|
||||
- **Success**: PR merged successfully
|
||||
- **Max fix iterations**: After 3 fix loop iterations, attempt merge with --admin regardless
|
||||
- **No changes to fix**: Fix loop produced no changes, merge with --admin
|
||||
- **No CI checks**: Repo has no checks configured, merge immediately
|
||||
- **Unrecoverable error**: PR creation fails, merge fails after retry, or gh CLI issues
|
||||
|
||||
---
|
||||
|
||||
## Progress Tracking
|
||||
|
||||
Display progress after each major step:
|
||||
|
||||
```
|
||||
╔════════════════════════════════════════════════════════════╗
|
||||
║ AUTO PR MERGE ║
|
||||
╠════════════════════════════════════════════════════════════╣
|
||||
║ Step 1 - Create PR: ✅ PR #123 created ║
|
||||
║ Step 2 - Wait for checks: ❌ 2/5 checks failed ║
|
||||
║ Step 3 - Fix Loop: ║
|
||||
║ Iteration 1/3: ║
|
||||
║ Diagnose: ✅ Type errors in 2 files ║
|
||||
║ Fix: ✅ Fixed type errors ║
|
||||
║ Typecheck: ✅ Passed ║
|
||||
║ Review: ✅ No critical issues ║
|
||||
║ Push: ✅ Updated PR ║
|
||||
║ Checks: ✅ All passed ║
|
||||
║ Step 4 - Merge: ✅ Merged with --admin --squash ║
|
||||
╠════════════════════════════════════════════════════════════╣
|
||||
║ PR URL: https://github.com/org/repo/pull/123 ║
|
||||
║ Result: Successfully merged! ║
|
||||
╚════════════════════════════════════════════════════════════╝
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Critical Instructions
|
||||
|
||||
1. **DO NOT ask for user confirmation** - Execute the entire workflow autonomously
|
||||
2. **DO NOT stop after /create-pr** - The PR creation is just step 1 of 4
|
||||
3. **DO NOT stop after checks pass** - Must complete the merge step
|
||||
4. **DO NOT run polling as a single long bash loop** - Each poll must be a separate Bash call to avoid the 2-minute Bash timeout. Use `sleep` in its own Bash call between polls.
|
||||
5. **DO wait the full polling period** - Don't skip check waiting; CI needs time
|
||||
6. **DO run typecheck before review** - Catch type errors early
|
||||
7. **DO run /review-code in each fix iteration** - Ensure code quality
|
||||
8. **DO check for actual changes before committing** - Skip commit/push if `git status --porcelain` is empty
|
||||
9. **DO use --admin for merge** - This is intentional to bypass protection rules
|
||||
10. **DO use --squash for merge** - Keep git history clean
|
||||
11. **DO delete branch after merge** - Clean up with --delete-branch
|
||||
12. **Maximum 3 fix iterations** - Don't loop forever; after 3, merge with --admin
|
||||
13. **DO NOT run /review-correctness or /review-via-codex** - This skill only uses `/review-code` for quick checks in the fix loop. Codex-based reviews are slow and belong in `/auto-review-fix`, not here.
|
||||
14. **DO actually execute sleep commands** - When the skill says `sleep 60`, you MUST run the bash command and wait for it to complete. Do not skip or fabricate the wait.
|
||||
15. **DO set timeout on Bash tool for sleep commands** - `sleep 60` requires `timeout: 120000`. The default 2-minute Bash timeout is sufficient for 1-minute sleeps but set it explicitly for safety.
|
||||
|
||||
---
|
||||
|
||||
## Error Handling
|
||||
|
||||
- If `/create-pr` fails: Report error and exit
|
||||
- If `gh` CLI is not installed or authenticated: Report and exit
|
||||
- If no CI checks are configured: Skip waiting and merge directly
|
||||
- If check polling times out: Enter fix loop to investigate
|
||||
- If typecheck loops more than 3 times within a single fix iteration: Move on to review
|
||||
- If fix loop produces no changes: Exit loop and merge with --admin
|
||||
- If merge fails due to conflicts: Try `/resolve-conflicts`, push, then retry merge once
|
||||
- If merge fails for other reasons: Report the error to the user with the full error output
|
||||
|
|
@ -1,650 +0,0 @@
|
|||
---
|
||||
name: auto-review-fix
|
||||
description: Automated iterative code review and fix loop with parallel review agents
|
||||
---
|
||||
|
||||
# auto-review-fix
|
||||
|
||||
Automated iterative code review and fix loop. Reviews all code changes on the current branch since it diverged from main, including uncommitted changes (staged and unstaged). Validates findings, fixes issues, and repeats until clean or max iterations reached.
|
||||
|
||||
**IMPORTANT**: Execute this entire process autonomously without asking the user for confirmation at any step. Just do the iterations.
|
||||
|
||||
---
|
||||
|
||||
## Process Overview
|
||||
|
||||
Run up to **4 iterations** of the review-validate-fix cycle. Stop early ONLY when a **follow-up review confirms no issues to fix** remain.
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────┐
|
||||
│ ITERATION LOOP (max 4 rounds) │
|
||||
│ ┌─────────────────────────────────────────────────────┐ │
|
||||
│ │ 0. SETUP PHASE (first iteration only) │ │
|
||||
│ │ - Fetch diff and create 00-review-context.md │ │
|
||||
│ │ - Categorize files by review area │ │
|
||||
│ │ - Deduplicate file assignments │ │
|
||||
│ └─────────────────────────────────────────────────────┘ │
|
||||
│ ↓ │
|
||||
│ ┌─────────────────────────────────────────────────────┐ │
|
||||
│ │ 1. PARALLEL REVIEW PHASE │ │
|
||||
│ │ - Spawn Task() subagents for each review area │ │
|
||||
│ │ - Each receives ONLY their relevant files │ │
|
||||
│ │ - All reference shared 00-review-context.md │ │
|
||||
│ └─────────────────────────────────────────────────────┘ │
|
||||
│ ↓ │
|
||||
│ ┌─────────────────────────────────────────────────────┐ │
|
||||
│ │ 2. COMBINE & DEDUPLICATE │ │
|
||||
│ │ - Aggregate all findings │ │
|
||||
│ │ - Deduplicate across review areas │ │
|
||||
│ │ - Exclude previously-skipped issues │ │
|
||||
│ └─────────────────────────────────────────────────────┘ │
|
||||
│ ↓ │
|
||||
│ ┌─────────────────────────────────────────────────────┐ │
|
||||
│ │ 3. VALIDATE PHASE (all severities) │ │
|
||||
│ │ - Validate all issues (Critical to Low) │ │
|
||||
│ │ - Skip issues already in "Skipped Issues" list │ │
|
||||
│ │ - Group by file, one agent per file │ │
|
||||
│ └─────────────────────────────────────────────────────┘ │
|
||||
│ ↓ │
|
||||
│ ┌─────────────────────────────────────────────────────┐ │
|
||||
│ │ 4. FIX PHASE │ │
|
||||
│ │ - Group issues by file (up to 5 per agent) │ │
|
||||
│ │ - Each fix done via Task() with opus │ │
|
||||
│ └─────────────────────────────────────────────────────┘ │
|
||||
│ ↓ │
|
||||
│ ┌─────────────────────────────────────────────────────┐ │
|
||||
│ │ 5. CHECK EXIT CONDITIONS │ │
|
||||
│ │ - Fixed issues? → MUST run another review │ │
|
||||
│ │ - Review shows no issues? → EXIT │ │
|
||||
│ │ - Iteration 4 reached? → EXIT │ │
|
||||
│ └─────────────────────────────────────────────────────┘ │
|
||||
└─────────────────────────────────────────────────────────────┘
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Phase 0: Setup (First Iteration Only)
|
||||
|
||||
### Step 1: Fetch the diff
|
||||
|
||||
```bash
|
||||
# Get the merge base (where the branch diverged from main)
|
||||
git diff $(git merge-base origin/main HEAD)
|
||||
```
|
||||
|
||||
### Step 1.5: Check diff size
|
||||
|
||||
```bash
|
||||
# Count changed files
|
||||
git diff $(git merge-base origin/main HEAD) --stat | tail -1
|
||||
# If >500 files changed, exit with recommendation to split PR
|
||||
```
|
||||
|
||||
If the diff contains more than 500 changed files, **EXIT immediately** with:
|
||||
|
||||
```
|
||||
❌ Diff too large (>500 files). Please split this PR into smaller, focused changes.
|
||||
```
|
||||
|
||||
### Step 2: Create shared context file
|
||||
|
||||
Create `00-review-context.md` in the working directory (see below for format).
|
||||
|
||||
### Step 2.5: Extract changed line ranges
|
||||
|
||||
Parse `git diff $(git merge-base origin/main HEAD) --unified=0` to extract line ranges (`@@ +A,B @@` → lines A to A+B-1). Add to context file below.
|
||||
|
||||
### Step 2 (continued): Context file format
|
||||
|
||||
Create `00-review-context.md` in the working directory with:
|
||||
|
||||
```markdown
|
||||
# Review Context
|
||||
|
||||
## Branch Info
|
||||
|
||||
- Base: origin/main
|
||||
- Current: [branch name]
|
||||
|
||||
## Changed Files Summary
|
||||
|
||||
[List all changed files with their change type: A/M/D]
|
||||
|
||||
## Changed Line Ranges (PR Scope)
|
||||
|
||||
<!-- In scope: issues on these lines OR caused by these changes. Out of scope: unrelated pre-existing issues -->
|
||||
|
||||
| File | Changed Lines |
|
||||
| ------------------ | ---------------- |
|
||||
| [path/to/file1.ts] | [45-67, 120-135] |
|
||||
| [path/to/file2.ts] | [10-25] |
|
||||
|
||||
## Review Standards Reference
|
||||
|
||||
- Follow /review-code standards
|
||||
- Focus on: correctness, security, performance, maintainability
|
||||
- Priority levels: Critical > High > Medium > Low
|
||||
|
||||
## File Categories
|
||||
|
||||
[Categorized list - see below]
|
||||
|
||||
## Skipped Issues (Do Not Re-validate)
|
||||
|
||||
<!-- Issues validated but deemed not worth fixing. Do not re-validate these in future iterations. -->
|
||||
<!-- Format: [file:line-range] | [severity] | [reason skipped] | [issue summary] -->
|
||||
<!-- NOTE: Skips should be RARE - only purely cosmetic issues with no functional impact -->
|
||||
|
||||
[Initially empty - populated during validation phase]
|
||||
|
||||
## Iteration State
|
||||
|
||||
<!-- Updated after each phase to enable crash recovery -->
|
||||
|
||||
Current iteration: 1
|
||||
Last completed phase: Setup
|
||||
Files fixed this iteration: []
|
||||
```
|
||||
|
||||
**IMPORTANT**: The "Skipped Issues" section persists across iterations. However, skips should be RARE - only purely cosmetic issues (naming, JSDoc, import ordering) may be skipped. Functional issues like error handling, type safety, and performance must ALWAYS be fixed.
|
||||
|
||||
### Step 3: Categorize and deduplicate files
|
||||
|
||||
Assign each file to **exactly ONE** file category (no duplicates):
|
||||
|
||||
| Priority | Category | File Patterns |
|
||||
| -------- | -------------- | ---------------------------------------------------------------- |
|
||||
| 1 | Electron/Main | `src/main/`, `src/preload/`, `electron.*` |
|
||||
| 2 | Backend/IPC | `*ipc*`, `*handler*`, `*service*` (in main process) |
|
||||
| 3 | Frontend/UI | `src/renderer/`, `components/`, `*.tsx`, `*.css` |
|
||||
| 4 | Config/Build | `*.config.*`, `package.json`, `tsconfig.*`, `electron-builder.*` |
|
||||
| 5 | Utility/Common | Everything else |
|
||||
|
||||
**Deduplication rule**: A file belongs to the FIRST matching category only.
|
||||
|
||||
**Tiebreaker rule**: If a file path matches multiple categories at different directory depths:
|
||||
|
||||
1. Use the **deepest matching directory** (e.g., `src/main/services/ui/dialog.ts` → Backend/IPC because `services/` is deeper than `main/`)
|
||||
2. If same depth, use priority order (lower number wins)
|
||||
|
||||
### Step 4: Map categories to review commands
|
||||
|
||||
Each file category gets reviewed by multiple specialized review commands:
|
||||
|
||||
| Category | Review Commands |
|
||||
| -------------- | ------------------------------------------------ |
|
||||
| Electron/Main | `/review-code`, `/review-algorithm-architecture` |
|
||||
| Backend/IPC | `/review-code`, `/review-algorithm-architecture` |
|
||||
| Frontend/UI | `/review-code`, `/review-algorithm-architecture` |
|
||||
| Config/Build | `/review-code` |
|
||||
| Utility/Common | `/review-code`, `/review-algorithm-architecture` |
|
||||
|
||||
**Total review agents**: (Categories with files) × (Applicable review commands per category)
|
||||
|
||||
---
|
||||
|
||||
## Phase 1: Parallel Review
|
||||
|
||||
Spawn review subagents using the Task tool. **CRITICAL**: Use the exact Task() definition format.
|
||||
|
||||
### Review Command Descriptions
|
||||
|
||||
| Command | Focus | Engine |
|
||||
| -------------------------------- | ------------------------------------------------------------------------ | ------ |
|
||||
| `/review-code` | Logical bugs, security issues, TypeScript best practices, error handling | Claude |
|
||||
| `/review-algorithm-architecture` | File organization, module boundaries, performance on hot paths | Claude |
|
||||
|
||||
### Spawn agents by category × review command
|
||||
|
||||
For each (category, review_command) pair where files exist, spawn a Task:
|
||||
|
||||
```
|
||||
Task(
|
||||
subagent_type: "general-purpose",
|
||||
model: "opus", // REQUIRED: opus for quality reviews
|
||||
description: "[REVIEW_COMMAND] for [CATEGORY] files",
|
||||
prompt: """
|
||||
Read 00-review-context.md for branch context and changed line ranges.
|
||||
|
||||
You are running [REVIEW_COMMAND] on [CATEGORY] files.
|
||||
|
||||
Review ONLY these files:
|
||||
[SPECIFIC FILE LIST FOR THIS CATEGORY]
|
||||
|
||||
**SCOPE RESTRICTION**:
|
||||
Report issues on changed lines OR caused by the changes (e.g., broken callers, type mismatches).
|
||||
Do NOT report pre-existing issues unrelated to this PR.
|
||||
For issues outside changed lines, include a "Causal Link" explaining the connection.
|
||||
|
||||
Changed line ranges: [FROM 00-review-context.md]
|
||||
|
||||
Follow the [REVIEW_COMMAND] standards exactly. Focus on issues relevant to that review type.
|
||||
|
||||
Output format for each finding:
|
||||
- File: path/to/file.ts
|
||||
- Line: 42
|
||||
- Severity: Critical|High|Medium|Low
|
||||
- Review Type: [REVIEW_COMMAND]
|
||||
- Causal Link: [required if outside changed lines]
|
||||
- Issue: [description]
|
||||
- Fix: [suggested fix]
|
||||
"""
|
||||
)
|
||||
```
|
||||
|
||||
### Example agent spawning for a PR with Electron, Backend, and Frontend files:
|
||||
|
||||
```
|
||||
# Electron/Main files → 2 agents
|
||||
Task(..., description: "/review-code for Electron/Main files", ...)
|
||||
Task(..., description: "/review-algorithm-architecture for Electron/Main files", ...)
|
||||
|
||||
# Backend/IPC files → 2 agents
|
||||
Task(..., description: "/review-code for Backend/IPC files", ...)
|
||||
Task(..., description: "/review-algorithm-architecture for Backend/IPC files", ...)
|
||||
|
||||
# Frontend/UI files → 2 agents
|
||||
Task(..., description: "/review-code for Frontend/UI files", ...)
|
||||
Task(..., description: "/review-algorithm-architecture for Frontend/UI files", ...)
|
||||
|
||||
Total: 6 parallel review agents
|
||||
```
|
||||
|
||||
**Launch ALL Task() calls in a single message block for true parallelism.**
|
||||
|
||||
**IMPORTANT — WAIT FOR ALL AGENTS**:
|
||||
|
||||
- Do NOT use `run_in_background: true` for any Phase 1 review agent.
|
||||
- Task() calls without `run_in_background` are blocking by default — they return only when the subagent finishes.
|
||||
- **VERIFICATION CHECKPOINT**: Before starting Phase 2, you MUST count the number of Task() results you received. It MUST equal the total number of Task() calls you made. If any result is missing, DO NOT proceed — wait or re-spawn the missing agent.
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: Combine & Deduplicate Results
|
||||
|
||||
**⛔ GATE CHECK**: Before starting Phase 2, verify you received results from ALL Task() agents spawned in Phase 1. Count: [N agents] = [total]. If any agent result is missing, DO NOT proceed. Log which agent(s) are missing and note it in the final report.
|
||||
|
||||
1. **Collect all subagent outputs** (from all review types)
|
||||
|
||||
2. **Scope filter**: Discard findings outside changed lines that lack a Causal Link. Pass others to validation.
|
||||
|
||||
3. **Deduplicate findings** across review types and categories:
|
||||
- Same file + same line range (within 5 lines) + similar issue = duplicate
|
||||
- Keep the more specific/actionable finding
|
||||
- Merge severity (take highest)
|
||||
- Preserve the review type that caught the issue
|
||||
|
||||
4. **Exclude previously-skipped issues**:
|
||||
- Read the "Skipped Issues" section from `00-review-context.md`
|
||||
- For each new finding, check if it matches a skipped issue (same file + overlapping line range + similar description)
|
||||
- If match found → exclude from validation (already determined not worth fixing)
|
||||
- This prevents re-validating the same low-priority issues every iteration
|
||||
|
||||
5. **Categorize remaining issues**:
|
||||
- 🔴 **Critical** - Must fix
|
||||
- 🟠 **High** - Should fix
|
||||
- 🟡 **Medium** - Consider fixing
|
||||
- 🟢 **Low** - Nice to have
|
||||
|
||||
6. **Create findings report** with all non-skipped issues (all severities), including which review type found each issue
|
||||
|
||||
7. **Severity escalation** (iterations 2+):
|
||||
- If an issue was marked "✅ Fix" in iteration N but reappears in iteration N+1:
|
||||
- Escalate severity by one level: Low → Medium → High → Critical
|
||||
- Add note: "⚠️ Persisted from iteration N - previous fix may have been incomplete"
|
||||
- This ensures recurring issues get higher priority attention
|
||||
|
||||
---
|
||||
|
||||
## Phase 3: Validation (All Severities)
|
||||
|
||||
Validate all issues that weren't excluded by the "Skipped Issues" list.
|
||||
|
||||
### Grouping strategy:
|
||||
|
||||
- Group findings by file
|
||||
- One validation agent per file (handles all findings for that file)
|
||||
- Maximum 10 validation agents total
|
||||
|
||||
```
|
||||
Task(
|
||||
subagent_type: "general-purpose",
|
||||
model: "opus", // REQUIRED: opus for accurate validation
|
||||
description: "Validate findings in [filename]",
|
||||
prompt: """
|
||||
Validate these findings for [FILE PATH]:
|
||||
|
||||
[LIST OF 1-N FINDINGS FOR THIS FILE]
|
||||
|
||||
**Changed line ranges**: [FROM 00-review-context.md]
|
||||
|
||||
**SCOPE CHECK (first step)**: Issues on changed lines are in scope. For issues outside changed lines, validate the Causal Link - reject as 🚫 Out of Scope if it's just pre-existing tech debt unrelated to PR changes.
|
||||
|
||||
For each IN-SCOPE finding:
|
||||
1. Read the actual code at the reported location
|
||||
2. Verify the issue actually exists
|
||||
3. Check if the suggested fix is correct
|
||||
4. Determine if it's worth fixing
|
||||
|
||||
Mark each as:
|
||||
- ✅ **Fix** - Valid issue in changed code (default - when in doubt, fix it)
|
||||
- ⏭️ **Skip** - Valid but genuinely not worth fixing (RARE - see strict criteria below)
|
||||
- ❌ **False Positive** - Issue doesn't actually exist
|
||||
- 🚫 **Out of Scope** - Issue exists but is in pre-existing code not changed by this PR
|
||||
|
||||
**STRICT Skip Criteria** - Only skip if ALL of these are true:
|
||||
1. The issue is purely cosmetic/stylistic with zero functional impact
|
||||
2. Fixing it would require changing >50 lines of unrelated code
|
||||
3. The pattern is consistently used throughout the codebase (not just this file)
|
||||
|
||||
**DO NOT SKIP these issues (always fix):**
|
||||
- ❌ Silent error swallowing or empty catch blocks
|
||||
- ❌ Type coercion hacks (e.g., `as unknown as X`, `as any`)
|
||||
- ❌ Missing error handling or error messages
|
||||
- ❌ Redundant code, dead code, or unused variables
|
||||
- ❌ Security issues of any severity
|
||||
- ❌ Race conditions or async issues
|
||||
- ❌ Memory leaks or resource cleanup issues
|
||||
- ❌ Any issue in NEW code (code added in this PR)
|
||||
- ❌ IPC security issues (missing validation, exposed handlers)
|
||||
|
||||
Examples of what CAN be skipped:
|
||||
- Pure naming preferences when existing codebase uses the same style
|
||||
- Adding JSDoc to functions that are already self-documenting
|
||||
- Reordering imports when not violating any lint rules
|
||||
|
||||
For each Skip decision, provide:
|
||||
- File and line range
|
||||
- Severity level
|
||||
- Reason for skipping (must match one of the allowed skip criteria above)
|
||||
- Brief issue summary
|
||||
"""
|
||||
)
|
||||
```
|
||||
|
||||
### After validation: Update Skipped Issues
|
||||
|
||||
For any issues marked as ⏭️ **Skip**, append them to the "Skipped Issues" section in `00-review-context.md`:
|
||||
|
||||
```markdown
|
||||
## Skipped Issues (Do Not Re-validate)
|
||||
|
||||
<!-- Format: [file:line-range] | [severity] | [reason skipped] | [issue summary] -->
|
||||
|
||||
src/utils/helper.ts:42-45 | Low | Stylistic preference | Variable naming convention
|
||||
```
|
||||
|
||||
**This ensures the same issues are not re-validated in subsequent iterations.**
|
||||
|
||||
**Out of Scope** issues are discarded (not tracked). Issues outside changed lines CAN be in scope if caused by PR changes.
|
||||
|
||||
### Early exit check
|
||||
|
||||
After validation completes, check if there are any actionable issues:
|
||||
|
||||
```
|
||||
IF (all_issues_marked_FP_or_Skip):
|
||||
→ Update Skipped Issues list in 00-review-context.md
|
||||
→ Update Iteration State: "Last completed phase: Validation (no actionable issues)"
|
||||
→ EXIT - No actionable issues found
|
||||
→ Do NOT proceed to Phase 4
|
||||
```
|
||||
|
||||
This prevents spawning fix agents when there's nothing to fix.
|
||||
|
||||
---
|
||||
|
||||
## Phase 4: Fix
|
||||
|
||||
**CRITICAL**: All fixes MUST be done via Task() subagents with opus. No direct edits.
|
||||
|
||||
### Step 1: Commit uncommitted changes first
|
||||
|
||||
```bash
|
||||
git status --porcelain
|
||||
# If output exists:
|
||||
git add -A && git commit -m "WIP: Changes before auto-review fixes"
|
||||
```
|
||||
|
||||
### Step 2: Build the fix manifest from validation output
|
||||
|
||||
**MANDATORY**: Create an explicit mapping from validation results to fix actions.
|
||||
|
||||
For every issue marked "✅ Fix" in validation:
|
||||
|
||||
1. Record: `[file] → [issue description] → [suggested fix]`
|
||||
2. Group by file
|
||||
|
||||
**Traceability requirement**: Every "✅ Fix" issue MUST appear in the fix manifest. Do not:
|
||||
|
||||
- Skip issues because you believe a different fix "solves the root cause"
|
||||
- Substitute your judgment for what validation explicitly said to fix
|
||||
- Rationalize away issues as "will be handled by another fix"
|
||||
|
||||
If validation says "✅ Fix" for file X, spawn a fix agent for file X. Period.
|
||||
|
||||
### Step 3: Group validated issues by file
|
||||
|
||||
- Group up to **5 issues per fix agent** (within same file)
|
||||
- If a file has >5 issues, split into multiple agents
|
||||
- **Maximum 10 fix agents** per iteration
|
||||
- If >10 files need fixes, prioritize by severity (Critical > High > Medium > Low)
|
||||
- Defer lower-priority files to the next iteration
|
||||
|
||||
### Conflict prevention
|
||||
|
||||
To avoid race conditions when multiple agents edit overlapping code:
|
||||
|
||||
- Each file should have **at most ONE fix agent running at a time**
|
||||
- If a file has >5 issues requiring multiple agents, spawn them **sequentially** (not in parallel)
|
||||
- After each fix agent completes for a multi-agent file, verify the file is syntactically valid before spawning the next
|
||||
- Update Iteration State after each agent completes: `Files fixed this iteration: [updated list]`
|
||||
|
||||
### Step 4: Spawn fix subagents
|
||||
|
||||
```
|
||||
Task(
|
||||
subagent_type: "general-purpose",
|
||||
model: "opus", // REQUIRED: opus for quality fixes
|
||||
description: "Fix issues in [filename]",
|
||||
prompt: """
|
||||
Fix the following validated issues in [FILE PATH]:
|
||||
|
||||
Issue 1: [description]
|
||||
- Line: XX
|
||||
- Fix: [suggested fix]
|
||||
|
||||
Issue 2: [description]
|
||||
- Line: YY
|
||||
- Fix: [suggested fix]
|
||||
|
||||
[Up to 5 issues]
|
||||
|
||||
Instructions:
|
||||
1. Read the file first
|
||||
2. Apply each fix using the Edit tool
|
||||
3. Verify fixes don't break syntax
|
||||
4. Report what was fixed
|
||||
"""
|
||||
)
|
||||
```
|
||||
|
||||
### Step 5: Track changes
|
||||
|
||||
Record all files modified for the iteration summary.
|
||||
|
||||
### Step 5.5: Verify fixes don't break build
|
||||
|
||||
After all fix agents complete, run a type check:
|
||||
|
||||
```bash
|
||||
npm run typecheck 2>&1
|
||||
```
|
||||
|
||||
If the type check fails:
|
||||
|
||||
- Log which fix agent(s) likely caused the failure
|
||||
- Continue to Phase 5 - the next iteration's review will catch the type errors as new issues
|
||||
- Update Iteration State: `Build status: FAILED (type errors in [files])`
|
||||
|
||||
### Step 6: Verify fix manifest completeness
|
||||
|
||||
Before proceeding to Phase 5, verify:
|
||||
|
||||
- Every file in the fix manifest had a fix agent spawned
|
||||
- Every "✅ Fix" issue from validation was addressed by a fix agent
|
||||
- No issues were skipped due to assumptions about "root cause" fixes elsewhere
|
||||
|
||||
If any "✅ Fix" issue was not addressed, spawn additional fix agents now.
|
||||
|
||||
---
|
||||
|
||||
## Phase 5: Exit Conditions
|
||||
|
||||
**CRITICAL LOGIC**: You cannot exit with "no issues" immediately after fixing. Must confirm with another review.
|
||||
|
||||
```
|
||||
IF (fixes_applied_this_iteration > 0):
|
||||
→ MUST run another iteration to confirm fixes work
|
||||
→ Cannot claim "no issues" without verification
|
||||
|
||||
IF (review_found_no_issues_to_fix AND fixes_applied_this_iteration == 0):
|
||||
→ EXIT - Clean (all new findings either fixed or added to Skipped Issues)
|
||||
|
||||
IF (iteration >= 4):
|
||||
→ EXIT - Max iterations reached
|
||||
|
||||
IF (same_issues_persist_for_2_iterations):
|
||||
→ EXIT - Stuck in loop
|
||||
|
||||
OTHERWISE:
|
||||
→ Continue to next iteration
|
||||
```
|
||||
|
||||
**The key insight**: "I fixed all issues" ≠ "There are no issues". Another review round must confirm.
|
||||
|
||||
**Note on convergence**: The loop converges because:
|
||||
|
||||
1. Fixed issues no longer appear in subsequent reviews
|
||||
2. Skipped issues are tracked and excluded from future validation
|
||||
3. Only genuinely new issues trigger additional work
|
||||
|
||||
---
|
||||
|
||||
## Iteration Tracking
|
||||
|
||||
Display progress after each phase:
|
||||
|
||||
```
|
||||
╔════════════════════════════════════════════════════════════╗
|
||||
║ ITERATION 1/4 ║
|
||||
╠════════════════════════════════════════════════════════════╣
|
||||
║ Phase 0 - Setup: ✅ Context file created ║
|
||||
║ Phase 1 - Review: ✅ 6 agents across 2 review types ║
|
||||
║ /review-code: 3 agents (all categories) ║
|
||||
║ /review-algorithm-architecture: 3 agents ║
|
||||
║ Phase 2 - Combine: ✅ 12 findings (3 out-of-scope, 0 skipped) ║
|
||||
║ Phase 3 - Validate: ✅ 8 fix, 2 skip, 1 out-of-scope, 1 FP ║
|
||||
║ Phase 4 - Fix: ✅ 8 issues fixed (opus) ║
|
||||
║ Phase 5 - Check: 🔁 Fixes applied → verify next round ║
|
||||
║ Skipped Issues: 📝 2 added to tracking list ║
|
||||
╚════════════════════════════════════════════════════════════╝
|
||||
→ Starting iteration 2 to verify fixes...
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Final Report
|
||||
|
||||
```
|
||||
╔════════════════════════════════════════════════════════════╗
|
||||
║ AUTO REVIEW-FIX COMPLETE ║
|
||||
╠════════════════════════════════════════════════════════════╣
|
||||
║ Iterations completed: X/4 ║
|
||||
║ Exit reason: [Clean review | Max iterations | Loop stuck] ║
|
||||
║ Total issues found: XX ║
|
||||
║ Out of scope: XX (pre-existing code, not in PR) ║
|
||||
║ Issues fixed: XX ║
|
||||
║ Issues skipped: XX (tracked, won't revalidate) ║
|
||||
║ Issues remaining: XX ║
|
||||
╠════════════════════════════════════════════════════════════╣
|
||||
║ Review agents by type: ║
|
||||
║ /review-code: X agents ║
|
||||
║ /review-algorithm-architecture: X agents ║
|
||||
║ Fix agents used: X (opus) ║
|
||||
╠════════════════════════════════════════════════════════════╣
|
||||
║ Issues found by review type: ║
|
||||
║ /review-code: XX issues ║
|
||||
║ /review-algorithm-architecture: XX issues ║
|
||||
╠════════════════════════════════════════════════════════════╣
|
||||
║ Files modified: ║
|
||||
║ - path/to/file1.ts ║
|
||||
║ - path/to/file2.ts ║
|
||||
╠════════════════════════════════════════════════════════════╣
|
||||
║ Skipped issues (should be RARE - cosmetic only): ║
|
||||
║ ⏭️ [file:line] Reason - Description ║
|
||||
╠════════════════════════════════════════════════════════════╣
|
||||
║ Remaining issues (if any): ║
|
||||
║ 🔴 [file:line] Description ║
|
||||
╚════════════════════════════════════════════════════════════╝
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Cleanup
|
||||
|
||||
After generating the final report, clean up the context file:
|
||||
|
||||
```bash
|
||||
# Create .context directory if it doesn't exist
|
||||
mkdir -p .context
|
||||
|
||||
# Archive the context file with timestamp for debugging/audit
|
||||
mv 00-review-context.md .context/auto-review-$(date +%Y%m%d-%H%M%S).md
|
||||
|
||||
# Optional: Keep only the last 5 archived reviews to prevent accumulation
|
||||
ls -t .context/auto-review-*.md 2>/dev/null | tail -n +6 | xargs rm -f 2>/dev/null
|
||||
```
|
||||
|
||||
**Important**: Clean up even on error - wrap the cleanup in the error handling flow.
|
||||
|
||||
---
|
||||
|
||||
## Critical Instructions
|
||||
|
||||
1. **DO NOT ask for user confirmation** - Execute autonomously
|
||||
2. **DO NOT skip the verification review** - After fixes, another review MUST confirm
|
||||
3. **DO NOT skip validated issues** - Every "✅ Fix" from validation MUST have a fix agent spawned for that file. No exceptions, no rationalization.
|
||||
4. **DO use Task() for ALL review/validate/fix work** - Never do these inline
|
||||
5. **DO use opus for all agents** - Quality matters
|
||||
6. **DO pass only relevant files to each area** - Never the full diff
|
||||
7. **DO deduplicate files across areas** - Each file to one area only
|
||||
8. **DO validate all severities** - But skip issues already in "Skipped Issues" list
|
||||
9. **DO update Skipped Issues** - When validator says "not worth fixing", add to list
|
||||
10. **DO group fixes by file** - Up to 5 issues per fix agent
|
||||
11. **DO clean up 00-review-context.md** when done
|
||||
12. **BE STRICT about skipping** - Skip should be RARE. If in doubt, fix it. Silent error swallowing, type hacks, and redundant code should NEVER be skipped.
|
||||
13. **DO set agent timeouts** - Review agents: 7min, Validation agents: 3min, Fix agents: 5min. If an agent times out, log the timeout and continue.
|
||||
14. **DO update Iteration State** - After each phase completes, update the Iteration State section in 00-review-context.md to enable crash recovery
|
||||
|
||||
---
|
||||
|
||||
## Token Efficiency Summary
|
||||
|
||||
| Before | After | Improvement |
|
||||
| -------------------------- | ----------------------------------- | --------------------------- |
|
||||
| Full diff to all agents | Relevant files only per category | ~6x reduction |
|
||||
| Generic review for all | Specialized review commands | Better issue coverage |
|
||||
| Re-validate skipped issues | Track in Skipped Issues list | ~2x reduction per iteration |
|
||||
| 1 agent per finding | 1 agent per file (up to 5 issues) | ~5x reduction |
|
||||
| No deduplication | Files assigned to one category only | ~2x reduction |
|
||||
|
||||
---
|
||||
|
||||
## Error Handling
|
||||
|
||||
- If a subagent fails, log the error and continue with others
|
||||
- If the diff is too large (>500 files), split into batches
|
||||
- If fixes cause build failures, revert and report
|
||||
- If stuck in a loop (same issues persist for 2 iterations), exit with report
|
||||
- Clean up 00-review-context.md even on error
|
||||
|
|
@ -1,48 +0,0 @@
|
|||
---
|
||||
name: auto-submit
|
||||
description: End-to-end autonomous pipeline that runs auto-review-fix, then auto-pr-merge
|
||||
---
|
||||
|
||||
# auto-submit
|
||||
|
||||
Autonomous pipeline: review+fix code, then create PR and merge. **Execute without user confirmation.**
|
||||
|
||||
## Steps
|
||||
|
||||
### 1. Auto Review-Fix
|
||||
|
||||
**IMPORTANT**: Run auto-review-fix as a sub-agent (not an inline skill) to ensure it gets its own isolated context window. This prevents instruction dilution and ensures fix phases properly spawn their own Task() sub-agents as required.
|
||||
|
||||
```
|
||||
Agent(
|
||||
subagent_type: "general-purpose",
|
||||
description: "Run auto-review-fix",
|
||||
prompt: "Run the /auto-review-fix skill. Follow ALL instructions exactly, especially: all fixes MUST be done via Task() subagents with opus-4-5. No direct edits."
|
||||
)
|
||||
```
|
||||
|
||||
Wait for the agent to complete. Then commit any changes:
|
||||
|
||||
```bash
|
||||
if [ -n "$(git status --porcelain)" ]; then
|
||||
git add -A && git commit -m "fix: address auto-review findings"
|
||||
fi
|
||||
```
|
||||
|
||||
**Continue to Step 2 — do not stop here.**
|
||||
|
||||
### 2. Auto PR-Merge
|
||||
|
||||
```
|
||||
Use the Skill tool: skill: "auto-pr-merge"
|
||||
```
|
||||
|
||||
Handles PR creation, CI polling, fix loops, and merge with `--admin --squash`.
|
||||
|
||||
## Rules
|
||||
|
||||
- Execute autonomously — no user confirmation
|
||||
- Both steps run sequentially — do not stop between them
|
||||
- Commit changes between steps
|
||||
- If Step 1 fails catastrophically, stop (don't create a broken PR)
|
||||
- If Step 2 fails, report the error with PR URL if available
|
||||
|
|
@ -1,310 +0,0 @@
|
|||
---
|
||||
name: electron
|
||||
description: Launch, automate, and validate Electron desktop apps using playwright-cli via Chrome DevTools Protocol. Use this skill to validate UI changes in Orca, test features in the running Electron app, verify code fixes work end-to-end, or automate any Electron app (VS Code, Slack, Discord, etc.). Triggers include "validate in Electron", "test in the app", "verify the fix", "check the UI", "/electron", "automate Slack app", "control VS Code", or any task requiring interaction with a running Electron application.
|
||||
allowed-tools: Bash(playwright-cli:*), Bash(npx playwright-cli:*), Bash(curl:*), Bash(lsof:*), Bash(open:*), Bash(ps:*), Bash(kill:*), Bash(node:*), Bash(pnpm:*), Read, Grep, Monitor
|
||||
---
|
||||
|
||||
# Electron App Automation
|
||||
|
||||
Automate any Electron desktop app using playwright-cli's CDP attach mode. Electron apps are built on Chromium and expose a Chrome DevTools Protocol (CDP) port that playwright-cli can connect to, enabling the same snapshot-interact workflow used for web pages.
|
||||
|
||||
## Critical Safety Rule: Never Kill Processes You Didn't Start
|
||||
|
||||
**You may be running inside an Electron app (e.g., Orca).** Killing the wrong process will terminate your own session.
|
||||
|
||||
- **NEVER** run `killall Electron`, `pkill Electron`, or any broad process-killing command.
|
||||
- **NEVER** kill a process unless you launched it yourself in this session and you recorded its PID.
|
||||
- Before killing, **always verify** the PID belongs to the process you started — check the command line includes the workspace path or args you used to launch it.
|
||||
- When quitting apps to relaunch with `--remote-debugging-port`, use `osascript -e 'quit app "AppName"'` for named apps (Slack, VS Code, etc.) — **never for Orca or the app you're running inside**.
|
||||
- If unsure whether a process is safe to kill, **ask the user**.
|
||||
|
||||
## Core Workflow
|
||||
|
||||
1. **Launch** the Electron app with remote debugging enabled (or find an already-running app with CDP)
|
||||
2. **Attach** playwright-cli to the CDP endpoint
|
||||
3. **Snapshot** to discover interactive elements
|
||||
4. **Interact** using element refs
|
||||
5. **Re-snapshot** after navigation or state changes
|
||||
|
||||
```bash
|
||||
# Launch an Electron app with remote debugging
|
||||
open -a "Slack" --args --remote-debugging-port=9222
|
||||
|
||||
# Wait for the app to initialize
|
||||
sleep 3
|
||||
|
||||
# Attach playwright-cli to the app via CDP
|
||||
command playwright-cli attach --cdp="http://localhost:9222"
|
||||
|
||||
# Standard workflow from here
|
||||
command playwright-cli snapshot
|
||||
command playwright-cli click e5
|
||||
command playwright-cli screenshot
|
||||
```
|
||||
|
||||
## Always prefix with `command`
|
||||
|
||||
Use `command playwright-cli …` to bypass shell aliases (e.g. `alias playwright-cli='playwright-cli --persistent'`) that leak flags into subcommands. Behaves identically when no alias is set. All examples below use this form.
|
||||
|
||||
## Pick a free CDP port
|
||||
|
||||
Port 9333 is a convention, not a guarantee — another worktree may hold it, and electron-vite will start without a debugger, letting you silently attach to the old app.
|
||||
|
||||
```bash
|
||||
for p in 9333 9334 9335 9336 9337 9338 9339 9340; do
|
||||
if ! lsof -i :$p >/dev/null 2>&1; then PORT=$p; break; fi
|
||||
done
|
||||
# After attach, verify you hit the right worktree:
|
||||
command playwright-cli eval "window.__store.getState().worktreesByRepo"
|
||||
```
|
||||
|
||||
## Launching Orca Dev Build with CDP
|
||||
|
||||
Orca uses electron-vite for dev builds. The correct way to launch with CDP:
|
||||
|
||||
```bash
|
||||
# Launch with remote debugging (PORT picked above)
|
||||
node config/scripts/run-electron-vite-dev.mjs --remote-debugging-port=$PORT 2>&1 &
|
||||
|
||||
# Wait for "DevTools listening on ws://..." in output, then attach
|
||||
command playwright-cli attach --cdp="http://localhost:$PORT"
|
||||
```
|
||||
|
||||
**Key details:**
|
||||
- Pass `--remote-debugging-port=NNNN` directly to the script — do NOT use `pnpm run dev -- --` (the double `--` breaks Chromium flag parsing)
|
||||
- electron-vite also supports `REMOTE_DEBUGGING_PORT` env var: `REMOTE_DEBUGGING_PORT=$PORT pnpm run dev`
|
||||
- The Zustand store is exposed at `window.__store` — use `window.__store.getState()` and `window.__store.getState().someAction()` to read/mutate state
|
||||
- Use port 9333 (not 9222) to avoid conflicts with other Electron apps
|
||||
|
||||
### Accessing Orca State via eval
|
||||
|
||||
```bash
|
||||
# Read store state
|
||||
command playwright-cli eval "(() => { const s = window.__store?.getState(); return JSON.stringify({ activeWorktreeId: s.activeWorktreeId, activeTabId: s.activeTabId, activeFileId: s.activeFileId, activeTabType: s.activeTabType }); })()"
|
||||
|
||||
# Open an editor file
|
||||
command playwright-cli eval "(() => { const s = window.__store?.getState(); const wtId = s.activeWorktreeId; s.openFile({ worktreeId: wtId, filePath: '/path/to/file', relativePath: 'file.ts', mode: 'edit', language: 'typescript' }); return 'done'; })()"
|
||||
|
||||
# Close a file
|
||||
command playwright-cli eval "(() => { window.__store.getState().closeFile('/path/to/file'); return 'closed'; })()"
|
||||
```
|
||||
|
||||
## Launching Electron Apps with CDP
|
||||
|
||||
Every Electron app supports the `--remote-debugging-port` flag since it's built into Chromium.
|
||||
|
||||
### macOS
|
||||
|
||||
```bash
|
||||
# Slack
|
||||
open -a "Slack" --args --remote-debugging-port=9222
|
||||
|
||||
# VS Code
|
||||
open -a "Visual Studio Code" --args --remote-debugging-port=9223
|
||||
|
||||
# Discord
|
||||
open -a "Discord" --args --remote-debugging-port=9224
|
||||
|
||||
# Figma
|
||||
open -a "Figma" --args --remote-debugging-port=9225
|
||||
|
||||
# Notion
|
||||
open -a "Notion" --args --remote-debugging-port=9226
|
||||
|
||||
# Spotify
|
||||
open -a "Spotify" --args --remote-debugging-port=9227
|
||||
```
|
||||
|
||||
### Linux
|
||||
|
||||
```bash
|
||||
slack --remote-debugging-port=9222
|
||||
code --remote-debugging-port=9223
|
||||
discord --remote-debugging-port=9224
|
||||
```
|
||||
|
||||
### Windows
|
||||
|
||||
```bash
|
||||
"C:\Users\%USERNAME%\AppData\Local\slack\slack.exe" --remote-debugging-port=9222
|
||||
"C:\Users\%USERNAME%\AppData\Local\Programs\Microsoft VS Code\Code.exe" --remote-debugging-port=9223
|
||||
```
|
||||
|
||||
**Important:** If the app is already running, quit it first, then relaunch with the flag. The `--remote-debugging-port` flag must be present at launch time.
|
||||
|
||||
## Connecting to an Already-Running App
|
||||
|
||||
If an Electron app was already launched with `--remote-debugging-port`, you can attach directly:
|
||||
|
||||
```bash
|
||||
# Check what's listening on a port
|
||||
lsof -i :9222
|
||||
|
||||
# Verify the CDP endpoint has targets
|
||||
curl -s http://localhost:9222/json
|
||||
|
||||
# Attach playwright-cli
|
||||
command playwright-cli attach --cdp="http://localhost:9222"
|
||||
```
|
||||
|
||||
## Attaching
|
||||
|
||||
```bash
|
||||
# Attach to a specific CDP port
|
||||
command playwright-cli attach --cdp="http://localhost:9222"
|
||||
|
||||
# Attach with a named session (for controlling multiple apps)
|
||||
command playwright-cli -s=slack attach --cdp="http://localhost:9222"
|
||||
command playwright-cli -s=vscode attach --cdp="http://localhost:9223"
|
||||
```
|
||||
|
||||
After `attach`, all subsequent commands (in that session) target the connected app.
|
||||
|
||||
## Tab Management
|
||||
|
||||
Electron apps may have multiple windows or webviews. Use tab commands to list and switch between them:
|
||||
|
||||
```bash
|
||||
# List all available targets
|
||||
command playwright-cli tab-list
|
||||
|
||||
# Switch to a specific tab by index
|
||||
command playwright-cli tab-select 2
|
||||
```
|
||||
|
||||
If `tab-list` doesn't show all targets, query the CDP endpoint directly to see everything:
|
||||
|
||||
```bash
|
||||
curl -s http://localhost:9222/json | python3 -c "
|
||||
import sys, json
|
||||
for i, t in enumerate(json.load(sys.stdin)):
|
||||
print(f'[{i}] ({t[\"type\"]}) {t[\"title\"][:60]} - {t[\"url\"][:60]}')
|
||||
"
|
||||
```
|
||||
|
||||
## Common Patterns
|
||||
|
||||
### Inspect and Navigate an App
|
||||
|
||||
```bash
|
||||
open -a "Slack" --args --remote-debugging-port=9222
|
||||
sleep 3
|
||||
command playwright-cli attach --cdp="http://localhost:9222"
|
||||
command playwright-cli snapshot
|
||||
# Read the snapshot output to identify UI elements
|
||||
command playwright-cli click e10 # Navigate to a section
|
||||
command playwright-cli snapshot # Re-snapshot after navigation
|
||||
```
|
||||
|
||||
### Take Screenshots of Desktop Apps
|
||||
|
||||
```bash
|
||||
command playwright-cli attach --cdp="http://localhost:9222"
|
||||
command playwright-cli screenshot
|
||||
command playwright-cli screenshot e5 # Screenshot a specific element
|
||||
command playwright-cli screenshot --filename=app-state.png
|
||||
```
|
||||
|
||||
### Extract Data from a Desktop App
|
||||
|
||||
```bash
|
||||
command playwright-cli attach --cdp="http://localhost:9222"
|
||||
command playwright-cli snapshot
|
||||
command playwright-cli eval "document.title"
|
||||
command playwright-cli eval "el => el.textContent" e5
|
||||
```
|
||||
|
||||
### Fill Forms in Desktop Apps
|
||||
|
||||
```bash
|
||||
command playwright-cli attach --cdp="http://localhost:9222"
|
||||
command playwright-cli snapshot
|
||||
command playwright-cli fill e3 "search query"
|
||||
command playwright-cli press Enter
|
||||
command playwright-cli snapshot
|
||||
```
|
||||
|
||||
### Run Multiple Apps Simultaneously
|
||||
|
||||
Use named sessions to control multiple Electron apps at the same time:
|
||||
|
||||
```bash
|
||||
# Attach to Slack
|
||||
command playwright-cli -s=slack attach --cdp="http://localhost:9222"
|
||||
|
||||
# Attach to VS Code
|
||||
command playwright-cli -s=vscode attach --cdp="http://localhost:9223"
|
||||
|
||||
# Interact with each independently
|
||||
command playwright-cli -s=slack snapshot
|
||||
command playwright-cli -s=vscode snapshot
|
||||
```
|
||||
|
||||
### Run Custom Playwright Code
|
||||
|
||||
For advanced scenarios, use `run-code` to execute arbitrary Playwright code:
|
||||
|
||||
```bash
|
||||
command playwright-cli run-code "async page => {
|
||||
await page.waitForSelector('.loading', { state: 'hidden' });
|
||||
const items = await page.locator('.item').allTextContents();
|
||||
return items;
|
||||
}"
|
||||
```
|
||||
|
||||
## Troubleshooting
|
||||
|
||||
### "Connection refused" or "Cannot connect"
|
||||
|
||||
- Make sure the app was launched with `--remote-debugging-port=NNNN`
|
||||
- If the app was already running, quit and relaunch with the flag
|
||||
- Check that the port isn't in use by another process: `lsof -i :9222`
|
||||
|
||||
### App launches but attach fails
|
||||
|
||||
- Wait a few seconds after launch before attaching (`sleep 3`)
|
||||
- Some apps take time to initialize their webview
|
||||
- Verify the endpoint is responding: `curl -s http://localhost:9222/json`
|
||||
|
||||
### Elements not appearing in snapshot
|
||||
|
||||
- The app may use multiple webviews. Use `playwright-cli tab-list` to list targets and switch
|
||||
- Use `curl -s http://localhost:<port>/json` to see all CDP targets if tab-list shows fewer
|
||||
- Try `playwright-cli snapshot` without flags first
|
||||
|
||||
### Cannot type in input fields
|
||||
|
||||
- Some Electron apps use custom input components
|
||||
- Try `playwright-cli press` for keyboard events
|
||||
- Use `playwright-cli run-code` for complex input scenarios
|
||||
|
||||
### Stale element refs after interaction
|
||||
|
||||
- Element refs change when the page state updates
|
||||
- Always re-snapshot after clicking, navigating, or filling forms
|
||||
- Use the new refs from the latest snapshot
|
||||
|
||||
## Supported Apps
|
||||
|
||||
Any app built on Electron works, including:
|
||||
|
||||
- **Communication:** Slack, Discord, Microsoft Teams, Signal, Telegram Desktop
|
||||
- **Development:** VS Code, GitHub Desktop, Postman, Insomnia
|
||||
- **Design:** Figma, Notion, Obsidian
|
||||
- **Media:** Spotify, Tidal
|
||||
- **Productivity:** Todoist, Linear, 1Password
|
||||
|
||||
If an app is built with Electron, it supports `--remote-debugging-port` and can be automated with playwright-cli.
|
||||
|
||||
## Cleaning Up
|
||||
|
||||
```bash
|
||||
# Close the playwright-cli session (does NOT kill the Electron app)
|
||||
command playwright-cli close
|
||||
|
||||
# Close a named session
|
||||
command playwright-cli -s=slack close
|
||||
|
||||
# Close all playwright-cli sessions
|
||||
command playwright-cli close-all
|
||||
```
|
||||
|
|
@ -1,341 +0,0 @@
|
|||
# React useEffect Best Practices
|
||||
|
||||
A comprehensive guide teaching when to use `useEffect` in React, and more importantly, when NOT to use it. This skill is based on official React documentation and provides practical alternatives to common useEffect anti-patterns.
|
||||
|
||||
## Purpose
|
||||
|
||||
Effects are an **escape hatch** from React's reactive paradigm. They let you synchronize with external systems like browser APIs, third-party widgets, or network requests. However, many developers overuse Effects for tasks that React handles better through other means.
|
||||
|
||||
This skill helps you:
|
||||
|
||||
- Identify when you truly need an Effect vs. when you don't
|
||||
- Recognize common anti-patterns and their fixes
|
||||
- Apply better alternatives like `useMemo`, `key` prop, and event handlers
|
||||
- Write Effects that are clean, maintainable, and free from race conditions
|
||||
|
||||
## When to Use This Skill
|
||||
|
||||
Use this skill when you're:
|
||||
|
||||
- Writing or reviewing `useEffect` code
|
||||
- Using `useState` to store derived values
|
||||
- Implementing data fetching or subscriptions
|
||||
- Synchronizing state between components
|
||||
- Facing bugs with stale data or race conditions
|
||||
- Wondering if your Effect is necessary
|
||||
|
||||
**Trigger phrases:**
|
||||
|
||||
- "Should I use useEffect for this?"
|
||||
- "How do I fix this useEffect?"
|
||||
- "My Effect is causing too many re-renders"
|
||||
- "Data fetching with useEffect"
|
||||
- "Reset state when props change"
|
||||
- "Derived state from props"
|
||||
|
||||
## How It Works
|
||||
|
||||
This skill provides guidance through three key resources:
|
||||
|
||||
1. **Quick Reference Table** - Fast lookup for common scenarios with DO/DON'T patterns
|
||||
2. **Decision Tree** - Visual flowchart to determine the right approach
|
||||
3. **Detailed Anti-Patterns** - 9 common mistakes with explanations and fixes
|
||||
4. **Better Alternatives** - 8 proven patterns to replace unnecessary Effects
|
||||
|
||||
The skill teaches you to ask the right questions:
|
||||
|
||||
- Is there an external system involved?
|
||||
- Am I responding to a user event or component appearance?
|
||||
- Can this value be calculated during render?
|
||||
- Do I need to reset state when a prop changes?
|
||||
|
||||
## Key Features
|
||||
|
||||
### 1. Quick Reference Guide
|
||||
|
||||
Visual table showing the DO/DON'T for common scenarios:
|
||||
|
||||
- Derived state from props/state
|
||||
- Expensive calculations
|
||||
- Resetting state on prop change
|
||||
- User event responses
|
||||
- Notifying parent components
|
||||
- Data fetching
|
||||
|
||||
### 2. Decision Tree
|
||||
|
||||
Clear flowchart that guides you from "Need to respond to something?" to the correct solution:
|
||||
|
||||
- User interaction → Event handler
|
||||
- Component appeared → Effect (for external sync/analytics)
|
||||
- Derived value needed → Calculate during render (+ useMemo if expensive)
|
||||
- Reset state on prop change → Key prop
|
||||
|
||||
### 3. Anti-Pattern Recognition
|
||||
|
||||
Detailed examples of 9 common mistakes:
|
||||
|
||||
1. Redundant state for derived values
|
||||
2. Filtering/transforming data in Effect
|
||||
3. Resetting state on prop change
|
||||
4. Event-specific logic in Effect
|
||||
5. Chains of Effects
|
||||
6. Notifying parent via Effect
|
||||
7. Passing data up to parent
|
||||
8. Fetching without cleanup (race conditions)
|
||||
9. App initialization in Effect
|
||||
|
||||
Each anti-pattern includes:
|
||||
|
||||
- Bad example with explanation
|
||||
- Good example with fix
|
||||
- Why the anti-pattern is problematic
|
||||
|
||||
### 4. Better Alternatives
|
||||
|
||||
8 proven patterns to replace unnecessary Effects:
|
||||
|
||||
1. Calculate during render for derived state
|
||||
2. `useMemo` for expensive calculations
|
||||
3. `key` prop to reset state
|
||||
4. Store ID instead of object for stable references
|
||||
5. Event handlers for user actions
|
||||
6. `useSyncExternalStore` for external stores
|
||||
7. Lifting state up for shared state
|
||||
8. Custom hooks for data fetching with cleanup
|
||||
|
||||
## Usage Examples
|
||||
|
||||
### Example 1: Derived State
|
||||
|
||||
**Bad - Unnecessary Effect:**
|
||||
|
||||
```tsx
|
||||
function Form() {
|
||||
const [firstName, setFirstName] = useState('Taylor')
|
||||
const [lastName, setLastName] = useState('Swift')
|
||||
const [fullName, setFullName] = useState('')
|
||||
|
||||
useEffect(() => {
|
||||
setFullName(firstName + ' ' + lastName)
|
||||
}, [firstName, lastName])
|
||||
}
|
||||
```
|
||||
|
||||
**Good - Calculate during render:**
|
||||
|
||||
```tsx
|
||||
function Form() {
|
||||
const [firstName, setFirstName] = useState('Taylor')
|
||||
const [lastName, setLastName] = useState('Swift')
|
||||
const fullName = firstName + ' ' + lastName // Just compute it
|
||||
}
|
||||
```
|
||||
|
||||
### Example 2: Resetting State
|
||||
|
||||
**Bad - Effect to reset:**
|
||||
|
||||
```tsx
|
||||
function ProfilePage({ userId }) {
|
||||
const [comment, setComment] = useState('')
|
||||
|
||||
useEffect(() => {
|
||||
setComment('')
|
||||
}, [userId])
|
||||
}
|
||||
```
|
||||
|
||||
**Good - Key prop:**
|
||||
|
||||
```tsx
|
||||
function ProfilePage({ userId }) {
|
||||
return <Profile userId={userId} key={userId} />
|
||||
}
|
||||
|
||||
function Profile({ userId }) {
|
||||
const [comment, setComment] = useState('') // Resets automatically
|
||||
}
|
||||
```
|
||||
|
||||
### Example 3: Data Fetching with Cleanup
|
||||
|
||||
**Bad - Race condition:**
|
||||
|
||||
```tsx
|
||||
function SearchResults({ query }) {
|
||||
const [results, setResults] = useState([])
|
||||
|
||||
useEffect(() => {
|
||||
fetchResults(query).then((json) => {
|
||||
setResults(json) // "hello" response may arrive after "hell"
|
||||
})
|
||||
}, [query])
|
||||
}
|
||||
```
|
||||
|
||||
**Good - Cleanup flag:**
|
||||
|
||||
```tsx
|
||||
function SearchResults({ query }) {
|
||||
const [results, setResults] = useState([])
|
||||
|
||||
useEffect(() => {
|
||||
let ignore = false
|
||||
|
||||
fetchResults(query).then((json) => {
|
||||
if (!ignore) setResults(json)
|
||||
})
|
||||
|
||||
return () => {
|
||||
ignore = true
|
||||
}
|
||||
}, [query])
|
||||
}
|
||||
```
|
||||
|
||||
### Example 4: Event Handler Instead of Effect
|
||||
|
||||
**Bad - Effect watching state:**
|
||||
|
||||
```tsx
|
||||
function ProductPage({ product, addToCart }) {
|
||||
useEffect(() => {
|
||||
if (product.isInCart) {
|
||||
showNotification(`Added ${product.name}!`)
|
||||
}
|
||||
}, [product])
|
||||
|
||||
function handleBuyClick() {
|
||||
addToCart(product)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Good - Handle in event:**
|
||||
|
||||
```tsx
|
||||
function ProductPage({ product, addToCart }) {
|
||||
function handleBuyClick() {
|
||||
addToCart(product)
|
||||
showNotification(`Added ${product.name}!`)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## When You DO Need Effects
|
||||
|
||||
Effects are appropriate for:
|
||||
|
||||
- **Synchronizing with external systems** - Browser APIs, third-party widgets, non-React code
|
||||
- **Subscriptions** - WebSocket connections, global event listeners (prefer `useSyncExternalStore`)
|
||||
- **Analytics/logging** - Code that needs to run because the component displayed
|
||||
- **Data fetching** - With proper cleanup (or use your framework's built-in mechanism)
|
||||
|
||||
## When You DON'T Need Effects
|
||||
|
||||
Avoid Effects for:
|
||||
|
||||
1. **Transforming data for rendering** - Calculate at the top level instead
|
||||
2. **Handling user events** - Use event handlers where you know exactly what happened
|
||||
3. **Deriving state** - Just compute it: `const fullName = firstName + ' ' + lastName`
|
||||
4. **Chaining state updates** - Calculate all next state in the event handler
|
||||
5. **Notifying parent components** - Call the callback in the same event handler
|
||||
6. **Resetting state** - Use the `key` prop to create a fresh component instance
|
||||
|
||||
## Best Practices
|
||||
|
||||
### 1. Start Without an Effect
|
||||
|
||||
Before adding an Effect, ask: "Is there an external system involved?" If no, you probably don't need an Effect.
|
||||
|
||||
### 2. Prefer Derived State
|
||||
|
||||
If you can calculate a value from props or state, don't store it in state with an Effect updating it.
|
||||
|
||||
### 3. Use the Right Tool
|
||||
|
||||
- Expensive calculation → `useMemo`
|
||||
- User interaction → Event handler
|
||||
- Reset on prop change → `key` prop
|
||||
- External subscription → `useSyncExternalStore`
|
||||
- Shared state → Lift state up
|
||||
|
||||
### 4. Always Clean Up
|
||||
|
||||
If your Effect subscribes, fetches, or sets timers, return a cleanup function to prevent memory leaks and race conditions.
|
||||
|
||||
### 5. Avoid Effect Chains
|
||||
|
||||
Multiple Effects triggering each other causes unnecessary re-renders and makes code hard to follow. Calculate everything in one place (usually an event handler).
|
||||
|
||||
### 6. Test in Strict Mode
|
||||
|
||||
React 18+ Strict Mode mounts components twice in development to expose missing cleanup. If your Effect breaks, you need cleanup.
|
||||
|
||||
### 7. Consider Framework Solutions
|
||||
|
||||
For data fetching, prefer your framework's built-in solution (Next.js, Remix) or libraries (React Query, SWR) over manual Effects.
|
||||
|
||||
## Reference Files
|
||||
|
||||
This skill includes three detailed reference documents:
|
||||
|
||||
1. **SKILL.md** - Quick reference table and decision tree
|
||||
2. **anti-patterns.md** - 9 common mistakes with detailed explanations
|
||||
3. **alternatives.md** - 8 better alternatives with code examples
|
||||
|
||||
## Common Pitfalls
|
||||
|
||||
### Multiple Re-renders
|
||||
|
||||
**Symptom:** Component re-renders many times in quick succession.
|
||||
|
||||
**Cause:** Effect that sets state based on state it depends on, creating a loop.
|
||||
|
||||
**Fix:** Calculate the final value in an event handler or during render.
|
||||
|
||||
### Stale Data
|
||||
|
||||
**Symptom:** UI shows outdated values briefly before updating.
|
||||
|
||||
**Cause:** Using Effect to update derived state causes an extra render pass.
|
||||
|
||||
**Fix:** Calculate derived values during render instead of in state.
|
||||
|
||||
### Race Conditions
|
||||
|
||||
**Symptom:** Fast typing shows results for old queries after new ones.
|
||||
|
||||
**Cause:** Missing cleanup in data fetching Effect.
|
||||
|
||||
**Fix:** Use cleanup flag (`ignore` variable) or AbortController.
|
||||
|
||||
### Runs Twice in Development
|
||||
|
||||
**Symptom:** Effect runs twice on component mount in development.
|
||||
|
||||
**Cause:** React 18 Strict Mode intentionally mounts components twice to expose bugs.
|
||||
|
||||
**Fix:** Add proper cleanup. If it's app initialization that shouldn't run twice, use a module-level guard.
|
||||
|
||||
## Resources
|
||||
|
||||
This skill is based on:
|
||||
|
||||
- [React Official Docs: You Might Not Need an Effect](https://react.dev/learn/you-might-not-need-an-effect)
|
||||
- [React Official Docs: Synchronizing with Effects](https://react.dev/learn/synchronizing-with-effects)
|
||||
- [React Official Docs: Lifecycle of Reactive Effects](https://react.dev/learn/lifecycle-of-reactive-effects)
|
||||
|
||||
## Summary
|
||||
|
||||
The golden rule: **Effects are an escape hatch from React.** If you're not synchronizing with an external system, you probably don't need an Effect.
|
||||
|
||||
Before writing `useEffect`, ask yourself:
|
||||
|
||||
1. Is this responding to a user interaction? → Use event handler
|
||||
2. Is this a value I can calculate from props/state? → Calculate during render
|
||||
3. Is this resetting state when a prop changes? → Use key prop
|
||||
4. Is this synchronizing with an external system? → Use Effect with cleanup
|
||||
|
||||
Follow these patterns, and your React code will be more maintainable, performant, and bug-free.
|
||||
|
|
@ -1,53 +0,0 @@
|
|||
---
|
||||
name: react-useeffect
|
||||
description: React useEffect best practices from official docs. Use when writing/reviewing useEffect, useState for derived values, data fetching, or state synchronization. Teaches when NOT to use Effect and better alternatives.
|
||||
---
|
||||
|
||||
# You Might Not Need an Effect
|
||||
|
||||
Effects are an **escape hatch** from React. They let you synchronize with external systems. If there is no external system involved, you shouldn't need an Effect.
|
||||
|
||||
## Quick Reference
|
||||
|
||||
| Situation | DON'T | DO |
|
||||
| ------------------------------ | ------------------------------ | ------------------------------------- |
|
||||
| Derived state from props/state | `useState` + `useEffect` | Calculate during render |
|
||||
| Expensive calculations | `useEffect` to cache | `useMemo` |
|
||||
| Reset state on prop change | `useEffect` with `setState` | `key` prop |
|
||||
| User event responses | `useEffect` watching state | Event handler directly |
|
||||
| Notify parent of changes | `useEffect` calling `onChange` | Call in event handler |
|
||||
| Fetch data | `useEffect` without cleanup | `useEffect` with cleanup OR framework |
|
||||
|
||||
## When You DO Need Effects
|
||||
|
||||
- Synchronizing with **external systems** (non-React widgets, browser APIs)
|
||||
- **Subscriptions** to external stores (use `useSyncExternalStore` when possible)
|
||||
- **Analytics/logging** that runs because component displayed
|
||||
- **Data fetching** with proper cleanup (or use framework's built-in mechanism)
|
||||
|
||||
## When You DON'T Need Effects
|
||||
|
||||
1. **Transforming data for rendering** - Calculate at top level, re-runs automatically
|
||||
2. **Handling user events** - Use event handlers, you know exactly what happened
|
||||
3. **Deriving state** - Just compute it: `const fullName = firstName + ' ' + lastName`
|
||||
4. **Chaining state updates** - Calculate all next state in the event handler
|
||||
|
||||
## Decision Tree
|
||||
|
||||
```
|
||||
Need to respond to something?
|
||||
├── User interaction (click, submit, drag)?
|
||||
│ └── Use EVENT HANDLER
|
||||
├── Component appeared on screen?
|
||||
│ └── Use EFFECT (external sync, analytics)
|
||||
├── Props/state changed and need derived value?
|
||||
│ └── CALCULATE DURING RENDER
|
||||
│ └── Expensive? Use useMemo
|
||||
└── Need to reset state when prop changes?
|
||||
└── Use KEY PROP on component
|
||||
```
|
||||
|
||||
## Detailed Guidance
|
||||
|
||||
- [Anti-Patterns](./anti-patterns.md) - Common mistakes with fixes
|
||||
- [Better Alternatives](./alternatives.md) - useMemo, key prop, lifting state, useSyncExternalStore
|
||||
|
|
@ -1,265 +0,0 @@
|
|||
# Better Alternatives to useEffect
|
||||
|
||||
## 1. Calculate During Render (Derived State)
|
||||
|
||||
For values derived from props or state, just compute them:
|
||||
|
||||
```tsx
|
||||
function Form() {
|
||||
const [firstName, setFirstName] = useState('Taylor')
|
||||
const [lastName, setLastName] = useState('Swift')
|
||||
|
||||
// Runs every render - that's fine and intentional
|
||||
const fullName = firstName + ' ' + lastName
|
||||
const isValid = firstName.length > 0 && lastName.length > 0
|
||||
}
|
||||
```
|
||||
|
||||
**When to use**: The value can be computed from existing props/state.
|
||||
|
||||
---
|
||||
|
||||
## 2. useMemo for Expensive Calculations
|
||||
|
||||
When computation is expensive, memoize it:
|
||||
|
||||
```tsx
|
||||
import { useMemo } from 'react'
|
||||
|
||||
function TodoList({ todos, filter }) {
|
||||
const visibleTodos = useMemo(() => getFilteredTodos(todos, filter), [todos, filter])
|
||||
}
|
||||
```
|
||||
|
||||
**How to know if it's expensive**:
|
||||
|
||||
```tsx
|
||||
console.time('filter')
|
||||
const visibleTodos = getFilteredTodos(todos, filter)
|
||||
console.timeEnd('filter')
|
||||
// If > 1ms, consider memoizing
|
||||
```
|
||||
|
||||
**Note**: React Compiler can auto-memoize, reducing manual useMemo needs.
|
||||
|
||||
---
|
||||
|
||||
## 3. Key Prop to Reset State
|
||||
|
||||
To reset ALL state when a prop changes, use key:
|
||||
|
||||
```tsx
|
||||
// Parent passes userId as key
|
||||
function ProfilePage({ userId }) {
|
||||
return (
|
||||
<Profile
|
||||
userId={userId}
|
||||
key={userId} // Different userId = different component instance
|
||||
/>
|
||||
)
|
||||
}
|
||||
|
||||
function Profile({ userId }) {
|
||||
// All state here resets when userId changes
|
||||
const [comment, setComment] = useState('')
|
||||
const [likes, setLikes] = useState([])
|
||||
}
|
||||
```
|
||||
|
||||
**When to use**: You want a "fresh start" when an identity prop changes.
|
||||
|
||||
---
|
||||
|
||||
## 4. Store ID Instead of Object
|
||||
|
||||
To preserve selection when list changes:
|
||||
|
||||
```tsx
|
||||
// BAD: Storing object that needs Effect to "adjust"
|
||||
function List({ items }) {
|
||||
const [selection, setSelection] = useState(null)
|
||||
|
||||
useEffect(() => {
|
||||
setSelection(null) // Reset when items change
|
||||
}, [items])
|
||||
}
|
||||
|
||||
// GOOD: Store ID, derive object
|
||||
function List({ items }) {
|
||||
const [selectedId, setSelectedId] = useState(null)
|
||||
|
||||
// Derived - no Effect needed
|
||||
const selection = items.find((item) => item.id === selectedId) ?? null
|
||||
}
|
||||
```
|
||||
|
||||
**Benefit**: If item with selectedId exists in new list, selection preserved.
|
||||
|
||||
---
|
||||
|
||||
## 5. Event Handlers for User Actions
|
||||
|
||||
User clicks/submits/drags should be handled in event handlers, not Effects:
|
||||
|
||||
```tsx
|
||||
// Event handler knows exactly what happened
|
||||
function ProductPage({ product, addToCart }) {
|
||||
function handleBuyClick() {
|
||||
addToCart(product)
|
||||
showNotification(`Added ${product.name}!`)
|
||||
analytics.track('product_added', { id: product.id })
|
||||
}
|
||||
|
||||
function handleCheckoutClick() {
|
||||
addToCart(product)
|
||||
showNotification(`Added ${product.name}!`)
|
||||
navigateTo('/checkout')
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Shared logic**: Extract a function, call from both handlers:
|
||||
|
||||
```tsx
|
||||
function buyProduct() {
|
||||
addToCart(product)
|
||||
showNotification(`Added ${product.name}!`)
|
||||
}
|
||||
|
||||
function handleBuyClick() {
|
||||
buyProduct()
|
||||
}
|
||||
function handleCheckoutClick() {
|
||||
buyProduct()
|
||||
navigateTo('/checkout')
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 6. useSyncExternalStore for External Stores
|
||||
|
||||
For subscribing to external data (browser APIs, third-party stores):
|
||||
|
||||
```tsx
|
||||
// Instead of manual Effect subscription
|
||||
function useOnlineStatus() {
|
||||
const [isOnline, setIsOnline] = useState(true)
|
||||
|
||||
useEffect(() => {
|
||||
function update() {
|
||||
setIsOnline(navigator.onLine)
|
||||
}
|
||||
window.addEventListener('online', update)
|
||||
window.addEventListener('offline', update)
|
||||
return () => {
|
||||
window.removeEventListener('online', update)
|
||||
window.removeEventListener('offline', update)
|
||||
}
|
||||
}, [])
|
||||
|
||||
return isOnline
|
||||
}
|
||||
|
||||
// Use purpose-built hook
|
||||
import { useSyncExternalStore } from 'react'
|
||||
|
||||
function subscribe(callback) {
|
||||
window.addEventListener('online', callback)
|
||||
window.addEventListener('offline', callback)
|
||||
return () => {
|
||||
window.removeEventListener('online', callback)
|
||||
window.removeEventListener('offline', callback)
|
||||
}
|
||||
}
|
||||
|
||||
function useOnlineStatus() {
|
||||
return useSyncExternalStore(
|
||||
subscribe,
|
||||
() => navigator.onLine, // Client value
|
||||
() => true // Server value (SSR)
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 7. Lifting State Up
|
||||
|
||||
When two components need synchronized state, lift it to common ancestor:
|
||||
|
||||
```tsx
|
||||
// Instead of syncing via Effects between siblings
|
||||
function Parent() {
|
||||
const [value, setValue] = useState('')
|
||||
|
||||
return (
|
||||
<>
|
||||
<Input value={value} onChange={setValue} />
|
||||
<Preview value={value} />
|
||||
</>
|
||||
)
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 8. Custom Hooks for Data Fetching
|
||||
|
||||
Extract fetch logic with proper cleanup:
|
||||
|
||||
```tsx
|
||||
function useData(url) {
|
||||
const [data, setData] = useState(null)
|
||||
const [error, setError] = useState(null)
|
||||
const [loading, setLoading] = useState(true)
|
||||
|
||||
useEffect(() => {
|
||||
let ignore = false
|
||||
setLoading(true)
|
||||
|
||||
fetch(url)
|
||||
.then((res) => res.json())
|
||||
.then((json) => {
|
||||
if (!ignore) {
|
||||
setData(json)
|
||||
setError(null)
|
||||
}
|
||||
})
|
||||
.catch((err) => {
|
||||
if (!ignore) setError(err)
|
||||
})
|
||||
.finally(() => {
|
||||
if (!ignore) setLoading(false)
|
||||
})
|
||||
|
||||
return () => {
|
||||
ignore = true
|
||||
}
|
||||
}, [url])
|
||||
|
||||
return { data, error, loading }
|
||||
}
|
||||
|
||||
// Usage
|
||||
function SearchResults({ query }) {
|
||||
const { data, error, loading } = useData(`/api/search?q=${query}`)
|
||||
}
|
||||
```
|
||||
|
||||
**Better**: Use framework's data fetching (React Query, SWR, Next.js, etc.)
|
||||
|
||||
---
|
||||
|
||||
## Summary: When to Use What
|
||||
|
||||
| Need | Solution |
|
||||
| ------------------------------ | ------------------------------------ |
|
||||
| Value from props/state | Calculate during render |
|
||||
| Expensive calculation | `useMemo` |
|
||||
| Reset all state on prop change | `key` prop |
|
||||
| Respond to user action | Event handler |
|
||||
| Sync with external system | `useEffect` with cleanup |
|
||||
| Subscribe to external store | `useSyncExternalStore` |
|
||||
| Share state between components | Lift state up |
|
||||
| Fetch data | Custom hook with cleanup / framework |
|
||||
|
|
@ -1,289 +0,0 @@
|
|||
# useEffect Anti-Patterns
|
||||
|
||||
## 1. Redundant State for Derived Values
|
||||
|
||||
```tsx
|
||||
// BAD: Extra state + Effect for derived value
|
||||
function Form() {
|
||||
const [firstName, setFirstName] = useState('Taylor')
|
||||
const [lastName, setLastName] = useState('Swift')
|
||||
const [fullName, setFullName] = useState('')
|
||||
|
||||
useEffect(() => {
|
||||
setFullName(firstName + ' ' + lastName)
|
||||
}, [firstName, lastName])
|
||||
}
|
||||
|
||||
// GOOD: Calculate during rendering
|
||||
function Form() {
|
||||
const [firstName, setFirstName] = useState('Taylor')
|
||||
const [lastName, setLastName] = useState('Swift')
|
||||
const fullName = firstName + ' ' + lastName // Just compute it
|
||||
}
|
||||
```
|
||||
|
||||
**Why it's bad**: Causes extra render pass with stale value, then re-renders with updated value.
|
||||
|
||||
---
|
||||
|
||||
## 2. Filtering/Transforming Data in Effect
|
||||
|
||||
```tsx
|
||||
// BAD: Effect to filter list
|
||||
function TodoList({ todos, filter }) {
|
||||
const [visibleTodos, setVisibleTodos] = useState([])
|
||||
|
||||
useEffect(() => {
|
||||
setVisibleTodos(getFilteredTodos(todos, filter))
|
||||
}, [todos, filter])
|
||||
}
|
||||
|
||||
// GOOD: Filter during render (memoize if expensive)
|
||||
function TodoList({ todos, filter }) {
|
||||
const visibleTodos = useMemo(() => getFilteredTodos(todos, filter), [todos, filter])
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 3. Resetting State on Prop Change
|
||||
|
||||
```tsx
|
||||
// BAD: Effect to reset state
|
||||
function ProfilePage({ userId }) {
|
||||
const [comment, setComment] = useState('')
|
||||
|
||||
useEffect(() => {
|
||||
setComment('')
|
||||
}, [userId])
|
||||
}
|
||||
|
||||
// GOOD: Use key prop
|
||||
function ProfilePage({ userId }) {
|
||||
return <Profile userId={userId} key={userId} />
|
||||
}
|
||||
|
||||
function Profile({ userId }) {
|
||||
const [comment, setComment] = useState('') // Resets automatically
|
||||
}
|
||||
```
|
||||
|
||||
**Why key works**: React treats components with different keys as different components, recreating state.
|
||||
|
||||
---
|
||||
|
||||
## 4. Event-Specific Logic in Effect
|
||||
|
||||
```tsx
|
||||
// BAD: Effect for button click result
|
||||
function ProductPage({ product, addToCart }) {
|
||||
useEffect(() => {
|
||||
if (product.isInCart) {
|
||||
showNotification(`Added ${product.name}!`)
|
||||
}
|
||||
}, [product])
|
||||
|
||||
function handleBuyClick() {
|
||||
addToCart(product)
|
||||
}
|
||||
}
|
||||
|
||||
// GOOD: Handle in event handler
|
||||
function ProductPage({ product, addToCart }) {
|
||||
function handleBuyClick() {
|
||||
addToCart(product)
|
||||
showNotification(`Added ${product.name}!`)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Why it's bad**: Effect fires on page refresh (isInCart is true), showing notification unexpectedly.
|
||||
|
||||
---
|
||||
|
||||
## 5. Chains of Effects
|
||||
|
||||
```tsx
|
||||
// BAD: Effects triggering each other
|
||||
function Game() {
|
||||
const [card, setCard] = useState(null)
|
||||
const [goldCardCount, setGoldCardCount] = useState(0)
|
||||
const [round, setRound] = useState(1)
|
||||
const [isGameOver, setIsGameOver] = useState(false)
|
||||
|
||||
useEffect(() => {
|
||||
if (card?.gold) setGoldCardCount((c) => c + 1)
|
||||
}, [card])
|
||||
|
||||
useEffect(() => {
|
||||
if (goldCardCount > 3) {
|
||||
setRound((r) => r + 1)
|
||||
setGoldCardCount(0)
|
||||
}
|
||||
}, [goldCardCount])
|
||||
|
||||
useEffect(() => {
|
||||
if (round > 5) setIsGameOver(true)
|
||||
}, [round])
|
||||
}
|
||||
|
||||
// GOOD: Calculate in event handler
|
||||
function Game() {
|
||||
const [card, setCard] = useState(null)
|
||||
const [goldCardCount, setGoldCardCount] = useState(0)
|
||||
const [round, setRound] = useState(1)
|
||||
const isGameOver = round > 5 // Derived!
|
||||
|
||||
function handlePlaceCard(nextCard) {
|
||||
if (isGameOver) throw Error('Game ended')
|
||||
|
||||
setCard(nextCard)
|
||||
if (nextCard.gold) {
|
||||
if (goldCardCount < 3) {
|
||||
setGoldCardCount(goldCardCount + 1)
|
||||
} else {
|
||||
setGoldCardCount(0)
|
||||
setRound(round + 1)
|
||||
if (round === 5) alert('Good game!')
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Why it's bad**: Multiple re-renders (setCard -> setGoldCardCount -> setRound -> setIsGameOver). Also fragile for features like history replay.
|
||||
|
||||
---
|
||||
|
||||
## 6. Notifying Parent via Effect
|
||||
|
||||
```tsx
|
||||
// BAD: Effect to notify parent
|
||||
function Toggle({ onChange }) {
|
||||
const [isOn, setIsOn] = useState(false)
|
||||
|
||||
useEffect(() => {
|
||||
onChange(isOn)
|
||||
}, [isOn, onChange])
|
||||
|
||||
function handleClick() {
|
||||
setIsOn(!isOn)
|
||||
}
|
||||
}
|
||||
|
||||
// GOOD: Notify in same event
|
||||
function Toggle({ onChange }) {
|
||||
const [isOn, setIsOn] = useState(false)
|
||||
|
||||
function updateToggle(nextIsOn) {
|
||||
setIsOn(nextIsOn)
|
||||
onChange(nextIsOn) // Same event, batched render
|
||||
}
|
||||
|
||||
function handleClick() {
|
||||
updateToggle(!isOn)
|
||||
}
|
||||
}
|
||||
|
||||
// BEST: Fully controlled component
|
||||
function Toggle({ isOn, onChange }) {
|
||||
function handleClick() {
|
||||
onChange(!isOn)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 7. Passing Data Up to Parent
|
||||
|
||||
```tsx
|
||||
// BAD: Child fetches, passes up via Effect
|
||||
function Parent() {
|
||||
const [data, setData] = useState(null)
|
||||
return <Child onFetched={setData} />
|
||||
}
|
||||
|
||||
function Child({ onFetched }) {
|
||||
const data = useSomeAPI()
|
||||
|
||||
useEffect(() => {
|
||||
if (data) onFetched(data)
|
||||
}, [onFetched, data])
|
||||
}
|
||||
|
||||
// GOOD: Parent fetches, passes down
|
||||
function Parent() {
|
||||
const data = useSomeAPI()
|
||||
return <Child data={data} />
|
||||
}
|
||||
```
|
||||
|
||||
**Why**: Data should flow down. Upward flow via Effects makes debugging hard.
|
||||
|
||||
---
|
||||
|
||||
## 8. Fetching Without Cleanup (Race Condition)
|
||||
|
||||
```tsx
|
||||
// BAD: No cleanup - race condition
|
||||
function SearchResults({ query }) {
|
||||
const [results, setResults] = useState([])
|
||||
|
||||
useEffect(() => {
|
||||
fetchResults(query).then((json) => {
|
||||
setResults(json) // "hello" response may arrive after "hell"
|
||||
})
|
||||
}, [query])
|
||||
}
|
||||
|
||||
// GOOD: Cleanup ignores stale responses
|
||||
function SearchResults({ query }) {
|
||||
const [results, setResults] = useState([])
|
||||
|
||||
useEffect(() => {
|
||||
let ignore = false
|
||||
|
||||
fetchResults(query).then((json) => {
|
||||
if (!ignore) setResults(json)
|
||||
})
|
||||
|
||||
return () => {
|
||||
ignore = true
|
||||
}
|
||||
}, [query])
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 9. App Initialization in Effect
|
||||
|
||||
```tsx
|
||||
// BAD: Runs twice in dev, may break auth
|
||||
function App() {
|
||||
useEffect(() => {
|
||||
loadDataFromLocalStorage()
|
||||
checkAuthToken() // May invalidate token on second call!
|
||||
}, [])
|
||||
}
|
||||
|
||||
// GOOD: Module-level guard
|
||||
let didInit = false
|
||||
|
||||
function App() {
|
||||
useEffect(() => {
|
||||
if (!didInit) {
|
||||
didInit = true
|
||||
loadDataFromLocalStorage()
|
||||
checkAuthToken()
|
||||
}
|
||||
}, [])
|
||||
}
|
||||
|
||||
// ALSO GOOD: Module-level execution
|
||||
if (typeof window !== 'undefined') {
|
||||
checkAuthToken()
|
||||
loadDataFromLocalStorage()
|
||||
}
|
||||
```
|
||||
|
|
@ -1,95 +0,0 @@
|
|||
---
|
||||
name: typescript
|
||||
description: This skill should be used when the user asks to "optimize TypeScript performance", "speed up tsc compilation", "configure tsconfig.json", "fix type errors", "improve async patterns", or encounters TS errors (TS2322, TS2339, "is not assignable to"). Also triggers on .ts, .tsx, .d.ts file work involving type definitions, module organization, or memory management. Does NOT cover TypeScript basics, framework-specific patterns, or testing.
|
||||
---
|
||||
|
||||
# TypeScript Best Practices
|
||||
|
||||
Comprehensive performance optimization guide for TypeScript applications. Contains 45 rules across 8 categories, prioritized by impact to guide automated refactoring and code generation.
|
||||
|
||||
## When to Apply
|
||||
|
||||
Reference these guidelines when:
|
||||
|
||||
- Configuring tsconfig.json for a new or existing project
|
||||
- Writing complex type definitions or generics
|
||||
- Optimizing async/await patterns and data fetching
|
||||
- Organizing modules and managing imports
|
||||
- Reviewing code for compilation or runtime performance
|
||||
|
||||
## Rule Categories by Priority
|
||||
|
||||
| Priority | Category | Impact | Prefix |
|
||||
| -------- | ----------------------- | ----------- | ----------- |
|
||||
| 1 | Type System Performance | CRITICAL | `type-` |
|
||||
| 2 | Compiler Configuration | CRITICAL | `tscfg-` |
|
||||
| 3 | Async Patterns | HIGH | `async-` |
|
||||
| 4 | Module Organization | HIGH | `module-` |
|
||||
| 5 | Type Safety Patterns | MEDIUM-HIGH | `safety-` |
|
||||
| 6 | Memory Management | MEDIUM | `mem-` |
|
||||
| 7 | Runtime Optimization | LOW-MEDIUM | `runtime-` |
|
||||
| 8 | Advanced Patterns | LOW | `advanced-` |
|
||||
|
||||
## Table of Contents
|
||||
|
||||
1. [Type System Performance](references/_sections.md#1-type-system-performance) — **CRITICAL**
|
||||
- 1.1 [Add Explicit Return Types to Exported Functions](references/type-explicit-return-types.md) — CRITICAL (30-50% faster declaration emit)
|
||||
- 1.2 [Avoid Deeply Nested Generic Types](references/type-avoid-deep-generics.md) — CRITICAL (prevents exponential instantiation cost)
|
||||
- 1.3 [Avoid Large Union Types](references/type-avoid-large-unions.md) — CRITICAL (quadratic O(n²) comparison cost)
|
||||
- 1.4 [Extract Conditional Types to Named Aliases](references/type-extract-conditional-types.md) — CRITICAL (enables compiler caching, prevents re-evaluation)
|
||||
- 1.5 [Limit Type Recursion Depth](references/type-limit-recursion-depth.md) — HIGH (prevents exponential type expansion when applicable)
|
||||
- 1.6 [Prefer Interfaces Over Type Intersections](references/type-interfaces-over-intersections.md) — CRITICAL (2-5× faster type resolution)
|
||||
- 1.7 [Simplify Complex Mapped Types](references/type-simplify-mapped-types.md) — HIGH (reduces type computation by 50-80% when applicable)
|
||||
2. [Compiler Configuration](references/_sections.md#2-compiler-configuration) — **CRITICAL**
|
||||
- 2.1 [Configure Include and Exclude Properly](references/tscfg-exclude-properly.md) — CRITICAL (prevents scanning thousands of unnecessary files)
|
||||
- 2.2 [Enable Incremental Compilation](references/tscfg-enable-incremental.md) — CRITICAL (50-90% faster rebuilds)
|
||||
- 2.3 [Enable isolatedDeclarations for Parallel Declaration Emit](references/tscfg-isolated-declarations.md) — CRITICAL (enables parallel .d.ts generation without type-checker)
|
||||
- 2.4 [Enable skipLibCheck for Faster Builds](references/tscfg-skip-lib-check.md) — CRITICAL (20-40% faster compilation)
|
||||
- 2.5 [Enable strictFunctionTypes for Faster Variance Checks](references/tscfg-strict-function-types.md) — CRITICAL (enables optimized variance checking)
|
||||
- 2.6 [Use erasableSyntaxOnly for Node.js Native TypeScript](references/tscfg-erasable-syntax-only.md) — HIGH (prevents 100% of Node.js type-stripping runtime errors)
|
||||
- 2.7 [Use isolatedModules for Single-File Transpilation](references/tscfg-isolate-modules.md) — CRITICAL (80-90% faster transpilation with bundlers)
|
||||
- 2.8 [Use Project References for Large Codebases](references/tscfg-project-references.md) — CRITICAL (60-80% faster incremental builds)
|
||||
3. [Async Patterns](references/_sections.md#3-async-patterns) — **HIGH**
|
||||
- 3.1 [Annotate Async Function Return Types](references/async-explicit-return-types.md) — HIGH (prevents runtime errors, improves inference)
|
||||
- 3.2 [Avoid await Inside Loops](references/async-avoid-loop-await.md) — HIGH (N× faster for N iterations, 10 users = 10× improvement)
|
||||
- 3.3 [Avoid Unnecessary async/await](references/async-avoid-unnecessary-async.md) — HIGH (eliminates trivial Promise wrappers and improves stack traces)
|
||||
- 3.4 [Defer await Until Value Is Needed](references/async-defer-await.md) — HIGH (enables implicit parallelization)
|
||||
- 3.5 [Use Promise.all for Independent Operations](references/async-parallel-promises.md) — HIGH (2-10× improvement in I/O-bound code)
|
||||
4. [Module Organization](references/_sections.md#4-module-organization) — **HIGH**
|
||||
- 4.1 [Avoid Barrel File Imports](references/module-avoid-barrel-imports.md) — HIGH (200-800ms import cost, 30-50% larger bundles)
|
||||
- 4.2 [Avoid Circular Dependencies](references/module-avoid-circular-dependencies.md) — HIGH (prevents runtime undefined errors and slow compilation)
|
||||
- 4.3 [Control @types Package Inclusion](references/module-control-types-inclusion.md) — HIGH (prevents type conflicts and reduces memory usage)
|
||||
- 4.4 [Use Dynamic Imports for Large Modules](references/module-dynamic-imports.md) — HIGH (reduces initial bundle by 30-70%)
|
||||
- 4.5 [Use Type-Only Imports for Types](references/module-use-type-imports.md) — HIGH (eliminates runtime imports for type information)
|
||||
5. [Type Safety Patterns](references/_sections.md#5-type-safety-patterns) — **MEDIUM-HIGH**
|
||||
- 5.1 [Enable noUncheckedIndexedAccess](references/safety-no-unchecked-indexed-access.md) — MEDIUM-HIGH (prevents 100% of unchecked index access errors at compile time)
|
||||
- 5.2 [Enable strictNullChecks](references/safety-strict-null-checks.md) — MEDIUM-HIGH (prevents null/undefined runtime errors)
|
||||
- 5.3 [Prefer unknown Over any](references/safety-prefer-unknown-over-any.md) — MEDIUM-HIGH (forces type narrowing, prevents runtime errors)
|
||||
- 5.4 [Use Assertion Functions for Validation](references/safety-assertion-functions.md) — MEDIUM-HIGH (reduces validation boilerplate by 50-70%)
|
||||
- 5.5 [Use const Assertions for Literal Types](references/safety-const-assertions.md) — MEDIUM-HIGH (preserves literal types, enables better inference)
|
||||
- 5.6 [Use Exhaustive Checks for Union Types](references/safety-exhaustive-checks.md) — MEDIUM-HIGH (prevents 100% of missing case errors at compile time)
|
||||
- 5.7 [Use Type Guards for Runtime Type Checking](references/safety-use-type-guards.md) — MEDIUM-HIGH (eliminates type assertions, catches errors at boundaries)
|
||||
6. [Memory Management](references/_sections.md#6-memory-management) — **MEDIUM**
|
||||
- 6.1 [Avoid Closure Memory Leaks](references/mem-avoid-closure-leaks.md) — MEDIUM (prevents retained references in long-lived callbacks)
|
||||
- 6.2 [Avoid Global State Accumulation](references/mem-avoid-global-state.md) — MEDIUM (prevents unbounded memory growth)
|
||||
- 6.3 [Clean Up Event Listeners](references/mem-cleanup-event-listeners.md) — MEDIUM (prevents unbounded memory growth)
|
||||
- 6.4 [Clear Timers and Intervals](references/mem-clear-timers.md) — MEDIUM (prevents callback retention and repeated execution)
|
||||
- 6.5 [Use WeakMap for Object Metadata](references/mem-use-weakmap-for-metadata.md) — MEDIUM (prevents memory leaks, enables automatic cleanup)
|
||||
7. [Runtime Optimization](references/_sections.md#7-runtime-optimization) — **LOW-MEDIUM**
|
||||
- 7.1 [Avoid Object Spread in Hot Loops](references/runtime-avoid-object-spread-in-loops.md) — LOW-MEDIUM (reduces object allocations by N×)
|
||||
- 7.2 [Cache Property Access in Loops](references/runtime-cache-property-access.md) — LOW-MEDIUM (reduces property lookups by N× in hot paths)
|
||||
- 7.3 [Prefer Native Array Methods Over Lodash](references/runtime-prefer-array-methods.md) — LOW-MEDIUM (eliminates library overhead, enables tree-shaking)
|
||||
- 7.4 [Use for-of for Simple Iteration](references/runtime-use-for-of-for-iteration.md) — LOW-MEDIUM (reduces iteration boilerplate by 30-50%)
|
||||
- 7.5 [Use Modern String Methods](references/runtime-use-string-methods.md) — LOW-MEDIUM (2-5× faster than regex for simple patterns)
|
||||
- 7.6 [Use Set/Map for O(1) Lookups](references/runtime-use-set-for-lookups.md) — LOW-MEDIUM (O(n) to O(1) per lookup)
|
||||
8. [Advanced Patterns](references/_sections.md#8-advanced-patterns) — **LOW**
|
||||
- 8.1 [Use Branded Types for Type-Safe IDs](references/advanced-branded-types.md) — LOW (prevents mixing incompatible ID types)
|
||||
- 8.2 [Use satisfies for Type Validation with Inference](references/advanced-satisfies-operator.md) — LOW (prevents property access errors, enables 100% autocomplete accuracy)
|
||||
- 8.3 [Use Template Literal Types for String Patterns](references/advanced-template-literal-types.md) — LOW (prevents 100% of string format errors at compile time)
|
||||
|
||||
## References
|
||||
|
||||
1. [https://github.com/microsoft/TypeScript/wiki/Performance](https://github.com/microsoft/TypeScript/wiki/Performance)
|
||||
2. [https://www.typescriptlang.org/docs/handbook/](https://www.typescriptlang.org/docs/handbook/)
|
||||
3. [https://v8.dev/blog](https://v8.dev/blog)
|
||||
4. [https://nodejs.org/en/learn/diagnostics/memory](https://nodejs.org/en/learn/diagnostics/memory)
|
||||
|
|
@ -1,26 +0,0 @@
|
|||
---
|
||||
title: Rule Title Here
|
||||
impact: MEDIUM
|
||||
impactDescription: Quantified impact (e.g., "2-10× improvement", "200ms savings")
|
||||
tags: prefix, technique, related-concepts
|
||||
---
|
||||
|
||||
## Rule Title Here
|
||||
|
||||
Brief explanation of the rule and why it matters (1-3 sentences). Focus on performance implications.
|
||||
|
||||
**Incorrect (description of what's wrong):**
|
||||
|
||||
```typescript
|
||||
// Bad code example here
|
||||
const badExample = inefficientOperation()
|
||||
```
|
||||
|
||||
**Correct (description of what's right):**
|
||||
|
||||
```typescript
|
||||
// Good code example here
|
||||
const goodExample = efficientOperation()
|
||||
```
|
||||
|
||||
Reference: [Link to documentation](https://example.com)
|
||||
|
|
@ -1,46 +0,0 @@
|
|||
# Sections
|
||||
|
||||
This file defines all sections, their ordering, impact levels, and descriptions.
|
||||
The section ID (in parentheses) is the filename prefix used to group rules.
|
||||
|
||||
---
|
||||
|
||||
## 1. Type System Performance (type)
|
||||
|
||||
**Impact:** CRITICAL
|
||||
**Description:** Complex types, deep generics, and large unions cause quadratic compilation time. Simplifying type definitions yields the largest compile-time gains.
|
||||
|
||||
## 2. Compiler Configuration (tscfg)
|
||||
|
||||
**Impact:** CRITICAL
|
||||
**Description:** Misconfigured tsconfig causes full rebuilds and unnecessary file scanning. Proper configuration reduces compile time by 50-80%.
|
||||
|
||||
## 3. Async Patterns (async)
|
||||
|
||||
**Impact:** HIGH
|
||||
**Description:** Sequential awaits create runtime waterfalls. Parallelizing async operations yields 2-10× improvement in I/O-bound code.
|
||||
|
||||
## 4. Module Organization (module)
|
||||
|
||||
**Impact:** HIGH
|
||||
**Description:** Barrel files and circular dependencies force excessive module loading. Direct imports reduce bundle size and improve tree-shaking.
|
||||
|
||||
## 5. Type Safety Patterns (safety)
|
||||
|
||||
**Impact:** MEDIUM-HIGH
|
||||
**Description:** Type guards, narrowing, and strict mode prevent runtime errors. Proper patterns eliminate defensive runtime checks.
|
||||
|
||||
## 6. Memory Management (mem)
|
||||
|
||||
**Impact:** MEDIUM
|
||||
**Description:** Object pooling, WeakMap usage, and closure hygiene reduce GC pressure and memory leaks in long-running applications.
|
||||
|
||||
## 7. Runtime Optimization (runtime)
|
||||
|
||||
**Impact:** LOW-MEDIUM
|
||||
**Description:** Loop optimization, property caching, and collection choice improve hot-path performance.
|
||||
|
||||
## 8. Advanced Patterns (advanced)
|
||||
|
||||
**Impact:** LOW
|
||||
**Description:** Branded types, variance annotations, and declaration merging for specialized use cases.
|
||||
|
|
@ -1,92 +0,0 @@
|
|||
---
|
||||
title: Use Branded Types for Type-Safe IDs
|
||||
impact: LOW
|
||||
impactDescription: prevents mixing incompatible ID types
|
||||
tags: advanced, branded-types, nominal-types, type-safety, ids
|
||||
---
|
||||
|
||||
## Use Branded Types for Type-Safe IDs
|
||||
|
||||
TypeScript uses structural typing, so `string` types are interchangeable even when they represent different concepts. Branded types add a unique marker to prevent mixing incompatible values.
|
||||
|
||||
**Incorrect (structural typing allows mixing):**
|
||||
|
||||
```typescript
|
||||
type UserId = string
|
||||
type OrderId = string
|
||||
type ProductId = string
|
||||
|
||||
function fetchUser(id: UserId): Promise<User> {
|
||||
/* ... */
|
||||
}
|
||||
function fetchOrder(id: OrderId): Promise<Order> {
|
||||
/* ... */
|
||||
}
|
||||
|
||||
const userId: UserId = 'user-123'
|
||||
const orderId: OrderId = 'order-456'
|
||||
|
||||
// No error - all strings are interchangeable
|
||||
fetchUser(orderId) // Bug: passed OrderId to UserId parameter
|
||||
fetchOrder(userId) // Bug: passed UserId to OrderId parameter
|
||||
```
|
||||
|
||||
**Correct (branded types prevent mixing):**
|
||||
|
||||
```typescript
|
||||
type Brand<K, T> = K & { __brand: T }
|
||||
|
||||
type UserId = Brand<string, 'UserId'>
|
||||
type OrderId = Brand<string, 'OrderId'>
|
||||
type ProductId = Brand<string, 'ProductId'>
|
||||
|
||||
function createUserId(id: string): UserId {
|
||||
return id as UserId
|
||||
}
|
||||
|
||||
function createOrderId(id: string): OrderId {
|
||||
return id as OrderId
|
||||
}
|
||||
|
||||
function fetchUser(id: UserId): Promise<User> {
|
||||
/* ... */
|
||||
}
|
||||
function fetchOrder(id: OrderId): Promise<Order> {
|
||||
/* ... */
|
||||
}
|
||||
|
||||
const userId = createUserId('user-123')
|
||||
const orderId = createOrderId('order-456')
|
||||
|
||||
fetchUser(orderId) // Error: Argument of type 'OrderId' is not assignable to 'UserId'
|
||||
fetchOrder(userId) // Error: Argument of type 'UserId' is not assignable to 'OrderId'
|
||||
fetchUser(userId) // OK
|
||||
```
|
||||
|
||||
**For numeric types:**
|
||||
|
||||
```typescript
|
||||
type Cents = Brand<number, 'Cents'>
|
||||
type Dollars = Brand<number, 'Dollars'>
|
||||
|
||||
function toCents(dollars: Dollars): Cents {
|
||||
return (dollars * 100) as Cents
|
||||
}
|
||||
|
||||
function formatPrice(cents: Cents): string {
|
||||
return `$${(cents / 100).toFixed(2)}`
|
||||
}
|
||||
|
||||
const price = 29.99 as Dollars
|
||||
formatPrice(price) // Error: Dollars not assignable to Cents
|
||||
formatPrice(toCents(price)) // OK: '$29.99'
|
||||
```
|
||||
|
||||
**When to use branded types:**
|
||||
|
||||
- Entity IDs that shouldn't be mixed
|
||||
- Currency/unit conversions
|
||||
- Validated strings (email, URL, slug)
|
||||
- Sensitive data that needs tracking
|
||||
|
||||
Reference: [TypeScript Playground - Nominal Typing](https://www.typescriptlang.org/play/typescript/language-extensions/nominal-typing.ts.html)
|
||||
|
|
@ -1,95 +0,0 @@
|
|||
---
|
||||
title: Use satisfies for Type Validation with Inference
|
||||
impact: LOW
|
||||
impactDescription: prevents property access errors, enables 100% autocomplete accuracy
|
||||
tags: advanced, satisfies, inference, validation, type-checking
|
||||
---
|
||||
|
||||
## Use satisfies for Type Validation with Inference
|
||||
|
||||
The `satisfies` operator validates that a value conforms to a type while preserving the narrower inferred type. This gives you both type safety and precise autocomplete.
|
||||
|
||||
**Incorrect (type annotation loses literal types):**
|
||||
|
||||
```typescript
|
||||
type ColorConfig = Record<string, [number, number, number]>
|
||||
|
||||
const colors: ColorConfig = {
|
||||
red: [255, 0, 0],
|
||||
green: [0, 255, 0],
|
||||
blue: [0, 0, 255]
|
||||
// Can't access colors.red - it's just string keys
|
||||
}
|
||||
|
||||
// TypeScript doesn't know 'red' is a valid key
|
||||
const redValue = colors.red // Type: [number, number, number]
|
||||
const pinkValue = colors.pink // No error! Type: [number, number, number]
|
||||
```
|
||||
|
||||
**Correct (satisfies preserves literal types):**
|
||||
|
||||
```typescript
|
||||
type ColorConfig = Record<string, [number, number, number]>
|
||||
|
||||
const colors = {
|
||||
red: [255, 0, 0],
|
||||
green: [0, 255, 0],
|
||||
blue: [0, 0, 255]
|
||||
} satisfies ColorConfig
|
||||
|
||||
// TypeScript knows exact keys
|
||||
const redValue = colors.red // Type: [number, number, number]
|
||||
const pinkValue = colors.pink // Error: Property 'pink' does not exist
|
||||
```
|
||||
|
||||
**For configuration objects:**
|
||||
|
||||
```typescript
|
||||
interface Route {
|
||||
path: string
|
||||
component: () => JSX.Element
|
||||
auth?: boolean
|
||||
}
|
||||
|
||||
// Without satisfies - loses literal path types
|
||||
const routes: Route[] = [
|
||||
{ path: '/', component: Home },
|
||||
{ path: '/users', component: Users }
|
||||
]
|
||||
// routes[0].path is just 'string'
|
||||
|
||||
// With satisfies - preserves literal paths
|
||||
const routes = [
|
||||
{ path: '/', component: Home },
|
||||
{ path: '/users', component: Users }
|
||||
] satisfies Route[]
|
||||
// routes[0].path is '/'
|
||||
|
||||
type RoutePath = (typeof routes)[number]['path'] // '/' | '/users'
|
||||
```
|
||||
|
||||
**Combining with as const:**
|
||||
|
||||
```typescript
|
||||
const config = {
|
||||
apiUrl: 'https://api.example.com',
|
||||
timeout: 5000,
|
||||
retries: 3
|
||||
} as const satisfies {
|
||||
apiUrl: string
|
||||
timeout: number
|
||||
retries: number
|
||||
}
|
||||
|
||||
// Both validated AND readonly with literal types
|
||||
config.apiUrl // Type: 'https://api.example.com' (not just string)
|
||||
config.timeout = 3000 // Error: Cannot assign to 'timeout' (readonly)
|
||||
```
|
||||
|
||||
**When to use satisfies vs type annotation:**
|
||||
|
||||
- Use `satisfies` when you want validation but need literal types
|
||||
- Use type annotation (`:`) when you want the variable to be exactly that type
|
||||
- Use `as const satisfies` for readonly config with validation
|
||||
|
||||
Reference: [TypeScript 4.9 satisfies](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-9.html#the-satisfies-operator)
|
||||
|
|
@ -1,105 +0,0 @@
|
|||
---
|
||||
title: Use Template Literal Types for String Patterns
|
||||
impact: LOW
|
||||
impactDescription: prevents 100% of string format errors at compile time
|
||||
tags: advanced, template-literals, string-types, patterns, validation
|
||||
---
|
||||
|
||||
## Use Template Literal Types for String Patterns
|
||||
|
||||
Template literal types allow defining string patterns at the type level. TypeScript validates that strings match the expected format at compile time.
|
||||
|
||||
**Incorrect (plain string allows any value):**
|
||||
|
||||
```typescript
|
||||
type EventHandler = {
|
||||
event: string
|
||||
handler: () => void
|
||||
}
|
||||
|
||||
const handler: EventHandler = {
|
||||
event: 'click', // OK
|
||||
handler: () => {}
|
||||
}
|
||||
|
||||
const badHandler: EventHandler = {
|
||||
event: 'clck', // Typo - no error
|
||||
handler: () => {}
|
||||
}
|
||||
|
||||
function addEventListener(event: string, handler: () => void): void {}
|
||||
addEventListener('onlcick', () => {}) // Typo compiles fine
|
||||
```
|
||||
|
||||
**Correct (template literal type validates pattern):**
|
||||
|
||||
```typescript
|
||||
type DOMEvent = 'click' | 'focus' | 'blur' | 'submit' | 'change'
|
||||
type EventHandlerName = `on${Capitalize<DOMEvent>}`
|
||||
|
||||
type EventHandler = {
|
||||
event: EventHandlerName
|
||||
handler: () => void
|
||||
}
|
||||
|
||||
const handler: EventHandler = {
|
||||
event: 'onClick', // OK
|
||||
handler: () => {}
|
||||
}
|
||||
|
||||
const badHandler: EventHandler = {
|
||||
event: 'onClck', // Error: Type '"onClck"' is not assignable to type 'EventHandlerName'
|
||||
handler: () => {}
|
||||
}
|
||||
```
|
||||
|
||||
**For CSS-like patterns:**
|
||||
|
||||
```typescript
|
||||
type CSSUnit = 'px' | 'em' | 'rem' | '%' | 'vh' | 'vw'
|
||||
type CSSValue = `${number}${CSSUnit}`
|
||||
|
||||
function setWidth(element: HTMLElement, width: CSSValue): void {
|
||||
element.style.width = width
|
||||
}
|
||||
|
||||
setWidth(div, '100px') // OK
|
||||
setWidth(div, '2.5rem') // OK
|
||||
setWidth(div, '100') // Error: Type '"100"' is not assignable to type 'CSSValue'
|
||||
setWidth(div, '100pixels') // Error
|
||||
```
|
||||
|
||||
**For API route patterns:**
|
||||
|
||||
```typescript
|
||||
type APIVersion = 'v1' | 'v2'
|
||||
type Resource = 'users' | 'orders' | 'products'
|
||||
type APIRoute = `/api/${APIVersion}/${Resource}`
|
||||
|
||||
function fetchResource(route: APIRoute): Promise<Response> {
|
||||
return fetch(route)
|
||||
}
|
||||
|
||||
fetchResource('/api/v1/users') // OK
|
||||
fetchResource('/api/v2/orders') // OK
|
||||
fetchResource('/api/v3/users') // Error: 'v3' not in APIVersion
|
||||
fetchResource('/users') // Error: doesn't match pattern
|
||||
```
|
||||
|
||||
**Combining with mapped types:**
|
||||
|
||||
```typescript
|
||||
type Getters<T> = {
|
||||
[K in keyof T as `get${Capitalize<string & K>}`]: () => T[K]
|
||||
}
|
||||
|
||||
interface User {
|
||||
name: string
|
||||
age: number
|
||||
}
|
||||
|
||||
type UserGetters = Getters<User>
|
||||
// { getName: () => string; getAge: () => number }
|
||||
```
|
||||
|
||||
Reference: [TypeScript 4.1 Template Literal Types](https://www.typescriptlang.org/docs/handbook/2/template-literal-types.html)
|
||||
|
|
@ -1,73 +0,0 @@
|
|||
---
|
||||
title: Avoid await Inside Loops
|
||||
impact: HIGH
|
||||
impactDescription: N× faster for N iterations, 10 users = 10× improvement
|
||||
tags: async, loops, batching, waterfalls, performance
|
||||
---
|
||||
|
||||
## Avoid await Inside Loops
|
||||
|
||||
Using `await` inside a loop creates N sequential operations. Collect promises and await them together, or use `Promise.all()` with `map()` for parallel execution.
|
||||
|
||||
**Incorrect (N sequential requests):**
|
||||
|
||||
```typescript
|
||||
async function enrichUsers(userIds: string[]): Promise<EnrichedUser[]> {
|
||||
const enrichedUsers: EnrichedUser[] = []
|
||||
|
||||
for (const userId of userIds) {
|
||||
const user = await fetchUser(userId) // Waits for each request
|
||||
const profile = await fetchProfile(userId)
|
||||
enrichedUsers.push({ ...user, profile })
|
||||
}
|
||||
// 10 users × 2 requests × 100ms = 2000ms
|
||||
|
||||
return enrichedUsers
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (parallel execution):**
|
||||
|
||||
```typescript
|
||||
async function enrichUsers(userIds: string[]): Promise<EnrichedUser[]> {
|
||||
const enrichedUsers = await Promise.all(
|
||||
userIds.map(async (userId) => {
|
||||
const [user, profile] = await Promise.all([fetchUser(userId), fetchProfile(userId)])
|
||||
return { ...user, profile }
|
||||
})
|
||||
)
|
||||
// 10 users processed in parallel = 100ms total
|
||||
|
||||
return enrichedUsers
|
||||
}
|
||||
```
|
||||
|
||||
**For rate-limited APIs (chunked batching):**
|
||||
|
||||
```typescript
|
||||
async function enrichUsers(userIds: string[]): Promise<EnrichedUser[]> {
|
||||
const BATCH_SIZE = 5
|
||||
const results: EnrichedUser[] = []
|
||||
|
||||
for (let i = 0; i < userIds.length; i += BATCH_SIZE) {
|
||||
const batch = userIds.slice(i, i + BATCH_SIZE)
|
||||
const batchResults = await Promise.all(
|
||||
batch.map(async (userId) => {
|
||||
const [user, profile] = await Promise.all([fetchUser(userId), fetchProfile(userId)])
|
||||
return { ...user, profile }
|
||||
})
|
||||
)
|
||||
results.push(...batchResults)
|
||||
}
|
||||
|
||||
return results
|
||||
}
|
||||
```
|
||||
|
||||
**When sequential loop await is acceptable:**
|
||||
|
||||
- Each iteration depends on the previous result
|
||||
- API strictly requires sequential calls
|
||||
- Processing order affects correctness
|
||||
|
||||
Reference: [ESLint no-await-in-loop](https://eslint.org/docs/rules/no-await-in-loop)
|
||||
|
|
@ -1,64 +0,0 @@
|
|||
---
|
||||
title: Avoid Unnecessary async/await
|
||||
impact: HIGH
|
||||
impactDescription: eliminates trivial Promise wrappers and improves stack traces
|
||||
tags: async, promises, overhead, optimization, return-await
|
||||
---
|
||||
|
||||
## Avoid Unnecessary async/await
|
||||
|
||||
Remove `async` from functions that only wrap a single Promise without using `await` for control flow. However, prefer `return await` over bare `return` inside try/catch blocks — it ensures errors are caught and produces better stack traces.
|
||||
|
||||
**Incorrect (trivial async wrapper with no logic):**
|
||||
|
||||
```typescript
|
||||
async function getUser(userId: string): Promise<User> {
|
||||
return userRepository.findById(userId)
|
||||
// async keyword creates unnecessary Promise wrapper
|
||||
// No await, no try/catch — async adds nothing here
|
||||
}
|
||||
|
||||
async function deleteUser(userId: string): Promise<void> {
|
||||
return userRepository.delete(userId)
|
||||
// Same pattern — async is pure overhead
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (remove async when it adds nothing):**
|
||||
|
||||
```typescript
|
||||
function getUser(userId: string): Promise<User> {
|
||||
return userRepository.findById(userId)
|
||||
// Direct Promise return, no wrapper
|
||||
}
|
||||
|
||||
function deleteUser(userId: string): Promise<void> {
|
||||
return userRepository.delete(userId)
|
||||
}
|
||||
```
|
||||
|
||||
**Keep async + return await in try/catch:**
|
||||
|
||||
```typescript
|
||||
// Correct — return await ensures the error is caught
|
||||
async function getUser(userId: string): Promise<User> {
|
||||
try {
|
||||
return await userRepository.findById(userId)
|
||||
// Without await, rejected promise skips the catch block
|
||||
} catch (error) {
|
||||
logger.error('Failed to fetch user', { userId, error })
|
||||
throw new UserNotFoundError(userId)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**When async IS needed:**
|
||||
|
||||
- Multiple sequential await statements
|
||||
- Try/catch around await (use `return await` here)
|
||||
- Conditional await logic
|
||||
- Complex control flow with early returns
|
||||
|
||||
**Note:** ESLint deprecated `no-return-await` in v8.46.0 because `return await` is both safe and produces better stack traces. Use the `@typescript-eslint/return-await` rule with `in-try-catch` setting for nuanced control.
|
||||
|
||||
Reference: [ESLint no-return-await deprecation](https://eslint.org/docs/latest/rules/no-return-await)
|
||||
|
|
@ -1,71 +0,0 @@
|
|||
---
|
||||
title: Defer await Until Value Is Needed
|
||||
impact: HIGH
|
||||
impactDescription: enables implicit parallelization
|
||||
tags: async, defer, promises, optimization, performance
|
||||
---
|
||||
|
||||
## Defer await Until Value Is Needed
|
||||
|
||||
Start async operations immediately but defer `await` until the value is actually required. This allows independent work to proceed while promises resolve in the background.
|
||||
|
||||
**Incorrect (blocks immediately, serializes independent work):**
|
||||
|
||||
```typescript
|
||||
async function processOrder(orderId: string): Promise<OrderResult> {
|
||||
const order = await fetchOrder(orderId) // Blocks here
|
||||
const config = await loadProcessingConfig() // Waits for order first, unnecessarily
|
||||
|
||||
// config doesn't depend on order — these could run in parallel
|
||||
if (order.priority === 'express') {
|
||||
return processExpress(order, config)
|
||||
}
|
||||
return processStandard(order, config)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (deferred await):**
|
||||
|
||||
```typescript
|
||||
async function processOrder(orderId: string): Promise<OrderResult> {
|
||||
const orderPromise = fetchOrder(orderId) // Start immediately
|
||||
const config = await loadProcessingConfig() // Runs while order fetches
|
||||
|
||||
const order = await orderPromise // Now await when needed
|
||||
|
||||
if (order.priority === 'express') {
|
||||
return processExpress(order, config)
|
||||
}
|
||||
return processStandard(order, config)
|
||||
}
|
||||
```
|
||||
|
||||
**Pattern for dependent-then-independent operations:**
|
||||
|
||||
```typescript
|
||||
async function loadUserContent(userId: string): Promise<Content> {
|
||||
// Start user fetch (needed for dependent calls)
|
||||
const userPromise = fetchUser(userId)
|
||||
|
||||
// Start independent operations immediately
|
||||
const settingsPromise = fetchGlobalSettings()
|
||||
const featuresPromise = fetchFeatureFlags()
|
||||
|
||||
// Await user for dependent operations
|
||||
const user = await userPromise
|
||||
const ordersPromise = fetchOrders(user.id)
|
||||
const prefsPromise = fetchPreferences(user.id)
|
||||
|
||||
// Await all remaining
|
||||
const [settings, features, orders, prefs] = await Promise.all([
|
||||
settingsPromise,
|
||||
featuresPromise,
|
||||
ordersPromise,
|
||||
prefsPromise
|
||||
])
|
||||
|
||||
return { user, settings, features, orders, prefs }
|
||||
}
|
||||
```
|
||||
|
||||
Reference: [V8 Blog - Fast Async](https://v8.dev/blog/fast-async)
|
||||
|
|
@ -1,77 +0,0 @@
|
|||
---
|
||||
title: Annotate Async Function Return Types
|
||||
impact: HIGH
|
||||
impactDescription: prevents runtime errors, improves inference
|
||||
tags: async, return-types, promises, type-safety, inference
|
||||
---
|
||||
|
||||
## Annotate Async Function Return Types
|
||||
|
||||
Explicit return types on async functions catch mismatches at the function boundary rather than at call sites. They also improve IDE performance by avoiding full function body inference.
|
||||
|
||||
**Incorrect (inferred Promise type):**
|
||||
|
||||
```typescript
|
||||
async function fetchUserOrders(userId: string) {
|
||||
const response = await fetch(`/api/users/${userId}/orders`)
|
||||
if (!response.ok) {
|
||||
return null // Implicit: Promise<Order[] | null>
|
||||
}
|
||||
return response.json() // Implicit: Promise<any>
|
||||
}
|
||||
|
||||
// Caller has unclear type: Promise<any>
|
||||
const orders = await fetchUserOrders('123')
|
||||
orders.map((o) => o.id) // No type error even if orders is null
|
||||
```
|
||||
|
||||
**Correct (explicit Promise type):**
|
||||
|
||||
```typescript
|
||||
interface Order {
|
||||
id: string
|
||||
total: number
|
||||
status: OrderStatus
|
||||
}
|
||||
|
||||
async function fetchUserOrders(userId: string): Promise<Order[] | null> {
|
||||
const response = await fetch(`/api/users/${userId}/orders`)
|
||||
if (!response.ok) {
|
||||
return null
|
||||
}
|
||||
return response.json() as Promise<Order[]>
|
||||
}
|
||||
|
||||
// Caller knows the exact type
|
||||
const orders = await fetchUserOrders('123')
|
||||
if (orders) {
|
||||
orders.map((o) => o.id) // Type-safe access
|
||||
}
|
||||
```
|
||||
|
||||
**For functions that might throw:**
|
||||
|
||||
```typescript
|
||||
type Result<T, E = Error> = { ok: true; value: T } | { ok: false; error: E }
|
||||
|
||||
async function fetchUserOrders(userId: string): Promise<Result<Order[]>> {
|
||||
try {
|
||||
const response = await fetch(`/api/users/${userId}/orders`)
|
||||
if (!response.ok) {
|
||||
return { ok: false, error: new Error(`HTTP ${response.status}`) }
|
||||
}
|
||||
const orders = (await response.json()) as Order[]
|
||||
return { ok: true, value: orders }
|
||||
} catch (error) {
|
||||
return { ok: false, error: error as Error }
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
|
||||
- Errors caught at function definition, not call sites
|
||||
- Better IDE autocomplete for consumers
|
||||
- Self-documenting API contracts
|
||||
|
||||
Reference: [TypeScript Performance Wiki - Using Type Annotations](https://github.com/microsoft/TypeScript/wiki/Performance#using-type-annotations)
|
||||
|
|
@ -1,64 +0,0 @@
|
|||
---
|
||||
title: Use Promise.all for Independent Operations
|
||||
impact: HIGH
|
||||
impactDescription: 2-10× improvement in I/O-bound code
|
||||
tags: async, promises, parallel, waterfalls, performance
|
||||
---
|
||||
|
||||
## Use Promise.all for Independent Operations
|
||||
|
||||
Sequential `await` statements create request waterfalls—each operation waits for the previous one to complete. Use `Promise.all()` to execute independent async operations concurrently.
|
||||
|
||||
**Incorrect (sequential execution, N round trips):**
|
||||
|
||||
```typescript
|
||||
async function loadDashboard(userId: string): Promise<Dashboard> {
|
||||
const user = await fetchUser(userId) // 200ms
|
||||
const orders = await fetchOrders(userId) // 300ms
|
||||
const notifications = await fetchNotifications(userId) // 150ms
|
||||
// Total: 650ms (sequential)
|
||||
|
||||
return { user, orders, notifications }
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (parallel execution, wall-clock time = max latency):**
|
||||
|
||||
```typescript
|
||||
async function loadDashboard(userId: string): Promise<Dashboard> {
|
||||
const [user, orders, notifications] = await Promise.all([
|
||||
fetchUser(userId), // 200ms ─┐
|
||||
fetchOrders(userId), // 300ms ─┼─ Run in parallel
|
||||
fetchNotifications(userId) // 150ms ─┘
|
||||
])
|
||||
// Total: 300ms (max of all operations)
|
||||
|
||||
return { user, orders, notifications }
|
||||
}
|
||||
```
|
||||
|
||||
**For error handling with partial success:**
|
||||
|
||||
```typescript
|
||||
async function loadDashboard(userId: string): Promise<Dashboard> {
|
||||
const results = await Promise.allSettled([
|
||||
fetchUser(userId),
|
||||
fetchOrders(userId),
|
||||
fetchNotifications(userId)
|
||||
])
|
||||
|
||||
return {
|
||||
user: results[0].status === 'fulfilled' ? results[0].value : null,
|
||||
orders: results[1].status === 'fulfilled' ? results[1].value : [],
|
||||
notifications: results[2].status === 'fulfilled' ? results[2].value : []
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**When sequential is correct:**
|
||||
|
||||
- Operations have data dependencies (need result A to make request B)
|
||||
- Rate limiting requires sequential requests
|
||||
- Order of execution matters for side effects
|
||||
|
||||
Reference: [MDN Promise.all](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all)
|
||||
|
|
@ -1,100 +0,0 @@
|
|||
---
|
||||
title: Avoid Closure Memory Leaks
|
||||
impact: MEDIUM
|
||||
impactDescription: prevents retained references in long-lived callbacks
|
||||
tags: mem, closures, memory-leaks, callbacks, garbage-collection
|
||||
---
|
||||
|
||||
## Avoid Closure Memory Leaks
|
||||
|
||||
Closures retain references to their outer scope variables. Long-lived callbacks can accidentally keep large objects alive, causing memory to grow unboundedly.
|
||||
|
||||
**Incorrect (closure retains entire scope):**
|
||||
|
||||
```typescript
|
||||
function createDataProcessor(largeDataset: DataRecord[]): () => void {
|
||||
const processedIds = new Set<string>()
|
||||
|
||||
return function processNext(): void {
|
||||
// This closure retains reference to largeDataset
|
||||
// even though it only needs processedIds
|
||||
const next = largeDataset.find((r) => !processedIds.has(r.id))
|
||||
if (next) {
|
||||
processedIds.add(next.id)
|
||||
sendToServer(next)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// largeDataset (100MB) stays in memory as long as processNext exists
|
||||
const processor = createDataProcessor(hugeDataset)
|
||||
setInterval(processor, 1000) // Runs forever, 100MB never freed
|
||||
```
|
||||
|
||||
**Correct (closure captures only what it needs):**
|
||||
|
||||
```typescript
|
||||
function createDataProcessor(largeDataset: DataRecord[]): () => void {
|
||||
// Build a queue of just the IDs and a lookup for individual records
|
||||
const pendingQueue: string[] = largeDataset.map((r) => r.id)
|
||||
const getRecord = (id: string): DataRecord | undefined => largeDataset.find((r) => r.id === id)
|
||||
|
||||
// Release the array reference — closure only captures pendingQueue and getRecord
|
||||
// Caller should also release their reference to largeDataset
|
||||
return function processNext(): void {
|
||||
const nextId = pendingQueue.shift()
|
||||
if (nextId) {
|
||||
const record = getRecord(nextId)
|
||||
if (record) sendToServer(record)
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Better pattern — accept an iterator to avoid holding the full dataset:**
|
||||
|
||||
```typescript
|
||||
function createDataProcessor(records: Iterable<DataRecord>): () => void {
|
||||
const iterator = records[Symbol.iterator]()
|
||||
|
||||
return function processNext(): void {
|
||||
const { value, done } = iterator.next()
|
||||
if (!done) {
|
||||
sendToServer(value)
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**For event handlers:**
|
||||
|
||||
```typescript
|
||||
// Incorrect - handler retains component instance forever
|
||||
class Dashboard {
|
||||
private largeCache: Map<string, Data> = new Map()
|
||||
|
||||
initialize(): void {
|
||||
window.addEventListener('resize', () => {
|
||||
this.handleResize() // 'this' keeps entire Dashboard alive
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// Correct - remove listener when done
|
||||
class Dashboard {
|
||||
private largeCache: Map<string, Data> = new Map()
|
||||
private resizeHandler: () => void
|
||||
|
||||
initialize(): void {
|
||||
this.resizeHandler = () => this.handleResize()
|
||||
window.addEventListener('resize', this.resizeHandler)
|
||||
}
|
||||
|
||||
destroy(): void {
|
||||
window.removeEventListener('resize', this.resizeHandler)
|
||||
this.largeCache.clear()
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Reference: [Node.js Memory Diagnostics](https://nodejs.org/en/learn/diagnostics/memory)
|
||||
|
|
@ -1,97 +0,0 @@
|
|||
---
|
||||
title: Avoid Global State Accumulation
|
||||
impact: MEDIUM
|
||||
impactDescription: prevents unbounded memory growth
|
||||
tags: mem, global-state, singletons, memory-leaks, caching
|
||||
---
|
||||
|
||||
## Avoid Global State Accumulation
|
||||
|
||||
Global variables and module-level state persist for the application's lifetime. Unbounded caches or collections at module scope grow indefinitely, causing memory exhaustion.
|
||||
|
||||
**Incorrect (unbounded global cache):**
|
||||
|
||||
```typescript
|
||||
// cache.ts
|
||||
const userCache = new Map<string, User>() // Never cleared
|
||||
|
||||
export function getCachedUser(id: string): User | undefined {
|
||||
return userCache.get(id)
|
||||
}
|
||||
|
||||
export function cacheUser(user: User): void {
|
||||
userCache.set(user.id, user)
|
||||
// Cache grows forever, never evicts old entries
|
||||
}
|
||||
|
||||
// After 1 million users, cache holds 1 million User objects
|
||||
```
|
||||
|
||||
**Correct (bounded cache with eviction):**
|
||||
|
||||
```typescript
|
||||
// cache.ts
|
||||
class LRUCache<K, V> {
|
||||
private cache = new Map<K, V>()
|
||||
private maxSize: number
|
||||
|
||||
constructor(maxSize: number) {
|
||||
this.maxSize = maxSize
|
||||
}
|
||||
|
||||
get(key: K): V | undefined {
|
||||
const value = this.cache.get(key)
|
||||
if (value !== undefined) {
|
||||
// Move to end (most recently used)
|
||||
this.cache.delete(key)
|
||||
this.cache.set(key, value)
|
||||
}
|
||||
return value
|
||||
}
|
||||
|
||||
set(key: K, value: V): void {
|
||||
if (this.cache.has(key)) {
|
||||
this.cache.delete(key)
|
||||
} else if (this.cache.size >= this.maxSize) {
|
||||
// Evict oldest (first) entry
|
||||
const oldest = this.cache.keys().next().value
|
||||
this.cache.delete(oldest)
|
||||
}
|
||||
this.cache.set(key, value)
|
||||
}
|
||||
}
|
||||
|
||||
const userCache = new LRUCache<string, User>(1000) // Max 1000 entries
|
||||
|
||||
export function getCachedUser(id: string): User | undefined {
|
||||
return userCache.get(id)
|
||||
}
|
||||
|
||||
export function cacheUser(user: User): void {
|
||||
userCache.set(user.id, user)
|
||||
}
|
||||
```
|
||||
|
||||
**For request-scoped state (Node.js):**
|
||||
|
||||
```typescript
|
||||
import { AsyncLocalStorage } from 'async_hooks'
|
||||
|
||||
interface RequestContext {
|
||||
userId: string
|
||||
cache: Map<string, unknown>
|
||||
}
|
||||
|
||||
const requestContext = new AsyncLocalStorage<RequestContext>()
|
||||
|
||||
export function runWithContext<T>(context: RequestContext, fn: () => T): T {
|
||||
return requestContext.run(context, fn)
|
||||
// Context is automatically cleaned up when request ends
|
||||
}
|
||||
|
||||
export function getRequestCache(): Map<string, unknown> {
|
||||
return requestContext.getStore()?.cache ?? new Map()
|
||||
}
|
||||
```
|
||||
|
||||
Reference: [Node.js Memory Management](https://nodejs.org/en/learn/diagnostics/memory)
|
||||
|
|
@ -1,111 +0,0 @@
|
|||
---
|
||||
title: Clean Up Event Listeners
|
||||
impact: MEDIUM
|
||||
impactDescription: prevents unbounded memory growth
|
||||
tags: mem, event-listeners, cleanup, memory-leaks, lifecycle
|
||||
---
|
||||
|
||||
## Clean Up Event Listeners
|
||||
|
||||
Event listeners hold references to their callback functions and bound objects. Failing to remove them when components unmount causes memory to grow with each mount/unmount cycle.
|
||||
|
||||
**Incorrect (listeners never removed):**
|
||||
|
||||
```typescript
|
||||
class WebSocketManager {
|
||||
private socket: WebSocket
|
||||
|
||||
connect(url: string): void {
|
||||
this.socket = new WebSocket(url)
|
||||
|
||||
this.socket.addEventListener('message', (event) => {
|
||||
this.handleMessage(event.data)
|
||||
})
|
||||
|
||||
this.socket.addEventListener('error', (event) => {
|
||||
this.handleError(event)
|
||||
})
|
||||
// Listeners keep 'this' alive even after disconnect
|
||||
}
|
||||
|
||||
disconnect(): void {
|
||||
this.socket.close()
|
||||
// Listeners still attached, WebSocketManager can't be GC'd
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (listeners removed on cleanup):**
|
||||
|
||||
```typescript
|
||||
class WebSocketManager {
|
||||
private socket: WebSocket
|
||||
private messageHandler: (event: MessageEvent) => void
|
||||
private errorHandler: (event: Event) => void
|
||||
|
||||
connect(url: string): void {
|
||||
this.socket = new WebSocket(url)
|
||||
|
||||
this.messageHandler = (event) => this.handleMessage(event.data)
|
||||
this.errorHandler = (event) => this.handleError(event)
|
||||
|
||||
this.socket.addEventListener('message', this.messageHandler)
|
||||
this.socket.addEventListener('error', this.errorHandler)
|
||||
}
|
||||
|
||||
disconnect(): void {
|
||||
this.socket.removeEventListener('message', this.messageHandler)
|
||||
this.socket.removeEventListener('error', this.errorHandler)
|
||||
this.socket.close()
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Using AbortController (modern pattern):**
|
||||
|
||||
```typescript
|
||||
class WebSocketManager {
|
||||
private socket: WebSocket
|
||||
private abortController: AbortController
|
||||
|
||||
connect(url: string): void {
|
||||
this.abortController = new AbortController()
|
||||
const { signal } = this.abortController
|
||||
|
||||
this.socket = new WebSocket(url)
|
||||
|
||||
this.socket.addEventListener('message', (e) => this.handleMessage(e.data), { signal })
|
||||
this.socket.addEventListener('error', (e) => this.handleError(e), { signal })
|
||||
// All listeners automatically removed when signal is aborted
|
||||
}
|
||||
|
||||
disconnect(): void {
|
||||
this.abortController.abort() // Removes all listeners at once
|
||||
this.socket.close()
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**React useEffect pattern:**
|
||||
|
||||
```typescript
|
||||
function useWebSocket(url: string): Data | null {
|
||||
const [data, setData] = useState<Data | null>(null)
|
||||
|
||||
useEffect(() => {
|
||||
const socket = new WebSocket(url)
|
||||
const handler = (event: MessageEvent) => setData(JSON.parse(event.data))
|
||||
|
||||
socket.addEventListener('message', handler)
|
||||
|
||||
return () => {
|
||||
socket.removeEventListener('message', handler)
|
||||
socket.close()
|
||||
}
|
||||
}, [url])
|
||||
|
||||
return data
|
||||
}
|
||||
```
|
||||
|
||||
Reference: [MDN AbortController](https://developer.mozilla.org/en-US/docs/Web/API/AbortController)
|
||||
|
|
@ -1,106 +0,0 @@
|
|||
---
|
||||
title: Clear Timers and Intervals
|
||||
impact: MEDIUM
|
||||
impactDescription: prevents callback retention and repeated execution
|
||||
tags: mem, timers, intervals, cleanup, memory-leaks
|
||||
---
|
||||
|
||||
## Clear Timers and Intervals
|
||||
|
||||
`setInterval` and `setTimeout` callbacks retain references to their closure scope. Failing to clear them causes callbacks to execute indefinitely and prevents garbage collection of referenced objects.
|
||||
|
||||
**Incorrect (intervals never cleared):**
|
||||
|
||||
```typescript
|
||||
class DataPoller {
|
||||
private data: LargeDataset
|
||||
|
||||
start(): void {
|
||||
setInterval(() => {
|
||||
this.data = fetchLatestData()
|
||||
this.updateDashboard()
|
||||
}, 5000)
|
||||
// No reference to interval ID, can't clear it
|
||||
}
|
||||
|
||||
stop(): void {
|
||||
// Can't stop the interval - it runs forever
|
||||
// 'this' is retained, DataPoller can't be GC'd
|
||||
}
|
||||
}
|
||||
|
||||
// Each new DataPoller instance creates another interval
|
||||
// Old instances can't be cleaned up
|
||||
```
|
||||
|
||||
**Correct (intervals tracked and cleared):**
|
||||
|
||||
```typescript
|
||||
class DataPoller {
|
||||
private data: LargeDataset
|
||||
private intervalId: ReturnType<typeof setInterval> | null = null
|
||||
|
||||
start(): void {
|
||||
if (this.intervalId) return // Prevent duplicate intervals
|
||||
|
||||
this.intervalId = setInterval(() => {
|
||||
this.data = fetchLatestData()
|
||||
this.updateDashboard()
|
||||
}, 5000)
|
||||
}
|
||||
|
||||
stop(): void {
|
||||
if (this.intervalId) {
|
||||
clearInterval(this.intervalId)
|
||||
this.intervalId = null
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**For multiple timers:**
|
||||
|
||||
```typescript
|
||||
class AnimationController {
|
||||
private timers = new Set<ReturnType<typeof setTimeout>>()
|
||||
|
||||
scheduleAnimation(delay: number, callback: () => void): void {
|
||||
const timerId = setTimeout(() => {
|
||||
this.timers.delete(timerId)
|
||||
callback()
|
||||
}, delay)
|
||||
this.timers.add(timerId)
|
||||
}
|
||||
|
||||
cancelAll(): void {
|
||||
for (const timerId of this.timers) {
|
||||
clearTimeout(timerId)
|
||||
}
|
||||
this.timers.clear()
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**React hook pattern:**
|
||||
|
||||
```typescript
|
||||
function usePolling(callback: () => void, interval: number): void {
|
||||
useEffect(() => {
|
||||
const id = setInterval(callback, interval)
|
||||
return () => clearInterval(id) // Cleanup on unmount
|
||||
}, [callback, interval])
|
||||
}
|
||||
|
||||
function useDebounce<T>(value: T, delay: number): T {
|
||||
const [debouncedValue, setDebouncedValue] = useState(value)
|
||||
|
||||
useEffect(() => {
|
||||
const timer = setTimeout(() => setDebouncedValue(value), delay)
|
||||
return () => clearTimeout(timer) // Clear on value change or unmount
|
||||
}, [value, delay])
|
||||
|
||||
return debouncedValue
|
||||
}
|
||||
```
|
||||
|
||||
Reference: [MDN clearInterval](https://developer.mozilla.org/en-US/docs/Web/API/clearInterval)
|
||||
|
|
@ -1,85 +0,0 @@
|
|||
---
|
||||
title: Use WeakMap for Object Metadata
|
||||
impact: MEDIUM
|
||||
impactDescription: prevents memory leaks, enables automatic cleanup
|
||||
tags: mem, weakmap, metadata, garbage-collection, memory-leaks
|
||||
---
|
||||
|
||||
## Use WeakMap for Object Metadata
|
||||
|
||||
WeakMap allows garbage collection of keys when no other references exist. Use it for associating metadata with objects without preventing their cleanup.
|
||||
|
||||
**Incorrect (Map retains object references):**
|
||||
|
||||
```typescript
|
||||
const userMetadata = new Map<User, UserMetadata>()
|
||||
|
||||
function trackUser(user: User): void {
|
||||
userMetadata.set(user, {
|
||||
lastSeen: Date.now(),
|
||||
pageViews: 0
|
||||
})
|
||||
}
|
||||
|
||||
function removeUser(user: User): void {
|
||||
// Even after user is "removed" from app state,
|
||||
// Map still holds reference, preventing GC
|
||||
userMetadata.delete(user) // Must manually clean up
|
||||
}
|
||||
|
||||
// If delete is forgotten, user objects leak forever
|
||||
```
|
||||
|
||||
**Correct (WeakMap allows GC):**
|
||||
|
||||
```typescript
|
||||
const userMetadata = new WeakMap<User, UserMetadata>()
|
||||
|
||||
function trackUser(user: User): void {
|
||||
userMetadata.set(user, {
|
||||
lastSeen: Date.now(),
|
||||
pageViews: 0
|
||||
})
|
||||
}
|
||||
|
||||
// No cleanup needed - when user object is GC'd,
|
||||
// WeakMap entry is automatically removed
|
||||
function processUsers(users: User[]): void {
|
||||
for (const user of users) {
|
||||
trackUser(user)
|
||||
}
|
||||
// When users array is cleared, all metadata is cleaned up automatically
|
||||
}
|
||||
```
|
||||
|
||||
**Common use cases:**
|
||||
|
||||
```typescript
|
||||
// DOM element metadata
|
||||
const elementState = new WeakMap<HTMLElement, ElementState>()
|
||||
|
||||
function attachState(element: HTMLElement): void {
|
||||
elementState.set(element, { isExpanded: false })
|
||||
// When element is removed from DOM and GC'd, state is cleaned up
|
||||
}
|
||||
|
||||
// Caching computed values
|
||||
const computedCache = new WeakMap<Config, ComputedConfig>()
|
||||
|
||||
function getComputedConfig(config: Config): ComputedConfig {
|
||||
let computed = computedCache.get(config)
|
||||
if (!computed) {
|
||||
computed = expensiveComputation(config)
|
||||
computedCache.set(config, computed)
|
||||
}
|
||||
return computed
|
||||
}
|
||||
```
|
||||
|
||||
**Limitations of WeakMap:**
|
||||
|
||||
- Keys must be objects (not primitives)
|
||||
- Not iterable (no `.keys()`, `.values()`, `.entries()`)
|
||||
- No `.size` property
|
||||
|
||||
Reference: [MDN WeakMap](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakMap)
|
||||
|
|
@ -1,71 +0,0 @@
|
|||
---
|
||||
title: Avoid Barrel File Imports
|
||||
impact: HIGH
|
||||
impactDescription: 200-800ms import cost, 30-50% larger bundles
|
||||
tags: module, barrel-files, imports, tree-shaking, bundling
|
||||
---
|
||||
|
||||
## Avoid Barrel File Imports
|
||||
|
||||
Barrel files (index.ts re-exports) defeat tree-shaking and force bundlers to load entire module graphs. Import directly from source files to enable proper dead-code elimination.
|
||||
|
||||
**Incorrect (imports entire module tree):**
|
||||
|
||||
```typescript
|
||||
// utils/index.ts (barrel file)
|
||||
export * from './string'
|
||||
export * from './date'
|
||||
export * from './validation'
|
||||
export * from './crypto' // Heavy, rarely used
|
||||
|
||||
// consumer.ts
|
||||
import { formatDate } from '@/utils'
|
||||
// Loads ALL utils modules, including crypto
|
||||
// Bundle includes 50KB of unused code
|
||||
```
|
||||
|
||||
**Correct (direct imports):**
|
||||
|
||||
```typescript
|
||||
// consumer.ts
|
||||
import { formatDate } from '@/utils/date'
|
||||
// Loads only the date module
|
||||
// Bundle includes only what's used
|
||||
```
|
||||
|
||||
**For icon libraries (common barrel offender):**
|
||||
|
||||
```typescript
|
||||
// Incorrect - loads all 1500+ icons
|
||||
import { Check, X } from 'lucide-react'
|
||||
|
||||
// Correct - loads only 2 icons
|
||||
import Check from 'lucide-react/dist/esm/icons/check'
|
||||
import X from 'lucide-react/dist/esm/icons/x'
|
||||
```
|
||||
|
||||
**Alternative (configure bundler optimization):**
|
||||
|
||||
```javascript
|
||||
// next.config.js
|
||||
module.exports = {
|
||||
experimental: {
|
||||
optimizePackageImports: ['lucide-react', '@mui/material', 'lodash']
|
||||
}
|
||||
}
|
||||
|
||||
// vite.config.ts
|
||||
export default {
|
||||
optimizeDeps: {
|
||||
include: ['lucide-react']
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**When barrels are acceptable:**
|
||||
|
||||
- Internal modules with few exports (< 10)
|
||||
- Package entry points for library consumers
|
||||
- When bundler is configured to optimize them
|
||||
|
||||
Reference: [Vercel - How we optimized package imports](https://vercel.com/blog/how-we-optimized-package-imports-in-next-js)
|
||||
|
|
@ -1,98 +0,0 @@
|
|||
---
|
||||
title: Avoid Circular Dependencies
|
||||
impact: HIGH
|
||||
impactDescription: prevents runtime undefined errors and slow compilation
|
||||
tags: module, circular, dependencies, architecture, compilation
|
||||
---
|
||||
|
||||
## Avoid Circular Dependencies
|
||||
|
||||
Circular dependencies cause undefined values at runtime (due to incomplete module initialization) and slow TypeScript compilation as the checker resolves cycles repeatedly.
|
||||
|
||||
**Incorrect (circular dependency):**
|
||||
|
||||
```typescript
|
||||
// user.ts
|
||||
import { Order } from './order'
|
||||
|
||||
export interface User {
|
||||
id: string
|
||||
orders: Order[]
|
||||
}
|
||||
|
||||
export function createUser(): User {
|
||||
/* ... */
|
||||
}
|
||||
|
||||
// order.ts
|
||||
import { User } from './user' // Circular!
|
||||
|
||||
export interface Order {
|
||||
id: string
|
||||
user: User
|
||||
}
|
||||
|
||||
export function createOrder(user: User): Order {
|
||||
// 'createUser' might be undefined if order.ts loads first
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (extract shared types):**
|
||||
|
||||
```typescript
|
||||
// types.ts (no dependencies)
|
||||
export interface User {
|
||||
id: string
|
||||
orders: Order[]
|
||||
}
|
||||
|
||||
export interface Order {
|
||||
id: string
|
||||
user: User
|
||||
}
|
||||
|
||||
// user.ts
|
||||
import { User, Order } from './types'
|
||||
|
||||
export function createUser(): User {
|
||||
/* ... */
|
||||
}
|
||||
|
||||
// order.ts
|
||||
import { User, Order } from './types'
|
||||
|
||||
export function createOrder(user: User): Order {
|
||||
/* ... */
|
||||
}
|
||||
```
|
||||
|
||||
**Alternative (interface segregation):**
|
||||
|
||||
```typescript
|
||||
// user-types.ts
|
||||
export interface UserBase {
|
||||
id: string
|
||||
name: string
|
||||
}
|
||||
|
||||
// order.ts
|
||||
import { UserBase } from './user-types'
|
||||
|
||||
export interface Order {
|
||||
id: string
|
||||
user: UserBase // Only needs base interface, not full User
|
||||
}
|
||||
```
|
||||
|
||||
**Detection tools:**
|
||||
|
||||
```bash
|
||||
# Madge - visualize circular dependencies
|
||||
npx madge --circular --extensions ts ./src
|
||||
|
||||
# ESLint plugin
|
||||
npm install eslint-plugin-import
|
||||
# Rule: import/no-cycle
|
||||
```
|
||||
|
||||
Reference: [Node.js Cycles Documentation](https://nodejs.org/api/modules.html#cycles)
|
||||
|
|
@ -1,93 +0,0 @@
|
|||
---
|
||||
title: Control @types Package Inclusion
|
||||
impact: HIGH
|
||||
impactDescription: prevents type conflicts and reduces memory usage
|
||||
tags: module, types, tsconfig, declaration-files, performance
|
||||
---
|
||||
|
||||
## Control @types Package Inclusion
|
||||
|
||||
By default, TypeScript loads all `@types/*` packages from `node_modules`. This causes conflicts between incompatible type versions and wastes memory loading unused declarations.
|
||||
|
||||
**Incorrect (loads all @types automatically):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"target": "ES2022",
|
||||
"module": "ESNext"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```bash
|
||||
# All @types/* packages loaded:
|
||||
# @types/node, @types/react, @types/express, @types/lodash,
|
||||
# @types/jest, @types/mocha (conflict!), @types/jasmine (conflict!)
|
||||
```
|
||||
|
||||
**Correct (explicit types inclusion):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"target": "ES2022",
|
||||
"module": "ESNext",
|
||||
"types": ["node", "react", "jest"]
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```bash
|
||||
# Only specified @types loaded
|
||||
# No conflicts between test frameworks
|
||||
```
|
||||
|
||||
**For different environments:**
|
||||
|
||||
```json
|
||||
// tsconfig.json (base)
|
||||
{
|
||||
"compilerOptions": {
|
||||
"types": []
|
||||
}
|
||||
}
|
||||
|
||||
// tsconfig.node.json (Node.js scripts)
|
||||
{
|
||||
"extends": "./tsconfig.json",
|
||||
"compilerOptions": {
|
||||
"types": ["node"]
|
||||
}
|
||||
}
|
||||
|
||||
// tsconfig.test.json (Jest tests)
|
||||
{
|
||||
"extends": "./tsconfig.json",
|
||||
"compilerOptions": {
|
||||
"types": ["node", "jest"]
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Using typeRoots for custom declarations:**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"typeRoots": [
|
||||
"./types", // Custom declarations first
|
||||
"./node_modules/@types" // Then @types
|
||||
],
|
||||
"types": ["node"]
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
|
||||
- Prevents type conflicts between similar packages
|
||||
- Reduces memory usage during compilation
|
||||
- Faster IDE responsiveness
|
||||
|
||||
Reference: [TypeScript Performance Wiki](https://github.com/microsoft/TypeScript/wiki/Performance#controlling-types-inclusion)
|
||||
|
|
@ -1,78 +0,0 @@
|
|||
---
|
||||
title: Use Dynamic Imports for Large Modules
|
||||
impact: HIGH
|
||||
impactDescription: reduces initial bundle by 30-70%
|
||||
tags: module, dynamic-import, code-splitting, lazy-loading, bundling
|
||||
---
|
||||
|
||||
## Use Dynamic Imports for Large Modules
|
||||
|
||||
Dynamic `import()` creates separate chunks that load on demand. Use them for large dependencies, route-specific code, and features that aren't needed immediately.
|
||||
|
||||
**Incorrect (static import, always loaded):**
|
||||
|
||||
```typescript
|
||||
import { PDFGenerator } from 'pdfkit' // 500KB
|
||||
import { ExcelExporter } from 'exceljs' // 800KB
|
||||
import { ChartLibrary } from 'chart.js' // 300KB
|
||||
|
||||
export async function exportReport(format: 'pdf' | 'excel' | 'chart') {
|
||||
if (format === 'pdf') {
|
||||
return new PDFGenerator().generate()
|
||||
}
|
||||
// All 1.6MB loaded even if user never exports
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (dynamic import, loaded on demand):**
|
||||
|
||||
```typescript
|
||||
export async function exportReport(format: 'pdf' | 'excel' | 'chart') {
|
||||
if (format === 'pdf') {
|
||||
const { PDFGenerator } = await import('pdfkit')
|
||||
return new PDFGenerator().generate()
|
||||
}
|
||||
|
||||
if (format === 'excel') {
|
||||
const { ExcelExporter } = await import('exceljs')
|
||||
return new ExcelExporter().export()
|
||||
}
|
||||
|
||||
const { ChartLibrary } = await import('chart.js')
|
||||
return new ChartLibrary().render()
|
||||
}
|
||||
// Only loads the module needed for the specific format
|
||||
```
|
||||
|
||||
**With TypeScript typing:**
|
||||
|
||||
```typescript
|
||||
async function loadPdfGenerator(): Promise<typeof import('pdfkit')> {
|
||||
return import('pdfkit')
|
||||
}
|
||||
|
||||
// Or with type-only import for the interface
|
||||
import type { PDFDocument } from 'pdfkit'
|
||||
|
||||
async function generatePdf(): Promise<PDFDocument> {
|
||||
const { default: PDFDocument } = await import('pdfkit')
|
||||
return new PDFDocument()
|
||||
}
|
||||
```
|
||||
|
||||
**Framework-specific patterns:**
|
||||
|
||||
```typescript
|
||||
// Next.js
|
||||
import dynamic from 'next/dynamic'
|
||||
|
||||
const HeavyChart = dynamic(() => import('@/components/HeavyChart'), {
|
||||
loading: () => <ChartSkeleton />,
|
||||
ssr: false // Skip server-side rendering
|
||||
})
|
||||
|
||||
// React
|
||||
const HeavyChart = React.lazy(() => import('@/components/HeavyChart'))
|
||||
```
|
||||
|
||||
Reference: [MDN Dynamic Import](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import)
|
||||
|
|
@ -1,81 +0,0 @@
|
|||
---
|
||||
title: Use Type-Only Imports for Types
|
||||
impact: HIGH
|
||||
impactDescription: eliminates runtime imports for type information
|
||||
tags: module, type-imports, tree-shaking, bundling, compilation
|
||||
---
|
||||
|
||||
## Use Type-Only Imports for Types
|
||||
|
||||
Type-only imports (`import type`) are completely erased during compilation, preventing unnecessary runtime module loading. Regular imports of types can force module execution even when only the type is needed.
|
||||
|
||||
**Incorrect (runtime import for type-only usage):**
|
||||
|
||||
```typescript
|
||||
// config.ts
|
||||
import { DatabaseConfig } from './database' // Loads entire database module
|
||||
import { Logger } from './logger' // Loads entire logger module
|
||||
|
||||
interface AppConfig {
|
||||
db: DatabaseConfig
|
||||
logger: Logger
|
||||
}
|
||||
|
||||
// Runtime: database.js and logger.js are both loaded
|
||||
// even though we only use their types
|
||||
```
|
||||
|
||||
**Correct (type-only imports):**
|
||||
|
||||
```typescript
|
||||
// config.ts
|
||||
import type { DatabaseConfig } from './database'
|
||||
import type { Logger } from './logger'
|
||||
|
||||
interface AppConfig {
|
||||
db: DatabaseConfig
|
||||
logger: Logger
|
||||
}
|
||||
|
||||
// Runtime: no modules loaded, types are erased
|
||||
```
|
||||
|
||||
**Mixed imports (types and values):**
|
||||
|
||||
```typescript
|
||||
// Incorrect - unclear what's type vs value
|
||||
import { User, createUser, UserRole } from './user'
|
||||
|
||||
// Correct - explicit separation
|
||||
import { createUser } from './user'
|
||||
import type { User, UserRole } from './user'
|
||||
|
||||
// Or inline type imports (TypeScript 4.5+)
|
||||
import { createUser, type User, type UserRole } from './user'
|
||||
```
|
||||
|
||||
**Enable enforcement:**
|
||||
|
||||
```json
|
||||
// tsconfig.json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"verbatimModuleSyntax": true
|
||||
}
|
||||
}
|
||||
|
||||
// .eslintrc
|
||||
{
|
||||
"rules": {
|
||||
"@typescript-eslint/consistent-type-imports": "error"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
|
||||
- Smaller bundles (unused modules not included)
|
||||
- Faster cold starts (fewer modules to parse)
|
||||
- Clearer code intent (types vs runtime values)
|
||||
|
||||
Reference: [TypeScript 3.8 Release Notes](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-8.html#type-only-imports-and-export)
|
||||
|
|
@ -1,86 +0,0 @@
|
|||
---
|
||||
title: Avoid Object Spread in Hot Loops
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: reduces object allocations by N×
|
||||
tags: runtime, object-spread, loops, allocation, performance
|
||||
---
|
||||
|
||||
## Avoid Object Spread in Hot Loops
|
||||
|
||||
Object spread (`...`) creates a new object on each use. In loops, this causes N object allocations and copies. Mutate objects directly when creating new instances isn't required.
|
||||
|
||||
**Incorrect (N object allocations):**
|
||||
|
||||
```typescript
|
||||
function enrichOrders(orders: Order[]): EnrichedOrder[] {
|
||||
return orders.map((order) => ({
|
||||
...order, // Creates new object
|
||||
...calculateTotals(order), // Spreads another object
|
||||
processedAt: new Date()
|
||||
}))
|
||||
}
|
||||
// 10,000 orders = 10,000 object spreads = significant GC pressure
|
||||
```
|
||||
|
||||
**Correct (direct assignment):**
|
||||
|
||||
```typescript
|
||||
interface EnrichedOrder extends Order {
|
||||
tax: number
|
||||
shipping: number
|
||||
total: number
|
||||
processedAt: Date
|
||||
}
|
||||
|
||||
function enrichOrders(orders: Order[]): EnrichedOrder[] {
|
||||
return orders.map((order) => {
|
||||
const totals = calculateTotals(order)
|
||||
|
||||
return {
|
||||
id: order.id,
|
||||
customerId: order.customerId,
|
||||
items: order.items,
|
||||
subtotal: order.subtotal,
|
||||
tax: totals.tax,
|
||||
shipping: totals.shipping,
|
||||
total: totals.total,
|
||||
processedAt: new Date()
|
||||
}
|
||||
})
|
||||
}
|
||||
```
|
||||
|
||||
**Note:** For immutable object creation, explicit property listing is the only spread-free option. This trades verbosity for performance in hot paths. If immutability isn't required, mutating the original object is faster still.
|
||||
|
||||
**For accumulation patterns:**
|
||||
|
||||
```typescript
|
||||
// Incorrect - spreads on every iteration
|
||||
const result = items.reduce(
|
||||
(acc, item) => ({
|
||||
...acc,
|
||||
[item.id]: item.value
|
||||
}),
|
||||
{}
|
||||
)
|
||||
// O(n²) - each spread copies growing object
|
||||
|
||||
// Correct - mutate accumulator
|
||||
const result = items.reduce(
|
||||
(acc, item) => {
|
||||
acc[item.id] = item.value
|
||||
return acc
|
||||
},
|
||||
{} as Record<string, number>
|
||||
)
|
||||
// O(n) - direct property assignment
|
||||
```
|
||||
|
||||
**When spread is acceptable:**
|
||||
|
||||
- Outside hot paths
|
||||
- Small objects (< 10 properties)
|
||||
- When immutability is required for state management
|
||||
- When readability significantly improves
|
||||
|
||||
Reference: [V8 Object Shapes](https://mathiasbynens.be/notes/shapes-ics)
|
||||
|
|
@ -1,67 +0,0 @@
|
|||
---
|
||||
title: Cache Property Access in Loops
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: reduces property lookups by N× in hot paths
|
||||
tags: runtime, loops, caching, property-access, optimization
|
||||
---
|
||||
|
||||
## Cache Property Access in Loops
|
||||
|
||||
Cache deeply nested or polymorphic property access before hot loops. **Note:** Modern V8's inline caches optimize monomorphic access efficiently — this optimization is only meaningful for 10,000+ iterations with deeply nested or polymorphic properties.
|
||||
|
||||
**Incorrect (repeated nested access in hot loop):**
|
||||
|
||||
```typescript
|
||||
function processOrders(orders: Order[], config: AppConfig): ProcessedOrder[] {
|
||||
const results: ProcessedOrder[] = []
|
||||
|
||||
for (const order of orders) {
|
||||
const tax = order.total * config.tax.rate // Nested access each iteration
|
||||
const shipping = config.shipping.rates[order.region] // Nested access again
|
||||
results.push({ ...order, tax, shipping, final: order.total + tax + shipping })
|
||||
}
|
||||
|
||||
return results
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (cached property access):**
|
||||
|
||||
```typescript
|
||||
function processOrders(orders: Order[], config: AppConfig): ProcessedOrder[] {
|
||||
const results: ProcessedOrder[] = []
|
||||
const taxRate = config.tax.rate
|
||||
const shippingRates = config.shipping.rates
|
||||
|
||||
for (const order of orders) {
|
||||
const tax = order.total * taxRate
|
||||
const shipping = shippingRates[order.region]
|
||||
results.push({ ...order, tax, shipping, final: order.total + tax + shipping })
|
||||
}
|
||||
|
||||
return results
|
||||
}
|
||||
```
|
||||
|
||||
**When V8 handles it automatically (no caching needed):**
|
||||
|
||||
```typescript
|
||||
// Monomorphic — all objects have same shape, V8 ICs optimize this
|
||||
function sumOrders(orders: Order[]): number {
|
||||
let total = 0
|
||||
for (let i = 0; i < orders.length; i++) {
|
||||
// orders.length is fine
|
||||
total += orders[i].total // Same shape every time
|
||||
}
|
||||
return total
|
||||
}
|
||||
```
|
||||
|
||||
**When to skip this optimization:**
|
||||
|
||||
- Arrays under 1,000 items
|
||||
- Monomorphic objects (same shape/class)
|
||||
- Non-hot paths executed infrequently
|
||||
- When readability suffers significantly
|
||||
|
||||
Reference: [V8 Hidden Classes](https://v8.dev/blog/fast-properties)
|
||||
|
|
@ -1,77 +0,0 @@
|
|||
---
|
||||
title: Prefer Native Array Methods Over Lodash
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: eliminates library overhead, enables tree-shaking
|
||||
tags: runtime, arrays, lodash, native-methods, bundling
|
||||
---
|
||||
|
||||
## Prefer Native Array Methods Over Lodash
|
||||
|
||||
Modern JavaScript includes most common array operations. Native methods are faster (no function call overhead) and don't add bundle weight. Use native methods when they provide equivalent functionality.
|
||||
|
||||
**Incorrect (lodash for native operations):**
|
||||
|
||||
```typescript
|
||||
import _ from 'lodash' // Imports entire library
|
||||
|
||||
const activeUsers = _.filter(users, (u) => u.isActive)
|
||||
const userNames = _.map(activeUsers, (u) => u.name)
|
||||
const firstAdmin = _.find(users, (u) => u.role === 'admin')
|
||||
const hasAdmin = _.some(users, (u) => u.role === 'admin')
|
||||
const allActive = _.every(users, (u) => u.isActive)
|
||||
const userIds = _.uniq(users.map((u) => u.id))
|
||||
```
|
||||
|
||||
**Correct (native methods):**
|
||||
|
||||
```typescript
|
||||
const activeUsers = users.filter((u) => u.isActive)
|
||||
const userNames = activeUsers.map((u) => u.name)
|
||||
const firstAdmin = users.find((u) => u.role === 'admin')
|
||||
const hasAdmin = users.some((u) => u.role === 'admin')
|
||||
const allActive = users.every((u) => u.isActive)
|
||||
const userIds = [...new Set(users.map((u) => u.id))]
|
||||
```
|
||||
|
||||
**Native replacements for common Lodash functions:**
|
||||
|
||||
```typescript
|
||||
// _.flatten / _.flattenDeep
|
||||
const flat = nestedArrays.flat(Infinity)
|
||||
|
||||
// _.chunk (still useful from lodash)
|
||||
function chunk<T>(array: T[], size: number): T[][] {
|
||||
return Array.from({ length: Math.ceil(array.length / size) }, (_, i) =>
|
||||
array.slice(i * size, i * size + size)
|
||||
)
|
||||
}
|
||||
|
||||
// _.groupBy
|
||||
function groupBy<T>(array: T[], key: keyof T): Record<string, T[]> {
|
||||
return array.reduce(
|
||||
(groups, item) => {
|
||||
const group = String(item[key])
|
||||
groups[group] = groups[group] ?? []
|
||||
groups[group].push(item)
|
||||
return groups
|
||||
},
|
||||
{} as Record<string, T[]>
|
||||
)
|
||||
}
|
||||
|
||||
// Object.groupBy (ES2024)
|
||||
const grouped = Object.groupBy(users, (user) => user.role)
|
||||
|
||||
// _.pick / _.omit
|
||||
const { password, ...userWithoutPassword } = user // omit
|
||||
const { id, name } = user // pick
|
||||
```
|
||||
|
||||
**When Lodash is still valuable:**
|
||||
|
||||
- `_.debounce`, `_.throttle` - complex timing logic
|
||||
- `_.cloneDeep` - deep object cloning
|
||||
- `_.merge` - deep object merging
|
||||
- `_.get` with default values (but optional chaining often suffices)
|
||||
|
||||
Reference: [You Don't Need Lodash](https://github.com/you-dont-need/You-Dont-Need-Lodash-Underscore)
|
||||
|
|
@ -1,89 +0,0 @@
|
|||
---
|
||||
title: Use for-of for Simple Iteration
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: reduces iteration boilerplate by 30-50%
|
||||
tags: runtime, loops, iteration, for-of, readability
|
||||
---
|
||||
|
||||
## Use for-of for Simple Iteration
|
||||
|
||||
`for-of` provides clean syntax for array iteration with performance comparable to traditional `for` loops. Use it when you don't need the index and aren't modifying the array.
|
||||
|
||||
**Incorrect (index-based when index isn't needed):**
|
||||
|
||||
```typescript
|
||||
function calculateTotal(orders: Order[]): number {
|
||||
let total = 0
|
||||
for (let i = 0; i < orders.length; i++) {
|
||||
total += orders[i].amount
|
||||
}
|
||||
return total
|
||||
}
|
||||
|
||||
function processUsers(users: User[]): void {
|
||||
for (let i = 0; i < users.length; i++) {
|
||||
sendNotification(users[i])
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (for-of for clean iteration):**
|
||||
|
||||
```typescript
|
||||
function calculateTotal(orders: Order[]): number {
|
||||
let total = 0
|
||||
for (const order of orders) {
|
||||
total += order.amount
|
||||
}
|
||||
return total
|
||||
}
|
||||
|
||||
function processUsers(users: User[]): void {
|
||||
for (const user of users) {
|
||||
sendNotification(user)
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**When to use each pattern:**
|
||||
|
||||
```typescript
|
||||
// for-of: when you only need values
|
||||
for (const item of items) {
|
||||
process(item)
|
||||
}
|
||||
|
||||
// forEach: when you want functional style (but can't break/return)
|
||||
items.forEach((item) => process(item))
|
||||
|
||||
// for-in: only for object keys (never for arrays)
|
||||
for (const key in config) {
|
||||
console.log(key, config[key])
|
||||
}
|
||||
|
||||
// Traditional for: when you need index, or need to modify loop
|
||||
for (let i = 0; i < items.length; i++) {
|
||||
if (items[i].id === targetId) {
|
||||
items[i] = updatedItem // Modifying array
|
||||
break // Early exit
|
||||
}
|
||||
}
|
||||
|
||||
// entries(): when you need both index and value
|
||||
for (const [index, item] of items.entries()) {
|
||||
console.log(`${index}: ${item.name}`)
|
||||
}
|
||||
```
|
||||
|
||||
**Avoid for-in for arrays:**
|
||||
|
||||
```typescript
|
||||
// NEVER do this
|
||||
for (const index in items) {
|
||||
// index is a string, not number
|
||||
// Iterates inherited properties
|
||||
// Wrong order not guaranteed
|
||||
}
|
||||
```
|
||||
|
||||
Reference: [MDN for...of](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of)
|
||||
|
|
@ -1,67 +0,0 @@
|
|||
---
|
||||
title: Use Set/Map for O(1) Lookups
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: O(n) to O(1) per lookup
|
||||
tags: runtime, set, map, lookup, performance
|
||||
---
|
||||
|
||||
## Use Set/Map for O(1) Lookups
|
||||
|
||||
Array methods like `.includes()` and `.find()` are O(n) operations. For frequent lookups, convert arrays to Set or Map for O(1) access.
|
||||
|
||||
**Incorrect (O(n) per lookup):**
|
||||
|
||||
```typescript
|
||||
const allowedRoles = ['admin', 'editor', 'viewer', 'moderator']
|
||||
|
||||
function hasPermission(userRole: string): boolean {
|
||||
return allowedRoles.includes(userRole) // O(n) every call
|
||||
}
|
||||
|
||||
// In a loop, this becomes O(n × m)
|
||||
function filterAuthorizedUsers(users: User[]): User[] {
|
||||
return users.filter((user) => allowedRoles.includes(user.role))
|
||||
// 1000 users × 4 roles = 4000 comparisons
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (O(1) per lookup):**
|
||||
|
||||
```typescript
|
||||
const allowedRoles = new Set(['admin', 'editor', 'viewer', 'moderator'])
|
||||
|
||||
function hasPermission(userRole: string): boolean {
|
||||
return allowedRoles.has(userRole) // O(1) every call
|
||||
}
|
||||
|
||||
function filterAuthorizedUsers(users: User[]): User[] {
|
||||
return users.filter((user) => allowedRoles.has(user.role))
|
||||
// 1000 users × O(1) = 1000 operations
|
||||
}
|
||||
```
|
||||
|
||||
**For object lookups by key:**
|
||||
|
||||
```typescript
|
||||
// Incorrect - O(n) search
|
||||
const users: User[] = [
|
||||
/* ... */
|
||||
]
|
||||
function findUserById(id: string): User | undefined {
|
||||
return users.find((u) => u.id === id) // Scans entire array
|
||||
}
|
||||
|
||||
// Correct - O(1) lookup
|
||||
const userById = new Map<string, User>(users.map((u) => [u.id, u]))
|
||||
function findUserById(id: string): User | undefined {
|
||||
return userById.get(id)
|
||||
}
|
||||
```
|
||||
|
||||
**When to stick with arrays:**
|
||||
|
||||
- Small collections (< 10 items)
|
||||
- One-time lookups where conversion cost exceeds benefit
|
||||
- When you need array methods like `.map()`, `.filter()`, `.slice()`
|
||||
|
||||
Reference: [MDN Set](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set)
|
||||
|
|
@ -1,78 +0,0 @@
|
|||
---
|
||||
title: Use Modern String Methods
|
||||
impact: LOW-MEDIUM
|
||||
impactDescription: 2-5× faster than regex for simple patterns
|
||||
tags: runtime, strings, methods, performance, readability
|
||||
---
|
||||
|
||||
## Use Modern String Methods
|
||||
|
||||
Modern string methods like `startsWith()`, `endsWith()`, `includes()`, and `padStart()` are clearer and often faster than regex or manual substring operations.
|
||||
|
||||
**Incorrect (regex or substring for simple checks):**
|
||||
|
||||
```typescript
|
||||
function isImageFile(filename: string): boolean {
|
||||
return /\.(jpg|png|gif)$/.test(filename)
|
||||
}
|
||||
|
||||
function hasHttpPrefix(url: string): boolean {
|
||||
return url.substring(0, 7) === 'http://' || url.substring(0, 8) === 'https://'
|
||||
}
|
||||
|
||||
function containsSearchTerm(text: string, term: string): boolean {
|
||||
return text.indexOf(term) !== -1
|
||||
}
|
||||
|
||||
function formatOrderId(id: number): string {
|
||||
return ('000000' + id).slice(-6) // Pad to 6 digits
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (modern string methods):**
|
||||
|
||||
```typescript
|
||||
function isImageFile(filename: string): boolean {
|
||||
return filename.endsWith('.jpg') || filename.endsWith('.png') || filename.endsWith('.gif')
|
||||
}
|
||||
|
||||
function hasHttpPrefix(url: string): boolean {
|
||||
return url.startsWith('http://') || url.startsWith('https://')
|
||||
}
|
||||
|
||||
function containsSearchTerm(text: string, term: string): boolean {
|
||||
return text.includes(term)
|
||||
}
|
||||
|
||||
function formatOrderId(id: number): string {
|
||||
return String(id).padStart(6, '0')
|
||||
}
|
||||
```
|
||||
|
||||
**Additional useful methods:**
|
||||
|
||||
```typescript
|
||||
// replaceAll (no global regex needed)
|
||||
const sanitized = input.replaceAll('<', '<').replaceAll('>', '>')
|
||||
|
||||
// at() for negative indexing
|
||||
const lastChar = filename.at(-1) // Last character
|
||||
const extension = filename.split('.').at(-1) // Last segment
|
||||
|
||||
// trimStart/trimEnd for directional trimming
|
||||
const trimmedLeft = ' text '.trimStart() // 'text '
|
||||
const trimmedRight = ' text '.trimEnd() // ' text'
|
||||
|
||||
// repeat for string multiplication
|
||||
const separator = '-'.repeat(40)
|
||||
const indent = ' '.repeat(depth)
|
||||
```
|
||||
|
||||
**When regex is still needed:**
|
||||
|
||||
- Complex pattern matching
|
||||
- Capture groups
|
||||
- Case-insensitive matching (`/pattern/i`)
|
||||
- Multiple conditions in one check
|
||||
|
||||
Reference: [MDN String Methods](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String)
|
||||
|
|
@ -1,93 +0,0 @@
|
|||
---
|
||||
title: Use Assertion Functions for Validation
|
||||
impact: MEDIUM-HIGH
|
||||
impactDescription: reduces validation boilerplate by 50-70%
|
||||
tags: safety, assertion-functions, asserts, validation, narrowing
|
||||
---
|
||||
|
||||
## Use Assertion Functions for Validation
|
||||
|
||||
Assertion functions (`asserts` return type) tell TypeScript that if the function returns, the condition is true. This narrows types in the calling scope without explicit if-checks.
|
||||
|
||||
**Incorrect (repeated if-throw pattern):**
|
||||
|
||||
```typescript
|
||||
function processOrder(order: Order | null): void {
|
||||
if (!order) {
|
||||
throw new Error('Order is required')
|
||||
}
|
||||
if (order.status !== 'pending') {
|
||||
throw new Error('Order must be pending')
|
||||
}
|
||||
if (!order.items.length) {
|
||||
throw new Error('Order must have items')
|
||||
}
|
||||
|
||||
// Finally can use order safely
|
||||
submitOrder(order)
|
||||
}
|
||||
|
||||
// Same checks repeated in every function that needs a valid order
|
||||
function shipOrder(order: Order | null): void {
|
||||
if (!order) throw new Error('Order is required')
|
||||
if (order.status !== 'pending') throw new Error('Order must be pending')
|
||||
// ...duplicate validation
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (assertion function):**
|
||||
|
||||
```typescript
|
||||
interface ValidOrder extends Order {
|
||||
status: 'pending'
|
||||
items: [OrderItem, ...OrderItem[]] // Non-empty array
|
||||
}
|
||||
|
||||
function assertValidOrder(order: Order | null): asserts order is ValidOrder {
|
||||
if (!order) {
|
||||
throw new Error('Order is required')
|
||||
}
|
||||
if (order.status !== 'pending') {
|
||||
throw new Error('Order must be pending')
|
||||
}
|
||||
if (!order.items.length) {
|
||||
throw new Error('Order must have items')
|
||||
}
|
||||
}
|
||||
|
||||
function processOrder(order: Order | null): void {
|
||||
assertValidOrder(order)
|
||||
// order is now typed as ValidOrder
|
||||
submitOrder(order) // Type-safe
|
||||
}
|
||||
|
||||
function shipOrder(order: Order | null): void {
|
||||
assertValidOrder(order)
|
||||
// Reuses validation, order is ValidOrder
|
||||
ship(order)
|
||||
}
|
||||
```
|
||||
|
||||
**For generic assertions:**
|
||||
|
||||
```typescript
|
||||
function assertDefined<T>(value: T | null | undefined, name: string): asserts value is T {
|
||||
if (value === null || value === undefined) {
|
||||
throw new Error(`${name} must be defined`)
|
||||
}
|
||||
}
|
||||
|
||||
function processUser(user: User | null): void {
|
||||
assertDefined(user, 'user')
|
||||
// user is now User, not User | null
|
||||
console.log(user.email)
|
||||
}
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
|
||||
- Centralizes validation logic
|
||||
- Automatic type narrowing after assertion
|
||||
- Clearer intent than if-throw patterns
|
||||
|
||||
Reference: [TypeScript 3.7 Assertion Functions](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html#assertion-functions)
|
||||
|
|
@ -1,78 +0,0 @@
|
|||
---
|
||||
title: Use const Assertions for Literal Types
|
||||
impact: MEDIUM-HIGH
|
||||
impactDescription: preserves literal types, enables better inference
|
||||
tags: safety, const-assertion, literals, inference, readonly
|
||||
---
|
||||
|
||||
## Use const Assertions for Literal Types
|
||||
|
||||
The `as const` assertion preserves literal types and makes arrays/objects readonly. This enables precise type inference and prevents accidental mutations.
|
||||
|
||||
**Incorrect (widened types):**
|
||||
|
||||
```typescript
|
||||
const config = {
|
||||
apiUrl: 'https://api.example.com',
|
||||
retries: 3,
|
||||
methods: ['GET', 'POST']
|
||||
}
|
||||
// Type: { apiUrl: string; retries: number; methods: string[] }
|
||||
|
||||
function makeRequest(method: 'GET' | 'POST'): void {}
|
||||
|
||||
makeRequest(config.methods[0])
|
||||
// Error: Argument of type 'string' is not assignable to 'GET' | 'POST'
|
||||
|
||||
const STATUS = {
|
||||
PENDING: 'pending',
|
||||
ACTIVE: 'active'
|
||||
}
|
||||
// Type: { PENDING: string; ACTIVE: string }
|
||||
```
|
||||
|
||||
**Correct (const assertion preserves literals):**
|
||||
|
||||
```typescript
|
||||
const config = {
|
||||
apiUrl: 'https://api.example.com',
|
||||
retries: 3,
|
||||
methods: ['GET', 'POST']
|
||||
} as const
|
||||
// Type: { readonly apiUrl: 'https://api.example.com'; readonly retries: 3; readonly methods: readonly ['GET', 'POST'] }
|
||||
|
||||
function makeRequest(method: 'GET' | 'POST'): void {}
|
||||
|
||||
makeRequest(config.methods[0]) // Works: 'GET' is assignable to 'GET' | 'POST'
|
||||
|
||||
const STATUS = {
|
||||
PENDING: 'pending',
|
||||
ACTIVE: 'active'
|
||||
} as const
|
||||
// Type: { readonly PENDING: 'pending'; readonly ACTIVE: 'active' }
|
||||
|
||||
type StatusType = (typeof STATUS)[keyof typeof STATUS] // 'pending' | 'active'
|
||||
```
|
||||
|
||||
**For function parameters:**
|
||||
|
||||
```typescript
|
||||
// Incorrect - tuple becomes array
|
||||
function setCoordinates(coords: [number, number]): void {}
|
||||
setCoordinates([10, 20]) // Error: number[] not assignable to [number, number]
|
||||
|
||||
// Correct - const preserves tuple
|
||||
setCoordinates([10, 20] as const) // Works
|
||||
|
||||
// Or inline
|
||||
function setCoordinates(coords: readonly [number, number]): void {}
|
||||
```
|
||||
|
||||
**When to use const assertions:**
|
||||
|
||||
- Configuration objects that shouldn't change
|
||||
- Enum-like objects with string values
|
||||
- Array/tuple literals passed to functions expecting specific types
|
||||
- Creating type-safe lookup tables
|
||||
|
||||
Reference: [TypeScript 3.4 Const Assertions](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-4.html#const-assertions)
|
||||
|
|
@ -1,84 +0,0 @@
|
|||
---
|
||||
title: Use Exhaustive Checks for Union Types
|
||||
impact: MEDIUM-HIGH
|
||||
impactDescription: prevents 100% of missing case errors at compile time
|
||||
tags: safety, exhaustive, never, discriminated-unions, switch
|
||||
---
|
||||
|
||||
## Use Exhaustive Checks for Union Types
|
||||
|
||||
Exhaustive checks ensure all union members are handled. When a new member is added, TypeScript errors on unhandled cases rather than falling through silently at runtime.
|
||||
|
||||
**Incorrect (missing case compiles but fails at runtime):**
|
||||
|
||||
```typescript
|
||||
type OrderStatus = 'pending' | 'processing' | 'shipped' | 'delivered'
|
||||
|
||||
function getStatusMessage(status: OrderStatus): string {
|
||||
switch (status) {
|
||||
case 'pending':
|
||||
return 'Order received'
|
||||
case 'processing':
|
||||
return 'Preparing your order'
|
||||
case 'shipped':
|
||||
return 'On the way'
|
||||
// 'delivered' case missing - no compile error
|
||||
// Returns undefined at runtime
|
||||
}
|
||||
}
|
||||
|
||||
// Later, someone adds 'cancelled' to OrderStatus
|
||||
// This function silently returns undefined for 'cancelled' and 'delivered'
|
||||
```
|
||||
|
||||
**Correct (exhaustive check with never):**
|
||||
|
||||
```typescript
|
||||
type OrderStatus = 'pending' | 'processing' | 'shipped' | 'delivered'
|
||||
|
||||
function assertNever(value: never): never {
|
||||
throw new Error(`Unhandled value: ${value}`)
|
||||
}
|
||||
|
||||
function getStatusMessage(status: OrderStatus): string {
|
||||
switch (status) {
|
||||
case 'pending':
|
||||
return 'Order received'
|
||||
case 'processing':
|
||||
return 'Preparing your order'
|
||||
case 'shipped':
|
||||
return 'On the way'
|
||||
case 'delivered':
|
||||
return 'Order complete'
|
||||
default:
|
||||
return assertNever(status) // Compile error if case missed
|
||||
}
|
||||
}
|
||||
|
||||
// Adding 'cancelled' to OrderStatus now causes compile error:
|
||||
// Argument of type 'string' is not assignable to parameter of type 'never'
|
||||
```
|
||||
|
||||
**For object mapping (alternative pattern):**
|
||||
|
||||
```typescript
|
||||
const statusMessages: Record<OrderStatus, string> = {
|
||||
pending: 'Order received',
|
||||
processing: 'Preparing your order',
|
||||
shipped: 'On the way',
|
||||
delivered: 'Order complete'
|
||||
// Missing key causes: Property 'cancelled' is missing in type
|
||||
}
|
||||
|
||||
function getStatusMessage(status: OrderStatus): string {
|
||||
return statusMessages[status]
|
||||
}
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
|
||||
- Compile-time error when union expands
|
||||
- Self-documenting: all cases explicitly handled
|
||||
- Runtime safety via assertNever fallback
|
||||
|
||||
Reference: [TypeScript Handbook - Exhaustiveness Checking](https://www.typescriptlang.org/docs/handbook/2/narrowing.html#exhaustiveness-checking)
|
||||
|
|
@ -1,86 +0,0 @@
|
|||
---
|
||||
title: Enable noUncheckedIndexedAccess
|
||||
impact: MEDIUM-HIGH
|
||||
impactDescription: prevents 100% of unchecked index access errors at compile time
|
||||
tags: safety, noUncheckedIndexedAccess, strict, arrays, undefined
|
||||
---
|
||||
|
||||
## Enable noUncheckedIndexedAccess
|
||||
|
||||
With `noUncheckedIndexedAccess` enabled, TypeScript adds `undefined` to the type of array elements and index signature properties. This catches one of the most common sources of runtime errors — accessing elements that may not exist.
|
||||
|
||||
**Incorrect (without noUncheckedIndexedAccess):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"strict": true
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```typescript
|
||||
const users = ['Alice', 'Bob', 'Charlie']
|
||||
const first = users[0] // Type: string (lies — could be undefined)
|
||||
console.log(first.toUpperCase()) // No error, but crashes if array is empty
|
||||
|
||||
const scores: Record<string, number> = { math: 95 }
|
||||
const science = scores['science'] // Type: number (lies — key doesn't exist)
|
||||
console.log(science.toFixed(2)) // No error, but crashes at runtime
|
||||
```
|
||||
|
||||
**Correct (with noUncheckedIndexedAccess):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"strict": true,
|
||||
"noUncheckedIndexedAccess": true
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```typescript
|
||||
const users = ['Alice', 'Bob', 'Charlie']
|
||||
const first = users[0] // Type: string | undefined
|
||||
console.log(first.toUpperCase()) // Error: 'first' is possibly undefined
|
||||
|
||||
// Handle the undefined case
|
||||
if (first) {
|
||||
console.log(first.toUpperCase()) // OK after narrowing
|
||||
}
|
||||
|
||||
const scores: Record<string, number> = { math: 95 }
|
||||
const science = scores['science'] // Type: number | undefined
|
||||
if (science !== undefined) {
|
||||
console.log(science.toFixed(2)) // OK after narrowing
|
||||
}
|
||||
```
|
||||
|
||||
**Common patterns with noUncheckedIndexedAccess:**
|
||||
|
||||
```typescript
|
||||
// Array destructuring — first element is T | undefined
|
||||
const [head, ...rest] = items
|
||||
if (head) {
|
||||
processItem(head)
|
||||
}
|
||||
|
||||
// Use non-null assertion only when you've validated
|
||||
function getRequired(items: string[], index: number): string {
|
||||
if (index < 0 || index >= items.length) {
|
||||
throw new RangeError(`Index ${index} out of bounds`)
|
||||
}
|
||||
return items[index]! // Safe — bounds checked above
|
||||
}
|
||||
|
||||
// Array.at() returns T | undefined regardless of this flag
|
||||
const last = items.at(-1) // Already T | undefined
|
||||
```
|
||||
|
||||
**When to disable:**
|
||||
|
||||
- Legacy codebases with heavy array indexing (migration cost too high)
|
||||
- Performance-critical inner loops where the narrowing pattern adds overhead
|
||||
|
||||
Reference: [TypeScript tsconfig - noUncheckedIndexedAccess](https://www.typescriptlang.org/tsconfig/noUncheckedIndexedAccess.html)
|
||||
|
|
@ -1,77 +0,0 @@
|
|||
---
|
||||
title: Prefer unknown Over any
|
||||
impact: MEDIUM-HIGH
|
||||
impactDescription: forces type narrowing, prevents runtime errors
|
||||
tags: safety, unknown, any, type-narrowing, type-safety
|
||||
---
|
||||
|
||||
## Prefer unknown Over any
|
||||
|
||||
The `any` type disables all type checking, allowing unsafe operations to pass silently. Use `unknown` to require explicit type narrowing before operations.
|
||||
|
||||
**Incorrect (any bypasses all checks):**
|
||||
|
||||
```typescript
|
||||
function processApiResponse(data: any): string {
|
||||
return data.user.name.toUpperCase()
|
||||
// No error even if data is null, has no user, or name isn't a string
|
||||
// Runtime: TypeError: Cannot read property 'name' of undefined
|
||||
}
|
||||
|
||||
async function fetchData(): Promise<any> {
|
||||
const response = await fetch('/api/data')
|
||||
return response.json() // Returns Promise<any>, loses all type info
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (unknown requires narrowing):**
|
||||
|
||||
```typescript
|
||||
interface ApiResponse {
|
||||
user: {
|
||||
name: string
|
||||
}
|
||||
}
|
||||
|
||||
function isApiResponse(data: unknown): data is ApiResponse {
|
||||
return (
|
||||
typeof data === 'object' &&
|
||||
data !== null &&
|
||||
'user' in data &&
|
||||
typeof (data as ApiResponse).user?.name === 'string'
|
||||
)
|
||||
}
|
||||
|
||||
function processApiResponse(data: unknown): string {
|
||||
if (!isApiResponse(data)) {
|
||||
throw new Error('Invalid API response')
|
||||
}
|
||||
return data.user.name.toUpperCase() // Type-safe access
|
||||
}
|
||||
```
|
||||
|
||||
**For JSON parsing:**
|
||||
|
||||
```typescript
|
||||
// Incorrect
|
||||
const config = JSON.parse(configString) as AppConfig // Unsafe assertion
|
||||
|
||||
// Correct
|
||||
function parseConfig(configString: string): AppConfig {
|
||||
const parsed: unknown = JSON.parse(configString)
|
||||
|
||||
if (!isValidConfig(parsed)) {
|
||||
throw new Error('Invalid config format')
|
||||
}
|
||||
|
||||
return parsed
|
||||
}
|
||||
```
|
||||
|
||||
**When any is acceptable:**
|
||||
|
||||
- Migrating JavaScript to TypeScript incrementally
|
||||
- Third-party library workarounds (with `// @ts-expect-error`)
|
||||
- Truly dynamic code where type is unknowable
|
||||
|
||||
Reference: [TypeScript Handbook - Unknown](https://www.typescriptlang.org/docs/handbook/2/narrowing.html#the-unknown-type)
|
||||
|
|
@ -1,72 +0,0 @@
|
|||
---
|
||||
title: Enable strictNullChecks
|
||||
impact: MEDIUM-HIGH
|
||||
impactDescription: prevents null/undefined runtime errors
|
||||
tags: safety, strictNullChecks, null, undefined, strict
|
||||
---
|
||||
|
||||
## Enable strictNullChecks
|
||||
|
||||
With `strictNullChecks`, TypeScript distinguishes between `T`, `T | null`, and `T | undefined`. This catches null pointer exceptions at compile time instead of runtime.
|
||||
|
||||
**Incorrect (strictNullChecks disabled):**
|
||||
|
||||
```typescript
|
||||
// tsconfig.json: { "strictNullChecks": false }
|
||||
|
||||
function getUser(id: string): User {
|
||||
return userMap.get(id) // Returns User | undefined, but typed as User
|
||||
}
|
||||
|
||||
const user = getUser('123')
|
||||
console.log(user.email) // No error, but crashes if user is undefined
|
||||
```
|
||||
|
||||
**Correct (strictNullChecks enabled):**
|
||||
|
||||
```typescript
|
||||
// tsconfig.json: { "strict": true } (includes strictNullChecks)
|
||||
|
||||
function getUser(id: string): User | undefined {
|
||||
return userMap.get(id) // Correctly typed as User | undefined
|
||||
}
|
||||
|
||||
const user = getUser('123')
|
||||
console.log(user.email) // Error: 'user' is possibly 'undefined'
|
||||
|
||||
// Must handle the undefined case
|
||||
if (user) {
|
||||
console.log(user.email) // Type narrowed to User
|
||||
}
|
||||
|
||||
// Or use optional chaining
|
||||
console.log(user?.email) // string | undefined
|
||||
|
||||
// Or assert when you're certain
|
||||
const confirmedUser = getUser('123')! // Non-null assertion (use sparingly)
|
||||
```
|
||||
|
||||
**Common patterns with strictNullChecks:**
|
||||
|
||||
```typescript
|
||||
// Default values
|
||||
function greet(name: string | undefined): string {
|
||||
return `Hello, ${name ?? 'Guest'}`
|
||||
}
|
||||
|
||||
// Guard clauses
|
||||
function processOrder(order: Order | null): void {
|
||||
if (!order) {
|
||||
throw new Error('Order is required')
|
||||
}
|
||||
// order is narrowed to Order
|
||||
ship(order)
|
||||
}
|
||||
|
||||
// Optional chaining with nullish coalescing
|
||||
const street = user?.address?.street ?? 'Unknown'
|
||||
```
|
||||
|
||||
**Note:** Always enable `strict: true` which includes `strictNullChecks` along with other safety checks.
|
||||
|
||||
Reference: [TypeScript Handbook - Strict Null Checks](https://www.typescriptlang.org/tsconfig#strictNullChecks)
|
||||
|
|
@ -1,86 +0,0 @@
|
|||
---
|
||||
title: Use Type Guards for Runtime Type Checking
|
||||
impact: MEDIUM-HIGH
|
||||
impactDescription: eliminates type assertions, catches errors at boundaries
|
||||
tags: safety, type-guards, narrowing, predicates, validation
|
||||
---
|
||||
|
||||
## Use Type Guards for Runtime Type Checking
|
||||
|
||||
Type guards provide runtime validation that TypeScript can use for static narrowing. They replace unsafe type assertions with checked operations.
|
||||
|
||||
**Incorrect (type assertions without validation):**
|
||||
|
||||
```typescript
|
||||
interface User {
|
||||
id: string
|
||||
email: string
|
||||
role: 'admin' | 'user'
|
||||
}
|
||||
|
||||
function handleUserEvent(event: MessageEvent): void {
|
||||
const user = event.data as User // Unsafe assertion
|
||||
sendEmail(user.email) // Crashes if data isn't actually a User
|
||||
}
|
||||
|
||||
function processResponse(data: unknown): User[] {
|
||||
return data as User[] // No runtime check
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (type guard with validation):**
|
||||
|
||||
```typescript
|
||||
function isUser(value: unknown): value is User {
|
||||
return (
|
||||
typeof value === 'object' &&
|
||||
value !== null &&
|
||||
typeof (value as User).id === 'string' &&
|
||||
typeof (value as User).email === 'string' &&
|
||||
['admin', 'user'].includes((value as User).role)
|
||||
)
|
||||
}
|
||||
|
||||
function handleUserEvent(event: MessageEvent): void {
|
||||
if (!isUser(event.data)) {
|
||||
console.error('Invalid user data received')
|
||||
return
|
||||
}
|
||||
sendEmail(event.data.email) // Type-safe: event.data is User
|
||||
}
|
||||
|
||||
function processResponse(data: unknown): User[] {
|
||||
if (!Array.isArray(data)) return []
|
||||
return data.filter(isUser)
|
||||
}
|
||||
```
|
||||
|
||||
**For discriminated unions:**
|
||||
|
||||
```typescript
|
||||
interface SuccessResult {
|
||||
status: 'success'
|
||||
data: User
|
||||
}
|
||||
|
||||
interface ErrorResult {
|
||||
status: 'error'
|
||||
message: string
|
||||
}
|
||||
|
||||
type ApiResult = SuccessResult | ErrorResult
|
||||
|
||||
function isSuccess(result: ApiResult): result is SuccessResult {
|
||||
return result.status === 'success'
|
||||
}
|
||||
|
||||
function handleResult(result: ApiResult): void {
|
||||
if (isSuccess(result)) {
|
||||
console.log(result.data.email) // Type narrowed to SuccessResult
|
||||
} else {
|
||||
console.error(result.message) // Type narrowed to ErrorResult
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Reference: [TypeScript Handbook - Narrowing](https://www.typescriptlang.org/docs/handbook/2/narrowing.html)
|
||||
|
|
@ -1,66 +0,0 @@
|
|||
---
|
||||
title: Enable Incremental Compilation
|
||||
impact: CRITICAL
|
||||
impactDescription: 50-90% faster rebuilds
|
||||
tags: tscfg, incremental, tsconfig, compilation, caching
|
||||
---
|
||||
|
||||
## Enable Incremental Compilation
|
||||
|
||||
Incremental compilation caches project graph information between builds in a `.tsbuildinfo` file. Subsequent compilations only recheck changed files and their dependents.
|
||||
|
||||
**Incorrect (full rebuild every time):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"target": "ES2022",
|
||||
"module": "ESNext",
|
||||
"strict": true
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```bash
|
||||
tsc # 15 seconds every build
|
||||
```
|
||||
|
||||
**Correct (incremental builds):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"target": "ES2022",
|
||||
"module": "ESNext",
|
||||
"strict": true,
|
||||
"incremental": true,
|
||||
"tsBuildInfoFile": "./dist/.tsbuildinfo"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```bash
|
||||
tsc # 15s first build, 1-3s subsequent builds
|
||||
```
|
||||
|
||||
**For monorepos (composite projects):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"composite": true,
|
||||
"declaration": true,
|
||||
"declarationMap": true
|
||||
},
|
||||
"references": [{ "path": "../shared" }, { "path": "../utils" }]
|
||||
}
|
||||
```
|
||||
|
||||
**Note:** The `composite` flag implies `incremental: true` and requires `declaration: true`.
|
||||
|
||||
**When to disable incremental:**
|
||||
|
||||
- CI environments where cache isn't preserved between runs
|
||||
- One-off type-checking scripts
|
||||
|
||||
Reference: [TypeScript Performance Wiki](https://github.com/microsoft/TypeScript/wiki/Performance#incremental-project-emit)
|
||||
|
|
@ -1,88 +0,0 @@
|
|||
---
|
||||
title: Use erasableSyntaxOnly for Node.js Native TypeScript
|
||||
impact: HIGH
|
||||
impactDescription: prevents 100% of Node.js type-stripping runtime errors
|
||||
tags: tscfg, erasableSyntaxOnly, node, type-stripping, enums
|
||||
---
|
||||
|
||||
## Use erasableSyntaxOnly for Node.js Native TypeScript
|
||||
|
||||
The `erasableSyntaxOnly` flag (TypeScript 5.8+) ensures your code only uses TypeScript syntax that can be removed by erasing type annotations — no code generation required. This is mandatory for Node.js `--experimental-strip-types` which strips types but cannot transform enums, namespaces, or parameter properties.
|
||||
|
||||
**Incorrect (non-erasable syntax fails with Node.js type-stripping):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"erasableSyntaxOnly": true
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```typescript
|
||||
// Error: non-erasable syntax
|
||||
export enum OrderStatus {
|
||||
Pending = 'pending',
|
||||
Shipped = 'shipped',
|
||||
Delivered = 'delivered'
|
||||
}
|
||||
|
||||
// Error: non-erasable syntax
|
||||
namespace Validation {
|
||||
export function isValid(input: string): boolean {
|
||||
return input.length > 0
|
||||
}
|
||||
}
|
||||
|
||||
// Error: non-erasable parameter property
|
||||
class UserService {
|
||||
constructor(private readonly repository: UserRepository) {}
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (erasable alternatives):**
|
||||
|
||||
```typescript
|
||||
// Union type instead of enum
|
||||
export type OrderStatus = 'pending' | 'shipped' | 'delivered'
|
||||
|
||||
// Object constant for runtime values
|
||||
export const OrderStatus = {
|
||||
Pending: 'pending',
|
||||
Shipped: 'shipped',
|
||||
Delivered: 'delivered'
|
||||
} as const satisfies Record<string, OrderStatus>
|
||||
|
||||
// Module-level functions instead of namespace
|
||||
export function isValid(input: string): boolean {
|
||||
return input.length > 0
|
||||
}
|
||||
|
||||
// Explicit property assignment instead of parameter property
|
||||
class UserService {
|
||||
readonly repository: UserRepository
|
||||
constructor(repository: UserRepository) {
|
||||
this.repository = repository
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Recommended configuration for Node.js native TS:**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"erasableSyntaxOnly": true,
|
||||
"verbatimModuleSyntax": true,
|
||||
"isolatedModules": true
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**When NOT to use this flag:**
|
||||
|
||||
- Projects using a bundler (esbuild, swc, Vite) that supports enum transformation
|
||||
- Libraries that need to support both bundled and unbundled consumers
|
||||
- Codebases with extensive enum usage where migration cost is high
|
||||
|
||||
Reference: [TypeScript 5.8 - erasableSyntaxOnly](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-8.html)
|
||||
|
|
@ -1,80 +0,0 @@
|
|||
---
|
||||
title: Configure Include and Exclude Properly
|
||||
impact: CRITICAL
|
||||
impactDescription: prevents scanning thousands of unnecessary files
|
||||
tags: tscfg, include, exclude, tsconfig, file-discovery
|
||||
---
|
||||
|
||||
## Configure Include and Exclude Properly
|
||||
|
||||
TypeScript walks through all included directories to discover files. Overly broad `include` patterns or missing `exclude` patterns force the compiler to scan irrelevant directories, significantly slowing startup.
|
||||
|
||||
**Incorrect (scans entire project tree):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"outDir": "dist"
|
||||
},
|
||||
"include": ["**/*"]
|
||||
}
|
||||
```
|
||||
|
||||
```bash
|
||||
# Scans node_modules, dist, coverage, .git...
|
||||
# Discovery time: 5+ seconds on large projects
|
||||
```
|
||||
|
||||
**Correct (targeted include with explicit exclude):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"outDir": "dist",
|
||||
"rootDir": "src"
|
||||
},
|
||||
"include": ["src/**/*"],
|
||||
"exclude": ["node_modules", "dist", "coverage", "**/*.test.ts", "**/*.spec.ts", "**/__tests__/**"]
|
||||
}
|
||||
```
|
||||
|
||||
```bash
|
||||
# Only scans src/ directory
|
||||
# Discovery time: <1 second
|
||||
```
|
||||
|
||||
**For separate test configuration:**
|
||||
|
||||
```json
|
||||
// tsconfig.json (production)
|
||||
{
|
||||
"include": ["src/**/*"],
|
||||
"exclude": ["**/*.test.ts"]
|
||||
}
|
||||
|
||||
// tsconfig.test.json
|
||||
{
|
||||
"extends": "./tsconfig.json",
|
||||
"include": ["src/**/*", "tests/**/*"]
|
||||
}
|
||||
```
|
||||
|
||||
**Diagnostic commands:**
|
||||
|
||||
```bash
|
||||
# List all files TypeScript will compile
|
||||
tsc --listFiles
|
||||
|
||||
# Explain why each file was included
|
||||
tsc --explainFiles
|
||||
```
|
||||
|
||||
**Common files to exclude:**
|
||||
|
||||
- `node_modules` (always)
|
||||
- Build output directories (`dist`, `build`, `out`)
|
||||
- Test files for production builds
|
||||
- Generated files (`.generated.ts`)
|
||||
- Coverage reports (`coverage`)
|
||||
|
||||
Reference: [TypeScript Performance Wiki](https://github.com/microsoft/TypeScript/wiki/Performance#configuring-tsconfigjson-or-jsconfigjson)
|
||||
|
|
@ -1,81 +0,0 @@
|
|||
---
|
||||
title: Use isolatedModules for Single-File Transpilation
|
||||
impact: CRITICAL
|
||||
impactDescription: 80-90% faster transpilation with bundlers
|
||||
tags: tscfg, isolatedModules, transpilation, bundlers, performance
|
||||
---
|
||||
|
||||
## Use isolatedModules for Single-File Transpilation
|
||||
|
||||
The `isolatedModules` flag ensures each file can be transpiled independently, enabling parallel transpilation by bundlers like esbuild, swc, or Babel. This bypasses TypeScript's slower multi-file analysis.
|
||||
|
||||
**Incorrect (requires cross-file analysis):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"target": "ES2022",
|
||||
"module": "ESNext"
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```typescript
|
||||
// constants.ts
|
||||
export const enum Status {
|
||||
Active = 'active',
|
||||
Inactive = 'inactive'
|
||||
}
|
||||
|
||||
// user.ts
|
||||
import { Status } from './constants'
|
||||
const status = Status.Active // Requires reading constants.ts to inline
|
||||
```
|
||||
|
||||
**Correct (single-file transpilable):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"target": "ES2022",
|
||||
"module": "ESNext",
|
||||
"isolatedModules": true,
|
||||
"verbatimModuleSyntax": true
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```typescript
|
||||
// constants.ts
|
||||
export enum Status {
|
||||
// Regular enum, not const enum
|
||||
Active = 'active',
|
||||
Inactive = 'inactive'
|
||||
}
|
||||
|
||||
// user.ts
|
||||
import { Status } from './constants'
|
||||
const status = Status.Active // Reference preserved, no cross-file read
|
||||
```
|
||||
|
||||
**Build pipeline integration:**
|
||||
|
||||
```javascript
|
||||
// vite.config.ts
|
||||
export default {
|
||||
esbuild: {
|
||||
// esbuild transpiles files in parallel
|
||||
// TypeScript only runs type-checking
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Code patterns blocked by isolatedModules:**
|
||||
|
||||
- `const enum` (use regular `enum` or union types instead)
|
||||
- `export =` / `import =` syntax
|
||||
- Re-exporting types without `type` keyword
|
||||
|
||||
**Note:** With `erasableSyntaxOnly` (TypeScript 5.8+), regular enums are also blocked. Use union types (`type Status = 'active' | 'inactive'`) as the universal safe alternative. See `tscfg-erasable-syntax-only.md`.
|
||||
|
||||
Reference: [TypeScript Performance Wiki](https://github.com/microsoft/TypeScript/wiki/Performance#isolated-file-emit)
|
||||
|
|
@ -1,73 +0,0 @@
|
|||
---
|
||||
title: Enable isolatedDeclarations for Parallel Declaration Emit
|
||||
impact: CRITICAL
|
||||
impactDescription: enables parallel .d.ts generation without type-checker
|
||||
tags: tscfg, isolatedDeclarations, declarations, parallel, performance
|
||||
---
|
||||
|
||||
## Enable isolatedDeclarations for Parallel Declaration Emit
|
||||
|
||||
The `isolatedDeclarations` flag (TypeScript 5.5+) ensures each file's exports are annotated sufficiently for tools to generate `.d.ts` files without running the type-checker. This enables parallel declaration emit via bundlers and dramatically speeds up builds in large codebases.
|
||||
|
||||
**Incorrect (declaration emit requires full type-check):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"declaration": true
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```typescript
|
||||
// utils.ts
|
||||
export function calculateTotal(items: CartItem[]) {
|
||||
// Return type inferred — requires type-checker to generate .d.ts
|
||||
return items.reduce((sum, item) => sum + item.price * item.quantity, 0)
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (explicit annotations enable parallel emit):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"declaration": true,
|
||||
"isolatedDeclarations": true
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```typescript
|
||||
// utils.ts
|
||||
export function calculateTotal(items: CartItem[]): number {
|
||||
// Explicit return type — .d.ts can be generated per-file, in parallel
|
||||
return items.reduce((sum, item) => sum + item.price * item.quantity, 0)
|
||||
}
|
||||
```
|
||||
|
||||
**What requires annotation under isolatedDeclarations:**
|
||||
|
||||
- Exported function return types
|
||||
- Exported variable types when not inferable from a literal
|
||||
- Exported class method return types
|
||||
|
||||
**What does NOT need annotation:**
|
||||
|
||||
- Local variables and functions (not exported)
|
||||
- Function parameters (already required by TypeScript)
|
||||
- Exports initialized with literals (`export const MAX = 100` is fine)
|
||||
|
||||
**Pair with isolatedModules for maximum build speed:**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"isolatedModules": true,
|
||||
"isolatedDeclarations": true,
|
||||
"declaration": true
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Reference: [TypeScript 5.5 - Isolated Declarations](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-5.html#isolated-declarations)
|
||||
|
|
@ -1,96 +0,0 @@
|
|||
---
|
||||
title: Use Project References for Large Codebases
|
||||
impact: CRITICAL
|
||||
impactDescription: 60-80% faster incremental builds
|
||||
tags: tscfg, project-references, monorepo, tsconfig, compilation
|
||||
---
|
||||
|
||||
## Use Project References for Large Codebases
|
||||
|
||||
Project references split a codebase into independent compilation units. Each project compiles separately, enabling parallel builds and preventing the compiler from loading the entire codebase at once.
|
||||
|
||||
**Incorrect (monolithic tsconfig):**
|
||||
|
||||
```text
|
||||
my-app/
|
||||
├── tsconfig.json # Single config for entire app
|
||||
├── packages/
|
||||
│ ├── api/src/
|
||||
│ ├── web/src/
|
||||
│ └── shared/src/
|
||||
```
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": { "outDir": "dist" },
|
||||
"include": ["packages/*/src/**/*"]
|
||||
}
|
||||
```
|
||||
|
||||
```bash
|
||||
# Loads ALL files into memory for every change
|
||||
# Change in api/ triggers full recompile
|
||||
```
|
||||
|
||||
**Correct (project references):**
|
||||
|
||||
```text
|
||||
my-app/
|
||||
├── tsconfig.json # Root config with references
|
||||
├── packages/
|
||||
│ ├── api/
|
||||
│ │ └── tsconfig.json # References shared
|
||||
│ ├── web/
|
||||
│ │ └── tsconfig.json # References shared
|
||||
│ └── shared/
|
||||
│ └── tsconfig.json # No references (leaf)
|
||||
```
|
||||
|
||||
```json
|
||||
// packages/shared/tsconfig.json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"composite": true,
|
||||
"declaration": true,
|
||||
"outDir": "dist"
|
||||
},
|
||||
"include": ["src/**/*"]
|
||||
}
|
||||
```
|
||||
|
||||
```json
|
||||
// packages/api/tsconfig.json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"composite": true,
|
||||
"declaration": true,
|
||||
"outDir": "dist"
|
||||
},
|
||||
"references": [{ "path": "../shared" }],
|
||||
"include": ["src/**/*"]
|
||||
}
|
||||
```
|
||||
|
||||
```json
|
||||
// tsconfig.json (root)
|
||||
{
|
||||
"files": [],
|
||||
"references": [
|
||||
{ "path": "packages/shared" },
|
||||
{ "path": "packages/api" },
|
||||
{ "path": "packages/web" }
|
||||
]
|
||||
}
|
||||
```
|
||||
|
||||
```bash
|
||||
tsc --build # Builds only changed projects
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
|
||||
- Parallel compilation of independent projects
|
||||
- Change in `shared/` only rebuilds dependents
|
||||
- Declaration files used as API boundaries
|
||||
|
||||
Reference: [TypeScript Performance Wiki](https://github.com/microsoft/TypeScript/wiki/Performance#using-project-references)
|
||||
|
|
@ -1,65 +0,0 @@
|
|||
---
|
||||
title: Enable skipLibCheck for Faster Builds
|
||||
impact: CRITICAL
|
||||
impactDescription: 20-40% faster compilation
|
||||
tags: tscfg, skipLibCheck, tsconfig, declaration-files, performance
|
||||
---
|
||||
|
||||
## Enable skipLibCheck for Faster Builds
|
||||
|
||||
The `skipLibCheck` option skips type-checking of declaration files (`.d.ts`). Since these files are pre-verified by library authors, checking them is redundant and wastes compilation time.
|
||||
|
||||
**Incorrect (checks all declaration files):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"target": "ES2022",
|
||||
"module": "ESNext",
|
||||
"strict": true
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```bash
|
||||
# Checks thousands of .d.ts files in node_modules
|
||||
# Compilation time: 25 seconds
|
||||
```
|
||||
|
||||
**Correct (skips declaration file checks):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"target": "ES2022",
|
||||
"module": "ESNext",
|
||||
"strict": true,
|
||||
"skipLibCheck": true
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```bash
|
||||
# Only checks your source files
|
||||
# Compilation time: 15 seconds (40% faster)
|
||||
```
|
||||
|
||||
**Alternative (more conservative):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"skipDefaultLibCheck": true
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
This only skips checking the default library files (lib.d.ts), not third-party declarations.
|
||||
|
||||
**When to disable skipLibCheck:**
|
||||
|
||||
- Debugging type conflicts between declaration files
|
||||
- Publishing a library where you want to verify `.d.ts` output
|
||||
- Encountering mysterious type errors that might originate in declarations
|
||||
|
||||
Reference: [TypeScript Performance Wiki](https://github.com/microsoft/TypeScript/wiki/Performance#skipping-d-ts-checking)
|
||||
|
|
@ -1,65 +0,0 @@
|
|||
---
|
||||
title: Enable strictFunctionTypes for Faster Variance Checks
|
||||
impact: CRITICAL
|
||||
impactDescription: enables optimized variance checking
|
||||
tags: tscfg, strict, strictFunctionTypes, variance, performance
|
||||
---
|
||||
|
||||
## Enable strictFunctionTypes for Faster Variance Checks
|
||||
|
||||
With `strictFunctionTypes` enabled, TypeScript uses fast variance-based checking for function parameters. Without it, TypeScript falls back to slower structural comparison for every function type.
|
||||
|
||||
**Incorrect (slow structural checking):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"strict": false,
|
||||
"strictFunctionTypes": false
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```typescript
|
||||
type Handler<T> = (event: T) => void
|
||||
|
||||
// Without strictFunctionTypes, TypeScript uses bidirectional
|
||||
// (bivariant) checking - comparing structures both ways
|
||||
const handler: Handler<MouseEvent> = (e: Event) => {} // Allowed but unsafe
|
||||
```
|
||||
|
||||
**Correct (fast variance checking):**
|
||||
|
||||
```json
|
||||
{
|
||||
"compilerOptions": {
|
||||
"strict": true
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
```typescript
|
||||
type Handler<T> = (event: T) => void
|
||||
|
||||
// With strictFunctionTypes, TypeScript uses contravariant
|
||||
// checking for parameters - faster and type-safe
|
||||
const handler: Handler<MouseEvent> = (e: Event) => {} // Error: Event is not MouseEvent
|
||||
```
|
||||
|
||||
**Note:** The `strict` flag enables `strictFunctionTypes` along with other strict options. Enable `strict` for all new projects.
|
||||
|
||||
**When bivariance is needed:**
|
||||
|
||||
```typescript
|
||||
// Use method syntax for intentional bivariance
|
||||
interface EventEmitter<T> {
|
||||
emit(event: T): void // Method syntax = bivariant
|
||||
}
|
||||
|
||||
// vs property syntax for contravariance
|
||||
interface StrictEmitter<T> {
|
||||
emit: (event: T) => void // Property syntax = contravariant
|
||||
}
|
||||
```
|
||||
|
||||
Reference: [TypeScript 2.6 - Strict Function Types](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-6.html#strict-function-types)
|
||||
|
|
@ -1,81 +0,0 @@
|
|||
---
|
||||
title: Avoid Deeply Nested Generic Types
|
||||
impact: CRITICAL
|
||||
impactDescription: prevents exponential instantiation cost
|
||||
tags: type, generics, nesting, instantiation, performance
|
||||
---
|
||||
|
||||
## Avoid Deeply Nested Generic Types
|
||||
|
||||
Each layer of generic nesting multiplies type instantiation cost. Flatten generic hierarchies or use intermediate type aliases to reduce the combinatorial explosion of type checking.
|
||||
|
||||
**Incorrect (deeply nested generics):**
|
||||
|
||||
```typescript
|
||||
type ApiResponse<T> = {
|
||||
data: T
|
||||
meta: ResponseMeta
|
||||
}
|
||||
|
||||
type PaginatedResponse<T> = ApiResponse<{
|
||||
items: T[]
|
||||
pagination: PaginationInfo
|
||||
}>
|
||||
|
||||
type CachedResponse<T> = PaginatedResponse<{
|
||||
value: T
|
||||
cachedAt: Date
|
||||
}>
|
||||
|
||||
// Usage creates 4+ levels of nesting
|
||||
function fetchUsers(): CachedResponse<User> {}
|
||||
// Compiler must resolve: CachedResponse<User> → PaginatedResponse<...> → ApiResponse<...>
|
||||
```
|
||||
|
||||
**Correct (flattened with composition):**
|
||||
|
||||
```typescript
|
||||
interface PaginationInfo {
|
||||
page: number
|
||||
totalPages: number
|
||||
}
|
||||
|
||||
interface CacheInfo {
|
||||
cachedAt: Date
|
||||
}
|
||||
|
||||
interface PaginatedData<T> {
|
||||
items: T[]
|
||||
pagination: PaginationInfo
|
||||
}
|
||||
|
||||
interface ApiResponse<T> {
|
||||
data: T
|
||||
meta: ResponseMeta
|
||||
}
|
||||
|
||||
// Compose at usage site instead of nesting
|
||||
type UserListResponse = ApiResponse<PaginatedData<User> & CacheInfo>
|
||||
|
||||
function fetchUsers(): UserListResponse {}
|
||||
// Single-level generic instantiation
|
||||
```
|
||||
|
||||
**Alternative (builder pattern for complex responses):**
|
||||
|
||||
```typescript
|
||||
interface ResponseBuilder<T> {
|
||||
data: T
|
||||
meta: ResponseMeta
|
||||
}
|
||||
|
||||
function withPagination<T>(items: T[], pagination: PaginationInfo): PaginatedData<T> {
|
||||
return { items, pagination }
|
||||
}
|
||||
|
||||
function withCache<T>(value: T): T & CacheInfo {
|
||||
return { ...value, cachedAt: new Date() }
|
||||
}
|
||||
```
|
||||
|
||||
Reference: [TypeScript Performance Wiki](https://github.com/microsoft/TypeScript/wiki/Performance)
|
||||
|
|
@ -1,62 +0,0 @@
|
|||
---
|
||||
title: Avoid Large Union Types
|
||||
impact: CRITICAL
|
||||
impactDescription: quadratic O(n²) comparison cost
|
||||
tags: type, unions, compilation, performance, discriminated-unions
|
||||
---
|
||||
|
||||
## Avoid Large Union Types
|
||||
|
||||
Union type checking is quadratic — TypeScript compares each union member pairwise. Unions with 50+ elements cause measurable compilation slowdowns and IDE lag. This commonly occurs with generated types (GraphQL schemas, API response codes, database enums).
|
||||
|
||||
**Incorrect (large generated union, O(n²) checks):**
|
||||
|
||||
```typescript
|
||||
// Auto-generated from GraphQL schema — 200+ event types
|
||||
type AnalyticsEvent =
|
||||
| 'page_view'
|
||||
| 'button_click'
|
||||
| 'form_submit'
|
||||
| 'scroll_depth'
|
||||
| 'video_play'
|
||||
| 'video_pause'
|
||||
| 'video_complete'
|
||||
| 'ad_impression'
|
||||
// ... 200 more event types from analytics schema
|
||||
// 200 members = 40,000 pairwise comparisons per usage
|
||||
```
|
||||
|
||||
**Correct (branded string type with runtime validation):**
|
||||
|
||||
```typescript
|
||||
type AnalyticsEvent = string & { readonly __brand: 'AnalyticsEvent' }
|
||||
|
||||
const VALID_EVENTS = new Set(['page_view', 'button_click', 'form_submit' /* ... */])
|
||||
|
||||
function createEvent(name: string): AnalyticsEvent {
|
||||
if (!VALID_EVENTS.has(name)) {
|
||||
throw new Error(`Unknown event: ${name}`)
|
||||
}
|
||||
return name as AnalyticsEvent
|
||||
}
|
||||
```
|
||||
|
||||
**For moderately large unions (20-50 members), use discriminated unions:**
|
||||
|
||||
```typescript
|
||||
// Group related values into categories
|
||||
type UserEvent = { category: 'user'; action: 'login' | 'logout' | 'signup' }
|
||||
type PageEvent = { category: 'page'; action: 'view' | 'scroll' | 'leave' }
|
||||
type FormEvent = { category: 'form'; action: 'submit' | 'validate' | 'reset' }
|
||||
|
||||
type AppEvent = UserEvent | PageEvent | FormEvent
|
||||
// Small union of 3 interfaces instead of 9+ string literals
|
||||
```
|
||||
|
||||
**When flat unions are fine:**
|
||||
|
||||
- Small unions (< 20 members) have negligible cost
|
||||
- Unions of primitive literals used in few places
|
||||
- `string | number | boolean` style utility unions
|
||||
|
||||
Reference: [TypeScript Performance Wiki](https://github.com/microsoft/TypeScript/wiki/Performance#preferring-base-types-over-unions)
|
||||
|
|
@ -1,66 +0,0 @@
|
|||
---
|
||||
title: Add Explicit Return Types to Exported Functions
|
||||
impact: CRITICAL
|
||||
impactDescription: 30-50% faster declaration emit
|
||||
tags: type, return-types, exports, inference, performance
|
||||
---
|
||||
|
||||
## Add Explicit Return Types to Exported Functions
|
||||
|
||||
Explicit return types accelerate compilation by eliminating inference overhead. Named types are more compact than inferred anonymous types, speeding up declaration file generation and consumption.
|
||||
|
||||
**Incorrect (inferred return type, slow declaration emit):**
|
||||
|
||||
```typescript
|
||||
export function fetchUserProfile(userId: string) {
|
||||
// Compiler must analyze entire function body to infer return type
|
||||
return fetch(`/api/users/${userId}`)
|
||||
.then((res) => res.json())
|
||||
.then((data) => ({
|
||||
id: data.id as string,
|
||||
name: data.name as string,
|
||||
email: data.email as string,
|
||||
createdAt: new Date(data.created_at),
|
||||
permissions: data.permissions as Permission[]
|
||||
}))
|
||||
}
|
||||
// Inferred: Promise<{ id: string; name: string; email: string; createdAt: Date; permissions: Permission[] }>
|
||||
```
|
||||
|
||||
**Correct (explicit return type, fast compilation):**
|
||||
|
||||
```typescript
|
||||
interface UserProfile {
|
||||
id: string
|
||||
name: string
|
||||
email: string
|
||||
createdAt: Date
|
||||
permissions: Permission[]
|
||||
}
|
||||
|
||||
export function fetchUserProfile(userId: string): Promise<UserProfile> {
|
||||
return fetch(`/api/users/${userId}`)
|
||||
.then((res) => res.json())
|
||||
.then((data) => ({
|
||||
id: data.id,
|
||||
name: data.name,
|
||||
email: data.email,
|
||||
createdAt: new Date(data.created_at),
|
||||
permissions: data.permissions
|
||||
}))
|
||||
}
|
||||
```
|
||||
|
||||
**When to skip explicit return types:**
|
||||
|
||||
- Private/internal functions with simple returns
|
||||
- Arrow functions in local scope
|
||||
- Functions where the return type is obvious (e.g., `(): void`)
|
||||
|
||||
**Benefits:**
|
||||
|
||||
- Declaration files use named type instead of expanded inline type
|
||||
- Faster incremental compilation when function body changes
|
||||
- Better error messages pointing to return type mismatch
|
||||
|
||||
Reference: [TypeScript Performance Wiki](https://github.com/microsoft/TypeScript/wiki/Performance#using-type-annotations)
|
||||
|
|
@ -1,54 +0,0 @@
|
|||
---
|
||||
title: Extract Conditional Types to Named Aliases
|
||||
impact: CRITICAL
|
||||
impactDescription: enables compiler caching, prevents re-evaluation
|
||||
tags: type, conditional-types, generics, caching, performance
|
||||
---
|
||||
|
||||
## Extract Conditional Types to Named Aliases
|
||||
|
||||
Inline conditional types are re-evaluated on every function call. Extracting them to named type aliases allows the compiler to cache results and reuse them across multiple call sites.
|
||||
|
||||
**Incorrect (inline conditional, re-evaluated each call):**
|
||||
|
||||
```typescript
|
||||
function processResponse<T>(
|
||||
response: T
|
||||
): T extends { data: infer D } ? (D extends Array<infer Item> ? Item[] : D) : never {
|
||||
// Compiler re-computes this complex conditional on every call
|
||||
return response.data
|
||||
}
|
||||
|
||||
function getFirstItem<T>(collection: T): T extends Array<infer U> ? U : T {
|
||||
// Re-evaluated for each getFirstItem() usage
|
||||
}
|
||||
```
|
||||
|
||||
**Correct (extracted, cacheable):**
|
||||
|
||||
```typescript
|
||||
type ExtractData<T> = T extends { data: infer D }
|
||||
? D extends Array<infer Item>
|
||||
? Item[]
|
||||
: D
|
||||
: never
|
||||
|
||||
function processResponse<T>(response: T): ExtractData<T> {
|
||||
// Compiler caches ExtractData<T> resolution
|
||||
return response.data
|
||||
}
|
||||
|
||||
type UnwrapArray<T> = T extends Array<infer U> ? U : T
|
||||
|
||||
function getFirstItem<T>(collection: T): UnwrapArray<T> {
|
||||
// Reuses cached UnwrapArray<T> computation
|
||||
}
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
|
||||
- Type alias acts as a cache boundary
|
||||
- Reduces duplicate computation across multiple call sites
|
||||
- Improves IDE responsiveness for autocomplete
|
||||
|
||||
Reference: [TypeScript Performance Wiki](https://github.com/microsoft/TypeScript/wiki/Performance#using-type-aliases)
|
||||
|
|
@ -1,42 +0,0 @@
|
|||
---
|
||||
title: Prefer Interfaces Over Type Intersections
|
||||
impact: CRITICAL
|
||||
impactDescription: 2-5× faster type resolution
|
||||
tags: type, interfaces, intersections, compilation, performance
|
||||
---
|
||||
|
||||
## Prefer Interfaces Over Type Intersections
|
||||
|
||||
Interfaces create a single flat object type that detects property conflicts at declaration. Intersections recursively merge properties on every use, forcing the compiler to recompute the combined type repeatedly.
|
||||
|
||||
**Incorrect (recursive intersection merging):**
|
||||
|
||||
```typescript
|
||||
type UserWithPermissions = User & Permissions & AuditInfo
|
||||
// Compiler merges all properties on every reference
|
||||
|
||||
type ExtendedOrder = Order & {
|
||||
metadata: OrderMetadata
|
||||
} & Timestamps
|
||||
// Each intersection adds another layer of computation
|
||||
```
|
||||
|
||||
**Correct (single flat interface):**
|
||||
|
||||
```typescript
|
||||
interface UserWithPermissions extends User, Permissions, AuditInfo {}
|
||||
// Single flat type, computed once
|
||||
|
||||
interface ExtendedOrder extends Order, Timestamps {
|
||||
metadata: OrderMetadata
|
||||
}
|
||||
// Extends create efficient inheritance chain
|
||||
```
|
||||
|
||||
**When to use intersections:**
|
||||
|
||||
- Combining function types or primitives (interfaces cannot extend these)
|
||||
- Creating mapped or conditional types
|
||||
- One-off type combinations not reused elsewhere
|
||||
|
||||
Reference: [TypeScript Performance Wiki](https://github.com/microsoft/TypeScript/wiki/Performance#preferring-interfaces-over-intersections)
|
||||
|
|
@ -1,59 +0,0 @@
|
|||
---
|
||||
title: Limit Type Recursion Depth
|
||||
impact: HIGH
|
||||
impactDescription: prevents exponential type expansion when applicable
|
||||
tags: type, recursion, generics, depth, performance
|
||||
---
|
||||
|
||||
## Limit Type Recursion Depth
|
||||
|
||||
Recursive types without depth limits can cause exponential type expansion, leading to compilation hangs or out-of-memory errors. Add explicit depth counters or use tail-recursive patterns.
|
||||
|
||||
**Incorrect (unbounded recursion):**
|
||||
|
||||
```typescript
|
||||
type DeepPartial<T> = {
|
||||
[P in keyof T]?: T[P] extends object ? DeepPartial<T[P]> : T[P]
|
||||
}
|
||||
// No depth limit - deeply nested objects cause exponential expansion
|
||||
|
||||
type JSONValue = string | number | boolean | null | JSONValue[] | { [key: string]: JSONValue }
|
||||
// Infinite recursion potential
|
||||
```
|
||||
|
||||
**Correct (bounded recursion with depth counter):**
|
||||
|
||||
```typescript
|
||||
type DeepPartial<T, Depth extends number[] = []> = Depth['length'] extends 5
|
||||
? T // Stop at depth 5
|
||||
: {
|
||||
[P in keyof T]?: T[P] extends object ? DeepPartial<T[P], [...Depth, 1]> : T[P]
|
||||
}
|
||||
|
||||
type JSONValue<Depth extends number[] = []> = Depth['length'] extends 10
|
||||
? unknown
|
||||
:
|
||||
| string
|
||||
| number
|
||||
| boolean
|
||||
| null
|
||||
| JSONValue<[...Depth, 1]>[]
|
||||
| { [key: string]: JSONValue<[...Depth, 1]> }
|
||||
```
|
||||
|
||||
**Alternative (use built-in utilities):**
|
||||
|
||||
```typescript
|
||||
// For simple cases, prefer built-in Partial over custom DeepPartial
|
||||
type Config = Partial<AppConfig>
|
||||
|
||||
// Use libraries like ts-toolbelt for complex recursive types
|
||||
// They implement optimized depth-limited versions
|
||||
```
|
||||
|
||||
**When unbounded recursion is acceptable:**
|
||||
|
||||
- Types with guaranteed shallow depth (max 2-3 levels)
|
||||
- Internal types not exposed in public APIs
|
||||
|
||||
Reference: [TypeScript Performance Wiki](https://github.com/microsoft/TypeScript/wiki/Performance)
|
||||
|
|
@ -1,67 +0,0 @@
|
|||
---
|
||||
title: Simplify Complex Mapped Types
|
||||
impact: HIGH
|
||||
impactDescription: reduces type computation by 50-80% when applicable
|
||||
tags: type, mapped-types, simplification, utility-types, performance
|
||||
---
|
||||
|
||||
## Simplify Complex Mapped Types
|
||||
|
||||
Overly complex mapped types with multiple conditional branches slow compilation significantly. Break them into smaller, focused utility types and compose them.
|
||||
|
||||
**Incorrect (monolithic mapped type):**
|
||||
|
||||
```typescript
|
||||
type ComplexTransform<T> = {
|
||||
[K in keyof T]: T[K] extends Function
|
||||
? T[K]
|
||||
: T[K] extends Array<infer U>
|
||||
? U extends object
|
||||
? ComplexTransform<U>[]
|
||||
: T[K]
|
||||
: T[K] extends object
|
||||
? T[K] extends Date
|
||||
? string
|
||||
: ComplexTransform<T[K]>
|
||||
: T[K] extends number
|
||||
? string
|
||||
: T[K]
|
||||
}
|
||||
// Multiple nested conditionals evaluated for every property
|
||||
```
|
||||
|
||||
**Correct (composed utility types):**
|
||||
|
||||
```typescript
|
||||
type TransformValue<T> = T extends Date ? string : T extends number ? string : T
|
||||
|
||||
type TransformObject<T> = {
|
||||
[K in keyof T]: TransformProperty<T[K]>
|
||||
}
|
||||
|
||||
type TransformProperty<T> = T extends Function
|
||||
? T
|
||||
: T extends Array<infer U>
|
||||
? TransformArray<U>
|
||||
: T extends object
|
||||
? TransformObject<T>
|
||||
: TransformValue<T>
|
||||
|
||||
type TransformArray<T> = T extends object ? TransformObject<T>[] : T[]
|
||||
|
||||
// Each utility is cached independently
|
||||
type TransformedUser = TransformObject<User>
|
||||
```
|
||||
|
||||
**Benefits:**
|
||||
|
||||
- Each small utility type is cached separately
|
||||
- Easier to debug type errors
|
||||
- More reusable across the codebase
|
||||
|
||||
**When complex mapped types are acceptable:**
|
||||
|
||||
- Internal utility types used in few places
|
||||
- Types that genuinely require complex logic
|
||||
|
||||
Reference: [TypeScript Performance Wiki](https://github.com/microsoft/TypeScript/wiki/Performance)
|
||||
|
|
@ -1 +0,0 @@
|
|||
../../.agents/skills/auto-review-fix
|
||||
|
|
@ -1 +0,0 @@
|
|||
../../.agents/skills/electron
|
||||
|
|
@ -1 +0,0 @@
|
|||
../../.agents/skills/react-useeffect
|
||||
|
|
@ -1,278 +0,0 @@
|
|||
---
|
||||
name: review-and-submit
|
||||
description: Lightweight review-fix loop (2 rounds, 1 agent each), then create PR and merge
|
||||
---
|
||||
|
||||
# review-and-submit
|
||||
|
||||
Lightweight autonomous pipeline: review code with a single agent, fix issues (up to 2 rounds), then create PR and merge. Designed for smaller PRs where full parallel review is overkill.
|
||||
|
||||
**IMPORTANT**: Execute this entire process autonomously without asking the user for confirmation at any step.
|
||||
|
||||
---
|
||||
|
||||
## Process Overview
|
||||
|
||||
```
|
||||
REVIEW-FIX (max 2 rounds, 1 agent each) → CREATE PR → WAIT FOR CI → MERGE
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Step 1: Review-Fix Loop (Max 2 Rounds)
|
||||
|
||||
### Round Setup (first round only)
|
||||
|
||||
Get the diff to understand what changed:
|
||||
|
||||
```bash
|
||||
MERGE_BASE=$(git merge-base origin/main HEAD)
|
||||
git diff $MERGE_BASE --stat
|
||||
git diff $MERGE_BASE
|
||||
```
|
||||
|
||||
### Each Round: Single-Agent Review
|
||||
|
||||
Spawn **one** review agent that covers all review concerns:
|
||||
|
||||
```
|
||||
Agent(
|
||||
subagent_type: "general-purpose",
|
||||
model: "opus",
|
||||
description: "Review all changes on branch",
|
||||
prompt: """
|
||||
Review all code changes on this branch vs origin/main.
|
||||
|
||||
Run: git diff $(git merge-base origin/main HEAD)
|
||||
|
||||
Review for:
|
||||
- Correctness and logical bugs
|
||||
- Security issues
|
||||
- Type safety issues
|
||||
- Error handling gaps
|
||||
- Performance problems
|
||||
- Dead code or unused imports
|
||||
|
||||
**SCOPE**: Only report issues in changed code or directly caused by the changes.
|
||||
Do NOT report pre-existing issues unrelated to this PR.
|
||||
|
||||
For each finding, output:
|
||||
- File: path/to/file.ts
|
||||
- Line: NN
|
||||
- Severity: Critical|High|Medium|Low
|
||||
- Issue: [description]
|
||||
- Fix: [suggested fix]
|
||||
|
||||
If no issues found, say "No issues found."
|
||||
"""
|
||||
)
|
||||
```
|
||||
|
||||
### Fix Phase
|
||||
|
||||
If the review found issues:
|
||||
|
||||
1. **Skip Low severity issues** — only fix Critical, High, and Medium
|
||||
2. Fix issues via a single fix agent:
|
||||
|
||||
```
|
||||
Agent(
|
||||
subagent_type: "general-purpose",
|
||||
model: "opus",
|
||||
description: "Fix review issues",
|
||||
prompt: """
|
||||
Fix the following review issues:
|
||||
|
||||
[LIST ALL CRITICAL/HIGH/MEDIUM ISSUES FROM REVIEW]
|
||||
|
||||
Instructions:
|
||||
1. Read each file
|
||||
2. Apply fixes using the Edit tool
|
||||
3. Verify fixes don't break syntax
|
||||
4. Report what was fixed
|
||||
"""
|
||||
)
|
||||
```
|
||||
|
||||
3. After fixes, run typecheck:
|
||||
|
||||
```bash
|
||||
pnpm typecheck 2>&1
|
||||
```
|
||||
|
||||
If typecheck fails, fix type errors (up to 2 attempts).
|
||||
|
||||
### Exit Conditions
|
||||
|
||||
```
|
||||
IF (review found no Critical/High/Medium issues):
|
||||
→ EXIT loop — code is clean
|
||||
|
||||
IF (fixes applied):
|
||||
→ Run another round to verify (unless already at round 2)
|
||||
|
||||
IF (round 2 reached):
|
||||
→ EXIT loop
|
||||
```
|
||||
|
||||
### Commit Changes
|
||||
|
||||
After the review-fix loop completes, commit any changes:
|
||||
|
||||
```bash
|
||||
if [ -n "$(git status --porcelain)" ]; then
|
||||
git add -A && git commit -m "fix: address review findings"
|
||||
fi
|
||||
```
|
||||
|
||||
**Continue to Step 2 — do not stop here.**
|
||||
|
||||
---
|
||||
|
||||
## Step 2: Create PR and Merge
|
||||
|
||||
### 2a. Remove local design docs
|
||||
|
||||
Design/planning docs are local-only scratch files and must not be checked in.
|
||||
Delete any untracked markdown files in `docs/` that match design/plan
|
||||
naming patterns before pushing:
|
||||
|
||||
```bash
|
||||
git ls-files --others --exclude-standard -- 'docs/*-design.md' 'docs/*-plan.md' 'docs/design-*.md' \
|
||||
| xargs -I{} rm -f {}
|
||||
```
|
||||
|
||||
### 2b. Create PR
|
||||
|
||||
**FIRST**: Push the branch to the remote so `gh pr create` doesn't fail with
|
||||
`aborted: you must first push the current branch to a remote`. The
|
||||
`create-pr` skill rebases locally but does not always push before invoking
|
||||
`gh pr create`, and `gh` refuses to create a PR for an un-pushed branch.
|
||||
|
||||
```bash
|
||||
git push --force-with-lease -u origin HEAD
|
||||
```
|
||||
|
||||
Then invoke:
|
||||
|
||||
```
|
||||
Use the Skill tool: skill: "create-pr"
|
||||
```
|
||||
|
||||
After `/create-pr` completes, extract PR info:
|
||||
|
||||
```bash
|
||||
gh pr view --json number,url --jq '.number, .url'
|
||||
```
|
||||
|
||||
### 2c. Wait for CI Checks
|
||||
|
||||
Wait **2.5 minutes** before the first poll:
|
||||
|
||||
```bash
|
||||
echo "Waiting 2.5 minutes for CI checks..." && sleep 150
|
||||
```
|
||||
|
||||
**CRITICAL**: Set `timeout: 210000` (3.5 minutes) on the Bash tool call for this sleep.
|
||||
|
||||
### Poll for results (up to 5 polls, 90s apart)
|
||||
|
||||
For each poll:
|
||||
|
||||
```bash
|
||||
CHECKS=$(gh pr checks --json name,state,bucket 2>&1)
|
||||
echo "$CHECKS"
|
||||
|
||||
if echo "$CHECKS" | jq -e 'length == 0' >/dev/null 2>&1; then
|
||||
echo "RESULT:NO_CHECKS"
|
||||
elif echo "$CHECKS" | jq -e '[.[].bucket] | all(. == "pass" or . == "skipping")' >/dev/null 2>&1; then
|
||||
echo "RESULT:PASSED"
|
||||
elif echo "$CHECKS" | jq -e '[.[].bucket] | any(. == "fail" or . == "cancel")' >/dev/null 2>&1; then
|
||||
echo "RESULT:FAILED"
|
||||
else
|
||||
echo "RESULT:PENDING"
|
||||
fi
|
||||
```
|
||||
|
||||
- `PASSED` → Proceed to merge
|
||||
- `FAILED` → Attempt one quick fix iteration (diagnose from `gh run view --log-failed`, fix, push, re-poll)
|
||||
- `PENDING` → Wait 90s (`sleep 90`, `timeout: 150000`), then poll again
|
||||
- `NO_CHECKS` → Proceed to merge
|
||||
|
||||
### 2d. Merge
|
||||
|
||||
```bash
|
||||
IS_WORKTREE=false
|
||||
if [ "$(git rev-parse --git-dir)" != "$(git rev-parse --git-common-dir)" ]; then
|
||||
IS_WORKTREE=true
|
||||
fi
|
||||
|
||||
BRANCH=$(git branch --show-current)
|
||||
|
||||
if [ "$IS_WORKTREE" = "true" ]; then
|
||||
gh pr merge --admin --squash
|
||||
git push origin --delete "$BRANCH" 2>/dev/null || true
|
||||
else
|
||||
gh pr merge --admin --squash --delete-branch
|
||||
fi
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## CI Fix Loop (if checks fail)
|
||||
|
||||
If CI fails after PR creation, run **one** fix iteration:
|
||||
|
||||
1. Diagnose:
|
||||
|
||||
```bash
|
||||
FAILED_RUN=$(gh run list --branch $(git branch --show-current) --limit 5 --json databaseId,conclusion,name --jq '[.[] | select(.conclusion == "failure")] | .[0].databaseId')
|
||||
if [ -n "$FAILED_RUN" ] && [ "$FAILED_RUN" != "null" ]; then
|
||||
gh run view "$FAILED_RUN" --log-failed 2>&1 | tail -200
|
||||
fi
|
||||
```
|
||||
|
||||
2. Fix the issues, run `pnpm typecheck`, commit and push:
|
||||
|
||||
```bash
|
||||
if [ -n "$(git status --porcelain)" ]; then
|
||||
git add -A && git commit -m "fix: address CI failures"
|
||||
git push --force-with-lease
|
||||
fi
|
||||
```
|
||||
|
||||
3. Wait 2.5 minutes and re-poll. If still failing after one fix attempt, merge with `--admin` anyway.
|
||||
|
||||
---
|
||||
|
||||
## Progress Tracking
|
||||
|
||||
```
|
||||
╔════════════════════════════════════════════════════════════╗
|
||||
║ REVIEW AND SUBMIT ║
|
||||
╠════════════════════════════════════════════════════════════╣
|
||||
║ Review-Fix Loop: ║
|
||||
║ Round 1: ✅ X issues found, Y fixed ║
|
||||
║ Round 2: ✅ Clean review ║
|
||||
║ Create PR: ✅ PR #123 ║
|
||||
║ CI Checks: ✅ Passed ║
|
||||
║ Merge: ✅ Merged with --admin --squash ║
|
||||
╠════════════════════════════════════════════════════════════╣
|
||||
║ PR URL: https://github.com/org/repo/pull/123 ║
|
||||
╚════════════════════════════════════════════════════════════╝
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Critical Instructions
|
||||
|
||||
1. **DO NOT ask for user confirmation** — Execute autonomously
|
||||
2. **Max 2 review rounds**, 1 agent per round — keep it lightweight
|
||||
3. **Only fix Critical/High/Medium** — skip Low severity
|
||||
4. **DO run typecheck** after fixes
|
||||
5. **DO use opus model** for review and fix agents
|
||||
6. **2.5 minute initial CI wait** (not 8 minutes) — this repo's CI is fast
|
||||
7. **DO use --admin --squash** for merge
|
||||
8. **DO set Bash timeouts** for sleep commands
|
||||
9. **Continue through all steps** — don't stop after review or PR creation
|
||||
10. **One CI fix iteration max** — don't loop forever on CI failures
|
||||
|
|
@ -1 +0,0 @@
|
|||
../../.agents/skills/typescript
|
||||
|
|
@ -80,3 +80,7 @@ docs/design-*.md
|
|||
# Playwright
|
||||
test-results/
|
||||
playwright-report/
|
||||
|
||||
# Agent skill installations (machine-local, populated by agent tooling)
|
||||
/.claude/skills/
|
||||
/.agents/skills/
|
||||
|
|
|
|||
|
|
@ -1,3 +1,7 @@
|
|||
scripts:
|
||||
setup: |
|
||||
pnpm install
|
||||
# Internal Stably dev setup: populates .claude/skills and .agents/skills
|
||||
# from a private repo. Silent no-op for public contributors (env unset).
|
||||
[ -n "$ORCA_INTERNAL_DEV_SETUP" ] && [ -x "$ORCA_INTERNAL_DEV_SETUP" ] && \
|
||||
"$ORCA_INTERNAL_DEV_SETUP" "$ORCA_WORKTREE_PATH" || true
|
||||
|
|
|
|||
Loading…
Reference in New Issue