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

Improve translations - flatten translation objects #25846

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,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 @@ -1660,6 +1683,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
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
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
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
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
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
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 @@ -1755,4 +1756,4 @@ export default {
selectSuggestedAddress: 'Please select a suggested address',
},
},
} as const;
} satisfies Translation;
3 changes: 2 additions & 1 deletion src/languages/es.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import CONST from '../CONST';
import * as ReportActionsUtils from '../libs/ReportActionsUtils';
import type {
Translation,
AddressLineParams,
CharacterLimitParams,
MaxParticipantsReachedParams,
Expand Down Expand Up @@ -2247,4 +2248,4 @@ export default {
selectSuggestedAddress: 'Por favor, selecciona una dirección sugerida',
},
},
};
} satisfies Translation;
44 changes: 42 additions & 2 deletions src/languages/translations.ts
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, 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

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: Record<string, unknown> = {};

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;
} 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] = '';
}
}
};

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

export default {
en,
es,
en: flattenObject(en),
es: flattenObject(es),
// eslint-disable-next-line @typescript-eslint/naming-convention
'es-ES': esES,
};
42 changes: 42 additions & 0 deletions src/languages/types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import en from './en';

type AddressLineParams = {
lineNumber: number;
};
Expand Down Expand Up @@ -190,7 +192,47 @@ type RemovedTheRequestParams = {valueName: string; oldValueToDisplay: string};

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

/* Translation Object types */
// eslint-disable-next-line @typescript-eslint/no-explicit-any
type TranslationBaseValue = string | string[] | ((...args: any[]) => 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 We need any[] here I think, I tried with unknown[] and every function starts failing.


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

/* Flat Translation Object types */
// Flattens an object and returns concatenations of all the keys of nested objects
type FlattenObject<TObject, TPrefix extends string = ''> = {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
[TKey in keyof TObject]: TObject[TKey] extends (...args: any[]) => any
? `${TPrefix}${TKey & string}`
: // eslint-disable-next-line @typescript-eslint/no-explicit-any
TObject[TKey] extends any[]
? `${TPrefix}${TKey & string}`
: // eslint-disable-next-line @typescript-eslint/ban-types
TObject[TKey] extends object
? FlattenObject<TObject[TKey], `${TPrefix}${TKey & string}.`>
: `${TPrefix}${TKey & string}`;
}[keyof TObject];

// Retrieves a type for a given key path (calculated from the type above)
type TranslateType<TObject, TPath extends string> = TPath extends keyof TObject
? TObject[TPath]
: TPath extends `${infer TKey}.${infer TRest}`
? TKey extends keyof TObject
? TranslateType<TObject[TKey], TRest>
: never
: never;

type TranslationsType = typeof en;

type TranslationPaths = FlattenObject<TranslationsType>;

type TranslationFlatObject = {
[TKey in TranslationPaths]: TranslateType<TranslationsType, TKey>;
};

export type {
Translation,
TranslationFlatObject,
AddressLineParams,
CharacterLimitParams,
MaxParticipantsReachedParams,
Expand Down
14 changes: 7 additions & 7 deletions src/libs/Localize/index.js
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
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
Loading
Loading