|
| 1 | +--- |
| 2 | +name: review-async |
| 3 | +description: Reviews code for async patterns including timers, data loading, and background services. Use when reviewing files with setInterval, async functions, or service components. |
| 4 | +--- |
| 5 | + |
| 6 | +Review the branch/pull request in context for async pattern conventions. |
| 7 | + |
| 8 | +## Context Expected |
| 9 | + |
| 10 | +You will receive: |
| 11 | +- Repository name |
| 12 | +- Branch name |
| 13 | +- List of changed files to review |
| 14 | + |
| 15 | +## How to Review |
| 16 | + |
| 17 | +1. Read the changed files provided in context |
| 18 | +2. Look for setInterval, async data loading, background work |
| 19 | +3. Verify async patterns follow conventions |
| 20 | +4. Report findings with specific file:line references |
| 21 | + |
| 22 | +--- |
| 23 | + |
| 24 | +## Never Use setInterval |
| 25 | + |
| 26 | +Always use `makePeriodicTask` instead, especially when async work is involved: |
| 27 | + |
| 28 | +```typescript |
| 29 | +// Incorrect |
| 30 | +setInterval(() => fetchData(), 5000) |
| 31 | + |
| 32 | +// Correct |
| 33 | +const task = makePeriodicTask(async () => { |
| 34 | + await fetchData() |
| 35 | +}, 5000) |
| 36 | +task.start() |
| 37 | +// ... later |
| 38 | +task.stop() |
| 39 | +``` |
| 40 | + |
| 41 | +--- |
| 42 | + |
| 43 | +## Use TanStack Query or useAsyncValue |
| 44 | + |
| 45 | +For async data loading in components, use TanStack Query: |
| 46 | + |
| 47 | +```typescript |
| 48 | +import { useQuery } from '@tanstack/react-query' |
| 49 | + |
| 50 | +const { data, isLoading } = useQuery({ |
| 51 | + queryKey: ['myData'], |
| 52 | + queryFn: fetchMyData |
| 53 | +}) |
| 54 | +``` |
| 55 | + |
| 56 | +Or use existing `useAsyncValue` hook. Don't re-implement async loading patterns. |
| 57 | + |
| 58 | +--- |
| 59 | + |
| 60 | +## Background Services |
| 61 | + |
| 62 | +Background services go in `components/services/` directory: |
| 63 | + |
| 64 | +> "If we did want a persistent background service, it would go in components/services. Making it a component means it gets mounted/unmounted cleanly when we log in/out, and we can also see all the background stuff in one place." |
| 65 | +
|
| 66 | +Avoid too much background work. Consider: |
| 67 | +- Triggering only when the user has pending items |
| 68 | +- Running only when on the relevant scene |
| 69 | + |
| 70 | +--- |
| 71 | + |
| 72 | +## Prevent Concurrent Execution |
| 73 | + |
| 74 | +Use a helper to prevent duplicate parallel calls when a function can be triggered multiple times (e.g., button presses, retries): |
| 75 | + |
| 76 | +```typescript |
| 77 | +// Helper to ensure only one execution at a time |
| 78 | +function runOnce<T>(fn: () => Promise<T>): () => Promise<T | undefined> { |
| 79 | + let running = false |
| 80 | + return async () => { |
| 81 | + if (running) return |
| 82 | + running = true |
| 83 | + try { |
| 84 | + return await fn() |
| 85 | + } finally { |
| 86 | + running = false |
| 87 | + } |
| 88 | + } |
| 89 | +} |
| 90 | + |
| 91 | +// Usage |
| 92 | +const handleSubmit = runOnce(async () => { |
| 93 | + await submitForm() |
| 94 | +}) |
| 95 | +``` |
| 96 | + |
| 97 | +This prevents race conditions from multiple simultaneous calls to the same async function. |
| 98 | + |
| 99 | +--- |
| 100 | + |
| 101 | +## Polling Race Conditions |
| 102 | + |
| 103 | +When implementing polling that can be cancelled, ensure the cancel flag is checked after each async operation: |
| 104 | + |
| 105 | +```typescript |
| 106 | +// Incorrect - race condition if cancelled during poll interval |
| 107 | +const pollForStatus = async (cancel: { cancelled: boolean }) => { |
| 108 | + while (!cancel.cancelled) { |
| 109 | + const status = await checkStatus() |
| 110 | + if (status === 'complete') return status |
| 111 | + await sleep(2000) |
| 112 | + // BUG: If cancelled during sleep, still continues |
| 113 | + } |
| 114 | +} |
| 115 | + |
| 116 | +// Correct - check cancel after every await |
| 117 | +const pollForStatus = async (cancel: { cancelled: boolean }) => { |
| 118 | + while (!cancel.cancelled) { |
| 119 | + const status = await checkStatus() |
| 120 | + if (cancel.cancelled) return |
| 121 | + if (status === 'complete') return status |
| 122 | + await sleep(2000) |
| 123 | + } |
| 124 | +} |
| 125 | +``` |
| 126 | + |
| 127 | +--- |
| 128 | + |
| 129 | +## Refresh State in Async Callbacks |
| 130 | + |
| 131 | +When a timeout or async callback needs current state, read it fresh inside the callback rather than capturing it in a closure: |
| 132 | + |
| 133 | +```typescript |
| 134 | +// Incorrect - captured state becomes stale |
| 135 | +const { wallets } = props.state // Captured at setup time |
| 136 | +setTimeout(() => { |
| 137 | + for (const walletId of walletIds) { |
| 138 | + const wallet = wallets[walletId] // Stale - may not reflect changes |
| 139 | + wallet.doSomething() |
| 140 | + } |
| 141 | +}, 15000) |
| 142 | + |
| 143 | +// Correct - read fresh state in callback |
| 144 | +setTimeout(() => { |
| 145 | + const { wallets } = props.state // Fresh read |
| 146 | + for (const walletId of walletIds) { |
| 147 | + const wallet = wallets[walletId] |
| 148 | + if (wallet == null) continue // Guard against removed wallets |
| 149 | + wallet.doSomething() |
| 150 | + } |
| 151 | +}, 15000) |
| 152 | +``` |
| 153 | + |
| 154 | +State captured in closures doesn't update when the source changes. This is especially important for timeouts and interval callbacks that run much later. |
0 commit comments