Merge branch 'main' of github.com:Expensify/App into stites-combineInvoicingPage

Author: stitesExpensify

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

@@ -522,6 +522,222 @@ const results = items.map((item) => {
 
 ---
 
+### [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)
+  - The Effect's only purpose is to read a value from that source and write it into state via `setState`
+  - The pattern follows: subscribe in setup, unsubscribe in cleanup, `setState` in the listener
+
+  **DO NOT flag if:**
+
+  - The subscription triggers side effects beyond reading a value (e.g., logging, navigation)
+  - 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 result is written to state via `setState`
+  - 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 sets state
+  - 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 dependency array is empty `[]` with no suppressed lint (no race possible — deps never change)
+  - The async operation doesn't set state (fire-and-forget)
+  - 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 guard variable prevents double execution (`if (didInit) return`)
+  - The logic is idempotent (safe to run twice with no side effects)
+  - 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`

.github/ISSUE_TEMPLATE/Standard.md

@@ -1,7 +1,7 @@
 ---
 name: Standard issue template
 about: A standard template to follow when creating a new issue in this repository
-labels: Bug, Daily
+labels: Bug, Daily, External
 ---
 
 If you haven’t already, check out our [contributing guidelines](https://github.com/Expensify/ReactNativeChat/blob/main/contributingGuides/CONTRIBUTING.md) for onboarding and email contributors@expensify.com to request to join our Slack channel!

.github/workflows/cherryPick.yml

@@ -116,10 +116,12 @@ jobs:
           VERSION_BUMP_COMMIT="$(git log -1 --format='%H' --author='OSBotify' --grep 'Update version to ${{ needs.createNewVersion.outputs.NEW_VERSION }}')"
           if [ -z "$VERSION_BUMP_COMMIT" ]; then
             echo "::error::❌ Could not find E/App version bump commit for ${{ needs.createNewVersion.outputs.NEW_VERSION }}"
-            git log --oneline
-          else
-            echo "::notice::👀 Found E/App version bump commit $VERSION_BUMP_COMMIT"
+            echo "::error::This may indicate E/App and Mobile-Expensify versions are out of sync"
+            echo "::error::Run the syncVersions workflow to fix, then retry this cherry-pick"
+            git log --oneline -20
+            exit 1
           fi
+          echo "::notice::👀 Found E/App version bump commit $VERSION_BUMP_COMMIT"
           echo "VERSION_BUMP_SHA=$VERSION_BUMP_COMMIT" >> "$GITHUB_OUTPUT"
 
       - name: Get Mobile-Expensify version bump commit
@@ -130,10 +132,12 @@ jobs:
           VERSION_BUMP_COMMIT="$(git log -1 --format='%H' --author='OSBotify' --grep 'Update version to ${{ needs.createNewVersion.outputs.NEW_VERSION }}')"
           if [ -z "$VERSION_BUMP_COMMIT" ]; then
             echo "::error::❌ Could not find Mobile-Expensify version bump commit for ${{ needs.createNewVersion.outputs.NEW_VERSION }}"
-            git log --oneline
-          else
-            echo "::notice::👀 Found Mobile-Expensify version bump commit $VERSION_BUMP_COMMIT"
+            echo "::error::This may indicate E/App and Mobile-Expensify versions are out of sync"
+            echo "::error::Run the syncVersions workflow to fix, then retry this cherry-pick"
+            git log --oneline -20
+            exit 1
           fi
+          echo "::notice::👀 Found Mobile-Expensify version bump commit $VERSION_BUMP_COMMIT"
           echo "VERSION_BUMP_SHA=$VERSION_BUMP_COMMIT" >> "$GITHUB_OUTPUT"
 
       - name: Get merge commit for pull request to CP

.github/workflows/createNewVersion.yml

@@ -127,23 +127,50 @@ jobs:
         run: |
           git add Mobile-Expensify
           git commit -m "Update Mobile-Expensify submodule version to ${{ steps.bumpVersion.outputs.NEW_VERSION }}"
-          if ! git push origin main; then
-            echo "Race condition! E/App main was updated while this workflow was running, so push failed. Fetching remote, rebasing, and retrying push once."
+          
+          MAX_RETRIES=5
+          RETRY_DELAY=2
+          
+          for i in $(seq 1 $MAX_RETRIES); do
+            if git push origin main; then
+              echo "::notice::Successfully pushed E/App changes on attempt $i"
+              exit 0
+            fi
+            
+            echo "::warning::Push attempt $i failed, retrying in ${RETRY_DELAY}s..."
+            sleep $RETRY_DELAY
+            RETRY_DELAY=$((RETRY_DELAY * 2))
+            
             if ! git fetch origin main; then
-              echo "::error:: ❌ Unable to fetch main"
-              echo "::error:: This can happen when Mobile-Expensify and E/App repos got out of sync."
-              echo "::error:: We likely need to manually bump the version in E/App to match Mobile-Expensify."
-              exit 1
+              echo "::error::Unable to fetch main on attempt $i"
+              continue
             fi
+            
             if ! git rebase origin/main; then
-              echo "::error:: Rebase failed while retrying E/App push"
-              exit 1
-            fi
-            if ! git push origin main; then
-              echo "::error:: E/App change failed to push after rebase"
-              exit 1
+              echo "::warning::Rebase failed on attempt $i, aborting and retrying..."
+              git rebase --abort || true
+              continue
             fi
+          done
+          
+          echo "::error::E/App push failed after $MAX_RETRIES attempts"
+          echo "::error::CRITICAL: Mobile-Expensify has version ${{ steps.bumpVersion.outputs.NEW_VERSION }} but E/App does not!"
+          echo "::error::Manual intervention required - run the syncVersions workflow to fix"
+          exit 1
+
+      - name: Verify version sync
+        run: |
+          APP_VERSION=$(jq -r .version package.json)
+          ME_VERSION=$(jq -r .meta.version Mobile-Expensify/app/config/config.json)
+          
+          if [ "$APP_VERSION" != "$ME_VERSION" ]; then
+            echo "::error::Version mismatch detected after push!"
+            echo "::error::E/App version: $APP_VERSION"
+            echo "::error::Mobile-Expensify version: $ME_VERSION"
+            echo "::error::Run the syncVersions workflow to fix this"
+            exit 1
           fi
+          echo "::notice::Versions verified: $APP_VERSION"
 
       - name: Announce failed workflow in Slack
         if: ${{ failure() }}