diff --git a/.claude/agents/code-inline-reviewer.md b/.claude/agents/code-inline-reviewer.md
index 487bba3a6977d..743407cd2cf86 100644
--- a/.claude/agents/code-inline-reviewer.md
+++ b/.claude/agents/code-inline-reviewer.md
@@ -10,1700 +10,53 @@ model: inherit
You are a **React Native Expert** — an AI trained to evaluate code contributions to Expensify and create inline comments for specific violations.
-Your job is to scan through changed files and create **inline comments** for specific violations based on the below rules.
+Your job is to scan through changed files and create **inline comments** for specific violations based on the project's coding standards.
## Rules
-Each rule includes:
+Coding standards are defined as individual rule files in `.claude/skills/coding-standards/rules/`.
-- A unique **Rule ID**
-- **Search patterns**: Grep patterns to efficiently locate potential violations in large files
-- **Pass/Fail condition**
-- **Reasoning**: Technical explanation of why the rule is important
-- Examples of good and bad usage
-
-### [PERF-1] No spread in list item's renderItem
-
-- **Search patterns**: `renderItem`, `...` (look for both in proximity)
-
-- **Condition**: Flag ONLY when ALL of these are true:
-
- - Code is inside a renderItem function (function passed to FlatList, SectionList, etc.)
- - A spread operator (...) is used on an object
- - That object is being passed as a prop to a component
- - The spread creates a NEW object literal inline
-
- **DO NOT flag if:**
-
- - Spread is used outside renderItem
- - Spread is on an array
- - Object is created once outside renderItem and reused
- - Spread is used to clone for local manipulation (not passed as prop)
-
-- **Reasoning**: `renderItem` functions execute for every visible list item on each render. Creating new objects with spread operators forces React to treat each item as changed, preventing reconciliation optimizations and causing unnecessary re-renders of child components.
-
-Good:
-
-```tsx
-
-```
-
-Bad:
-
-```tsx
-
-```
-
----
-
-### [PERF-2] Return early before expensive work
-
-- **Search patterns**: Function bodies where `if (!param)` or `if (param === undefined)` appears AFTER function calls that use `param`
-
-- **Condition**: Flag ONLY when ALL of these are true:
-
- - Code performs expensive work (function calls, iterations, API/Onyx reads)
- - A simple check could short-circuit earlier
- - The simple check happens AFTER the expensive work
-
- **DO NOT flag if**:
-
- - Simple checks already come first
- - Validation requires the computed result
- - Expensive work must run for side effects
-
-- **Reasoning**: Early returns prevent wasted computation. Validate inputs before passing them to expensive operations.
-
-Good:
-
-```ts
-function clearReportActionErrors(reportID: string, reportAction: ReportAction) {
- if (!reportAction?.reportActionID) {
- return;
- }
-
- const originalReportID = getOriginalReportID(reportID, reportAction);
- // ...
-}
-```
-
-Bad:
-
-```ts
-function clearReportActionErrors(reportID: string, reportAction: ReportAction) {
- const originalReportID = getOriginalReportID(reportID, reportAction);
-
- if (!reportAction?.reportActionID) {
- return;
- }
- // ...
-}
-```
-
----
-
-### [PERF-3] Use OnyxListItemProvider hooks instead of useOnyx in renderItem
-
-- **Search patterns**: `useOnyx` within components used in `renderItem`
-
-- **Condition**: Components rendered inside `renderItem` functions should use dedicated hooks from `OnyxListItemProvider` instead of individual `useOnyx` calls.
-- **Reasoning**: Individual `useOnyx` calls in renderItem create separate subscriptions for each list item, causing memory overhead and update cascades. `OnyxListItemProvider` hooks provide optimized data access patterns specifically designed for list rendering performance.
-
-Good:
-
-```tsx
-const personalDetails = usePersonalDetails();
-```
-
-Bad:
-
-```tsx
-const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST);
-```
-
----
-
-### [PERF-5] Use shallow comparisons instead of deep comparisons
-
-- **Search patterns**: `React.memo`, `deepEqual`
-
-- **Condition**: In `React.memo` and similar optimization functions, compare only specific relevant properties instead of using deep equality checks.
-- **Reasoning**: Deep equality checks recursively compare all nested properties, creating performance overhead that often exceeds the re-render cost they aim to prevent. Shallow comparisons of specific relevant properties provide the same optimization benefits with minimal computational cost.
-
-Good:
-
-```tsx
-memo(ReportActionItem, (prevProps, nextProps) =>
- prevProps.report.type === nextProps.report.type &&
- prevProps.report.reportID === nextProps.report.reportID &&
- prevProps.isSelected === nextProps.isSelected
-)
-```
-
-Bad:
-
-```tsx
-memo(ReportActionItem, (prevProps, nextProps) =>
- deepEqual(prevProps.report, nextProps.report) &&
- prevProps.isSelected === nextProps.isSelected
-)
-```
-
----
-
-### [PERF-6] Derive state from props
-
-- **Condition**: Flag when useEffect updates state based on props or other state, when the value could be computed directly
-
-- **Reasoning**: Computing derived values directly in the component body ensures they're always synchronized with props/state and avoids unnecessary re-renders.
-
-Good:
-
-```tsx
-function Form() {
- const [firstName, setFirstName] = useState('Taylor');
- const [lastName, setLastName] = useState('Swift');
-
- // ✅ Good: calculated during rendering
- const fullName = firstName + ' ' + lastName;
-}
-```
-
-Bad:
-
-```tsx
-function Form() {
- const [firstName, setFirstName] = useState('Taylor');
- const [lastName, setLastName] = useState('Swift');
-
- // 🔴 Avoid: redundant state and unnecessary Effect
- const [fullName, setFullName] = useState('');
- useEffect(() => {
- setFullName(firstName + ' ' + lastName);
- }, [firstName, lastName]);
-}
-```
-
----
-
-### [PERF-7] Control component resets via key prop
-
-- **Condition**:
- - Flag when useEffect resets all or most component state when a prop changes
- - Should use `key` prop instead to reset the entire component
-
-- **Reasoning**: Using `key` prop for full resets is more React-idiomatic. When a prop changes and you need to reset all component state, the `key` prop causes React to unmount and remount the component, automatically resetting all state without needing useEffect.
-
-Good:
-
-```tsx
-function ProfilePage({ userId }) {
- return ;
-}
-
-function ProfileView({ userId }) {
- const [comment, setComment] = useState('');
- const [rating, setRating] = useState(0);
- // Component resets when userId changes due to key prop
-}
-```
-
-Bad:
-
-```tsx
-// 🔴 Avoid: resetting all state with useEffect
-function ProfilePage({ userId }) {
- return ;
-}
-
-function ProfileView({ userId }) {
- const [comment, setComment] = useState('');
- const [rating, setRating] = useState(0);
-
- useEffect(() => {
- setComment(''); // Reset when userId changes
- setRating(0);
- }, [userId]);
-}
-```
-
----
-
-### [PERF-8] Handle events in event handlers
-
-- **Condition**: Flag when useEffect responds to user events that should be handled in event handlers
-
-- **Reasoning**: Event handlers provide immediate response and clearer code flow. useEffect adds unnecessary render cycles and makes the relationship between user action and response less clear.
-
-Good:
-
-```tsx
-function BuyButton({ productId, onBuy }) {
- function handleClick() {
- // ✅ Good: handle event directly in event handler
- onBuy();
- showNotification('Item purchased!');
- }
-
- return ;
-}
-```
-
-Bad:
-
-```tsx
-function BuyButton({ productId, onBuy }) {
- const [isBuying, setIsBuying] = useState(false);
-
- // 🔴 Avoid: handling events in useEffect
- useEffect(() => {
- if (isBuying) {
- onBuy();
- showNotification('Item purchased!');
- }
- }, [isBuying, onBuy]);
-
- return ;
-}
-```
-
----
-
-### [PERF-9] Avoid useEffect chains
-
-- **Condition**: Flag when multiple useEffects form a chain where one effect's state update triggers another effect
-
-- **Reasoning**: Chains of effects create complex dependencies, timing issues, and unnecessary renders. Logic should be restructured to avoid interdependent effects.
-
-Good:
-
-```tsx
-function Form() {
- const [firstName, setFirstName] = useState('');
- const [lastName, setLastName] = useState('');
-
- // ✅ Good: compute derived values directly
- const fullName = firstName + ' ' + lastName;
- const isValid = firstName.length > 0 && lastName.length > 0;
-
- return (
-
- );
-}
-```
-
-Bad:
-
-```tsx
-function Form() {
- const [firstName, setFirstName] = useState('');
- const [lastName, setLastName] = useState('');
- const [fullName, setFullName] = useState('');
- const [isValid, setIsValid] = useState(false);
-
- // 🔴 Avoid: chain of effects
- useEffect(() => {
- setFullName(firstName + ' ' + lastName);
- }, [firstName, lastName]);
-
- useEffect(() => {
- setIsValid(fullName.length > 0);
- }, [fullName]);
-}
-```
-
----
-
-### [PERF-10] Communicate with parent components without useEffect
-
-- **Condition**: Flag when useEffect calls parent callbacks to communicate state changes or pass data to parent components
-
-- **Reasoning**: Parent-child communication should not use useEffect. Instead, lift the state up to the parent component and pass it down as props. This follows React's unidirectional data flow pattern, eliminates synchronization issues, reduces unnecessary renders, and makes the data flow clearer. Use useEffect only when synchronizing with external systems, not for parent-child communication.
-
-Good:
-
-```tsx
-// Lifting state up
-function Parent() {
- const [value, setValue] = useState('');
- return ;
-}
-
-function Child({ value, onChange }) {
- return onChange(e.target.value)} />;
-}
-```
-
-Bad:
-
-```tsx
-// 🔴 Avoid: passing data via useEffect
-function Child({ onValueChange }) {
- const [value, setValue] = useState('');
-
- useEffect(() => {
- onValueChange(value);
- }, [value, onValueChange]);
-
- return setValue(e.target.value)} />;
-}
-```
-
----
-
-### [PERF-11] Optimize data selection and handling
-
-- **Search patterns**: `useOnyx`, `selector`, `.filter(`, `.map(`
-
-- **Condition**: Flag ONLY when ALL of these are true:
-
- - A component uses a broad data structure (e.g., entire object) without selecting specific fields
- - This causes unnecessary re-renders when unrelated fields change
- - OR unnecessary data filtering/fetching is performed (excluding necessary data, fetching already available data)
-
- **DO NOT flag if:**
-
- - Specific fields are already being selected or the data structure is static
- - The filtering is necessary for correct functionality
- - The fetched data is required and cannot be derived from existing data
- - The function requires the entire object for valid operations
-
-- **Reasoning**: Using broad data structures or performing unnecessary data operations causes excessive re-renders and degrades performance. Selecting specific fields and avoiding redundant operations reduces render cycles and improves efficiency.
-
-Good:
-
-```tsx
-function UserProfile({ userId }) {
- const [user] = useOnyx(`${ONYXKEYS.USER}${userId}`, {
- selector: (user) => ({
- name: user?.name,
- avatar: user?.avatar,
- }),
- });
- return {user?.name};
-}
-```
-
-Bad:
-
-```tsx
-function UserProfile({ userId }) {
- const [user] = useOnyx(`${ONYXKEYS.USER}${userId}`);
- // Component re-renders when any user field changes, even unused ones
- return {user?.name};
-}
-```
-
----
-
-### [PERF-12] Prevent memory leaks in components and plugins
-
-- **Search patterns**: `setInterval`, `setTimeout`, `addEventListener`, `subscribe`, `useEffect` with missing cleanup
-
-- **Condition**: Flag ONLY when ALL of these are true:
-
- - A resource (timeout, interval, event listener, subscription, etc.) is created in a component
- - The resource is not cleared upon component unmount
- - Asynchronous operations are initiated without a corresponding cleanup mechanism
-
- **DO NOT flag if:**
-
- - The resource is cleared properly in a cleanup function (e.g., inside `useEffect` return)
- - The resource is not expected to persist beyond the component's lifecycle
- - The resource is managed by a library that handles cleanup automatically
- - The operation is guaranteed to complete before the component unmounts
-
-- **Reasoning**: Failing to clear resources causes memory leaks, leading to increased memory consumption and potential crashes, especially in long-lived components or components that mount/unmount frequently.
-
-Good:
-
-```tsx
-function TimerComponent() {
- useEffect(() => {
- const intervalId = setInterval(() => {
- updateTimer();
- }, 1000);
-
- return () => {
- clearInterval(intervalId);
- };
- }, []);
-
- return Timer;
-}
-```
-
-Bad:
-
-```tsx
-function TimerComponent() {
- useEffect(() => {
- const intervalId = setInterval(() => {
- updateTimer();
- }, 1000);
- // Missing cleanup - interval will continue after unmount
- }, []);
-
- return Timer;
-}
-```
-
----
-
-### [PERF-13] Avoid iterator-independent function calls in array methods
-
-- **Search patterns**: `.map(`, `.reduce(`, `.filter(`, `.some(`, `.every(`, `.find(`, `.findIndex(`, `.flatMap(`, `.forEach(`
-
-- **Condition**: Flag when ALL of these are true:
-
- **For side-effect-free methods** (`.map`, `.reduce`, `.filter`, `.some`, `.every`, `.find`, `.findIndex`, `.flatMap`):
- - A function call exists inside the callback
- - The function call does NOT receive:
- - The iterator variable directly (e.g., `transform(item)`)
- - A property/value derived from the iterator (e.g., `format(item.name)`)
- - The index parameter when used meaningfully (e.g., `generateId(index)`)
- - The function is not a method called ON the iterator or iterator-derived value (e.g., `item.getValue()`)
-
- **For `.forEach`**:
- - Same conditions as above, BUT also verify the side effect doesn't depend on iteration context
- - If the function call would produce the same effect regardless of which iteration it runs in, flag it
-
- **DO NOT flag if:**
-
- - Function uses iterator, its parts or derived value based on iterator (e.g. `func(item.process())`)
- - Function call depends on iterator (e.g. `item.value ?? getDefault()`)
- - Function is used when mapping to new entities (e.g. `const thing = { id: createID() }`)
- - Above but applied to index instead of iterator
-
-- **Reasoning**: Function calls inside iteration callbacks that don't use the iterator variable execute redundantly - producing the same result. This creates O(n) overhead that scales with data size. Hoisting these calls outside the loop eliminates redundant computation and improves performance, especially on large datasets like transaction lists or report collections.
-
-Good:
-
-```ts
-// Hoist iterator-independent calls outside the loop
-const config = getConfig();
-
-const results = items.map((item) => item.value * config.multiplier);
-```
-
-```ts
-// Function receives iterator or iterator-derived value
-const formatted = items.map((item) => formatCurrency(item.amount));
-```
-
-```ts
-// Index used meaningfully
-const indexed = items.map((item, index) => ({ ...item, id: generateId(index) }));
-```
-
-Bad:
-
-```ts
-// getConfig() called on every iteration but doesn't use item
-const results = items.map((item) => {
- const config = getConfig();
- return item.value * config.multiplier;
-});
-```
-
----
-
-### [PERF-14] Use `useSyncExternalStore` for external store subscriptions
-
-- **Search patterns**: `addEventListener`, `subscribe`, `useEffect`, `useState`
-
-- **Condition**: Flag ONLY when ALL of these are true:
-
- - A `useEffect` subscribes to an external source (DOM events, third-party store, browser API)
- - Inside the listener, at least one `setState` call directly mirrors an external value
- (e.g., `setWidth(window.innerWidth)`, `setOnline(navigator.onLine)`)
- - The pattern for that state variable follows: subscribe in setup, unsubscribe in cleanup,
- `setState(externalValue)` in the listener
- - Evaluate each state variable independently — if one state variable is a raw mirror of an
- external value, flag it even if the same effect also manages other state for different purposes
-
- **DO NOT flag if:**
-
- - The specific state variable being flagged undergoes transformation, debouncing, or derives
- from computation rather than directly mirroring the external value. Other state variables
- in the same effect that DO directly mirror external values should still be flagged.
- - The external API doesn't fit the `subscribe` / `getSnapshot` contract
- - The code already uses `useSyncExternalStore`
- - The subscription is managed by a library (e.g., Onyx's `useOnyx`)
-
-- **Reasoning**: [`useSyncExternalStore`](https://react.dev/learn/you-might-not-need-an-effect#subscribing-to-an-external-store) is React's purpose-built hook for reading external store values. It handles concurrent rendering edge cases (tearing), avoids the extra render pass of `useEffect` + `setState`, and makes the subscription contract explicit. Manual subscriptions via `useEffect` are more error-prone and miss these guarantees.
-
-Good:
-
-```tsx
-function subscribe(callback) {
- window.addEventListener('online', callback);
- window.addEventListener('offline', callback);
- return () => {
- window.removeEventListener('online', callback);
- window.removeEventListener('offline', callback);
- };
-}
-
-function useOnlineStatus() {
- // ✅ Good: Subscribing to an external store with a built-in Hook
- return useSyncExternalStore(
- subscribe, // React won't resubscribe for as long as you pass the same function
- () => navigator.onLine, // How to get the value on the client
- () => true // How to get the value on the server
- );
-}
-
-function ChatIndicator() {
- const isOnline = useOnlineStatus();
- // ...
-}
-```
-
-Bad:
-
-```tsx
-function useOnlineStatus() {
- // Not ideal: Manual store subscription in an Effect
- const [isOnline, setIsOnline] = useState(true);
- useEffect(() => {
- function updateState() {
- setIsOnline(navigator.onLine);
- }
-
- updateState();
-
- window.addEventListener('online', updateState);
- window.addEventListener('offline', updateState);
- return () => {
- window.removeEventListener('online', updateState);
- window.removeEventListener('offline', updateState);
- };
- }, []);
- return isOnline;
-}
-
-function ChatIndicator() {
- const isOnline = useOnlineStatus();
- // ...
-}
-```
-
----
-
-### [PERF-15] Clean up async Effects to prevent race conditions
-
-- **Search patterns**: `useEffect`, `fetch(`, `async`, `await`, `.then(`, `setState`, `eslint-disable`
-
-- **Condition**: Flag when EITHER of these is true:
-
- **Case 1 — Missing cleanup:**
- - A `useEffect` performs async work (fetch, promise chain, async/await)
- - The async callback performs side effects (setState, navigation, data mutations, deletions)
- - There is no cleanup mechanism to discard stale responses (no `ignore` flag, no `AbortController`, no cancellation token)
-
- **Case 2 — Suppressed dependency lint:**
- - A `useEffect` performs async work and triggers side effects (setState, navigation, mutations)
- - The dependency array has an `eslint-disable` comment suppressing `react-hooks/exhaustive-deps`
- - This hides a dependency that could change and cause a race condition
-
- **DO NOT flag if:**
-
- - The Effect includes an `ignore`/`cancelled` boolean checked before `setState`
- - The Effect uses `AbortController` to cancel the request on cleanup
- - The async operation is truly fire-and-forget (no setState, no navigation, no mutations —
- just logging or analytics that are safe to complete after unmount)
- - The dependency array is empty `[]` with no suppressed lint, AND the async callback only
- performs idempotent/safe operations (no navigation, no destructive mutations that could
- fire after unmount)
- - Data fetching is handled by a library/framework (e.g., Onyx, React Query)
-
-- **Reasoning**: When an Effect's dependencies change, the previous async operation [may still be in flight](https://react.dev/learn/you-might-not-need-an-effect#fetching-data). Without cleanup, a slow earlier response can overwrite the result of a faster later response, showing stale data. This is especially dangerous for search inputs and navigation where dependencies change rapidly.
-
-Good (ignore flag):
-
-```tsx
-useEffect(() => {
- let ignore = false;
-
- fetchResults(query).then((json) => {
- if (!ignore) {
- setResults(json);
- }
- });
-
- return () => {
- ignore = true;
- };
-}, [query]);
-```
-
-Good (AbortController):
-
-```tsx
-useEffect(() => {
- const controller = new AbortController();
-
- fetch(`/api/search?q=${query}`, {signal: controller.signal})
- .then((res) => res.json())
- .then((data) => setResults(data))
- .catch((e) => {
- if (e.name !== 'AbortError') {
- setError(e);
- }
- });
-
- return () => controller.abort();
-}, [query]);
-```
-
-Bad:
-
-```tsx
-useEffect(() => {
- fetchResults(query).then((json) => {
- setResults(json);
- });
-}, [query]);
-```
-
----
-
-### [PERF-16] Guard initialization logic against double-execution
-
-- **Search patterns**: `useEffect` with empty dependency array `[]` containing initialization logic (API calls, storage access, authentication checks, SDK setup, global configuration, event handler registration)
-
-- **Condition**: Flag ONLY when ALL of these are true:
-
- - A `useEffect` with empty dependency array `[]` runs non-idempotent initialization logic
- - The logic would cause problems if executed twice (e.g., double API calls, invalidated tokens, duplicate SDK init, duplicate analytics sessions, duplicate deep link registrations)
- - There is no guard mechanism (module-level flag or module-level execution)
- - This applies at any level — app-wide init, feature screens, or individual components
-
- **DO NOT flag if:**
-
- - A module-level or ref-based guard variable prevents double execution. A proper execution
- guard follows this pattern: `if (didInit) return; didInit = true;` — it checks a flag AND
- sets it. Conditional checks on data/props (e.g., `if (!transaction) return`,
- `if (action !== 'CREATE') return`) are NOT execution guards — they validate preconditions
- but don't prevent the logic from running again if the same preconditions hold in a second
- invocation (which happens in React Strict Mode).
- - The logic is idempotent (safe to run twice with no side effects)
- - NOTE: Navigation calls (e.g., `navigate()`), data deletion (e.g., `removeDraftTransactions()`),
- and similar mutations are NOT idempotent — running them twice produces different/undesirable results.
- - The logic is at module level, outside any component
- - The Effect has non-empty dependencies (not one-time init)
-
-- **Reasoning**: React Strict Mode [intentionally double-invokes Effects](https://react.dev/learn/you-might-not-need-an-effect#initializing-the-application) in development to surface missing cleanup. Non-idempotent initialization — whether app-wide (auth tokens, global config) or per-feature (SDK setup, analytics session creation, deep link handler registration) — can break when executed twice. Guarding with a module-level flag or moving initialization outside the component ensures it runs exactly once regardless of rendering mode.
-
-Good (module-level guard):
-
-```tsx
-let didInit = false;
-
-function App() {
- useEffect(() => {
- if (didInit) {
- return;
- }
- didInit = true;
-
- loadDataFromLocalStorage();
- checkAuthToken();
- }, []);
-}
-```
-
-Good (module-level execution):
-
-```tsx
-if (typeof window !== 'undefined') {
- checkAuthToken();
- loadDataFromLocalStorage();
-}
-
-function App() {
- // ...
-}
-```
-
-Bad:
-
-```tsx
-function App() {
- useEffect(() => {
- loadDataFromLocalStorage();
- checkAuthToken();
- }, []);
-}
-```
-
----
-
-### [CONSISTENCY-1] Avoid platform-specific checks within components
-
-- **Search patterns**: `Platform.OS`, `isAndroid`, `isIOS`, `Platform\.select`
-
-- **Condition**: Flag ONLY when ALL of these are true:
+Each rule file contains:
- - Platform detection checks (e.g., `Platform.OS`, `isAndroid`, `isIOS`) are present within a component
- - The checks lead to hardcoded values or styles specific to a platform
- - The component is not structured to handle platform-specific logic through file extensions or separate components
-
- **DO NOT flag if:**
-
- - The logic is handled through platform-specific file extensions (e.g., `index.web.tsx`, `index.native.tsx`)
-
-- **Reasoning**: Mixing platform-specific logic within components increases maintenance overhead, complexity, and bug risk. Separating concerns through dedicated files or components improves maintainability and reduces platform-specific bugs.
-
-Good:
-
-```tsx
-// Platform-specific file: Button.desktop.tsx
-function Button() {
- return ;
-}
-
-// Platform-specific file: Button.mobile.tsx
-function Button() {
- return ;
-}
-```
-
-Bad:
-
-```tsx
-function Button() {
- const isAndroid = Platform.OS === 'android';
- return isAndroid ? (
-
- ) : (
-
- );
-}
-```
-
----
-
-### [CONSISTENCY-2] Avoid magic numbers and strings
-
-- **Search patterns**: Hardcoded numbers/strings (context-dependent, look for numeric literals > 1, string literals that aren't obvious)
-
-- **Condition**: Flag ONLY when ALL of these are true:
-
- - Hardcoded strings or numbers are used without documentation or comments
- - The value is not defined as a constant elsewhere in the codebase
- - The value is not self-explanatory (e.g., `0`, `1`, `Math.PI`)
-
- **DO NOT flag if:**
-
- - The value is self-explanatory (e.g., `Math.PI`, `0`, `1`, `true`, `false`)
- - The value is part of configuration or environment variables
- - The value is documented with clear comments explaining its purpose
- - The value is defined as a named constant in the same file or imported module
-
-- **Reasoning**: Magic numbers and strings reduce code readability and maintainability. Replacing them with named constants or documented values improves clarity and makes future changes easier.
-
-Good:
-
-```tsx
-const MAX_RETRY_ATTEMPTS = 3;
-const API_TIMEOUT_MS = 5000;
-
-function fetchData() {
- if (attempts < MAX_RETRY_ATTEMPTS) {
- return apiCall({ timeout: API_TIMEOUT_MS });
- }
-}
-```
-
-Bad:
-
-```tsx
-function fetchData() {
- if (attempts < 3) {
- return apiCall({ timeout: 5000 });
- }
-}
-```
-
----
-
-### [CONSISTENCY-3] Eliminate code duplication
-
-- **Search patterns**: Similar code patterns, repeated logic (context-dependent analysis)
-
-- **Condition**: Flag ONLY when ALL of these are true:
-
- - Code contains duplicated logic, constants, or components in multiple locations
- - The duplicated code performs similar operations or serves the same purpose
- - The duplicated code is not abstracted into a reusable function or component
- - There is no justification for the duplication
-
- **DO NOT flag if:**
-
- - The duplicated code serves distinct purposes or has different requirements
- - The code is intentionally duplicated for performance reasons or due to external constraints
- - The duplication is in test or mock code
- - The duplication is a temporary measure with a plan for refactoring
-
-- **Reasoning**: Code duplication increases maintenance overhead, raises bug risk, and complicates the codebase. Consolidating similar logic into reusable functions or components adheres to the DRY principle, making code easier to maintain and understand.
-
-Good:
-
-```tsx
-function formatCurrency(amount: number, currency: string) {
- return new Intl.NumberFormat('en-US', {
- style: 'currency',
- currency,
- }).format(amount);
-}
-
-function TransactionList({ transactions }) {
- return transactions.map(t => formatCurrency(t.amount, t.currency));
-}
-
-function SummaryCard({ total }) {
- return {formatCurrency(total, 'USD')};
-}
-```
-
-Bad:
-
-```tsx
-function TransactionList({ transactions }) {
- return transactions.map(t =>
- new Intl.NumberFormat('en-US', {
- style: 'currency',
- currency: t.currency,
- }).format(t.amount)
- );
-}
-
-function SummaryCard({ total }) {
- return (
-
- {new Intl.NumberFormat('en-US', {
- style: 'currency',
- currency: 'USD',
- }).format(total)}
-
- );
-}
-```
-
----
-
-### [CONSISTENCY-4] Eliminate unused and redundant props
-
-- **Search patterns**: Component prop definitions, unused props in destructuring
-
-- **Condition**: Flag ONLY when ALL of these are true:
-
- - A component defines props that are not referenced in its implementation
- - The prop is not conditionally used or part of a larger interface
- - The prop is not prepared for future use or part of an ongoing refactor
-
- **DO NOT flag if:**
-
- - Props are conditionally used or part of a larger interface
- - Props are prepared for future use or part of an ongoing refactor
- - The prop is necessary for functionality or future extensibility
- - The prop is redundant but serves a distinct purpose (e.g., backward compatibility)
-
-- **Reasoning**: Unused props increase component complexity and maintenance overhead. Simplifying component interfaces improves code clarity and makes the component API easier to understand.
-
-Good:
-
-```tsx
-type ButtonProps = {
- title: string;
- onPress: () => void;
- disabled?: boolean;
-};
-
-function Button({ title, onPress, disabled = false }: ButtonProps) {
- return (
-
- {title}
-
- );
-}
-```
-
-Bad:
-
-```tsx
-type ButtonProps = {
- title: string;
- onPress: () => void;
- unusedProp: string; // Never used in component
- anotherUnused: number; // Never used in component
-};
-
-function Button({ title, onPress }: ButtonProps) {
- return (
-
- {title}
-
- );
-}
-```
-
----
-
-### [CONSISTENCY-5] Justify ESLint rule disables
-
-- **Search patterns**: `eslint-disable`, `eslint-disable-next-line`, `eslint-disable-line`
-
-- **Condition**: Flag ONLY when ALL of these are true:
-
- - An ESLint rule is disabled (via `eslint-disable`, `eslint-disable-next-line`, etc.)
- - The disable statement lacks an accompanying comment explaining the reason
-
- **DO NOT flag if:**
-
- - The disablement is justified with a clear comment explaining why the rule is disabled
-
-- **Reasoning**: ESLint rule disables without justification can mask underlying issues and reduce code quality. Clear documentation ensures team members understand exceptions, promoting better maintainability.
-
-Good:
-
-```tsx
-// eslint-disable-next-line react-hooks/exhaustive-deps
-// Dependencies are intentionally omitted - this effect should only run on mount
-useEffect(() => {
- initializeComponent();
-}, []);
-```
-
-Bad:
-
-```tsx
-// eslint-disable-next-line react-hooks/exhaustive-deps
-useEffect(() => {
- initializeComponent();
-}, []);
-```
-
----
-
-### [CONSISTENCY-6] Ensure proper error handling
-
-- **Search patterns**: `try`, `catch`, `async`, `await`, `Promise`, `.then(`, `.catch(`
-
-- **Condition**: Flag ONLY when ALL of these are true:
-
- - Error handling logic exists but errors are not logged or handled appropriately
- - OR error states are not communicated to the user or developer clearly
- - OR a critical function (e.g., API call, authentication, data mutation) lacks error handling
-
- **DO NOT flag if:**
-
- - Errors are logged and handled properly with user feedback
- - Errors are intentionally suppressed with clear justification
- - Error handling is managed by a higher-level function or middleware
- - The operation is non-critical and errors are acceptable to ignore
-
-- **Reasoning**: Proper error handling prevents silent failures, enhances debuggability, and improves user experience. Failing to handle errors can lead to crashes, data loss, and confusion for both developers and users.
-
-Good:
-
-```tsx
-async function submitForm(data: FormData) {
- try {
- await API.submit(data);
- showSuccessMessage('Form submitted successfully');
- } catch (error) {
- Log.error('Form submission failed', error);
- showErrorMessage('Failed to submit form. Please try again.');
- }
-}
-```
-
-Bad:
-
-```tsx
-async function submitForm(data: FormData) {
- await API.submit(data);
- // No error handling - failures are silent
- showSuccessMessage('Form submitted successfully');
-}
-```
-
----
-
-### [CLEAN-REACT-PATTERNS-1] Favor composition over configuration
-
-- **Search patterns**: `shouldShow`, `shouldEnable`, `canSelect`, `enable`, `disable`, configuration props patterns
-
-- **Condition**: Flag ONLY when ALL of these are true:
-
- - A **new feature** is being introduced OR an **existing component's API is being expanded with new props**
- - The change adds configuration properties (flags, conditional logic)
- - These configuration options control feature presence or behavior within the component
- - These features could instead be expressed as composable child components
-
- **Features that should be expressed as child components:**
- - Optional UI elements that could be composed in
- - New behavior that could be introduced as new children
- - Features that currently require parent component code changes
-
- **DO NOT flag if:**
- - Props are narrow, stable values needed for coordination between composed parts (e.g., `reportID`, `data`, `columns`)
- - The component uses composition and child components for features
- - Parent components stay stable as features are added
-
-- **Reasoning**: 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".
-
-Good (composition):
-
-- Features expressed as composable children
-- Parent stays stable; add features by adding children
-
-```tsx
-
-
-
-
-
-```
-
-```tsx
-
-
-
-
-```
-
-Bad (configuration):
-
-- Features controlled by boolean flags
-- Adding a new feature requires modifying the Table component's API
-
-```tsx
-
-
-type TableProps = {
- data: Item[];
- columns: Column[];
- shouldShowSearchBar?: boolean; // ❌ Could be
- shouldShowHeader?: boolean; // ❌ Could be
- shouldEnableSorting?: boolean; // ❌ Configuration for header behavior
- shouldShowPagination?: boolean; // ❌ Could be
- shouldHighlightOnHover?: boolean; // ❌ Configuration for styling behavior
-};
-```
-
-```tsx
-
-
-type SelectionListProps = {
- shouldShowTextInput?: boolean; // ❌ Could be
- shouldShowConfirmButton?: boolean; // ❌ Could be
- textInputOptions?: {...}; // ❌ Configuration object for the above
-};
-```
-
-Good (children manage their own state):
-
-```tsx
-// Children are self-contained and manage their own state
-// Parent only passes minimal data (IDs)
-// Adding new features doesn't require changing the parent
-function ReportScreen({ params: { reportID }}) {
- return (
- <>
-
- // other features
-
-
- );
-}
-
-// Component accesses stores and calculates its own state
-// Parent doesn't know the internals
-function ReportActionsView({ reportID }) {
- const [reportOnyx] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`);
- const reportActions = getFilteredReportActionsForReportView(unfilteredReportActions);
- // ...
-}
-```
-
-Bad (parent manages child state):
-
-```tsx
-// Parent fetches and manages state for its children
-// Parent has to know child implementation details
-function ReportScreen({ params: { reportID }}) {
- const [reportOnyx] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {allowStaleData: true, canBeMissing: true});
- const reportActions = useMemo(() => getFilteredReportActionsForReportView(unfilteredReportActions), [unfilteredReportActions]);
- const [reportMetadata = defaultReportMetadata] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${reportIDFromRoute}`, {canBeMissing: true, allowStaleData: true});
- const {reportActions: unfilteredReportActions, linkedAction, sortedAllReportActions, hasNewerActions, hasOlderActions} = usePaginatedReportActions(reportID, reportActionIDFromRoute);
- const parentReportAction = useParentReportAction(reportOnyx);
- const transactionThreadReportID = getOneTransactionThreadReportID(report, chatReport, reportActions ?? [], isOffline, reportTransactionIDs);
- const isTransactionThreadView = isReportTransactionThread(report);
- // other onyx connections etc
-
- return (
- <>
-
- // other features
-
-
- );
-}
-```
-
----
-
-### [CLEAN-REACT-PATTERNS-2] Let components own their behavior and effects
-
-- **Search patterns**: Large prop counts in JSX, props named `*Report`, `*Policy`, `*Transaction`, `*Actions`, `useOnyx`/context results passed directly as props
-
-- **Condition**: 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.
-
- **Signs of violation:**
- - Parent imports hooks/contexts only to satisfy child's data needs
- - Props that are direct pass-throughs of hook results (e.g., `report={reportOnyx}`)
- - Component receives props that are just passed through to children or that it could fetch itself
- - Removing or commenting out the child would leave unused variables in the parent
-
- **DO NOT flag if:**
- - Props are minimal, domain-relevant identifiers (e.g., `reportID`, `transactionID`, `policyID`)
- - Props are callback/event handlers for coordination (e.g., `onSelectRow`, `onLayout`, `onPress`)
- - Props are structural/presentational that can't be derived internally (e.g., `style`, `testID`)
- - Parent genuinely uses the data for its own rendering or logic
- - Data is shared coordination state that parent legitimately owns (e.g., selection state managed by parent)
-
-- **Reasoning**: 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."
-
-**Distinction from CLEAN-REACT-PATTERNS-3**: This rule is about data flow DOWN (parent → child) — "Don't pass data the child can get itself."
-
-Good (component owns its behavior):
-
-- Component receives only IDs and handlers
-- Internally accesses stores, contexts, and computes values
-- Children follow the same pattern
-
-```tsx
-
-```
-
-```tsx
-function OptionRowLHNData({reportID, onSelectRow}) {
- // Component fetches its own data
- const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`);
- const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`);
- const [viewMode] = useOnyx(ONYXKEYS.NVP_VIEW_MODE);
- // ... other data this component needs
-
- return (
-
- {/* Children own their state too */}
-
-
-
-
- );
-}
-```
-
-Bad (parent micromanages child's state):
-
-- Parent gathers, computes, and dictates the child's entire contextual awareness
-- Parent imports hooks/stores only because the child needs the information
-- Double coupling: parent → child's dependencies, child → prop names/types
-
-```tsx
-
-```
-
-In this example:
-- The parent fetches `fullReport`, `policy`, `transaction`, `reportActions`, `personalDetails`, `transactionViolations`, and routing/layout state
-- These dependencies exist only because the child needs them — the parent is a data intermediary
-- If `OptionRowLHNData` requirements change, the parent must change too
-
----
-
-### [CLEAN-REACT-PATTERNS-3] Design context-free component contracts
-
-- **Search patterns**: Callback props with consumer-specific signatures like `(index: number) => void`, props used only to extract values for callbacks, refs used to access external component state, useImperativeHandle
-
-- **Condition**: Flag ONLY when BOTH of these are true:
-
- 1. A component's interface is shaped around a specific consumer's implementation rather than abstract capabilities
- 2. AND at least ONE of the following manifestations is present:
- - The component receives data only to extract values for callbacks (doesn't use it for rendering)
- - Callback signatures encode consumer-specific assumptions (e.g., `(index: number) => void` for navigation)
- - The component accesses external state through refs or imperative handles
-
- **Signs of violation:**
- - Callback signatures that encode consumer assumptions: `navigateToWaypoint(index: number)` instead of `onAddWaypoint()`
- - Props passed only to extract values for callbacks, not for rendering (e.g., `transaction` passed just to compute `waypoints.length`)
- - Imperative access to external state via refs or `useImperativeHandle`
- - Component requires modification to work in a different context
-
- **DO NOT flag if:**
- - Component signals events with data it naturally owns (e.g., `onChange(value)` for an input, `onSelectItem(item)` for a list)
- - Callbacks are abstract actions the component can trigger (e.g., `onAddStop()`, `onSubmit()`)
- - State coordination happens at a higher level with clear data flow
-
- **What makes a contract "abstract":**
- - Callback describes *what happened* in component terms: `onAddStop`, `onSelect`, `onChange`
- - Callback does NOT describe *what consumer should do*: `navigateToWaypoint(index)`, `updateParentState(value)`
- - Props are used for rendering or internal logic, not just to compute callback arguments
- - Component works without modification in a different context
-
-- **Reasoning**: A component's contract should expose its capabilities abstractly, not encode assumptions about how it will be used. When interfaces leak consumer-specific details, the component becomes coupled to that context and requires modification for reuse. Good contracts signal *what the component can do*, not *what the consumer needs*.
-
-**Distinction from CLEAN-REACT-PATTERNS-2**: PATTERNS-2 ensures components fetch their own data. This rule ensures components expose abstract capabilities, not consumer-specific interfaces.
-
-Good (abstract contract):
-
-- Interface exposes capability: "user can add a stop"
-- Implementation details (index computation, navigation) stay with consumer
-- Component is reusable in any context needing an "add stop" action
-
-```tsx
- {
- const nextIndex = Object.keys(transaction?.comment?.waypoints ?? {}).length;
- Navigation.navigate(ROUTES.MONEY_REQUEST_STEP_WAYPOINT.getRoute(..., nextIndex.toString(), ...));
- }}
-/>
-
-// in DistanceRequestFooter
-
-```
-
-Bad (contract leaks consumer assumptions):
-
-- Callback `navigateToWaypointEditPage(index: number)` encodes routing assumption
-- `transaction` prop exists only to compute index for callback
-- Requires modification if consumer navigates differently
-
-```tsx
-type DistanceRequestFooterProps = {
- waypoints?: WaypointCollection;
- navigateToWaypointEditPage: (index: number) => void; // Encodes routing assumption
- transaction: OnyxEntry;
- policy: OnyxEntry;
-};
-
-// in IOURequestStepDistance
-
-
-// in DistanceRequestFooter - computes value for consumer's callback
-