Skip to content

Commit a93a316

Browse files
JulesssssJulesssss
authored
Merge pull request Expensify#81604 from callstack-internal/feat/convert-ai-reviewer-rules-into-skill
[NoQA] Convert code-inline-reviewer rules into proactive coding guidelines for Claude
2 parents 4f2b1d5 + 0c50794 commit a93a316

32 files changed

+2161
-1692
lines changed

‎.claude/agents/code-inline-reviewer.md‎

Lines changed: 34 additions & 1677 deletions
Large diffs are not rendered by default.

‎.claude/skills/coding-standards/SKILL.md‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
name: coding-standards
3+
description: Provides coding standards for React Native — performance patterns, consistency rules, and clean React architecture. Use when writing, modifying, or reviewing code.
4+
alwaysApply: true
5+
---
6+
7+
# Expensify Coding Standards
8+
9+
Coding standards for the Expensify App. Each standard is a standalone file in `rules/` with reasoning, examples, and applicability conditions.
10+
11+
## Categories
12+
13+
| Category | Prefix | Focus |
14+
|----------|--------|-------|
15+
| Performance | `PERF-*` | Render optimization, memo patterns, useEffect hygiene, data selection |
16+
| Consistency | `CONSISTENCY-*` | Platform checks, magic values, unused props, ESLint discipline |
17+
| Clean React Patterns | `CLEAN-REACT-PATTERNS-*` | Composition, component ownership, state structure |
18+
19+
## Quick Reference
20+
21+
### Performance
22+
- [PERF-1](rules/perf-1-no-spread-in-renderitem.md) — No spread in renderItem
23+
- [PERF-2](rules/perf-2-early-return.md) — Return early before expensive work
24+
- [PERF-3](rules/perf-3-onyx-list-item-provider.md) — Use OnyxListItemProvider in renderItem
25+
- [PERF-5](rules/perf-5-shallow-comparison.md) — Shallow over deep comparisons
26+
- [PERF-6](rules/perf-6-derive-state-from-props.md) — Derive state from props
27+
- [PERF-7](rules/perf-7-reset-via-key-prop.md) — Reset via key prop
28+
- [PERF-8](rules/perf-8-events-in-handlers.md) — Handle events in handlers
29+
- [PERF-9](rules/perf-9-no-useeffect-chains.md) — No useEffect chains
30+
- [PERF-10](rules/perf-10-no-useeffect-parent-comm.md) — No useEffect parent communication
31+
- [PERF-11](rules/perf-11-optimize-data-selection.md) — Optimize data selection
32+
- [PERF-12](rules/perf-12-prevent-memory-leaks.md) — Prevent memory leaks
33+
- [PERF-13](rules/perf-13-hoist-iterator-calls.md) — Hoist iterator-independent calls
34+
- [PERF-14](rules/perf-14-use-sync-external-store.md) — Use useSyncExternalStore
35+
- [PERF-15](rules/perf-15-cleanup-async-effects.md) — Clean up async Effects
36+
- [PERF-16](rules/perf-16-guard-double-init.md) — Guard double initialization
37+
38+
### Consistency
39+
- [CONSISTENCY-1](rules/consistency-1-no-platform-checks.md) — No platform-specific checks in components
40+
- [CONSISTENCY-2](rules/consistency-2-no-magic-values.md) — No magic numbers/strings
41+
- [CONSISTENCY-3](rules/consistency-3-no-code-duplication.md) — No code duplication
42+
- [CONSISTENCY-4](rules/consistency-4-no-unused-props.md) — No unused props
43+
- [CONSISTENCY-5](rules/consistency-5-justify-eslint-disable.md) — Justify ESLint disables
44+
- [CONSISTENCY-6](rules/consistency-6-proper-error-handling.md) — Proper error handling
45+
46+
### Clean React Patterns
47+
- [CLEAN-REACT-PATTERNS-1](rules/clean-react-1-composition-over-config.md) — Composition over configuration
48+
- [CLEAN-REACT-PATTERNS-2](rules/clean-react-2-own-behavior.md) — Components own their behavior
49+
- [CLEAN-REACT-PATTERNS-3](rules/clean-react-3-context-free-contracts.md) — Context-free component contracts
50+
- [CLEAN-REACT-PATTERNS-4](rules/clean-react-4-no-side-effect-spaghetti.md) — No side-effect spaghetti
51+
- [CLEAN-REACT-PATTERNS-5](rules/clean-react-5-narrow-state.md) — Keep state narrow
52+
53+
## Usage
54+
55+
**During development**: When writing or modifying `src/` files, consult the relevant standard files for detailed conditions, examples, and exceptions.
56+
57+
**During review**: The code-inline-reviewer agent loads all standards from this directory. See `.claude/agents/code-inline-reviewer.md`.

‎.claude/skills/coding-standards/rules/clean-react-1-composition-over-config.md‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
---
2+
ruleId: CLEAN-REACT-PATTERNS-1
3+
title: Favor composition over configuration
4+
---
5+
6+
## [CLEAN-REACT-PATTERNS-1] Favor composition over configuration
7+
8+
### Reasoning
9+
10+
When new features are implemented by adding configuration (props, flags, conditional logic) to existing components, if requirements change, then those components must be repeatedly modified, increasing coupling, surface area, and regression risk. Composition ensures features scale horizontally, limits the scope of changes, and prevents components from becoming configuration-driven "mega components".
11+
12+
### Incorrect
13+
14+
#### Incorrect (configuration)
15+
16+
- Features controlled by boolean flags
17+
- Adding a new feature requires modifying the Table component's API
18+
19+
```tsx
20+
<Table
21+
data={items}
22+
columns={columns}
23+
shouldShowSearchBar
24+
shouldShowHeader
25+
shouldEnableSorting
26+
shouldShowPagination
27+
shouldHighlightOnHover
28+
/>
29+
30+
type TableProps = {
31+
data: Item[];
32+
columns: Column[];
33+
shouldShowSearchBar?: boolean; // Could be <Table.SearchBar />
34+
shouldShowHeader?: boolean; // Could be <Table.Header />
35+
shouldEnableSorting?: boolean; // Configuration for header behavior
36+
shouldShowPagination?: boolean; // Could be <Table.Pagination />
37+
shouldHighlightOnHover?: boolean; // Configuration for styling behavior
38+
};
39+
```
40+
41+
```tsx
42+
<SelectionList
43+
data={items}
44+
shouldShowTextInput
45+
shouldShowTooltips
46+
shouldScrollToFocusedIndex
47+
shouldDebounceScrolling
48+
shouldUpdateFocusedIndex
49+
canSelectMultiple
50+
disableKeyboardShortcuts
51+
/>
52+
53+
type SelectionListProps = {
54+
shouldShowTextInput?: boolean; // Could be <SelectionList.TextInput />
55+
shouldShowConfirmButton?: boolean; // Could be <SelectionList.ConfirmButton />
56+
textInputOptions?: {...}; // Configuration object for the above
57+
};
58+
```
59+
60+
#### Incorrect (parent manages child state)
61+
62+
```tsx
63+
// Parent fetches and manages state for its children
64+
// Parent has to know child implementation details
65+
function ReportScreen({ params: { reportID }}) {
66+
const [reportOnyx] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {allowStaleData: true, canBeMissing: true});
67+
const reportActions = useMemo(() => getFilteredReportActionsForReportView(unfilteredReportActions), [unfilteredReportActions]);
68+
const [reportMetadata = defaultReportMetadata] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportIDFromRoute}`, {canBeMissing: true, allowStaleData: true});
69+
const {reportActions: unfilteredReportActions, linkedAction, sortedAllReportActions, hasNewerActions, hasOlderActions} = usePaginatedReportActions(reportID, reportActionIDFromRoute);
70+
const parentReportAction = useParentReportAction(reportOnyx);
71+
const transactionThreadReportID = getOneTransactionThreadReportID(report, chatReport, reportActions ?? [], isOffline, reportTransactionIDs);
72+
const isTransactionThreadView = isReportTransactionThread(report);
73+
// other onyx connections etc
74+
75+
return (
76+
<>
77+
<ReportActionsView
78+
report={report}
79+
reportActions={reportActions}
80+
isLoadingInitialReportActions={reportMetadata?.isLoadingInitialReportActions}
81+
hasNewerActions={hasNewerActions}
82+
hasOlderActions={hasOlderActions}
83+
parentReportAction={parentReportAction}
84+
transactionThreadReportID={transactionThreadReportID}
85+
isReportTransactionThread={isTransactionThreadView}
86+
/>
87+
// other features
88+
<Composer />
89+
</>
90+
);
91+
}
92+
```
93+
94+
### Correct
95+
96+
#### Correct (composition)
97+
98+
- Features expressed as composable children
99+
- Parent stays stable; add features by adding children
100+
101+
```tsx
102+
<Table data={items} columns={columns}>
103+
<Table.SearchBar />
104+
<Table.Header />
105+
<Table.Body />
106+
</Table>
107+
```
108+
109+
```tsx
110+
<SelectionList data={items}>
111+
<SelectionList.TextInput />
112+
<SelectionList.Body />
113+
</SelectionList>
114+
```
115+
116+
#### Correct (children manage their own state)
117+
118+
```tsx
119+
// Children are self-contained and manage their own state
120+
// Parent only passes minimal data (IDs)
121+
// Adding new features doesn't require changing the parent
122+
function ReportScreen({ params: { reportID }}) {
123+
return (
124+
<>
125+
<ReportActionsView reportID={reportID} />
126+
// other features
127+
<Composer />
128+
</>
129+
);
130+
}
131+
132+
// Component accesses stores and calculates its own state
133+
// Parent doesn't know the internals
134+
function ReportActionsView({ reportID }) {
135+
const [reportOnyx] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`);
136+
const reportActions = getFilteredReportActionsForReportView(unfilteredReportActions);
137+
// ...
138+
}
139+
```
140+
141+
---
142+
143+
### Review Metadata
144+
145+
#### Condition
146+
147+
Flag ONLY when ALL of these are true:
148+
149+
- Any of these scenarios apply:
150+
- A **new feature** is being introduced
151+
- An **existing component's API** is being expanded with new props
152+
- A **refactoring** creates a new component that still has boolean configuration props matching the search patterns controlling branching logic — refactoring is an opportunity to eliminate configuration flags, not preserve them
153+
- The component contains boolean props matching the search patterns that cause `if/else` or ternary branching inside the component body
154+
- These configuration options control feature presence, layout strategy, or behavior within the component
155+
156+
**Features that should NOT be controlled by boolean flags:**
157+
- Optional UI elements that could be composed in
158+
- New behavior that could be introduced as new children
159+
- Features that currently require parent component code changes
160+
- Layout strategy variants
161+
162+
**DO NOT flag if:**
163+
- Props are non-boolean data values needed for coordination between composed parts (e.g., `reportID`, `data`, `columns`).
164+
- The component uses composition and child components for features
165+
- Parent components stay stable as features are added
166+
167+
**Search Patterns** (hints for reviewers):
168+
- `should\w+` (any prop starting with `should`)
169+
- `canSelect`
170+
- `enable`
171+
- `disable`

‎.claude/skills/coding-standards/rules/clean-react-2-own-behavior.md‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
---
2+
ruleId: CLEAN-REACT-PATTERNS-2
3+
title: Let components own their behavior and effects
4+
---
5+
6+
## [CLEAN-REACT-PATTERNS-2] Let components own their behavior and effects
7+
8+
### Reasoning
9+
10+
When parent components compute and pass behavioral state to children, if a child's requirements change, then parent components must change as well, increasing coupling and causing behavior to leak across concerns. Letting components own their behavior keeps logic local, allows independent evolution, and follows the principle: "If removing a child breaks parent behavior, coupling exists."
11+
12+
**Distinction from CLEAN-REACT-PATTERNS-3**: This rule is about data flow DOWN (parent -> child) — "Don't pass data the child can get itself."
13+
14+
### Incorrect
15+
16+
#### Incorrect (parent micromanages child's state)
17+
18+
- Parent gathers, computes, and dictates the child's entire contextual awareness
19+
- Parent imports hooks/stores only because the child needs the information
20+
- Double coupling: parent -> child's dependencies, child -> prop names/types
21+
22+
```tsx
23+
<OptionRowLHNData
24+
reportID={reportID}
25+
fullReport={item}
26+
reportAttributes={itemReportAttributes}
27+
oneTransactionThreadReport={itemOneTransactionThreadReport}
28+
reportNameValuePairs={itemReportNameValuePairs}
29+
reportActions={itemReportActions}
30+
parentReportAction={itemParentReportAction}
31+
iouReportReportActions={itemIouReportReportActions}
32+
policy={itemPolicy}
33+
invoiceReceiverPolicy={itemInvoiceReceiverPolicy}
34+
personalDetails={personalDetails ?? {}}
35+
transaction={itemTransaction}
36+
lastReportActionTransaction={lastReportActionTransaction}
37+
receiptTransactions={transactions}
38+
viewMode={optionMode}
39+
isOptionFocused={!shouldDisableFocusOptions}
40+
lastMessageTextFromReport={lastMessageTextFromReport}
41+
onSelectRow={onSelectRow}
42+
preferredLocale={preferredLocale}
43+
hasDraftComment={hasDraftComment}
44+
transactionViolations={transactionViolations}
45+
onLayout={onLayoutItem}
46+
shouldShowRBRorGBRTooltip={shouldShowRBRorGBRTooltip}
47+
activePolicyID={activePolicyID}
48+
onboardingPurpose={introSelected?.choice}
49+
isFullscreenVisible={isFullscreenVisible}
50+
isReportsSplitNavigatorLast={isReportsSplitNavigatorLast}
51+
isScreenFocused={isScreenFocused}
52+
localeCompare={localeCompare}
53+
testID={index}
54+
isReportArchived={isReportArchived}
55+
lastAction={lastAction}
56+
lastActionReport={lastActionReport}
57+
/>
58+
```
59+
60+
In this example:
61+
- The parent fetches `fullReport`, `policy`, `transaction`, `reportActions`, `personalDetails`, `transactionViolations`, and routing/layout state
62+
- These dependencies exist only because the child needs them — the parent is a data intermediary
63+
- If `OptionRowLHNData` requirements change, the parent must change too
64+
65+
### Correct
66+
67+
#### Correct (component owns its behavior)
68+
69+
- Component receives only IDs and handlers
70+
- Internally accesses stores, contexts, and computes values
71+
- Children follow the same pattern
72+
73+
```tsx
74+
<OptionRowLHNData
75+
reportID={reportID}
76+
onSelectRow={onSelectRow}
77+
/>
78+
```
79+
80+
```tsx
81+
function OptionRowLHNData({reportID, onSelectRow}) {
82+
// Component fetches its own data
83+
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`);
84+
const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`);
85+
const [viewMode] = useOnyx(ONYXKEYS.NVP_VIEW_MODE);
86+
// ... other data this component needs
87+
88+
return (
89+
<View>
90+
{/* Children own their state too */}
91+
<Avatars reportID={reportID} />
92+
<DisplayNames reportID={reportID} />
93+
<Status reportID={reportID} />
94+
</View>
95+
);
96+
}
97+
```
98+
99+
---
100+
101+
### Review Metadata
102+
103+
#### Condition
104+
105+
Flag when a parent component acts as a pure data intermediary — fetching or computing state only to pass it to children without using it for its own logic.
106+
107+
**Signs of violation:**
108+
- Parent imports hooks/contexts only to satisfy child's data needs
109+
- Props that are direct pass-throughs of hook results (e.g., `report={reportOnyx}`)
110+
- Component receives props that are just passed through to children or that it could fetch itself
111+
- Removing or commenting out the child would leave unused variables in the parent
112+
113+
**DO NOT flag if:**
114+
- Props are minimal, domain-relevant identifiers (e.g., `reportID`, `transactionID`, `policyID`)
115+
- Props are callback/event handlers for coordination (e.g., `onSelectRow`, `onLayout`, `onPress`)
116+
- Props are structural/presentational that can't be derived internally (e.g., `style`, `testID`)
117+
- Parent genuinely uses the data for its own rendering or logic
118+
- Data is shared coordination state that parent legitimately owns (e.g., selection state managed by parent)
119+
120+
**Search Patterns** (hints for reviewers):
121+
- `Report`
122+
- `Policy`
123+
- `Transaction`
124+
- `Actions`
125+
- `useOnyx`

0 commit comments

Comments
 (0)