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,7 +1,6 @@
import lodashGet from 'lodash/get';
import * as OnfidoSDK from 'onfido-sdk-ui';
import React, {forwardRef, useEffect} from 'react';
import _ from 'underscore';
import React, {ForwardedRef, forwardRef, useEffect} from 'react';
import type {LocaleContextProps} from '@components/LocaleContextProvider';
import useLocalize from '@hooks/useLocalize';
import Log from '@libs/Log';
import fontFamily from '@styles/fontFamily';
Expand All @@ -10,9 +9,15 @@ import themeColors from '@styles/themes/default';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import './index.css';
import onfidoPropTypes from './onfidoPropTypes';
import type {OnfidoElement, OnfidoProps} from './types';

function initializeOnfido({sdkToken, onSuccess, onError, onUserExit, preferredLocale, translate}) {
type LocaleProps = Pick<LocaleContextProps, 'translate' | 'preferredLocale'>;

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

function initializeOnfido({sdkToken, onSuccess, onError, onUserExit, preferredLocale, translate}: OnfidoProps & LocaleProps) {
OnfidoSDK.init({
token: sdkToken,
containerId: CONST.ONFIDO.CONTAINER_ID,
Expand All @@ -22,7 +27,7 @@ function initializeOnfido({sdkToken, onSuccess, onError, onUserExit, preferredLo
fontFamilySubtitle: `${fontFamily.EXP_NEUE}, -apple-system, serif`,
fontFamilyBody: `${fontFamily.EXP_NEUE}, -apple-system, serif`,
fontSizeTitle: `${variables.fontSizeLarge}px`,
fontWeightTitle: fontWeightBold,
fontWeightTitle: Number(fontWeightBold),
fontWeightSubtitle: 400,
fontSizeSubtitle: `${variables.fontSizeNormal}px`,
colorContentTitle: themeColors.text,
Expand All @@ -47,7 +52,6 @@ function initializeOnfido({sdkToken, onSuccess, onError, onUserExit, preferredLo
colorBorderLinkUnderline: themeColors.link,
colorBackgroundLinkHover: themeColors.link,
colorBackgroundLinkActive: themeColors.link,
authAccentColor: themeColors.link,
colorBackgroundInfoPill: themeColors.link,
VickyStash marked this conversation as resolved.
Show resolved Hide resolved
colorBackgroundSelector: themeColors.appBG,
colorBackgroundDocTypeButton: themeColors.success,
Expand All @@ -59,11 +63,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 @@ -78,17 +81,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 (!Object.keys(data).length) {
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 @@ -101,32 +102,33 @@ 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();

useEffect(() => {
initializeOnfido({
sdkToken: props.sdkToken,
onSuccess: props.onSuccess,
onError: props.onError,
onUserExit: props.onUserExit,
sdkToken,
onSuccess,
onError,
onUserExit,
preferredLocale,
translate,
});
Expand All @@ -143,8 +145,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,15 +1,13 @@
import {OnfidoCaptureType, OnfidoCountryCode, OnfidoDocumentType, Onfido as OnfidoSDK} from '@onfido/react-native-sdk';
import lodashGet from 'lodash/get';
import React, {useEffect} from 'react';
import {Alert, Linking} from 'react-native';
import _ from 'underscore';
import FullscreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
import useLocalize from '@hooks/useLocalize';
import Log from '@libs/Log';
import CONST from '@src/CONST';
import onfidoPropTypes from './onfidoPropTypes';
import type {OnfidoProps} from './types';

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

useEffect(() => {
Expand All @@ -28,19 +26,20 @@ 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;
}

// Handle user camera permission on iOS and Android
if (_.contains([CONST.ONFIDO.ERROR.USER_CAMERA_PERMISSION, CONST.ONFIDO.ERROR.USER_CAMERA_DENINED, CONST.ONFIDO.ERROR.USER_CAMERA_CONSENT_DENIED], errorMessage)) {
if ([CONST.ONFIDO.ERROR.USER_CAMERA_PERMISSION, CONST.ONFIDO.ERROR.USER_CAMERA_DENINED, CONST.ONFIDO.ERROR.USER_CAMERA_CONSENT_DENIED].includes(errorMessage)) {
Alert.alert(
translate('onfidoStep.cameraPermissionsNotGranted'),
translate('onfidoStep.cameraRequestMessage'),
Expand Down Expand Up @@ -71,7 +70,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.

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

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: OnfidoSDK.SdkResponse | OnfidoResult | OnfidoError) => void;

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

export type {OnfidoProps, OnfidoElement};
Loading