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 useLocalize #29180

Merged
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
d26ce71
Migrate to TypeScript
MaciejSWM Oct 10, 2023
a8e3b97
Export Locale as a type and handle default values
MaciejSWM Oct 10, 2023
8367198
Better Onyx typing
MaciejSWM Oct 11, 2023
369edd8
Comments describing props
MaciejSWM Oct 17, 2023
8185ab7
Drop spaces at the end of the comment
MaciejSWM Oct 17, 2023
9db5a94
More strict string types
MaciejSWM Oct 19, 2023
60a8bb7
Merge branch 'main' into ts-migration/useLocalize
MaciejSWM Oct 19, 2023
5d6eed5
More strict locale and timezone types
MaciejSWM Oct 19, 2023
c78c8bf
Linter
MaciejSWM Oct 19, 2023
48213da
Restore displayName property
MaciejSWM Oct 25, 2023
2208b60
Type using Pick instead of creating new type
MaciejSWM Oct 25, 2023
f19ef94
Export two more types
MaciejSWM Oct 25, 2023
79d2ab4
Better TS for Translate type
MaciejSWM Oct 25, 2023
72dc905
Prettier
MaciejSWM Oct 25, 2023
935180c
Type similarly to types from Localize/index.ts
MaciejSWM Oct 26, 2023
67e5e89
Eslint
MaciejSWM Oct 26, 2023
728d99d
Merge branch 'main' into ts-migration/useLocalize
MaciejSWM Oct 26, 2023
8da6fde
Merge branch 'main' into ts-migration/useLocalize
blazejkustra Oct 26, 2023
46a51e5
Fix useLocalize typecheck
blazejkustra Oct 26, 2023
572e4b5
Improve Translate type
blazejkustra Oct 26, 2023
c4910dd
Clean types a bit
blazejkustra Oct 26, 2023
61c393f
Fix tests
blazejkustra Oct 26, 2023
dc8ebfb
Merge branch 'main' into ts-migration/useLocalize
MaciejSWM Oct 31, 2023
54471bc
Import using alias syntax
MaciejSWM Oct 31, 2023
74556fc
Merge branch 'ts-migration/useLocalize' of https://github.com/softwar…
MaciejSWM Oct 31, 2023
8db09d3
Reorder imports
MaciejSWM Oct 31, 2023
627d589
Merge branch 'main' into ts-migration/useLocalize
blazejkustra Nov 7, 2023
79f3317
Use types from Localize
blazejkustra Nov 7, 2023
c521d17
Add a missing context type
blazejkustra Nov 8, 2023
c610b37
Refactor LocaleContext types declaration
blazejkustra Nov 8, 2023
1ad4186
Merge branch 'main' into ts-migration/useLocalize
blazejkustra Nov 8, 2023
493fef5
Merge branch 'main' into ts-migration/useLocalize
blazejkustra Nov 9, 2023
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
135 changes: 0 additions & 135 deletions src/components/LocaleContextProvider.js

This file was deleted.

144 changes: 144 additions & 0 deletions src/components/LocaleContextProvider.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import React, {createContext, useMemo} from 'react';
import {withOnyx} from 'react-native-onyx';
import {ValueOf} from 'type-fest';
import ONYXKEYS from '../ONYXKEYS';
import * as Localize from '../libs/Localize';
import DateUtils from '../libs/DateUtils';
import * as NumberFormatUtils from '../libs/NumberFormatUtils';
import * as LocaleDigitUtils from '../libs/LocaleDigitUtils';
import CONST from '../CONST';
import compose from '../libs/compose';
import withCurrentUserPersonalDetails, {WithCurrentUserPersonalDetailsProps} from './withCurrentUserPersonalDetails';
import * as LocalePhoneNumber from '../libs/LocalePhoneNumber';
import {TranslationPaths, TranslationFlatObject} from '../languages/types';

type LocaleContextProviderOnyxProps = {
/** The user's preferred locale e.g. 'en', 'es-ES' */
preferredLocale: ValueOf<typeof CONST.LOCALES>;
};

type LocaleContextProviderProps = LocaleContextProviderOnyxProps &
WithCurrentUserPersonalDetailsProps & {
/** Actual content wrapped by this component */
children: React.ReactNode;
};

type PhraseParameters<T> = T extends (...args: infer A) => string ? A : never[];

type Phrase<TKey extends TranslationPaths> = TranslationFlatObject[TKey] extends (...args: infer A) => unknown ? (...args: A) => string : string;

type Translate = <TKey extends TranslationPaths>(phrase: TKey, variables?: PhraseParameters<Phrase<TKey>>) => string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
type Translate = <TKey extends TranslationPaths>(phrase: TKey, variables?: PhraseParameters<Phrase<TKey>>) => string;
type Translate = <TKey extends TranslationPaths>(phraseKey: TKey, ...phraseParameters: PhraseParameters<Phrase<TKey>>) => string;


type NumberFormat = (number: number, options: Intl.NumberFormatOptions) => string;

type DatetimeToRelative = (datetime: string) => string;

type DatetimeToCalendarTime = (datetime: string, includeTimezone: boolean, isLowercase: boolean) => string;

type UpdateLocale = () => void;

type FormatPhoneNumber = (phoneNumber: string) => string;
Copy link
Contributor

Choose a reason for hiding this comment

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

@blazejkustra Defining types for each function like this seems a little redundant to me. Is this a general pattern we're using throughout our app? I would just add these typings when defining the function.

Copy link
Contributor

Choose a reason for hiding this comment

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

All these types are used in LocaleContextProps type and their respective implementation in useMemos. I agree it looks rather clunky, I refactored the code a bit, wdyt? cc @BartoszGrajdek / @kubabutkiewicz


type ToLocaleDigit = (digit: string) => string;

type FromLocaleDigit = (digit: string) => string;

type LocaleContextProps = {
/** Returns translated string for given locale and phrase */
translate: Translate;

/** Formats number formatted according to locale and options */
numberFormat: NumberFormat;

/** Converts a datetime into a localized string representation that's relative to current moment in time */
datetimeToRelative: DatetimeToRelative;

/** Formats a datetime to local date and time string */
datetimeToCalendarTime: DatetimeToCalendarTime;

/** Updates date-fns internal locale */
updateLocale: UpdateLocale;

/** Returns a locally converted phone number for numbers from the same region
* and an internationally converted phone number with the country code for numbers from other regions */
formatPhoneNumber: FormatPhoneNumber;

/** Gets the locale digit corresponding to a standard digit */
toLocaleDigit: ToLocaleDigit;

/** Gets the standard digit corresponding to a locale digit */
fromLocaleDigit: FromLocaleDigit;

/** The user's preferred locale e.g. 'en', 'es-ES' */
preferredLocale: ValueOf<typeof CONST.LOCALES>;
};

const LocaleContext = createContext<LocaleContextProps>({
translate: () => '',
numberFormat: () => '',
datetimeToRelative: () => '',
datetimeToCalendarTime: () => '',
updateLocale: () => '',
formatPhoneNumber: () => '',
toLocaleDigit: () => '',
fromLocaleDigit: () => '',
preferredLocale: CONST.LOCALES.DEFAULT,
});

function LocaleContextProvider({preferredLocale = CONST.LOCALES.DEFAULT, currentUserPersonalDetails = {}, children}: LocaleContextProviderProps) {
const selectedTimezone = useMemo(() => currentUserPersonalDetails?.timezone?.selected, [currentUserPersonalDetails]);

const translate = useMemo<Translate>(() => (phrase, variables) => Localize.translate(preferredLocale, phrase, variables), [preferredLocale]);
Copy link
Contributor

@blazejkustra blazejkustra Oct 26, 2023

Choose a reason for hiding this comment

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

Suggested change
const translate = useMemo<Translate>(() => (phrase, variables) => Localize.translate(preferredLocale, phrase, variables), [preferredLocale]);
const translate = useMemo<Translate>(() => (phraseKey, ...phraseParameters) => Localize.translate(preferredLocale, phraseKey, ...phraseParameters), [preferredLocale]);


const numberFormat = useMemo<NumberFormat>(() => (number, options) => NumberFormatUtils.format(preferredLocale, number, options), [preferredLocale]);
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that some of these util functions (lines 95-116) accept strings, this is something we haven't noticed before, so I'd recommend going through all of them and making sure that they are strict enough - in cases where we use timezones, make use of that new type, and where we use LOCALES make use of ValueOf


const datetimeToRelative = useMemo<DatetimeToRelative>(() => (datetime) => DateUtils.datetimeToRelative(preferredLocale, datetime), [preferredLocale]);

const datetimeToCalendarTime = useMemo<DatetimeToCalendarTime>(
() =>
(datetime, includeTimezone, isLowercase = false) =>
DateUtils.datetimeToCalendarTime(preferredLocale, datetime, includeTimezone, selectedTimezone, isLowercase),
[preferredLocale, selectedTimezone],
);

const updateLocale = useMemo<UpdateLocale>(() => () => DateUtils.setLocale(preferredLocale), [preferredLocale]);

const formatPhoneNumber = useMemo<FormatPhoneNumber>(() => (phoneNumber) => LocalePhoneNumber.formatPhoneNumber(phoneNumber), []);

const toLocaleDigit = useMemo<ToLocaleDigit>(() => (digit) => LocaleDigitUtils.toLocaleDigit(preferredLocale, digit), [preferredLocale]);

const fromLocaleDigit = useMemo<FromLocaleDigit>(() => (localeDigit) => LocaleDigitUtils.fromLocaleDigit(preferredLocale, localeDigit), [preferredLocale]);

const contextValue = useMemo(
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you miss to type this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, good catch. I fixed it!

() => ({
translate,
numberFormat,
datetimeToRelative,
datetimeToCalendarTime,
updateLocale,
formatPhoneNumber,
toLocaleDigit,
fromLocaleDigit,
preferredLocale,
}),
[translate, numberFormat, datetimeToRelative, datetimeToCalendarTime, updateLocale, formatPhoneNumber, toLocaleDigit, fromLocaleDigit, preferredLocale],
);

return <LocaleContext.Provider value={contextValue}>{children}</LocaleContext.Provider>;
}

const Provider = compose(
withOnyx<LocaleContextProviderProps, LocaleContextProviderOnyxProps>({
preferredLocale: {
key: ONYXKEYS.NVP_PREFERRED_LOCALE,
selector: (preferredLocale) => preferredLocale ?? CONST.LOCALES.DEFAULT,
},
}),
withCurrentUserPersonalDetails,
)(LocaleContextProvider);

mountiny marked this conversation as resolved.
Show resolved Hide resolved
Provider.displayName = 'withOnyx(LocaleContextProvider)';

export {Provider as LocaleContextProvider, LocaleContext};

export type {LocaleContextProps};
5 changes: 3 additions & 2 deletions src/components/withCurrentUserPersonalDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ type HOCProps = {
currentUserPersonalDetails: CurrentUserPersonalDetails;
};

type ComponentProps = OnyxProps & HOCProps;
type WithCurrentUserPersonalDetailsProps = OnyxProps & HOCProps;

// TODO: remove when all components that use it will be migrated to TS
const withCurrentUserPersonalDetailsPropTypes = {
Expand All @@ -30,7 +30,7 @@ const withCurrentUserPersonalDetailsDefaultProps: HOCProps = {
currentUserPersonalDetails: {},
};

export default function <TProps extends ComponentProps, TRef>(
export default function <TProps extends WithCurrentUserPersonalDetailsProps, TRef>(
WrappedComponent: ComponentType<TProps & RefAttributes<TRef>>,
): ComponentType<Omit<Omit<TProps, keyof HOCProps> & RefAttributes<TRef>, keyof OnyxProps>> {
function WithCurrentUserPersonalDetails(props: Omit<TProps, keyof HOCProps>, ref: ForwardedRef<TRef>) {
Expand Down Expand Up @@ -65,3 +65,4 @@ export default function <TProps extends ComponentProps, TRef>(
}

export {withCurrentUserPersonalDetailsPropTypes, withCurrentUserPersonalDetailsDefaultProps};
export type {WithCurrentUserPersonalDetailsProps};
6 changes: 0 additions & 6 deletions src/hooks/useLocalize.js

This file was deleted.

6 changes: 6 additions & 0 deletions src/hooks/useLocalize.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import {useContext} from 'react';
import {LocaleContext, LocaleContextProps} from '../components/LocaleContextProvider';

export default function useLocalize(): LocaleContextProps {
Copy link
Contributor

Choose a reason for hiding this comment

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

Typing this is redundant. I think the type would be inferred here. No?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but specifying return types aligns with TS guidelines. We decided to almost always specify return types for two reasons:

  1. Better visibility
  2. Improved TS performance when type checking

return useContext(LocaleContext);
}
2 changes: 2 additions & 0 deletions src/languages/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,8 @@ type TranslationFlatObject = {

export type {
TranslationBase,
TranslateType,
TranslationPaths,
EnglishTranslation,
TranslationFlatObject,
AddressLineParams,
Expand Down
Loading
Loading