From fadfc2cdac32db90e8a5460bc86493ed537f571a Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Tue, 10 Sep 2024 14:52:11 +0200 Subject: [PATCH 1/4] fix: synchronous navigator mount --- package-lock.json | 10 +++++++ package.json | 1 + src/libs/Navigation/AppNavigator/index.tsx | 32 ++++++++++++++++------ 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index de12a7d768a9..b1e4c14f6f4f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -76,6 +76,7 @@ "react-native": "0.75.2", "react-native-android-location-enabler": "^2.0.1", "react-native-blob-util": "0.19.4", + "react-native-bundle-splitter": "^3.0.1", "react-native-collapsible": "^1.6.2", "react-native-config": "1.5.0", "react-native-dev-menu": "^4.1.1", @@ -34411,6 +34412,15 @@ "url": "https://github.com/sponsors/isaacs" } }, + "node_modules/react-native-bundle-splitter": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/react-native-bundle-splitter/-/react-native-bundle-splitter-3.0.1.tgz", + "integrity": "sha512-YvG30oL+3uhPoYisRzMJLHjs5+X7NK8yLHqLKxLIvqZ9wGAvIJ+sJG09iQpLK+UCjEAEVP5he0F0vnzuokHyBw==", + "peerDependencies": { + "react": "*", + "react-native": ">=0.59.1" + } + }, "node_modules/react-native-clean-project": { "version": "4.0.1", "dev": true, diff --git a/package.json b/package.json index 4c1bf98cc976..c1c2d053be02 100644 --- a/package.json +++ b/package.json @@ -133,6 +133,7 @@ "react-native": "0.75.2", "react-native-android-location-enabler": "^2.0.1", "react-native-blob-util": "0.19.4", + "react-native-bundle-splitter": "^3.0.1", "react-native-collapsible": "^1.6.2", "react-native-config": "1.5.0", "react-native-dev-menu": "^4.1.1", diff --git a/src/libs/Navigation/AppNavigator/index.tsx b/src/libs/Navigation/AppNavigator/index.tsx index 05961528ca11..bdf411f98659 100644 --- a/src/libs/Navigation/AppNavigator/index.tsx +++ b/src/libs/Navigation/AppNavigator/index.tsx @@ -1,22 +1,38 @@ -import React, {lazy, memo, Suspense} from 'react'; +import React, {lazy, memo, Suspense, useEffect, useState} from 'react'; +import {preload, register} from 'react-native-bundle-splitter'; import lazyRetry from '@src/utils/lazyRetry'; -const AuthScreens = lazy(() => lazyRetry(() => import('./AuthScreens'))); +// const AuthScreens = lazy(() => lazyRetry(() => import('./AuthScreens'))); const PublicScreens = lazy(() => lazyRetry(() => import('./PublicScreens'))); +const AuthScreens = register({ + name: 'AuthScreens', + // TODO: add retry logic + loader: () => import('./AuthScreens'), +}); + type AppNavigatorProps = { /** If we have an authToken this is true */ authenticated: boolean; }; function AppNavigator({authenticated}: AppNavigatorProps) { - if (authenticated) { + const [canNavigateToProtectedRoutes, setNavigateToProtectedRoutes] = useState(false); + + useEffect(() => { + // Preload Auth Screens to be sure navigator can be mounted synchronously to avoid problems + // described in https://github.com/Expensify/App/issues/44600 + preload() + .component('AuthScreens') + .then(() => { + setNavigateToProtectedRoutes(true); + }); + }, []); + + if (authenticated && canNavigateToProtectedRoutes) { // These are the protected screens and only accessible when an authToken is present - return ( - - - - ); + // Navigate to them only when route is preloaded + return ; } return ( From 9bc4bc122730d6b14d21a10d74c80e59c44cb035 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Tue, 10 Sep 2024 16:50:52 +0200 Subject: [PATCH 2/4] fix: patch react-navigation to perform async cleanup as prior to StrictMode enabling --- ...per-navigator-unmount-in-strict-mode.patch | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 patches/@react-navigation+core+6.4.11+002+proper-navigator-unmount-in-strict-mode.patch diff --git a/patches/@react-navigation+core+6.4.11+002+proper-navigator-unmount-in-strict-mode.patch b/patches/@react-navigation+core+6.4.11+002+proper-navigator-unmount-in-strict-mode.patch new file mode 100644 index 000000000000..817fd1ea1c7f --- /dev/null +++ b/patches/@react-navigation+core+6.4.11+002+proper-navigator-unmount-in-strict-mode.patch @@ -0,0 +1,52 @@ +diff --git a/node_modules/@react-navigation/core/lib/module/useNavigationBuilder.js b/node_modules/@react-navigation/core/lib/module/useNavigationBuilder.js +index 6fb49e0..1f7c859 100644 +--- a/node_modules/@react-navigation/core/lib/module/useNavigationBuilder.js ++++ b/node_modules/@react-navigation/core/lib/module/useNavigationBuilder.js +@@ -114,6 +114,16 @@ const getRouteConfigsFromChildren = (children, groupKey, groupOptions) => { + return configs; + }; + ++const useFullyMountedRef = () => { ++ const isFullyMountedRef = React.useRef(false); ++ ++ React.useEffect(() => { ++ isFullyMountedRef.current = true; ++ }, []); ++ ++ return isFullyMountedRef; ++}; ++ + /** + * Hook for building navigators. + * +@@ -122,6 +132,7 @@ const getRouteConfigsFromChildren = (children, groupKey, groupOptions) => { + * @returns An object containing `state`, `navigation`, `descriptors` objects. + */ + export default function useNavigationBuilder(createRouter, options) { ++ const isFullyMountedRef = useFullyMountedRef(); + const navigatorKey = useRegisterNavigator(); + const route = React.useContext(NavigationRouteContext); + const { +@@ -298,10 +309,18 @@ export default function useNavigationBuilder(createRouter, options) { + setState(nextState); + } + return () => { +- // We need to clean up state for this navigator on unmount +- if (getCurrentState() !== undefined && getKey() === navigatorKey) { +- setCurrentState(undefined); +- stateCleanedUp.current = true; ++ const cleanup = () => { ++ // We need to clean up state for this navigator on unmount ++ if (getCurrentState() !== undefined && getKey() === navigatorKey) { ++ setCurrentState(undefined); ++ stateCleanedUp.current = true; ++ } ++ } ++ ++ if (!isFullyMountedRef.current) { ++ cleanup(); ++ } else { ++ setTimeout(cleanup, 0); + } + }; + // eslint-disable-next-line react-hooks/exhaustive-deps From fa3c56aaf68be7b82af3c95184b8dfdcf84efa01 Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Tue, 10 Sep 2024 16:52:25 +0200 Subject: [PATCH 3/4] refactor: simplify StrictMode wrapper --- src/App.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/App.tsx b/src/App.tsx index 177cc00c7dee..33b75ec74338 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -54,7 +54,7 @@ LogBox.ignoreLogs([ const fill = {flex: 1}; -const StrictModeWrapper = CONFIG.USE_REACT_STRICT_MODE_IN_DEV ? React.StrictMode : ({children}: {children: React.ReactElement}) => children; +const StrictModeWrapper = CONFIG.USE_REACT_STRICT_MODE_IN_DEV ? React.StrictMode : React.Fragment; function App({url}: AppProps) { useDefaultDragAndDrop(); From 1f2db183103235ac14e8b6a0d376d0f998b81ead Mon Sep 17 00:00:00 2001 From: kirillzyusko Date: Wed, 18 Sep 2024 17:20:49 +0200 Subject: [PATCH 4/4] fix: resolve own todos, code cleanup --- src/libs/Navigation/AppNavigator/index.tsx | 15 +++++++-------- src/utils/lazyRetry.ts | 11 +++++++++-- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/libs/Navigation/AppNavigator/index.tsx b/src/libs/Navigation/AppNavigator/index.tsx index bdf411f98659..ac70d7167246 100644 --- a/src/libs/Navigation/AppNavigator/index.tsx +++ b/src/libs/Navigation/AppNavigator/index.tsx @@ -1,14 +1,13 @@ import React, {lazy, memo, Suspense, useEffect, useState} from 'react'; import {preload, register} from 'react-native-bundle-splitter'; -import lazyRetry from '@src/utils/lazyRetry'; +import lazyRetry, {retryImport} from '@src/utils/lazyRetry'; -// const AuthScreens = lazy(() => lazyRetry(() => import('./AuthScreens'))); const PublicScreens = lazy(() => lazyRetry(() => import('./PublicScreens'))); +const AUTH_SCREENS = 'AuthScreens'; const AuthScreens = register({ - name: 'AuthScreens', - // TODO: add retry logic - loader: () => import('./AuthScreens'), + name: AUTH_SCREENS, + loader: () => retryImport(() => import('./AuthScreens')), }); type AppNavigatorProps = { @@ -20,10 +19,10 @@ function AppNavigator({authenticated}: AppNavigatorProps) { const [canNavigateToProtectedRoutes, setNavigateToProtectedRoutes] = useState(false); useEffect(() => { - // Preload Auth Screens to be sure navigator can be mounted synchronously to avoid problems - // described in https://github.com/Expensify/App/issues/44600 + // Preload Auth Screens in advance to be sure that navigator can be mounted synchronously + // to avoid problems described in https://github.com/Expensify/App/issues/44600 preload() - .component('AuthScreens') + .component(AUTH_SCREENS) .then(() => { setNavigateToProtectedRoutes(true); }); diff --git a/src/utils/lazyRetry.ts b/src/utils/lazyRetry.ts index 35c6b444e612..5bcf8d3d83d0 100644 --- a/src/utils/lazyRetry.ts +++ b/src/utils/lazyRetry.ts @@ -5,6 +5,8 @@ type Import = Promise<{default: T}>; type ComponentImport = () => Import; /** + * Common retry mechanism for importing components. + * * Attempts to lazily import a React component with a retry mechanism on failure. * If the initial import fails the function will refresh the page once and retry the import. * If the import fails again after the refresh, the error is propagated. @@ -12,8 +14,7 @@ type ComponentImport = () => Import; * @param componentImport - A function that returns a promise resolving to a lazily imported React component. * @returns A promise that resolves to the imported component or rejects with an error after a retry attempt. */ -// eslint-disable-next-line @typescript-eslint/no-explicit-any -const lazyRetry = function >(componentImport: ComponentImport): Import { +function retryImport(componentImport: ComponentImport): Import { return new Promise((resolve, reject) => { // Retrieve the retry status from sessionStorage, defaulting to 'false' if not set const hasRefreshed = JSON.parse(sessionStorage.getItem(CONST.SESSION_STORAGE_KEYS.RETRY_LAZY_REFRESHED) ?? 'false') as boolean; @@ -37,6 +38,12 @@ const lazyRetry = function >(componentImport: Compo } }); }); +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +const lazyRetry = function >(componentImport: ComponentImport): Import { + return retryImport(componentImport); }; export default lazyRetry; +export {retryImport};