Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 10 additions & 38 deletions src/libs/Localize/index.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import * as RNLocalize from 'react-native-localize';
import Onyx from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import Log from '@libs/Log';
import memoize from '@libs/memoize';
import type {MessageElementBase, MessageTextElement} from '@libs/MessageElement';
import Config from '@src/CONFIG';
import CONST from '@src/CONST';
import translations from '@src/languages/translations';
import type {PluralForm, TranslationParameters, TranslationPaths} from '@src/languages/types';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Locale} from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import LocaleListener from './LocaleListener';
import BaseLocaleListener from './LocaleListener/BaseLocaleListener';

Expand Down Expand Up @@ -45,28 +46,6 @@ function init() {
}, {});
}

/**
* Map to store translated values for each locale.
* This is used to avoid translating the same phrase multiple times.
*
* The data is stored in the following format:
*
* {
* "en": {
* "name": "Name",
* }
*
* Note: We are not storing any translated values for phrases with variables,
* as they have higher chance of being unique, so we'll end up wasting space
* in our cache.
*/
const translationCache = new Map<ValueOf<typeof CONST.LOCALES>, Map<TranslationPaths, string>>(
Object.values(CONST.LOCALES).reduce((cache, locale) => {
cache.push([locale, new Map<TranslationPaths, string>()]);
return cache;
}, [] as Array<[ValueOf<typeof CONST.LOCALES>, Map<TranslationPaths, string>]>),
);

/**
* Helper function to get the translated string for given
* locale and phrase. This function is used to avoid
Expand All @@ -86,18 +65,6 @@ function getTranslatedPhrase<TKey extends TranslationPaths>(
fallbackLanguage: 'en' | 'es' | null,
...parameters: TranslationParameters<TKey>
): string | null {
// Get the cache for the above locale
const cacheForLocale = translationCache.get(language);

// Directly access and assign the translated value from the cache, instead of
// going through map.has() and map.get() to avoid multiple lookups.
const valueFromCache = cacheForLocale?.get(phraseKey);

// If the phrase is already translated, return the translated value
if (valueFromCache) {
return valueFromCache;
}

const translatedPhrase = translations?.[language]?.[phraseKey];

if (translatedPhrase) {
Expand Down Expand Up @@ -138,8 +105,6 @@ function getTranslatedPhrase<TKey extends TranslationPaths>(
return translateResult.other(phraseObject.count);
}

// We set the translated value in the cache only for the phrases without parameters.
cacheForLocale?.set(phraseKey, translatedPhrase);
return translatedPhrase;
}

Expand All @@ -162,6 +127,13 @@ function getTranslatedPhrase<TKey extends TranslationPaths>(
return getTranslatedPhrase(CONST.LOCALES.DEFAULT, phraseKey, null, ...parameters);
}

const memoizedGetTranslatedPhrase = memoize(getTranslatedPhrase, {
maxArgs: 2,
equality: 'shallow',
// eslint-disable-next-line @typescript-eslint/no-unused-vars
skipCache: (params) => !isEmptyObject(params.at(3)),
});

/**
* Return translated string for given locale and phrase
*
Expand All @@ -174,7 +146,7 @@ function translate<TPath extends TranslationPaths>(desiredLanguage: 'en' | 'es'
// Phrase is not found in full locale, search it in fallback language e.g. es
const languageAbbreviation = desiredLanguage.substring(0, 2) as 'en' | 'es';

const translatedPhrase = getTranslatedPhrase(language, path, languageAbbreviation, ...parameters);
const translatedPhrase = memoizedGetTranslatedPhrase(language, path, languageAbbreviation, ...parameters);
if (translatedPhrase !== null && translatedPhrase !== undefined) {
return translatedPhrase;
}
Expand Down
11 changes: 11 additions & 0 deletions src/libs/memoize/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,17 @@ function memoize<Fn extends IsomorphicFn, MaxArgs extends number = NonPartial<Is
// Detect if memoized function was called with `new` keyword. If so we need to call the original function as constructor.
const constructable = !!new.target;

// If skipCache is set, check if we should skip the cache
if (options.skipCache?.(args)) {
const fnTimeStart = performance.now();
const result = (constructable ? new (fn as Constructable)(...args) : (fn as Callable)(...args)) as IsomorphicReturnType<Fn>;

statsEntry.trackTime('processingTime', fnTimeStart);
statsEntry.track('didHit', false);

return result;
}

const truncatedArgs = truncateArgs(args, options.maxArgs);

const key = options.transformKey ? options.transformKey(truncatedArgs) : (truncatedArgs as Key);
Expand Down
7 changes: 7 additions & 0 deletions src/libs/memoize/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ type Options<Fn extends IsomorphicFn, MaxArgs extends number, Key> = {
* @returns Key to use for caching
*/
transformKey?: (truncatedArgs: TakeFirst<IsomorphicParameters<Fn>, MaxArgs>) => Key;

/**
* Checks if the cache should be skipped for the given arguments.
* @param args Tuple of arguments passed to the memoized function. Does not work with constructable (see description).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@kacper-mikolajczak Can you tell me reason for this? I just copied this from transformKey description.

@kacper-mikolajczak kacper-mikolajczak Oct 14, 2024 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@shubham1206agra IIRC the transformKey did not work for constructables but it was fixed.

One of the exceptions is Intl.NumberFormat from NumberFormatUtils module.
However, for other constructables and it works:

  • custom classes
  • other constructables APIs (e.g. ResizeObserver, URL)

In this case I think we can remove the warning or change it to be more descriptive 👍

@kacper-mikolajczak kacper-mikolajczak Oct 14, 2024 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Update!

After testing it more carefully, it looks like it has an issue with resolving arguments for other APIs as well. For example FormData arguments are merged into union as it happens for NumberFormat.

What they have in common is that all the arguments are optional - it may be a good starting point for investigations

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What they have in common is that all the arguments are optional - it may be a good starting point for investigations

@kacper-mikolajczak I assume we will investigate this point in another issue, won't we?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Definitely, let's make it separate issue!

* @returns boolean to whether to skip cache lookup and execute the function if true
*/
skipCache?: (args: IsomorphicParameters<Fn>) => boolean;
} & InternalOptions;

type ClientOptions<Fn extends IsomorphicFn, MaxArgs extends number, Key> = Partial<Omit<Options<Fn, MaxArgs, Key>, keyof InternalOptions>>;
Expand Down