[HOLD for payment 2024-08-02] Moize - general purpose memoization tool

[kacper-mikolajczak]

In order to properly evaluate if a new library can be added to package.json, please fill out this request form. It will be automatically assigned someone from our review team that will go through and vet the library.

In order to add any new production dependency, it must be approved by the App Deployer team. They will evaluate the library and decide if it's something we want to move forward with or if other alternatives should be explored.

Note: This is only for production dependencies. While we don't want people to add packages to dev-dependencies willy-nilly, we recognize that there isn't as great of a need there to secure them.

Name of library: Moize

Details

• Link to package: https://github.com/planttheidea/moize
• Problem solved by using this package: App as of now, does not have a standardized way of handling memoization outside of React. It boils down to using custom-made solutions like Maps or serialization. This is a problem because engineers implementing and reviewing solutions for memoization outside of React have to sink time into writing and creating a custom solution that may not be consistent with other places in the codebase. This leads to inconsistency, overhead, and at times sub-optimal solutions.
• Number of stars in GH: 880
• Number of monthly downloads: 222,725 weekly!
• Number of releases in the last year: 5
• Level of activity in the repo: 17 contributors, 313 deployments, 3 issues, 6 PRs
• Alternatives: List of alternatives and benchmarks
• Are security concerns brought up and addressed in the library's repo? No
• How many dependencies does this lib use that will be brought into our code? At most 1 (It uses 2 deps: fast-equals and micro-memoize but fast-equals is already used, not sure about micro-memoize)
• What will the effect be on the bundle size of our code? Bundlephobia report of current moize version

Issue Owner

Current Issue Owner: @puneetlath

[melvin-bot]

Triggered auto assignment to @puneetlath (AutoAssignerAppLibraryReview), see https://stackoverflowteams.com/c/expensify/questions/17737 for more details.

[melvin-bot]

New Library Review

• Are all the answers in the main description filled out properly and make sense?
• Who maintains the library and how well is it maintained?
• How viable are the alternatives?
• Should we build it ourselves instead?

Once these questions are answered, start a thread in #engineering-chat, ping the @app_deployers group, and call for a vote to accept the new library. Once the vote is complete, update this issue with the outcome and procede accordingly. Here is a sample post:

Hey @app_deployers,

There is a request to add a new library to App that we need to consider. Please look at this GH and then vote :+1: or :-1: on accepting this new library or not.

GH_LINK

[roryabraham]

After researching this, one of the things I like about moize is how the base API is extremely simple, but it's also configurable for just about any kind of memoization you might need.

[puneetlath]

App as of now, does not have a standardized way of handling memoization outside of React. It boils down to using custom-made solutions like Maps or serialization. This is a problem because engineers implementing and reviewing solutions for memoization outside of React have to sink time into writing and creating a custom solution that may not be consistent with other places in the codebase. This leads to inconsistency, overhead, and at times sub-optimal solutions.

Can you give some examples of such scenarios and where we encounter this problem?

[roryabraham]

Can you give some examples of such scenarios and where we encounter this problem?

Here are just a few examples of different caching solutions we've built ad-hoc:

• https://github.com/Expensify/react-native-onyx/blob/main/lib/OnyxCache.ts

• App/src/libs/Localize/index.ts

  Lines 68 to 73 in f5cf3fb

  	const translationCache = new Map<ValueOf<typeof CONST.LOCALES>, Map<TranslationPaths, string>>(
  	Object.values(CONST.LOCALES).reduce((cache, locale) => {
  	cache.push([locale, new Map()]);
  	return cache;
  	}, [] as Array<[ValueOf<typeof CONST.LOCALES>, Map<TranslationPaths, string>]>),
  	);

• Cache measured sizes in ThumbnailImage #40643

• App/src/libs/PersonalDetailsUtils.ts

  Lines 27 to 32 in 22f6d15

  	emailToPersonalDetailsCache = personalDetails.reduce((acc: Record<string, PersonalDetails>, detail) => {
  	if (detail?.login) {
  	acc[detail.login.toLowerCase()] = detail;
  	}
  	return acc;
  	}, {});

• [HOLD #42200] Fix Intl NumberFormat calls #40341

A case study I was involved with can be found in this PR. The diff in src/libs/Localize/index.ts could have been reduced to this:

const translateMemoized = moize(translate);
export {translateMemoized as translate};

and lots of discussion could have been avoided with this standardization.

[janicduplessis]

Other case I added recently that could use this

App/src/components/ThumbnailImage.tsx

Line 19 in e8ae3c5

const thumbnailDimensionsCache = new Map<string, {width: number; height: number}>();

[puneetlath]

@kacper-mikolajczak can you please post this in the #expensify-open-source channel in problem/solution format along with all the information provided in the OP? And cc @app-deployers on it. Thank you!

[kacper-mikolajczak]

Sure thing @puneetlath! Here is a P/S post.

[tgolen]

Number of releases in the last year: 5

From looking at https://github.com/planttheidea/moize/releases, it looks like the last release was May 2023, last year. Correct?

[puneetlath]

Looks like the discussion is ongoing.

[kacper-mikolajczak]

Proposal

Intro

Goal of the proposal is to find API for a future memoization standard. By looking at the current use-cases in the codebase, we will try to distill minimal API surface which is going to be used for the implementation.

Examples

Lodash memoize helpers

There are two instances of lodash memoize helper in the app. Both of them are meant to be replaced with suggested solution.

The getEmojiUnicode helper has simple API with just one input, so it is going to be straightforward to replace it. No extra requirements.

App/src/libs/EmojiUtils.ts

Line 69 in 92a7b15

const getEmojiUnicode = memoize((input: string) => {

Same goes for second use-case which is getLocaleDigits.

App/src/libs/LocaleDigitUtils.ts

Line 16 in 92a7b15

const getLocaleDigits = _.memoize((locale: Locale): string[] => {

Intl-like modules

Intl as a module needs to be memoized in order not to re-create new instances on every call (especially when used in a iteration with same configuration, e.g. rendering list of components).

#40341

This use-case requires deep comparison of arguments, because options passed to Intl methods are possibly going to be the same across the app, but have different references.

Maps

Many samples of using Maps were provided, like ones below:

• App/src/libs/Localize/index.ts

  Line 67 in 92a7b15

  const translationCache = new Map<ValueOf<typeof CONST.LOCALES>, Map<TranslationPaths, string>>(

• https://github.com/Expensify/App/pull/40643/files

Based on those examples we would need to employ value caching mechanism (instead of function caching) which could be achieved by overloading memoize API or creating a separate like Cache.

Requirements

Here are additional concerns and requirements that were discovered either during discussion or exploration of similar solutions (moize, memoizee etc.):

• Very simple API like moize (just wrap the function, and that's it)
• Arbitrary data (strings, numbers, arrays, objects, maps) as input arguments
• Performance considerations regarding Array vs Map throughput
• Key serialization
  • Sensitive to order of arguments
  • Circular dependencies
• Cache invalidation strategy
• Original function static properties
  • It may be a good idea to copy static properties but based on current use-cases it is not needed see

Conclusion

Based on findings above and recent discussion, we can conclude that in simplest form the API could look like it:

type MapCacheConfig = {
    cacheMode: 'map';
    // Map keys are singlular values, so we cannot compare tuple of arguments directly. We are forced to serialize arguments and then compare strings.
    // It may be worth to choose if you anticipate large cache size.
    equalityCheck: 'deep';
};

type ArrayCacheConfig = {
    cacheMode: 'array';
    equalityCheck: 'shallow' | 'deep';
};

type CacheConfig = MapCacheConfig | ArrayCacheConfig;

type MemoizeConfig = {
    maxSize: number;
    maxAge: number;
    replacementPolicy: 'LRU';
} & CacheConfig;

type MemoizeStaticInstance<T> = {
    get: (key: string) => T | undefined;
    set: (key: string, value: T) => void;
    clear: () => void;
} 

type MemoizeInstance<Fn extends () => {}> = Fn & MemoizeStaticInstance<ReturnType<Fn>>;

type Memoize<Fn extends () => {}> = (f: Fn, config: MemoizeConfig) => MemoizeInstance<Fn>;
type Memoize<T> = (initialValue T[], config: MemoizeConfig) => MemoizeStaticInstance<T>;

In the next comment I will add information about possible implementation obstacles. Let me know what you think, thanks!

[melvin-bot]

@puneetlath Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mountiny]

@kacper-mikolajczak What is your ETA for this POC?

[kacper-mikolajczak]

Hi @mountiny I have posted POC here: #43538.

I will cross-post the update in the Slack thread as well - let me know what you think, thanks!

[puneetlath]

@kacper-mikolajczak what's the next step here?

[roryabraham]

@kacper-mikolajczak has a POC for a custom-build general purpose memoization tool (not moize) here. It went through some reviews but he's OOO this week so I assume it will pick back up next week

[puneetlath]

How's it going @kacper-mikolajczak?

[mountiny]

@puneetlath i believe this is now waiting for @roryabraham to check the changes made after his previous review

[kacper-mikolajczak]

Hi @puneetlath, exactly as @mountiny said 🫡

[roryabraham]

I reviewed and approved the PR last week. Looks like @kacper-mikolajczak has an updated TODO list here

[mountiny]

We are getting close, I think we can merge the PR tomorrow

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.12-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• General purpose memoization tool #43868

If no regressions arise, payment will be issued on 2024-08-02. 🎊

For reference, here are some details about the assignees on this issue:

• @kacper-mikolajczak does not require payment (Contractor)

[puneetlath]

Adding @mananjadhav as it looks like they were the C+ here.

[melvin-bot]

Payment Summary

Upwork Job

• Reviewer: @mananjadhav owed $250 via NewDot
• Contributor: @kacper-mikolajczak is from an agency-contributor and not due payment

BugZero Checklist (@puneetlath)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[puneetlath]

Payment summary:

• C+ - @mananjadhav - $250 - to be paid via NewDot.

[JmillsExpensify]

$250 approved for @mananjadhav