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

[TS migration] Migrate 'Onfido' component to TypeScript #31378

Merged
merged 10 commits into from
Mar 15, 2024
Original file line number Diff line number Diff line change
@@ -1,17 +1,28 @@
import lodashGet from 'lodash/get';
import * as OnfidoSDK from 'onfido-sdk-ui';
import React, {forwardRef, useEffect} from 'react';
import _ from 'underscore';
import type {ForwardedRef} from 'react';
import type {LocaleContextProps} from '@components/LocaleContextProvider';
import useLocalize from '@hooks/useLocalize';
import useTheme from '@hooks/useTheme';
import Log from '@libs/Log';
import type {ThemeColors} from '@styles/theme/types';
import FontUtils from '@styles/utils/FontUtils';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import './index.css';
import onfidoPropTypes from './onfidoPropTypes';
import type {OnfidoElement, OnfidoProps} from './types';

function initializeOnfido({sdkToken, onSuccess, onError, onUserExit, preferredLocale, translate, theme}) {
type InitializeOnfidoProps = OnfidoProps &
Pick<LocaleContextProps, 'translate' | 'preferredLocale'> & {
theme: ThemeColors;
};

type OnfidoEvent = Event & {
detail?: Record<string, unknown>;
};

function initializeOnfido({sdkToken, onSuccess, onError, onUserExit, preferredLocale, translate, theme}: InitializeOnfidoProps) {
OnfidoSDK.init({
token: sdkToken,
containerId: CONST.ONFIDO.CONTAINER_ID,
Expand All @@ -21,7 +32,7 @@ function initializeOnfido({sdkToken, onSuccess, onError, onUserExit, preferredLo
fontFamilySubtitle: `${FontUtils.fontFamily.platform.EXP_NEUE}, -apple-system, serif`,
fontFamilyBody: `${FontUtils.fontFamily.platform.EXP_NEUE}, -apple-system, serif`,
fontSizeTitle: `${variables.fontSizeLarge}px`,
fontWeightTitle: FontUtils.fontWeight.bold,
fontWeightTitle: Number(FontUtils.fontWeight.bold),
fontWeightSubtitle: 400,
fontSizeSubtitle: `${variables.fontSizeNormal}px`,
colorContentTitle: theme.text,
Expand All @@ -46,7 +57,6 @@ function initializeOnfido({sdkToken, onSuccess, onError, onUserExit, preferredLo
colorBorderLinkUnderline: theme.link,
colorBackgroundLinkHover: theme.link,
colorBackgroundLinkActive: theme.link,
authAccentColor: theme.link,
colorBackgroundInfoPill: theme.link,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This param wasn't found by TS, I haven't found it in lib/docs as well.
Onfido SDK source code is not is not publicly available to check this param there, but I can see the last version this params was mentioned in Onfido docs is 10.1.0, after that it wasn't there anymore.

colorBackgroundSelector: theme.appBG,
colorBackgroundDocTypeButton: theme.success,
Expand All @@ -58,11 +68,10 @@ function initializeOnfido({sdkToken, onSuccess, onError, onUserExit, preferredLo
{
type: CONST.ONFIDO.TYPE.DOCUMENT,
options: {
useLiveDocumentCapture: true,
forceCrossDevice: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was removed from lib some time ago https://documentation.onfido.com/sdk/web/#no--1100

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, but we should make sure isn't being used in Onfido SDK, because it can be an issue with TS typings but not the code itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

From what I understood it was deprecated in 10.1.10, but we are at 8.3.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fabioh8010 8.3.0. is a version of @onfido/react-native-sdk which is used by mobile platforms.
The version of onfido-sdk-ui used by E/App for web is 13.1.0, so it's deprecated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for explanation @VickyStash !

hideCountrySelection: true,
country: 'USA',
documentTypes: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This param wasn't found by TS

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, but we should make sure isn't being used in Onfido SDK, because it can be an issue with TS typings but not the code itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since I don't have access to the source code it's hard to say if it's used. But based on the docs you can pass country to specific document, which is done here.

// eslint-disable-next-line @typescript-eslint/naming-convention
driving_licence: {
country: 'USA',
},
Expand All @@ -77,17 +86,15 @@ function initializeOnfido({sdkToken, onSuccess, onError, onUserExit, preferredLo
},
},
],
smsNumberCountryCode: CONST.ONFIDO.SMS_NUMBER_COUNTRY_CODE.US,
showCountrySelection: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have CONST.ONFIDO.SMS_NUMBER_COUNTRY_CODE.US, we have only CONST.ONFIDO.SMS_NUMBER_COUNTRY_CODE, so all this time the undefined was used

Copy link
Contributor

Choose a reason for hiding this comment

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

and what about showCountrySelection?

Copy link
Contributor Author

@VickyStash VickyStash Nov 16, 2023

Choose a reason for hiding this comment

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

showCountrySelection was removed between 8.1.1 and 8.2.0 web sdk versions
image

Copy link
Contributor

Choose a reason for hiding this comment

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

ohh right, thanks!

onComplete: (data) => {
VickyStash marked this conversation as resolved.
Show resolved Hide resolved
if (_.isEmpty(data)) {
if (isEmptyObject(data)) {
Log.warn('Onfido completed with no data');
}
onSuccess(data);
},
onError: (error) => {
const errorMessage = lodashGet(error, 'message', CONST.ERROR.UNKNOWN_ERROR);
const errorType = lodashGet(error, 'type');
const errorMessage = error.message ?? CONST.ERROR.UNKNOWN_ERROR;
const errorType = error.type;
Log.hmmm('Onfido error', {errorType, errorMessage});
onError(errorMessage);
},
Expand All @@ -100,33 +107,34 @@ function initializeOnfido({sdkToken, onSuccess, onError, onUserExit, preferredLo
},
language: {
// We need to use ES_ES as locale key because the key `ES` is not a valid config key for Onfido
locale: preferredLocale === CONST.LOCALES.ES ? CONST.LOCALES.ES_ES_ONFIDO : preferredLocale,
locale: preferredLocale === CONST.LOCALES.ES ? CONST.LOCALES.ES_ES_ONFIDO : (preferredLocale as OnfidoSDK.SupportedLanguages),

// Provide a custom phrase for the back button so that the first letter is capitalized,
// and translate the phrase while we're at it. See the issue and documentation for more context.
// https://github.com/Expensify/App/issues/17244
// https://documentation.onfido.com/sdk/web/#custom-languages
phrases: {
// eslint-disable-next-line @typescript-eslint/naming-convention
'generic.back': translate('common.back'),
},
},
});
}

function logOnFidoEvent(event) {
function logOnFidoEvent(event: OnfidoEvent) {
Log.hmmm('Receiving Onfido analytic event', event.detail);
}

const Onfido = forwardRef((props, ref) => {
function Onfido({sdkToken, onSuccess, onError, onUserExit}: OnfidoProps, ref: ForwardedRef<OnfidoElement>) {
const {preferredLocale, translate} = useLocalize();
const theme = useTheme();

useEffect(() => {
initializeOnfido({
sdkToken: props.sdkToken,
onSuccess: props.onSuccess,
onError: props.onError,
onUserExit: props.onUserExit,
sdkToken,
onSuccess,
onError,
onUserExit,
preferredLocale,
translate,
theme,
Expand All @@ -144,8 +152,8 @@ const Onfido = forwardRef((props, ref) => {
ref={ref}
/>
);
});
}

Onfido.displayName = 'Onfido';
Onfido.propTypes = onfidoPropTypes;
export default Onfido;

export default forwardRef(Onfido);
11 changes: 0 additions & 11 deletions src/components/Onfido/index.desktop.js
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this file isn't necessary anymore, the issue it mentioned is resolved.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's test it to ensure no regressions are created!

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
import {OnfidoCaptureType, OnfidoCountryCode, OnfidoDocumentType, Onfido as OnfidoSDK} from '@onfido/react-native-sdk';
import lodashGet from 'lodash/get';
import {OnfidoCaptureType, OnfidoCountryCode, OnfidoDocumentType, Onfido as OnfidoSDK, OnfidoTheme} from '@onfido/react-native-sdk';
import React, {useEffect} from 'react';
import {Alert, Linking} from 'react-native';
import {checkMultiple, PERMISSIONS, RESULTS} from 'react-native-permissions';
import _ from 'underscore';
import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import useLocalize from '@hooks/useLocalize';
import getPlatform from '@libs/getPlatform';
import Log from '@libs/Log';
import CONST from '@src/CONST';
import onfidoPropTypes from './onfidoPropTypes';
import type {TranslationPaths} from '@src/languages/types';
import type {OnfidoProps} from './types';

function Onfido({sdkToken, onUserExit, onSuccess, onError}) {
function Onfido({sdkToken, onUserExit, onSuccess, onError}: OnfidoProps) {
const {translate} = useLocalize();

useEffect(() => {
OnfidoSDK.start({
sdkToken,
theme: OnfidoTheme.AUTOMATIC,
flowSteps: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

theme is a required prop in OnfidoSDK.start config. I've checked the docs and OnfidoTheme.AUTOMATIC is a default value, so I've added theme prop with a default value.

welcome: true,
captureFace: {
Expand All @@ -30,24 +30,25 @@ function Onfido({sdkToken, onUserExit, onSuccess, onError}) {
})
.then(onSuccess)
.catch((error) => {
const errorMessage = lodashGet(error, 'message', CONST.ERROR.UNKNOWN_ERROR);
const errorType = lodashGet(error, 'type');
const errorMessage = error.message ?? CONST.ERROR.UNKNOWN_ERROR;
const errorType = error.type;

Log.hmmm('Onfido error on native', {errorType, errorMessage});

// If the user cancels the Onfido flow we won't log this error as it's normal. In the React Native SDK the user exiting the flow will trigger this error which we can use as
// our "user exited the flow" callback. On web, this event has it's own callback passed as a config so we don't need to bother with this there.
if (_.contains([CONST.ONFIDO.ERROR.USER_CANCELLED, CONST.ONFIDO.ERROR.USER_TAPPED_BACK, CONST.ONFIDO.ERROR.USER_EXITED], errorMessage)) {
if ([CONST.ONFIDO.ERROR.USER_CANCELLED, CONST.ONFIDO.ERROR.USER_TAPPED_BACK, CONST.ONFIDO.ERROR.USER_EXITED].includes(errorMessage)) {
onUserExit();
return;
}

if (!_.isEmpty(errorMessage) && getPlatform() === CONST.PLATFORM.IOS) {
if (!!errorMessage && getPlatform() === CONST.PLATFORM.IOS) {
checkMultiple([PERMISSIONS.IOS.MICROPHONE, PERMISSIONS.IOS.CAMERA])
.then((statuses) => {
const isMicAllowed = statuses[PERMISSIONS.IOS.MICROPHONE] === RESULTS.GRANTED;
const isCameraAllowed = statuses[PERMISSIONS.IOS.CAMERA] === RESULTS.GRANTED;
let alertTitle = '';
let alertMessage = '';
let alertTitle: TranslationPaths | '' = '';
let alertMessage: TranslationPaths | '' = '';
if (!isCameraAllowed) {
alertTitle = 'onfidoStep.cameraPermissionsNotGranted';
alertMessage = 'onfidoStep.cameraRequestMessage';
Expand All @@ -56,7 +57,7 @@ function Onfido({sdkToken, onUserExit, onSuccess, onError}) {
alertMessage = 'onfidoStep.microphoneRequestMessage';
}

if (!_.isEmpty(alertTitle) && !_.isEmpty(alertMessage)) {
if (!!alertTitle && !!alertMessage) {
Alert.alert(
translate(alertTitle),
translate(alertMessage),
Expand Down Expand Up @@ -93,7 +94,6 @@ function Onfido({sdkToken, onUserExit, onSuccess, onError}) {
return <FullscreenLoadingIndicator />;
}

Onfido.propTypes = onfidoPropTypes;
Onfido.displayName = 'Onfido';

export default Onfido;
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import lodashGet from 'lodash/get';
import React, {useEffect, useRef} from 'react';
import BaseOnfidoWeb from './BaseOnfidoWeb';
import onfidoPropTypes from './onfidoPropTypes';
import type {OnfidoElement, OnfidoProps} from './types';

function Onfido({sdkToken, onSuccess, onError, onUserExit}) {
const baseOnfidoRef = useRef(null);
function Onfido({sdkToken, onSuccess, onError, onUserExit}: OnfidoProps) {
const baseOnfidoRef = useRef<OnfidoElement>(null);

useEffect(
() => () => {
const onfidoOut = lodashGet(baseOnfidoRef.current, 'onfidoOut');
const onfidoOut = baseOnfidoRef.current?.onfidoOut;

if (!onfidoOut) {
return;
}
Expand All @@ -29,7 +29,6 @@ function Onfido({sdkToken, onSuccess, onError, onUserExit}) {
);
}

Onfido.propTypes = onfidoPropTypes;
Onfido.displayName = 'Onfido';

export default Onfido;
15 changes: 0 additions & 15 deletions src/components/Onfido/onfidoPropTypes.js

This file was deleted.

27 changes: 27 additions & 0 deletions src/components/Onfido/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import type {OnfidoResult} from '@onfido/react-native-sdk';
import type * as OnfidoSDK from 'onfido-sdk-ui';
import type {OnyxEntry} from 'react-native-onyx';

type OnfidoData = OnfidoSDK.SdkResponse | OnfidoResult;

type OnfidoDataWithApplicantID = OnfidoData & {
applicantID: OnyxEntry<string>;
};

type OnfidoElement = HTMLDivElement & {onfidoOut?: OnfidoSDK.SdkHandle};

type OnfidoProps = {
/** Token used to initialize the Onfido SDK */
sdkToken: string;

/** Called when the user intentionally exits the flow without completing it */
onUserExit: (userExitCode?: OnfidoSDK.UserExitCode) => void;

/** Called when the user is totally done with Onfido */
onSuccess: (data: OnfidoData) => void;

/** Called when Onfido throws an error */
onError: (error?: string) => void;
};

export type {OnfidoProps, OnfidoElement, OnfidoData, OnfidoDataWithApplicantID};
3 changes: 2 additions & 1 deletion src/libs/actions/BankAccounts.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Onyx from 'react-native-onyx';
import type {OnfidoDataWithApplicantID} from '@components/Onfido/types';
import * as API from '@libs/API';
import type {
AddPersonalBankAccountParams,
Expand Down Expand Up @@ -436,7 +437,7 @@ function connectBankAccountManually(bankAccountID: number, bankAccount: PlaidBan
/**
* Verify the user's identity via Onfido
*/
function verifyIdentityForBankAccount(bankAccountID: number, onfidoData: Record<string, unknown>, policyID: string) {
function verifyIdentityForBankAccount(bankAccountID: number, onfidoData: OnfidoDataWithApplicantID, policyID: string) {
const parameters: VerifyIdentityForBankAccountParams = {
bankAccountID,
onfidoData: JSON.stringify(onfidoData),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import {withOnyx} from 'react-native-onyx';
import FullPageOfflineBlockingView from '@components/BlockingViews/FullPageOfflineBlockingView';
import HeaderWithBackButton from '@components/HeaderWithBackButton';
import InteractiveStepSubHeader from '@components/InteractiveStepSubHeader';
// @ts-expect-error TODO: Remove this once Onfido (https://github.com/Expensify/App/issues/25136) is migrated to TypeScript.
import Onfido from '@components/Onfido';
import type {OnfidoData} from '@components/Onfido/types';
import ScreenWrapper from '@components/ScreenWrapper';
import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
Expand Down Expand Up @@ -40,7 +40,7 @@ function VerifyIdentity({reimbursementAccount, onBackButtonPress, onfidoApplican

const policyID = reimbursementAccount?.achData?.policyID ?? '';
const handleOnfidoSuccess = useCallback(
(onfidoData: Record<string, unknown>) => {
(onfidoData: OnfidoData) => {
BankAccounts.verifyIdentityForBankAccount(Number(reimbursementAccount?.achData?.bankAccountID ?? '0'), {...onfidoData, applicantID: onfidoApplicantID}, policyID);
BankAccounts.updateReimbursementAccountDraft({isOnfidoSetupComplete: true});
},
Expand Down Expand Up @@ -74,7 +74,7 @@ function VerifyIdentity({reimbursementAccount, onBackButtonPress, onfidoApplican
<FullPageOfflineBlockingView>
<ScrollView contentContainerStyle={styles.flex1}>
<Onfido
sdkToken={onfidoToken}
sdkToken={onfidoToken ?? ''}
onUserExit={handleOnfidoUserExit}
onError={handleOnfidoError}
onSuccess={handleOnfidoSuccess}
Expand Down
Loading