[Performance] [Audit] withLocalizeHOC recreates props at each render, causing extraneous commits

[jsamr]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

This report is part of #3957, scenario "Rendering Individual chat messages".

Commit Log

The full commit log can be inspected with Flipper or React Devtools, see #3957 (comment) for instructions.

I noticed that SidebarLinks component would often rerender. If you look at c72 and select SidebarLinks, you'll notice it's rendered 6 times during the session. 3 of those 6 commits are caused by props from withLocalizeHOC, see screenshot below:

Cause

withLocalize is a central HOC used massively throughout the App. Yet, each prop it passes (translate, numberFormat, timestampToRelative...) is an inline function recreated on each render.

https://github.com/Expensify/Expensify.cash/blob/92429b5411bc8d050e2aae157dae27da2a3d0f68/src/components/withLocalize.js#L33-L46

Proposal 1: use memoization

Make sure those props are memoized via a class component bound methods or hooks.

Proposal 2, test rendering performance of withLocalizeHOC

Test performance-critical withLocalizeHOC with react-performance-testing to enforce rendering metrics, such as number of renders in controlled conditions.

EDIT: added Proposal 2

[MelvinBot]

Triggered auto assignment to @alexpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[parasharrajat]

Wow! nice catch. useMemo or 'useCallback' will do the trick.

Edit:
Just checked, I think this will make a big impact.

[MelvinBot]

Triggered auto assignment to @NikkiWines (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[marcaaron]

Looks pretty straight forward and has an easy solution. Curious to see how many net re-renders we could prevent by memoizing these callbacks. Should be a good metric to justify implementing.

[jsamr]

Curious to see how many net re-renders we could prevent by memoizing these callbacks. Should be a good metric to justify implementing.

@marcaaron That would be very cumbersome to investigate, but not impossible! However we know that this HOC is referenced in 90 files, I believe the impact will be pretty significant!

[MelvinBot]

Triggered auto assignment to @NicMendonca (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[marcaaron]

That would be very cumbersome to investigate, but not impossible

For sure, apologies, to be clear I wasn't asking for you to do this - just that it would be interesting to see data :)

[MelvinBot]

Triggered auto assignment to @NikkiWines (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[NicMendonca]

Posted to Upwork: https://www.upwork.com/jobs/~012f859a8ff2c9153a

[marcaaron]

Sorry, I'm taking this over 😈

[MelvinBot]

@NikkiWines, @NicMendonca Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[parasharrajat]

I think we should remove the Help Wanted label and close this issue as well. The fix is patched.

[MelvinBot]

@marcaaron Still overdue 6 days?! Let's take care of this!

[marcaaron]

This issue is on staging now so bumping down to weekly it should be fixed soon.

[marcaaron]

This can be closed I think.