-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Investigate infinite credentials Onyx loading state when migrating SignInPage to useOnyx #48725
Comments
Update: Still investigating it, it seems a problem around the cache / Onyx.clear() |
Update: I got back to this issue today and I was able to replicate through an new unit test, will investigate and work in the solution now. |
Update: Still looking for a solution to solve the racing condition around the connection / cache / Onyx.clear() |
Update: Working on and testing a possible solution I found today. |
I found another example for this when working on #49103, here is a diff for you to test it: diff --git a/src/libs/Navigation/AppNavigator/AuthScreens.tsx b/src/libs/Navigation/AppNavigator/AuthScreens.tsx
index ccc8290a93c..a4d78926558 100644
--- a/src/libs/Navigation/AppNavigator/AuthScreens.tsx
+++ b/src/libs/Navigation/AppNavigator/AuthScreens.tsx
@@ -1,10 +1,11 @@
import React, {memo, useEffect, useMemo, useRef, useState} from 'react';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
-import Onyx, {withOnyx} from 'react-native-onyx';
+import Onyx, {useOnyx, withOnyx} from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import ActiveGuidesEventListener from '@components/ActiveGuidesEventListener';
import ComposeProviders from '@components/ComposeProviders';
+import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import OptionsListContextProvider from '@components/OptionListContextProvider';
import {SearchContextProvider} from '@components/Search/SearchContext';
import SearchRouter from '@components/Search/SearchRouter/SearchRouter';
@@ -51,6 +52,7 @@ import SCREENS from '@src/SCREENS';
import type * as OnyxTypes from '@src/types/onyx';
import type {SelectedTimezone, Timezone} from '@src/types/onyx/PersonalDetails';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
+import isLoadingOnyxValue from '@src/types/utils/isLoadingOnyxValue';
import type ReactComponentModule from '@src/types/utils/ReactComponentModule';
import beforeRemoveReportOpenedFromSearchRHP from './beforeRemoveReportOpenedFromSearchRHP';
import CENTRAL_PANE_SCREENS from './CENTRAL_PANE_SCREENS';
@@ -588,14 +590,27 @@ AuthScreens.displayName = 'AuthScreens';
const AuthScreensMemoized = memo(AuthScreens, () => true);
-export default withOnyx<AuthScreensProps, AuthScreensProps>({
- session: {
- key: ONYXKEYS.SESSION,
- },
- lastOpenedPublicRoomID: {
- key: ONYXKEYS.LAST_OPENED_PUBLIC_ROOM_ID,
- },
- initialLastUpdateIDAppliedToClient: {
- key: ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT,
- },
-})(AuthScreensMemoized);
+export default function AuthScreensWithOnyx() {
+ const [session, sessionStatus] = useOnyx(ONYXKEYS.SESSION);
+ const [lastOpenedPublicRoomID, lastOpenedPublicRoomIDStatus] = useOnyx(ONYXKEYS.LAST_OPENED_PUBLIC_ROOM_ID);
+ const [initialLastUpdateIDAppliedToClient, initialLastUpdateIDAppliedToClientStatus] = useOnyx(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT);
+
+ console.log(
+ `sessionStatus, lastOpenedPublicRoomIDStatus, initialLastUpdateIDAppliedToClientStatus = `,
+ sessionStatus,
+ lastOpenedPublicRoomIDStatus,
+ initialLastUpdateIDAppliedToClientStatus,
+ );
+
+ if (isLoadingOnyxValue(sessionStatus, lastOpenedPublicRoomIDStatus, initialLastUpdateIDAppliedToClientStatus)) {
+ return <FullScreenLoadingIndicator />;
+ }
+
+ return (
+ <AuthScreensMemoized
+ session={session}
+ lastOpenedPublicRoomID={lastOpenedPublicRoomID}
+ initialLastUpdateIDAppliedToClient={initialLastUpdateIDAppliedToClient}
+ />
+ );
+} The outcome is that there is an indefinite loading because one of the keys is in constant loading state: Screen.Recording.2024-10-01.at.09.17.33.mov |
Thanks for this report @blazejkustra , I will look into it tomorrow because I'm not sure if my fix will cover this and maybe the problem is a broader one than expected. |
@fabioh8010 Any updates? |
Hi @blazejkustra, sorry for the late here. After cracking my head over these issues for some time, here's my findings: Case 1ProblemRacing conditions between an This happens because we clear the cache during From what I've seen this problem doesn't happen for SolutionNo definitive solutions yet. I first tried to use Case 2ProblemThe cache clearing performed by Specifically, if we are reusing the connection of a key that is still exists in the connection manager after an When logging out a lot of Onyx connections will be removed from the connection manager as expected, but some of them persists because they don't belong to the React's lifecycle e.g. any This problem doesn't happen for SolutionI first tried to "signal" the connection manager after My second and current approach is to introduce the concept of "Session ID" to the connection manager. This Session ID will be used when generating the Connection IDs with Not sure if it's the best approach but it solves this case for now. Draft PR of the solution: Expensify/react-native-onyx#588 I will continue exploring for other solutions and also explore if we can solve this at the cache's side. I will reach out Margelo's team eventually too as they have more context about the cache system and can suggest better solutions. |
When migrating SignInPage.tsx to
useOnyx
for testing purposes, @blazejkustra noticed that the screen becomes blank when logging out the user. Refreshing the page solves the problem.It seems that the
credentials
data returned byuseOnyx
is always onloading
state, so if we put theisLoadingOnyxValue
condition to only render the page when all Onyx data is available the page will never render.Screen.Recording.2024-09-06.at.13.42.35-compressed.mov
We must investigate why this is happening as it could be a bug in Onyx internals.
The text was updated successfully, but these errors were encountered: