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.

Sign up for GitHub

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

Jump to bottom

Improve translations - flatten translation objects #25846

Merged
merged 19 commits into from
Sep 13, 2023
Merged

Improve translations - flatten translation objects #25846

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
e3b5852
test: unit test for translation flatten object function
BeeMargarida Aug 23, 2023
82919b7
feat: flatten object function to flatten translation
BeeMargarida Aug 23, 2023
463daff
feat: remove lodashGet from translate
BeeMargarida Aug 23, 2023
62d4e91
feat: migrate use of languages translate to new solution
BeeMargarida Aug 23, 2023
370d679
feat: migrate use of pronouns translate to new solution
BeeMargarida Aug 23, 2023
9f84e42
feat: migrate use of countries and states translate to new solution
BeeMargarida Aug 23, 2023
6548241
style: prettier
BeeMargarida Aug 23, 2023
f38a0bf
fix: small error
BeeMargarida Aug 24, 2023
2fef893
refactor: move unit test to translate unit suite
BeeMargarida Aug 24, 2023
9d8b224
fix: remove unused usage
BeeMargarida Aug 24, 2023
ad78499
feat: rename const
BeeMargarida Aug 25, 2023
0088b39
feat: switch us states const to an array of ISOs
BeeMargarida Sep 4, 2023
e86a966
Merge branch 'main' into feat/25466-flatten_translation_objects
BeeMargarida Sep 4, 2023
c6bf5df
Merge branch 'main' into feat/25466-flatten_translation_objects
BeeMargarida Sep 7, 2023
8db1cda
Merge branch 'main' into feat/25466-flatten_translation_objects
BeeMargarida Sep 11, 2023
1ac2cde
Merge branch 'main' into feat/25466-flatten_translation_objects
BeeMargarida Sep 12, 2023
be487e7
feat: improve translation types
BeeMargarida Sep 12, 2023
2931a45
refactor: rename generics
BeeMargarida Sep 12, 2023
8f8420a
style: prettier
BeeMargarida Sep 12, 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
78 changes: 78 additions & 0 deletions src/CONST.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1080,6 +1080,29 @@ const CONST = {
DEFAULT: 'en',
},

LANGUAGES: ['en', 'es'],

PRONOUNS_LIST: [
'coCos',
'eEyEmEir',
'heHimHis',
'heHimHisTheyThemTheirs',
'sheHerHers',
'sheHerHersTheyThemTheirs',
'merMers',
'neNirNirs',
'neeNerNers',
'perPers',
'theyThemTheirs',
'thonThons',
'veVerVis',
'viVir',
'xeXemXyr',
'zeZieZirHir',
'zeHirHirs',
'callMeByMyName',
],

POLICY: {
TYPE: {
FREE: 'free',
Expand Down Expand Up @@ -1651,6 +1674,61 @@ const CONST = {
ZW: 'Zimbabwe',
},

ALL_US_ISO_STATES: [
'AK',
'AL',
'AR',
'AZ',
'CA',
'CO',
'CT',
'DE',
'FL',
'GA',
'HI',
'IA',
'ID',
'IL',
'IN',
'KS',
'KY',
'LA',
'MA',
'MD',
'ME',
'MI',
'MN',
'MO',
'MS',
'MT',
'NC',
'ND',
'NE',
'NH',
'NJ',
'NM',
'NV',
'NY',
'OH',
'OK',
'OR',
'PA',
'PR',
'RI',
'SC',
'SD',
'TN',
'TX',
'UT',
'VA',
'VT',
'WA',
'WI',
'WV',
'WY',
'DC',
],

// Sources: https://github.com/Expensify/App/issues/14958#issuecomment-1442138427
// https://github.com/Expensify/App/issues/14958#issuecomment-1456026810
COUNTRY_ZIP_REGEX_DATA: {
Expand Down
17 changes: 10 additions & 7 deletions src/components/CountryPicker/CountrySelectorModal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,16 @@ function CountrySelectorModal({currentCountry, isVisible, onClose, onCountrySele

const countries = useMemo(
() =>
_.map(translate('allCountries'), (countryName, countryISO) => ({
value: countryISO,
keyForList: countryISO,
text: countryName,
isSelected: currentCountry === countryISO,
searchValue: StringUtils.sanitizeString(`${countryISO}${countryName}`),
})),
_.map(_.keys(CONST.ALL_COUNTRIES), (countryISO) => {
const countryName = translate(`allCountries.${countryISO}`);
return {
value: countryISO,
keyForList: countryISO,
text: countryName,
isSelected: currentCountry === countryISO,
searchValue: StringUtils.sanitizeString(`${countryISO}${countryName}`),
};
}),
[translate, currentCountry],
);

Expand Down
3 changes: 1 addition & 2 deletions src/components/CountryPicker/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const defaultProps = {

function CountryPicker({value, errorText, onInputChange, forwardedRef}) {
const {translate} = useLocalize();
const allCountries = translate('allCountries');
const [isPickerVisible, setIsPickerVisible] = useState(false);
const [searchValue, setSearchValue] = useState('');

Expand All @@ -48,7 +47,7 @@ function CountryPicker({value, errorText, onInputChange, forwardedRef}) {
hidePickerModal();
};

const title = allCountries[value] || '';
const title = value ? translate(`allCountries.${value}`) : '';
const descStyle = title.length === 0 ? styles.textNormal : null;

return (
Expand Down
8 changes: 5 additions & 3 deletions src/components/LocalePicker.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@ const defaultProps = {
};

function LocalePicker(props) {
const localesToLanguages = _.map(props.translate('languagePage.languages'), (language, key) => ({
value: key,
label: language.label,
const localesToLanguages = _.map(CONST.LANGUAGES, (language) => ({
value: language,
label: props.translate(`languagePage.languages.${language}.label`),
keyForList: language,
isSelected: props.preferredLocale === language,
}));
return (
<Picker
Expand Down
18 changes: 11 additions & 7 deletions src/components/StatePicker/StateSelectorModal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,17 @@ function StateSelectorModal({currentState, isVisible, onClose, onStateSelected,

const countryStates = useMemo(
() =>
_.map(translate('allStates'), (state) => ({
value: state.stateISO,
keyForList: state.stateISO,
text: state.stateName,
isSelected: currentState === state.stateISO,
searchValue: StringUtils.sanitizeString(`${state.stateISO}${state.stateName}`),
})),
_.map(CONST.ALL_US_ISO_STATES, (state) => {
const stateName = translate(`allStates.${state}.stateName`);
const stateISO = translate(`allStates.${state}.stateISO`);
return {
value: stateISO,
keyForList: stateISO,
text: stateName,
isSelected: currentState === stateISO,
searchValue: StringUtils.sanitizeString(`${stateISO}${stateName}`),
};
}),
[translate, currentState],
);

Expand Down
3 changes: 1 addition & 2 deletions src/components/StatePicker/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ const defaultProps = {

function StatePicker({value, errorText, onInputChange, forwardedRef, label}) {
const {translate} = useLocalize();
const allStates = translate('allStates');
const [isPickerVisible, setIsPickerVisible] = useState(false);
const [searchValue, setSearchValue] = useState('');

Expand All @@ -52,7 +51,7 @@ function StatePicker({value, errorText, onInputChange, forwardedRef, label}) {
hidePickerModal();
};

const title = allStates[value] ? allStates[value].stateName : '';
const title = value ? translate(`allStates.${value}.stateName`) : '';
const descStyle = title.length === 0 ? styles.textNormal : null;

return (
Expand Down
3 changes: 2 additions & 1 deletion src/languages/en.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {CONST as COMMON_CONST} from 'expensify-common/lib/CONST';
import CONST from '../CONST';
import type {
Translation,
AddressLineParams,
CharacterLimitParams,
MaxParticipantsReachedParams,
Expand Down Expand Up @@ -1765,4 +1766,4 @@ export default {
heroBody: 'Use New Expensify for event updates, networking, social chatter, and to get paid back for your ride to or from the show!',
},
},
} as const;
} satisfies Translation;
44 changes: 42 additions & 2 deletions src/languages/translations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,50 @@
import en from './en';
import es from './es';
import esES from './es-ES';
import type {Translation, TranslationBaseValue, TranslationFlatObject} from './types';

/**
* Converts an object to it's flattened version.
*
* Ex:
* Input: { common: { yes: "Yes", no: "No" }}
* Output: { "common.yes": "Yes", "common.no": "No" }
*/
// Necessary to export so that it is accessible to the unit tests
// eslint-disable-next-line rulesdir/no-inline-named-export
export function flattenObject(obj: Translation): TranslationFlatObject {
Copy link
Contributor

@blazejkustra blazejkustra Sep 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

TranslationFlatObject is way too generic (along with Translation & TranslationBaseValue types). The problem is that we lose type safety using flattenObject function:

const translations = flattenObject(en);
const text2 = translations.nonExistingKey; // TranslationFlatObject, meanwhile it's undefined in reality

This is a big problem as we should infer what keys are available to stay type safe when using Localize inside of components.

What we should do is to flatten the object in Typescript. Something like this (link):

type Join<K, P> = K extends string | number ? (P extends string | number ? `${K}${'' extends P ? '' : '.'}${P}` : never) : never;

type Prev = [never, 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, ...Array<0>];

type Leaves<T, D extends number = 10> = [D] extends [never] ? never : T extends object ? {[K in keyof T]-?: Join<K, Leaves<T[K], Prev[D]>>}[keyof T] : '';

declare function flattenObject<TTranslation extends Translation>(obj: TTranslation): Record<Leaves<TTranslation>, string | ...>;

const enFlattened = flattenObject2(en);

⚠️ These types might be quite slow, so be careful and test before using.

Copy link
Contributor

Choose a reason for hiding this comment

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

However this will be quite irritating to use inside of components as the value is not inferred and it's too generic: string | string[] | Function. So we would have to always check the types inside of components 😒

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I had a similar problem in my side project, something like this could work here but the type is quite complicated ngl 😅 (this would have to be tested as I used it a little bit differentl in my project)

// Flatten object type
type FlattenObject<T> = CollapseEntries<CreateObjectEntries<T, T>>;

type Entry = {key: string; value: unknown};
type EmptyEntry<T> = {key: ''; value: T};

// Transforms entries to one flattened type
type CollapseEntries<T extends Entry> = {
    [E in T as E['key']]: E['value'];
} extends infer V
    ? {[K in keyof V]: V[K]}
    : never;

type CreateObjectEntries<T, I> = T extends infer U
    ? // Checks that U is an object
      U extends object
        ? {
              // Checks that Key is of type string
              [K in keyof U]-?: K extends string
                  ? // Nested key can be an object, run recursively to the bottom
                    CreateObjectEntries<U[K], I> extends infer E
                      ? E extends Entry
                          ? {
                                key: E['key'] extends '' ? K : `${K}.${E['key']}`;
                                value: E['value'];
                            }
                          : never
                      : never
                  : never;
          }[keyof U] // Builds entry for each key
        : EmptyEntry<U>
    : never;

declare function flattenObject2<TTranslation extends Translation>(obj: TTranslation): FlattenObject<TTranslation>;

const enFlattened = flattenObject2(es);

const type = enFlattened['common.websiteExample']; // string

Copy link
Contributor

Choose a reason for hiding this comment

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

Also cc @fabioh8010 as this is quite important to get right 😅

const result: TranslationFlatObject = {};

const recursive = (data: Translation, key: string): void => {
// If the data is a function or not a object (eg. a string or array),
// it's the final value for the key being built and there is no need
// for more recursion
if (typeof data === 'function' || Array.isArray(data) || !(typeof data === 'object' && !!data)) {
result[key] = data as TranslationBaseValue;
} else {
let isEmpty = true;

// Recursive call to the keys and connect to the respective data
Object.keys(data).forEach((k) => {
isEmpty = false;
recursive(data[k] as Translation, key ? `${key}.${k}` : k);
});

// Check for when the object is empty but a key exists, so that
// it defaults to an empty object
if (isEmpty && key) {
result[key] = {} as TranslationBaseValue;
}
}
};

recursive(obj, '');
return result;
}

export default {
en,
es,
en: flattenObject(en),
es: flattenObject(es),
// eslint-disable-next-line @typescript-eslint/naming-convention
'es-ES': esES,
};
10 changes: 10 additions & 0 deletions src/languages/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,17 @@ type RemovedTheRequestParams = {valueName: string; oldValueToDisplay: string};

type UpdatedTheRequestParams = {valueName: string; newValueToDisplay: string; oldValueToDisplay: string};

// eslint-disable-next-line @typescript-eslint/no-explicit-any
type TranslationBaseValue = string | string[] | ((args: any) => string);

type Translation = {[key: string]: TranslationBaseValue | Translation};

type TranslationFlatObject = Record<string, TranslationBaseValue>;

export type {
Translation,
TranslationBaseValue,
TranslationFlatObject,
AddressLineParams,
CharacterLimitParams,
MaxParticipantsReachedParams,
Expand Down
14 changes: 7 additions & 7 deletions src/libs/Localize/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function init() {
* Return translated string for given locale and phrase
*
* @param {String} [desiredLanguage] eg 'en', 'es-ES'
* @param {String|Array} phraseKey
* @param {String} phraseKey
* @param {Object} [phraseParameters] Parameters to supply if the phrase is a template literal.
* @returns {String}
*/
Expand All @@ -47,15 +47,15 @@ function translate(desiredLanguage = CONST.LOCALES.DEFAULT, phraseKey, phrasePar
let translatedPhrase;

// Search phrase in full locale e.g. es-ES
const desiredLanguageDictionary = lodashGet(translations, desiredLanguage);
translatedPhrase = lodashGet(desiredLanguageDictionary, phraseKey);
const desiredLanguageDictionary = translations[desiredLanguage] || {};
translatedPhrase = desiredLanguageDictionary[phraseKey];
if (translatedPhrase) {
return Str.result(translatedPhrase, phraseParameters);
}

// Phrase is not found in full locale, search it in fallback language e.g. es
const fallbackLanguageDictionary = lodashGet(translations, languageAbbreviation);
translatedPhrase = lodashGet(fallbackLanguageDictionary, phraseKey);
const fallbackLanguageDictionary = translations[languageAbbreviation] || {};
translatedPhrase = fallbackLanguageDictionary[phraseKey];
if (translatedPhrase) {
return Str.result(translatedPhrase, phraseParameters);
}
Expand All @@ -64,8 +64,8 @@ function translate(desiredLanguage = CONST.LOCALES.DEFAULT, phraseKey, phrasePar
}

// Phrase is not translated, search it in default language (en)
const defaultLanguageDictionary = lodashGet(translations, CONST.LOCALES.DEFAULT, {});
translatedPhrase = lodashGet(defaultLanguageDictionary, phraseKey);
const defaultLanguageDictionary = translations[CONST.LOCALES.DEFAULT] || {};
translatedPhrase = defaultLanguageDictionary[phraseKey];
if (translatedPhrase) {
return Str.result(translatedPhrase, phraseParameters);
}
Expand Down
11 changes: 6 additions & 5 deletions src/pages/settings/Preferences/LanguagePage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import withLocalize, {withLocalizePropTypes} from '../../../components/withLocal
import * as App from '../../../libs/actions/App';
import Navigation from '../../../libs/Navigation/Navigation';
import ROUTES from '../../../ROUTES';
import CONST from '../../../CONST';
import SelectionList from '../../../components/SelectionList';

const propTypes = {
Expand All @@ -17,11 +18,11 @@ const propTypes = {
};

function LanguagePage(props) {
const localesToLanguages = _.map(props.translate('languagePage.languages'), (language, key) => ({
value: key,
text: language.label,
keyForList: key,
isSelected: props.preferredLocale === key,
const localesToLanguages = _.map(CONST.LANGUAGES, (language) => ({
value: language,
text: props.translate(`languagePage.languages.${language}.label`),
keyForList: language,
isSelected: props.preferredLocale === language,
}));

return (
Expand Down
7 changes: 2 additions & 5 deletions src/pages/settings/Preferences/PreferencesPage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ function PreferencesPage(props) {
const {isProduction} = useEnvironment();
const {translate, preferredLocale} = useLocalize();

const priorityModes = translate('priorityModePage.priorityModes');
const languages = translate('languagePage.languages');

return (
<IllustratedHeaderPageLayout
title={translate('common.preferences')}
Expand Down Expand Up @@ -71,13 +68,13 @@ function PreferencesPage(props) {
</View>
<MenuItemWithTopDescription
shouldShowRightIcon
title={priorityModes[props.priorityMode].label}
title={translate(`priorityModePage.priorityModes.${props.priorityMode}.label`)}
description={translate('priorityModePage.priorityMode')}
onPress={() => Navigation.navigate(ROUTES.SETTINGS_PRIORITY_MODE)}
/>
<MenuItemWithTopDescription
shouldShowRightIcon
title={languages[preferredLocale].label}
title={translate(`languagePage.languages.${preferredLocale}.label`)}
description={translate('languagePage.language')}
onPress={() => Navigation.navigate(ROUTES.SETTINGS_LANGUAGE)}
/>
Expand Down
6 changes: 5 additions & 1 deletion src/pages/settings/Profile/ProfilePage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ function ProfilePage(props) {
if (pronounsKey.startsWith(CONST.PRONOUNS.PREFIX)) {
pronounsKey = pronounsKey.slice(CONST.PRONOUNS.PREFIX.length);
}
return lodashGet(props.translate('pronouns'), pronounsKey, props.translate('profilePage.selectYourPronouns'));

if (!pronounsKey) {
return props.translate('profilePage.selectYourPronouns');
}
return props.translate(`pronouns.${pronounsKey}`);
};
const currentUserDetails = props.currentUserPersonalDetails || {};
const contactMethodBrickRoadIndicator = UserUtils.getLoginListBrickRoadIndicator(props.loginList);
Expand Down
Loading
Loading