|
| 1 | +$NOTE |
| 2 | +# Anti-Patterns |
| 3 | + |
| 4 | +This defines repeatedly observed "do not do this" patterns. |
| 5 | +Each anti-pattern includes why it is problematic and what should be done instead. |
| 6 | + |
| 7 | +--- |
| 8 | + |
| 9 | +## 1. Abandoning Type Safety |
| 10 | + |
| 11 | +### Careless Use of `any` |
| 12 | + |
| 13 | +```ts |
| 14 | +// ❌ Creates a hole in the type system |
| 15 | +const data: any = await fetchData() |
| 16 | +data.nonExistent.property // Explodes at runtime |
| 17 | + |
| 18 | +// ✔ unknown + type guard |
| 19 | +const data: unknown = await fetchData() |
| 20 | +if (isValidResponse(data)) { |
| 21 | + data.property // Type-safe |
| 22 | +} |
| 23 | +``` |
| 24 | + |
| 25 | +**Why it's problematic**: `any` disables type checking. A single `any` propagates through type inference — when `any` is assigned to a variable, every expression derived from it also becomes `any`, silently disabling checks across the entire call chain without compiler warnings. |
| 26 | +**Exception**: Inadequate type definitions in external libraries. Always attach a `// FIXME` comment. |
| 27 | + |
| 28 | +--- |
| 29 | + |
| 30 | +## 2. Architectural Deviation |
| 31 | + |
| 32 | +### Direct fetch from Components |
| 33 | + |
| 34 | +```ts |
| 35 | +// ❌ Skipping layers |
| 36 | +function MyComponent() { |
| 37 | + const [data, setData] = useState(null) |
| 38 | + useEffect(() => { |
| 39 | + fetch('/api/data').then(r => r.json()).then(setData) |
| 40 | + }, []) |
| 41 | +} |
| 42 | + |
| 43 | +// ✔ Go through Server Actions + Hooks |
| 44 | +function MyComponent() { |
| 45 | + const { data } = useListData({ action: fetchDataAction }) |
| 46 | +} |
| 47 | +``` |
| 48 | + |
| 49 | +**Why it's problematic**: Error handling, authentication, caching, and loading states end up implemented inconsistently. |
| 50 | + |
| 51 | +### Introducing Global State Management Libraries |
| 52 | + |
| 53 | +```ts |
| 54 | +// ❌ Introducing Redux / Zustand / Recoil |
| 55 | +const useStore = create((set) => ({ ... })) |
| 56 | + |
| 57 | +// ✔ Server Actions + local state + Feature Context |
| 58 | +const [state, setState] = useState(initial) |
| 59 | +// or |
| 60 | +const { value } = useFeatureContext() |
| 61 | +``` |
| 62 | + |
| 63 | +**Why it's problematic**: Global state obscures the origin of data and undermines Server Actions' role as the source of truth. |
| 64 | + |
| 65 | +--- |
| 66 | + |
| 67 | +## 3. Lack of Observability |
| 68 | + |
| 69 | +### Missing Trace ID |
| 70 | + |
| 71 | +```ts |
| 72 | +// ❌ Cannot identify which request caused the error |
| 73 | +logger.error('Something went wrong') |
| 74 | + |
| 75 | +// ✔ Traceable via Request ID |
| 76 | +logger.error({ requestId, userId, action }, 'Payment processing failed') |
| 77 | +``` |
| 78 | + |
| 79 | +**Why it's problematic**: Without it, requests cannot be traced end-to-end during incident investigation. |
| 80 | + |
| 81 | +--- |
| 82 | + |
| 83 | +## 4. Over-Abstraction |
| 84 | + |
| 85 | +### Generalizing Code Used in Only One Place |
| 86 | + |
| 87 | +```ts |
| 88 | +// ❌ Abstraction with no reuse potential |
| 89 | +function useSinglePurposeHook() { ... } |
| 90 | +function createSingleUseFactory(options) { ... } |
| 91 | + |
| 92 | +// ✔ Write it inline. Extract only after duplication appears in 3 places |
| 93 | +``` |
| 94 | + |
| 95 | +**Why it's problematic**: Abstraction increases comprehension cost. An abstraction used in only one place leaves only the cost. |
| 96 | + |
| 97 | +### Bloated Configuration Objects |
| 98 | + |
| 99 | +```ts |
| 100 | +// ❌ Too many options = the abstraction is heading in the wrong direction |
| 101 | +createAction({ |
| 102 | + schema, handler, middleware, errorHandler, |
| 103 | + retryPolicy, cachePolicy, rateLimitPolicy, auditPolicy, |
| 104 | +}) |
| 105 | + |
| 106 | +// ✔ Extract only the common parts; implement special cases individually |
| 107 | +const baseAction = createBaseAction(commonOptions) |
| 108 | +const specialAction = async (input) => { |
| 109 | + // Special logic |
| 110 | + return baseAction(transformedInput) |
| 111 | +} |
| 112 | +``` |
| 113 | + |
| 114 | +**Why it's problematic**: A massive configuration object is not "flexible" — it is "complex." |
| 115 | + |
| 116 | +--- |
| 117 | + |
| 118 | +## 5. Misuse of Naming and Comments |
| 119 | + |
| 120 | +### Vague Naming |
| 121 | + |
| 122 | +```ts |
| 123 | +// ❌ |
| 124 | +const data = await getData() |
| 125 | +const result = process(data) |
| 126 | +function handleClick() { ... } |
| 127 | + |
| 128 | +// ✔ |
| 129 | +const userProfile = await fetchUserProfile() |
| 130 | +const validatedOrder = validateOrder(rawOrder) |
| 131 | +function handleSubmitPayment() { ... } |
| 132 | +``` |
| 133 | + |
| 134 | +**Why it's problematic**: `data`, `result`, `handle` convey nothing. The reader must read the implementation to understand the intent. |
| 135 | + |
| 136 | +### Comments That Explain Logic |
| 137 | + |
| 138 | +```ts |
| 139 | +// ❌ Stating what the code already says |
| 140 | +// Check if user's age is 18 or above |
| 141 | +if (user.age >= 18) { ... } |
| 142 | + |
| 143 | +// ✔ Document intent, exceptions, and side effects |
| 144 | +// Legal requirement: minors cannot use the payment feature (see compliance guidelines) |
| 145 | +if (user.age >= 18) { ... } |
| 146 | +``` |
| 147 | + |
| 148 | +**Why it's problematic**: "What it does" is told by the code. Comments should tell "why it does it that way." |
| 149 | + |
| 150 | +--- |
| 151 | + |
| 152 | +## 6. Inadequate Testing |
| 153 | + |
| 154 | +### Testing Only the Happy Path |
| 155 | + |
| 156 | +```ts |
| 157 | +// ❌ Only the success case |
| 158 | +test('creates user', async () => { |
| 159 | + const user = await createUser(validInput) |
| 160 | + expect(user).toBeDefined() |
| 161 | +}) |
| 162 | + |
| 163 | +// ✔ Include error cases, boundary values, and security scenarios |
| 164 | +test('rejects invalid email', ...) |
| 165 | +test('prevents IDOR access', ...) |
| 166 | +test('handles duplicate webhook events', ...) |
| 167 | +test('fails gracefully on external API timeout', ...) |
| 168 | +``` |
| 169 | + |
| 170 | +**Why it's problematic**: Bugs occur in error cases and at boundary values, not in the happy path. |
| 171 | + |
| 172 | +--- |
| 173 | + |
| 174 | +## 7. Ignoring Performance |
| 175 | + |
| 176 | +### Huge Bundle Sizes |
| 177 | + |
| 178 | +```ts |
| 179 | +// ❌ Importing the entire library |
| 180 | +import _ from 'lodash' |
| 181 | +import moment from 'moment' |
| 182 | + |
| 183 | +// ✔ Import only the needed functions / use lightweight alternatives |
| 184 | +import { debounce } from 'lodash/debounce' |
| 185 | +import dayjs from 'dayjs' |
| 186 | +``` |
| 187 | + |
| 188 | +**Why it's problematic**: Bundle size directly impacts load time. As a reference point, research by Google and Akamai shows that each additional 100KB of JavaScript adds roughly 100–300ms of parse/compile time on mobile devices, and pages loading in over 3 seconds see significantly higher bounce rates. Tree-shaking and selective imports keep the bundle lean. |
0 commit comments