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
- The pattern for that state variable follows: subscribe in setup, unsubscribe in cleanup,
535
+
`setState(externalValue)` in the listener
536
+
- Evaluate each state variable independently — if one state variable is a raw mirror of an
537
+
external value, flag it even if the same effect also manages other state for different purposes
538
+
539
+
**DO NOT flag if:**
540
+
541
+
- The specific state variable being flagged undergoes transformation, debouncing, or derives
542
+
from computation rather than directly mirroring the external value. Other state variables
543
+
in the same effect that DO directly mirror external values should still be flagged.
544
+
- The external API doesn't fit the `subscribe` / `getSnapshot` contract
545
+
- The code already uses `useSyncExternalStore`
546
+
- The subscription is managed by a library (e.g., Onyx's `useOnyx`)
547
+
548
+
-**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.
549
+
550
+
Good:
551
+
552
+
```tsx
553
+
function subscribe(callback) {
554
+
window.addEventListener('online', callback);
555
+
window.addEventListener('offline', callback);
556
+
return () => {
557
+
window.removeEventListener('online', callback);
558
+
window.removeEventListener('offline', callback);
559
+
};
560
+
}
561
+
562
+
function useOnlineStatus() {
563
+
// ✅ Good: Subscribing to an external store with a built-in Hook
564
+
returnuseSyncExternalStore(
565
+
subscribe, // React won't resubscribe for as long as you pass the same function
566
+
() =>navigator.onLine, // How to get the value on the client
567
+
() =>true// How to get the value on the server
568
+
);
569
+
}
570
+
571
+
function ChatIndicator() {
572
+
const isOnline =useOnlineStatus();
573
+
// ...
574
+
}
575
+
```
576
+
577
+
Bad:
578
+
579
+
```tsx
580
+
function useOnlineStatus() {
581
+
// Not ideal: Manual store subscription in an Effect
-**Condition**: Flag when EITHER of these is true:
613
+
614
+
**Case 1 — Missing cleanup:**
615
+
- A `useEffect` performs async work (fetch, promise chain, async/await)
616
+
- The async callback performs side effects (setState, navigation, data mutations, deletions)
617
+
- There is no cleanup mechanism to discard stale responses (no `ignore` flag, no `AbortController`, no cancellation token)
618
+
619
+
**Case 2 — Suppressed dependency lint:**
620
+
- A `useEffect` performs async work and triggers side effects (setState, navigation, mutations)
621
+
- The dependency array has an `eslint-disable` comment suppressing `react-hooks/exhaustive-deps`
622
+
- This hides a dependency that could change and cause a race condition
623
+
624
+
**DO NOT flag if:**
625
+
626
+
- The Effect includes an `ignore`/`cancelled` boolean checked before `setState`
627
+
- The Effect uses `AbortController` to cancel the request on cleanup
628
+
- The async operation is truly fire-and-forget (no setState, no navigation, no mutations —
629
+
just logging or analytics that are safe to complete after unmount)
630
+
- The dependency array is empty `[]` with no suppressed lint, AND the async callback only
631
+
performs idempotent/safe operations (no navigation, no destructive mutations that could
632
+
fire after unmount)
633
+
- Data fetching is handled by a library/framework (e.g., Onyx, React Query)
634
+
635
+
-**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.
-**Condition**: Flag ONLY when ALL of these are true:
691
+
692
+
- A `useEffect` with empty dependency array `[]` runs non-idempotent initialization logic
693
+
- 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)
694
+
- There is no guard mechanism (module-level flag or module-level execution)
695
+
- This applies at any level — app-wide init, feature screens, or individual components
696
+
697
+
**DO NOT flag if:**
698
+
699
+
- A module-level or ref-based guard variable prevents double execution. A proper execution
700
+
guard follows this pattern: `if (didInit) return; didInit = true;` — it checks a flag AND
701
+
sets it. Conditional checks on data/props (e.g., `if (!transaction) return`,
702
+
`if (action !== 'CREATE') return`) are NOT execution guards — they validate preconditions
703
+
but don't prevent the logic from running again if the same preconditions hold in a second
704
+
invocation (which happens in React Strict Mode).
705
+
- The logic is idempotent (safe to run twice with no side effects)
706
+
- NOTE: Navigation calls (e.g., `navigate()`), data deletion (e.g., `removeDraftTransactions()`),
707
+
and similar mutations are NOT idempotent — running them twice produces different/undesirable results.
708
+
- The logic is at module level, outside any component
709
+
- The Effect has non-empty dependencies (not one-time init)
710
+
711
+
-**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.
712
+
713
+
Good (module-level guard):
714
+
715
+
```tsx
716
+
let didInit =false;
717
+
718
+
function App() {
719
+
useEffect(() => {
720
+
if (didInit) {
721
+
return;
722
+
}
723
+
didInit=true;
724
+
725
+
loadDataFromLocalStorage();
726
+
checkAuthToken();
727
+
}, []);
728
+
}
729
+
```
730
+
731
+
Good (module-level execution):
732
+
733
+
```tsx
734
+
if (typeofwindow!=='undefined') {
735
+
checkAuthToken();
736
+
loadDataFromLocalStorage();
737
+
}
738
+
739
+
function App() {
740
+
// ...
741
+
}
742
+
```
743
+
744
+
Bad:
745
+
746
+
```tsx
747
+
function App() {
748
+
useEffect(() => {
749
+
loadDataFromLocalStorage();
750
+
checkAuthToken();
751
+
}, []);
752
+
}
753
+
```
754
+
755
+
---
756
+
525
757
### [CONSISTENCY-1] Avoid platform-specific checks within components
0 commit comments