You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy file name to clipboardExpand all lines: .claude/skills/coding-standards/rules/clean-react-4-no-side-effect-spaghetti.md
+42Lines changed: 42 additions & 0 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -91,6 +91,41 @@ In this example:
91
91
- Effects could be extracted to focused hooks: `useTelemetrySpans`, `useDeepLinking`, `useAudioMode`, etc.
92
92
- Entry points don't get special treatment — extracting effects into named hooks improves clarity and makes it possible to understand what each effect does and how to safely modify it
93
93
94
+
#### Incorrect (internal render helper functions)
95
+
96
+
- Internal `render*` functions signal the component owns multiple rendering responsibilities that should be separate components
97
+
- They close over the entire component scope — the React Compiler cannot independently memoize them
98
+
- They hide the component's render tree — the return statement doesn't show what actually renders
99
+
- The fix is to extract each helper into its own component with explicit props
@@ -154,6 +189,9 @@ Flag when a component, hook, or utility aggregates multiple unrelated responsibi
154
189
- Unrelated state variables are interdependent or updated together
155
190
- Logic mixes data fetching, navigation, UI state, and lifecycle behavior in one place
156
191
- Removing one piece of functionality requires careful untangling from others
192
+
- Component defines internal `render*` functions or arrow functions that return JSX and calls them in its return statement (e.g., `const renderContent = () => ...`, `{renderContent()}`)
193
+
- These functions close over the component's entire scope, preventing the React Compiler from independently memoizing them
194
+
- The component's return statement calls these helpers instead of showing the render tree directly
157
195
158
196
**What counts as "unrelated":**
159
197
- Group by responsibility (what the code does), NOT by timing (when it runs)
@@ -163,7 +201,11 @@ Flag when a component, hook, or utility aggregates multiple unrelated responsibi
163
201
**DO NOT flag if:**
164
202
- Component is a thin orchestration layer that ONLY composes child components (no business logic, no effects beyond rendering)
165
203
- Effects are extracted into focused custom hooks with single responsibilities (e.g., `useDebugShortcut`, `usePriorityMode`) — inline `useEffect` calls are a code smell and should be named hooks
204
+
- The internal function is a **callback or event handler** (e.g., `handlePress`, `onSubmit`), not a render helper — only functions that return JSX qualify
205
+
- The internal function is a **single early return** for a guard clause (e.g., `if (!data) return <EmptyState />;` at the top of the component) — simple guards in the component body are not render helpers
166
206
167
207
**Search Patterns** (hints for reviewers):
168
208
-`useEffect`
169
209
-`useOnyx`
210
+
-`const render\w+\s*=` or `function render\w+` inside a component body (internal render helpers)
211
+
-`{render\w+\(\)}` in JSX return statements (helper invocations)
issueBody += `\r\n- [${isFirebaseChecked ? 'x' : ' '}] I checked [Firebase Crashlytics](https://console.firebase.google.com/u/0/project/expensify-mobile-app/crashlytics/app/ios:com.expensify.expensifylite/issues?state=open&time=last-seven-days&types=crash&tag=all&sort=eventCount) for **this release version** and verified that this release does not introduce any new crashes. More detailed instructions on this verification can be found [here](https://stackoverflowteams.com/c/expensify/questions/15095/15096).`;
issueBody += `\r\n- [${isFirebaseChecked ? 'x' : ' '}] I checked [Firebase Crashlytics](https://console.firebase.google.com/u/0/project/expensify-mobile-app/crashlytics/app/ios:com.expensify.expensifylite/issues?state=open&time=last-seven-days&types=crash&tag=all&sort=eventCount) for **this release version** and verified that this release does not introduce any new crashes. More detailed instructions on this verification can be found [here](https://stackoverflowteams.com/c/expensify/questions/15095/15096).`;
issueBody += `\r\n- [${isFirebaseChecked ? 'x' : ' '}] I checked [Firebase Crashlytics](https://console.firebase.google.com/u/0/project/expensify-mobile-app/crashlytics/app/ios:com.expensify.expensifylite/issues?state=open&time=last-seven-days&types=crash&tag=all&sort=eventCount) for **this release version** and verified that this release does not introduce any new crashes. More detailed instructions on this verification can be found [here](https://stackoverflowteams.com/c/expensify/questions/15095/15096).`;
issueBody += `\r\n- [${isFirebaseChecked ? 'x' : ' '}] I checked [Firebase Crashlytics](https://console.firebase.google.com/u/0/project/expensify-mobile-app/crashlytics/app/ios:com.expensify.expensifylite/issues?state=open&time=last-seven-days&types=crash&tag=all&sort=eventCount) for **this release version** and verified that this release does not introduce any new crashes. More detailed instructions on this verification can be found [here](https://stackoverflowteams.com/c/expensify/questions/15095/15096).`;
issueBody += `\r\n- [${isFirebaseChecked ? 'x' : ' '}] I checked [Firebase Crashlytics](https://console.firebase.google.com/u/0/project/expensify-mobile-app/crashlytics/app/ios:com.expensify.expensifylite/issues?state=open&time=last-seven-days&types=crash&tag=all&sort=eventCount) for **this release version** and verified that this release does not introduce any new crashes. More detailed instructions on this verification can be found [here](https://stackoverflowteams.com/c/expensify/questions/15095/15096).`;
0 commit comments