-
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
[TS migration] Migrate useLocalize #29180
Changes from 3 commits
d26ce71
a8e3b97
8367198
369edd8
8185ab7
9db5a94
60a8bb7
5d6eed5
c78c8bf
48213da
2208b60
f19ef94
79d2ab4
72dc905
935180c
67e5e89
728d99d
8da6fde
46a51e5
572e4b5
c4910dd
61c393f
dc8ebfb
54471bc
74556fc
8db09d3
627d589
79f3317
c521d17
c610b37
1ad4186
493fef5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,129 @@ | ||||||||||||||
import React, {createContext, useMemo} from 'react'; | ||||||||||||||
import {withOnyx} from 'react-native-onyx'; | ||||||||||||||
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 from './withCurrentUserPersonalDetails'; | ||||||||||||||
import * as LocalePhoneNumber from '../libs/LocalePhoneNumber'; | ||||||||||||||
|
||||||||||||||
type SelectedTimezone = { | ||||||||||||||
/** Value of the selected timezone */ | ||||||||||||||
selected: string; | ||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
type CurrentUserPersonalDetails = { | ||||||||||||||
/** Timezone of the current user */ | ||||||||||||||
timezone?: SelectedTimezone; | ||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
type LocaleContextProviderOnyxProps = { | ||||||||||||||
/** The user's preferred locale e.g. 'en', 'es-ES' */ | ||||||||||||||
preferredLocale: string; | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would suggest doing something like this
Suggested change
|
||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
type LocaleContextProviderProps = LocaleContextProviderOnyxProps & { | ||||||||||||||
/** The current user's personalDetails */ | ||||||||||||||
currentUserPersonalDetails: CurrentUserPersonalDetails; | ||||||||||||||
|
||||||||||||||
/** Actual content wrapped by this component */ | ||||||||||||||
children: React.ReactNode; | ||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
type Translate = (phrase: string, variables: Record<string, string>) => string; | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MaciejSWM let's make this type smarter, here is my idea:
Suggested change
This type will allow us to only use the keys that exist and, when is a key that needs variables, we will have type inference too. translate('addDebitCardPage.nameOnCard');
translate('chronos.oooEventSummaryFullDay', {
summary: '',
dayCount: 1,
date: '',
}); Note: You will have to export There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @fabioh8010 We implemented a similar approach in this PR (src/libs/Localize/index.ts, 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;
function translate<TKey extends TranslationPaths>(desiredLanguage: 'en' | 'es' | 'es-ES' | 'es_ES', phraseKey: TKey, ...phraseParameters: PhraseParameters<Phrase<TKey>>): string { ☝️ edit: just updated Localize PR, it's ready for cross review 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MaciejSWM @blazejkustra The way it was implemented in that PR works better, let's use it then! |
||||||||||||||
|
||||||||||||||
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; | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All these types are used in |
||||||||||||||
|
||||||||||||||
type ToLocaleDigit = (digit: string) => string; | ||||||||||||||
|
||||||||||||||
type FromLocaleDigit = (digit: string) => string; | ||||||||||||||
|
||||||||||||||
type LocaleContextProps = { | ||||||||||||||
translate: Translate; | ||||||||||||||
numberFormat: NumberFormat; | ||||||||||||||
datetimeToRelative: DatetimeToRelative; | ||||||||||||||
datetimeToCalendarTime: DatetimeToCalendarTime; | ||||||||||||||
updateLocale: UpdateLocale; | ||||||||||||||
formatPhoneNumber: FormatPhoneNumber; | ||||||||||||||
toLocaleDigit: ToLocaleDigit; | ||||||||||||||
fromLocaleDigit: FromLocaleDigit; | ||||||||||||||
preferredLocale: string; | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MaciejSWM Can you add comments to describe these props? /** Returns translated string for given locale and phrase */
translate: type
/** Formats number formatted according to locale and options */
numberFormat: type
/** Converts a datetime into a localized string representation that's relative to current moment in time */
datetimeToRelative: type
/** Formats a datetime to local date and time string */
datetimeToCalendarTime: type
/** Updates date-fns internal locale */
updateLocale: type
/** 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: type
/** Gets the standard digit corresponding to a locale digit */
fromLocaleDigit: type
/** Gets the locale digit corresponding to a standard digit */
toLocaleDigit: type |
||||||||||||||
}; | ||||||||||||||
|
||||||||||||||
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]); | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
|
||||||||||||||
const numberFormat = useMemo<NumberFormat>(() => (number, options) => NumberFormatUtils.format(preferredLocale, number, options), [preferredLocale]); | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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( | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you miss to type this? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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( | ||||||||||||||
withCurrentUserPersonalDetails, | ||||||||||||||
withOnyx<LocaleContextProviderProps, LocaleContextProviderOnyxProps>({ | ||||||||||||||
preferredLocale: { | ||||||||||||||
key: ONYXKEYS.NVP_PREFERRED_LOCALE, | ||||||||||||||
selector: (preferredLocale) => preferredLocale ?? CONST.LOCALES.DEFAULT, | ||||||||||||||
}, | ||||||||||||||
}), | ||||||||||||||
)(LocaleContextProvider); | ||||||||||||||
|
||||||||||||||
mountiny marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
export {Provider as LocaleContextProvider, LocaleContext}; | ||||||||||||||
|
||||||||||||||
export type {LocaleContextProps}; |
This file was deleted.
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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
|
||
return useContext(LocaleContext); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use timezones from TIMEZONES.js to make it more strict?