Skip to content
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

[No QA] [TS migration] Migrate 'SignInRedirect.js' lib to TypeScript #28388

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import Onyx from 'react-native-onyx';
import lodashGet from 'lodash/get';
import _ from 'underscore';
import ONYXKEYS from '../../ONYXKEYS';
import ONYXKEYS, {OnyxKey} from '../../ONYXKEYS';
import * as MainQueue from '../Network/MainQueue';
import * as PersistedRequests from './PersistedRequests';
import NetworkConnection from '../NetworkConnection';
Expand All @@ -12,27 +10,21 @@ import Navigation from '../Navigation/Navigation';
import * as ErrorUtils from '../ErrorUtils';
import * as SessionUtils from '../SessionUtils';

let currentIsOffline;
let currentShouldForceOffline;
let currentIsOffline: boolean | undefined;
let currentShouldForceOffline: boolean | undefined;
Julesssss marked this conversation as resolved.
Show resolved Hide resolved
Onyx.connect({
key: ONYXKEYS.NETWORK,
callback: (network) => {
Comment on lines 15 to 17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the callback we are converting network.shouldForceOffline to Boolean. I don't think that's necessary. There is also an early return. Can we use optional chaining instead?

        currentIsOffline = network?.isOffline;
        currentShouldForceOffline = network?.shouldForceOffline;

if (!network) {
return;
}
currentIsOffline = network.isOffline;
currentShouldForceOffline = Boolean(network.shouldForceOffline);
currentIsOffline = network?.isOffline;
currentShouldForceOffline = network?.shouldForceOffline;
},
});

/**
* @param {String} errorMessage
*/
function clearStorageAndRedirect(errorMessage) {
function clearStorageAndRedirect(errorMessage?: string) {
// Under certain conditions, there are key-values we'd like to keep in storage even when a user is logged out.
// We pass these into the clear() method in order to avoid having to reset them on a delayed tick and getting
// flashes of unwanted default state.
const keysToPreserve = [];
const keysToPreserve: OnyxKey[] = [];
keysToPreserve.push(ONYXKEYS.NVP_PREFERRED_LOCALE);
keysToPreserve.push(ONYXKEYS.ACTIVE_CLIENTS);
keysToPreserve.push(ONYXKEYS.DEVICE_ID);
Expand All @@ -58,15 +50,15 @@ function clearStorageAndRedirect(errorMessage) {
*/
function resetHomeRouteParams() {
Navigation.isNavigationReady().then(() => {
const routes = navigationRef.current && lodashGet(navigationRef.current.getState(), 'routes');
const homeRoute = _.find(routes, (route) => route.name === SCREENS.HOME);
const routes = navigationRef.current?.getState().routes;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coming from #45619, we should use optional chaining operator here to avoid accessing routes when nav state is null

const homeRoute = routes?.find((route) => route.name === SCREENS.HOME);

const emptyParams = {};
_.keys(lodashGet(homeRoute, 'params')).forEach((paramKey) => {
const emptyParams: Record<string, undefined> = {};
Object.keys(homeRoute?.params ?? {}).forEach((paramKey) => {
emptyParams[paramKey] = undefined;
});

Navigation.setParams(emptyParams, lodashGet(homeRoute, 'key', ''));
Navigation.setParams(emptyParams, homeRoute?.key ?? '');
Onyx.set(ONYXKEYS.IS_CHECKING_PUBLIC_ROOM, false);
});
}
Expand All @@ -79,9 +71,9 @@ function resetHomeRouteParams() {
*
* Normally this method would live in Session.js, but that would cause a circular dependency with Network.js.
*
* @param {String} [errorMessage] error message to be displayed on the sign in page
* @param [errorMessage] error message to be displayed on the sign in page
*/
function redirectToSignIn(errorMessage) {
function redirectToSignIn(errorMessage?: string) {
Julesssss marked this conversation as resolved.
Show resolved Hide resolved
MainQueue.clear();
HttpUtils.cancelPendingRequests();
PersistedRequests.clear();
Expand Down
4 changes: 4 additions & 0 deletions src/types/onyx/Session.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import * as OnyxCommon from './OnyxCommon';

type Session = {
/** The user's email for the current session */
email?: string;
Expand All @@ -12,6 +14,8 @@ type Session = {
accountID?: number;

autoAuthState?: string;
/** Server side errors keyed by microtime */
errors?: OnyxCommon.Errors;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can only import Errors from this common namespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everywhere we are are importing it as OnyxCommon.Errors so I would follow this pattern 😄

};

export default Session;
Loading