From 7949f2240ab8c5856de6cfae3774b7573031d17f Mon Sep 17 00:00:00 2001 From: Alex Risch Date: Sat, 19 Oct 2024 08:31:40 -0400 Subject: [PATCH 1/2] fix: Xmtp Engine Rerenders, Race Conditions, Crashes Refactored Xmtp Engine to be mostly outside of React Context Adds subscriptions Moves app state into folder and adds new app state util --- App.tsx | 4 +- App.web.tsx | 5 +- components/ExternalWalletPicker.tsx | 2 +- components/XmtpEngine.tsx | 201 ++++++++++-------- data/db/index.ts | 2 +- data/store/appStore.ts | 4 +- utils/alert.ts | 2 +- utils/appState/appStateIsBlurred.ts | 6 + .../waitUntilAppActive.ts} | 3 +- 9 files changed, 128 insertions(+), 101 deletions(-) create mode 100644 utils/appState/appStateIsBlurred.ts rename utils/{appState.ts => appState/waitUntilAppActive.ts} (96%) diff --git a/App.tsx b/App.tsx index 78c0b7e17..ef394577a 100644 --- a/App.tsx +++ b/App.tsx @@ -29,7 +29,7 @@ import { Provider as PaperProvider } from "react-native-paper"; import { ThirdwebProvider } from "thirdweb/react"; import "./utils/splash/splash"; -import XmtpEngine from "./components/XmtpEngine"; +import { XmtpCron } from "./components/XmtpEngine"; import config from "./config"; import { useAppStore } from "./data/store/appStore"; import { useSelect } from "./data/store/storeHelpers"; @@ -99,7 +99,7 @@ const App = () => { return ( - +
diff --git a/App.web.tsx b/App.web.tsx index 5901313a6..85f115c21 100644 --- a/App.web.tsx +++ b/App.web.tsx @@ -9,7 +9,7 @@ import { SafeAreaProvider } from "react-native-safe-area-context"; import "./assets/web.css"; import "./polyfills"; -import XmtpEngine from "./components/XmtpEngine"; +import { XmtpCron } from "./components/XmtpEngine"; import config from "./config"; import Main from "./screens/Main"; @@ -31,6 +31,7 @@ createWeb3Modal({ export default function App() { const colorScheme = useColorScheme(); + return ( @@ -57,7 +58,7 @@ export default function App() { > <>
- + diff --git a/components/ExternalWalletPicker.tsx b/components/ExternalWalletPicker.tsx index 6f4165e4b..d89cd8fa4 100644 --- a/components/ExternalWalletPicker.tsx +++ b/components/ExternalWalletPicker.tsx @@ -6,7 +6,7 @@ import { textPrimaryColor, } from "@styles/colors"; import { PictoSizes } from "@styles/sizes"; -import { waitUntilAppActive } from "@utils/appState"; +import { waitUntilAppActive } from "@utils/appState/waitUntilAppActive"; import { converseEventEmitter } from "@utils/events"; import { thirdwebClient } from "@utils/thirdweb"; import { Image } from "expo-image"; diff --git a/components/XmtpEngine.tsx b/components/XmtpEngine.tsx index ae7635b2d..8a0067694 100644 --- a/components/XmtpEngine.tsx +++ b/components/XmtpEngine.tsx @@ -2,19 +2,20 @@ import { dropConverseDbConnections, reconnectConverseDbConnections, } from "@data/db/driver"; +import { appStateIsBlurredState } from "@utils/appState/appStateIsBlurred"; import logger from "@utils/logger"; import { stopStreamingAllMessage } from "@utils/xmtpRN/messages"; import { useCallback, useEffect, useRef } from "react"; -import { AppState, Platform } from "react-native"; +import { + AppState, + AppStateStatus, + NativeEventSubscription, + Platform, +} from "react-native"; import { getExistingDataSource } from "../data/db/datasource"; -import { - getAccountsList, - getChatStore, - useAccountsList, -} from "../data/store/accountsStore"; +import { getAccountsList, getChatStore } from "../data/store/accountsStore"; import { useAppStore } from "../data/store/appStore"; -import { useSelect } from "../data/store/storeHelpers"; import { getTopicsData } from "../utils/api"; import { loadSavedNotificationMessagesToContext } from "../utils/notifications"; import { @@ -25,109 +26,129 @@ import { import { sendPendingMessages } from "../utils/xmtpRN/send"; import { syncXmtpClient } from "../utils/xmtpRN/sync"; -export default function XmtpEngine() { - const appState = useRef(AppState.currentState); - const accounts = useAccountsList(); - const syncedAccounts = useRef<{ [account: string]: boolean }>({}); - const syncingAccounts = useRef<{ [account: string]: boolean }>({}); - const { hydrationDone, isInternetReachable } = useAppStore( - useSelect(["hydrationDone", "isInternetReachable"]) - ); +class XmtpEngine { + appStoreSubscription: () => void; + appStateSubscription: NativeEventSubscription; + isInternetReachable: boolean; + hydrationDone: boolean; + syncedAccounts: { [account: string]: boolean }; + syncingAccounts: { [account: string]: boolean }; + appState: AppStateStatus; - const syncAccounts = useCallback((accountsToSync: string[]) => { - accountsToSync.forEach((a) => { - if (!syncingAccounts.current[a]) { - getTopicsData(a).then((topicsData) => { - getChatStore(a).getState().setTopicsData(topicsData, true); - }); - syncedAccounts.current[a] = true; - syncingAccounts.current[a] = true; - syncXmtpClient(a) - .then(() => { - syncingAccounts.current[a] = false; - }) - .catch(() => { - syncingAccounts.current[a] = false; - }); - } - }); - }, []); + constructor() { + logger.debug("[XmtpEngine] Initializing"); + this.syncedAccounts = {}; + this.syncingAccounts = {}; - // Sync accounts on load and when a new one is added - useEffect(() => { - // Let's remove accounts that dont exist anymore from ref - Object.keys(syncedAccounts.current).forEach((account) => { - if (!accounts.includes(account)) { - delete syncedAccounts.current[account]; - } - }); - if (hydrationDone) { - const unsyncedAccounts = accounts.filter( - (a) => !syncedAccounts.current[a] - ); - syncAccounts(unsyncedAccounts); - } - }, [accounts, hydrationDone, syncAccounts]); + const { isInternetReachable, hydrationDone } = useAppStore.getState(); + this.isInternetReachable = isInternetReachable; + this.hydrationDone = hydrationDone; + this.appStoreSubscription = useAppStore.subscribe( + (state, previousState) => { + this.isInternetReachable = state.isInternetReachable; + this.hydrationDone = state.hydrationDone; - const isInternetReachableRef = useRef(isInternetReachable); + if (previousState.isInternetReachable !== state.isInternetReachable) { + this.onInternetReachabilityChange(state.isInternetReachable); + } + if (previousState.hydrationDone !== state.hydrationDone) { + this.onHydrationDone(state.hydrationDone); + } + } + ); - // When app back active, resync all, in case we lost sync - // And also save data from notifications - useEffect(() => { - const subscription = AppState.addEventListener( + this.appState = AppState.currentState; + this.appStateSubscription = AppState.addEventListener( "change", - async (nextAppState) => { + (nextAppState: AppStateStatus) => { + const previousAppState = this.appState; + this.appState = nextAppState; logger.debug( - `[AppState] App is was ${appState.current} - now ${nextAppState}` + `[XmtpEngine] App is now ${nextAppState} - was ${previousAppState}` ); if ( nextAppState === "active" && - appState.current.match(/inactive|background/) + appStateIsBlurredState(previousAppState) ) { - reconnectConverseDbConnections(); - if (hydrationDone) { - loadSavedNotificationMessagesToContext(); - if (isInternetReachableRef.current) { - syncAccounts(accounts); - } - } + this.onAppFocus(); } else if ( - nextAppState.match(/inactive|background/) && - appState.current === "active" + appStateIsBlurredState(nextAppState) && + previousAppState === "active" ) { - for (const account of accounts) { - await Promise.all([ - stopStreamingAllMessage(account), - stopStreamingConversations(account), - stopStreamingGroups(account), - ]); - } - dropConverseDbConnections(); + this.onAppBlur(); } - appState.current = nextAppState; } ); + } + + onInternetReachabilityChange(isInternetReachable: boolean) { + logger.debug( + `[XmtpEngine] Internet reachability changed: ${isInternetReachable}` + ); + this.syncAccounts(getAccountsList()); + } - return () => { - subscription.remove(); - }; - }, [syncAccounts, accounts, hydrationDone]); + onHydrationDone(hydrationDone: boolean) { + logger.debug(`[XmtpEngine] Hydration done changed: ${hydrationDone}`); + this.syncAccounts(getAccountsList()); + } - // If lost connection, resync - useEffect(() => { - if ( - !isInternetReachableRef.current && - isInternetReachable && - hydrationDone - ) { - // We're back online! - syncAccounts(accounts); + onAppFocus() { + logger.debug("[XmtpEngine] App is now active, reconnecting db connections"); + reconnectConverseDbConnections(); + if (this.hydrationDone) { + loadSavedNotificationMessagesToContext(); + if (this.isInternetReachable) { + this.syncAccounts(getAccountsList()); + } } - isInternetReachableRef.current = isInternetReachable; - }, [accounts, hydrationDone, isInternetReachable, syncAccounts]); + } - // Cron + async onAppBlur() { + logger.debug( + "[XmtpEngine] App is now inactive, stopping xmtp streams and db connections" + ); + for (const account of getAccountsList()) { + await Promise.all([ + stopStreamingAllMessage(account), + stopStreamingConversations(account), + stopStreamingGroups(account), + ]); + } + dropConverseDbConnections(); + } + + async syncAccounts(accountsToSync: string[]) { + accountsToSync.forEach((a) => { + if (!this.syncingAccounts[a]) { + getTopicsData(a).then((topicsData) => { + getChatStore(a).getState().setTopicsData(topicsData, true); + }); + this.syncedAccounts[a] = true; + this.syncingAccounts[a] = true; + syncXmtpClient(a) + .then(() => { + this.syncingAccounts[a] = false; + }) + .catch(() => { + this.syncingAccounts[a] = false; + }); + } + }); + } + + destroy() { + // Normal app usage won't call this, but hot reloading will + logger.debug("[XmtpEngine] Removing subscriptions"); + this.appStoreSubscription(); + this.appStateSubscription.remove(); + } +} + +export const xmtpEngine = new XmtpEngine(); +export function XmtpCron() { + // Cron const lastCronTimestamp = useRef(0); const runningCron = useRef(false); diff --git a/data/db/index.ts b/data/db/index.ts index 2876f8dfc..841f50db1 100644 --- a/data/db/index.ts +++ b/data/db/index.ts @@ -1,4 +1,4 @@ -import { waitUntilAppActive } from "@utils/appState"; +import { waitUntilAppActive } from "@utils/appState/waitUntilAppActive"; import logger from "@utils/logger"; import { AppState, Platform } from "react-native"; import RNFS from "react-native-fs"; diff --git a/data/store/appStore.ts b/data/store/appStore.ts index 36b2c943e..3ce5f843e 100644 --- a/data/store/appStore.ts +++ b/data/store/appStore.ts @@ -19,13 +19,13 @@ type AppStoreType = { ) => void; splashScreenHidden: boolean; - setSplashScreenHidden: (hidden: boolean) => void; + setSplashScreenHidden: (hidden: true) => void; isInternetReachable: boolean; setIsInternetReachable: (reachable: boolean) => void; hydrationDone: boolean; - setHydrationDone: (done: boolean) => void; + setHydrationDone: (done: true) => void; lastVersionOpen: string; setLastVersionOpen: (version: string) => void; diff --git a/utils/alert.ts b/utils/alert.ts index 681c5ff53..e663e4eb2 100644 --- a/utils/alert.ts +++ b/utils/alert.ts @@ -1,6 +1,6 @@ import { Alert, AlertButton } from "react-native"; -import { waitUntilAppActive } from "./appState"; +import { waitUntilAppActive } from "./appState/waitUntilAppActive"; export const awaitableAlert = ( title: string, diff --git a/utils/appState/appStateIsBlurred.ts b/utils/appState/appStateIsBlurred.ts new file mode 100644 index 000000000..c990c4869 --- /dev/null +++ b/utils/appState/appStateIsBlurred.ts @@ -0,0 +1,6 @@ +import { AppStateStatus } from "react-native"; + +const blurredStateRegex = /inactive|background/; + +export const appStateIsBlurredState = (appState: AppStateStatus) => + appState.match(blurredStateRegex); diff --git a/utils/appState.ts b/utils/appState/waitUntilAppActive.ts similarity index 96% rename from utils/appState.ts rename to utils/appState/waitUntilAppActive.ts index 393c4be95..8849baf2d 100644 --- a/utils/appState.ts +++ b/utils/appState/waitUntilAppActive.ts @@ -1,7 +1,6 @@ +import logger from "@utils/logger"; import { AppState } from "react-native"; -import logger from "./logger"; - /* Method to wait until the app is in Foreground (Active) helpful to make sure we're not doing critical database stuff From 5ec62995dd4c7eb1f27500e09ebada1b4874f18a Mon Sep 17 00:00:00 2001 From: Alex Risch Date: Sat, 19 Oct 2024 08:56:32 -0400 Subject: [PATCH 2/2] Moved cron to class component --- App.tsx | 6 ++-- App.web.tsx | 6 ++-- components/XmtpEngine.tsx | 75 +++++++++++++++++++++------------------ 3 files changed, 49 insertions(+), 38 deletions(-) diff --git a/App.tsx b/App.tsx index ef394577a..9d141dece 100644 --- a/App.tsx +++ b/App.tsx @@ -29,7 +29,7 @@ import { Provider as PaperProvider } from "react-native-paper"; import { ThirdwebProvider } from "thirdweb/react"; import "./utils/splash/splash"; -import { XmtpCron } from "./components/XmtpEngine"; +import { xmtpCron, xmtpEngine } from "./components/XmtpEngine"; import config from "./config"; import { useAppStore } from "./data/store/appStore"; import { useSelect } from "./data/store/storeHelpers"; @@ -59,6 +59,9 @@ initSentry(); const coinbaseUrl = new URL(`https://${config.websiteDomain}/coinbase`); +xmtpEngine.start(); +xmtpCron.start(); + const App = () => { const styles = useStyles(); const debugRef = useRef(); @@ -99,7 +102,6 @@ const App = () => { return ( -
diff --git a/App.web.tsx b/App.web.tsx index 85f115c21..abddf6a33 100644 --- a/App.web.tsx +++ b/App.web.tsx @@ -9,7 +9,7 @@ import { SafeAreaProvider } from "react-native-safe-area-context"; import "./assets/web.css"; import "./polyfills"; -import { XmtpCron } from "./components/XmtpEngine"; +import { xmtpCron, xmtpEngine } from "./components/XmtpEngine"; import config from "./config"; import Main from "./screens/Main"; @@ -29,6 +29,9 @@ createWeb3Modal({ projectId: config.walletConnectConfig.projectId, }); +xmtpEngine.start(); +xmtpCron.start(); + export default function App() { const colorScheme = useColorScheme(); @@ -58,7 +61,6 @@ export default function App() { > <>
- diff --git a/components/XmtpEngine.tsx b/components/XmtpEngine.tsx index 8a0067694..568c6d83e 100644 --- a/components/XmtpEngine.tsx +++ b/components/XmtpEngine.tsx @@ -5,7 +5,6 @@ import { import { appStateIsBlurredState } from "@utils/appState/appStateIsBlurred"; import logger from "@utils/logger"; import { stopStreamingAllMessage } from "@utils/xmtpRN/messages"; -import { useCallback, useEffect, useRef } from "react"; import { AppState, AppStateStatus, @@ -27,16 +26,22 @@ import { sendPendingMessages } from "../utils/xmtpRN/send"; import { syncXmtpClient } from "../utils/xmtpRN/sync"; class XmtpEngine { - appStoreSubscription: () => void; - appStateSubscription: NativeEventSubscription; - isInternetReachable: boolean; - hydrationDone: boolean; - syncedAccounts: { [account: string]: boolean }; - syncingAccounts: { [account: string]: boolean }; - appState: AppStateStatus; - - constructor() { - logger.debug("[XmtpEngine] Initializing"); + appStoreSubscription: (() => void) | null = null; + appStateSubscription: NativeEventSubscription | null = null; + isInternetReachable: boolean = false; + hydrationDone: boolean = false; + syncedAccounts: { [account: string]: boolean } = {}; + syncingAccounts: { [account: string]: boolean } = {}; + appState: AppStateStatus = AppState.currentState; + started: boolean = false; + + start() { + logger.debug("[XmtpEngine] Starting"); + if (this.started) { + return; + } + + this.started = true; this.syncedAccounts = {}; this.syncingAccounts = {}; @@ -138,28 +143,27 @@ class XmtpEngine { } destroy() { - // Normal app usage won't call this, but hot reloading will logger.debug("[XmtpEngine] Removing subscriptions"); - this.appStoreSubscription(); - this.appStateSubscription.remove(); + this.appStoreSubscription?.(); + this.appStateSubscription?.remove(); } } export const xmtpEngine = new XmtpEngine(); -export function XmtpCron() { - // Cron - const lastCronTimestamp = useRef(0); - const runningCron = useRef(false); +class XmtpCron { + private lastCronTimestamp: number = 0; + private runningCron: boolean = false; + private interval: NodeJS.Timeout | null = null; - const xmtpCron = useCallback(async () => { + private async xmtpCron() { if ( !useAppStore.getState().splashScreenHidden || AppState.currentState.match(/inactive|background/) ) { return; } - runningCron.current = true; + this.runningCron = true; const accounts = getAccountsList(); for (const account of accounts) { if ( @@ -174,22 +178,25 @@ export function XmtpCron() { } } } - lastCronTimestamp.current = new Date().getTime(); - runningCron.current = false; - }, []); - - useEffect(() => { - // Launch cron - const interval = setInterval(() => { - if (runningCron.current) return; + this.lastCronTimestamp = new Date().getTime(); + this.runningCron = false; + } + + start() { + logger.debug("[XmtpCron] Starting"); + this.interval = setInterval(() => { + if (this.runningCron) return; const now = new Date().getTime(); - if (now - lastCronTimestamp.current > 1000) { - xmtpCron(); + if (now - this.lastCronTimestamp > 1000) { + this.xmtpCron(); } }, 300); + } - return () => clearInterval(interval); - }, [xmtpCron]); - - return null; + destroy() { + logger.debug("[XmtpCron] Destroying"); + this.interval && clearInterval(this.interval); + } } + +export const xmtpCron = new XmtpCron();