PRESS 2 498 | Audit memoization candidates #157
Conversation
-> Sidebar Skeleton -> Checkbox Skeleton Removed console.log() from Live Preview
This reverts commit 9ff7324.
officiallygod
added
WIP
There was a problem hiding this comment.
Hey Allen, added one code comment and then these are a few things we need to take a look at
Top 3 Potential Memoization Opportunities (I am not an expert so might be wrong on a few of these)
-
The Common Layout used across onboarding is being re-rendered causing a domino effect re-rendering all its children as well?
-
Sidebars are rendered 3 times on every page + 1 render on open.
-
The drawer is being rendered multiple times?
Important Note
Can we check if the changes here are the correct way to use memo? If I am not wrong effective memoization flows starting from the parent to the child so that a small parent re-render does not result into a big expensive child re-render(like our previews and the common layout), please take a look at a few of these links that helped me understand memoization, https://dmitripavlutin.com/use-react-memo-wisely/, https://medium.com/@vitaliysteffensen/how-i-eliminate-all-unnecessary-rerenders-in-react-79505deeedea and WordPress/gutenberg#25780
Based on the note I am not sure if we have missed more potential opportunities, please do take a look at those other than the top 3 as well.
| @@ -1,7 +1,7 @@ | |||
| /** | |||
| * External dependencies | |||
| */ | |||
| import { get, isEmpty, kebabCase, pickBy, reduce, set } from 'lodash'; | |||
| import { get, isEmpty, kebabCase, pickBy, reduce, set, memoize } from 'lodash'; | |||
There was a problem hiding this comment.
Sorry for pointing this out a bit late, let's avoid adding more dependencies on lodash? It seems like Gutenberg has made a commitment to remove the lodash dependency from it's packages which we are keeping an eye on. I was going through some of the PR's there and found that https://www.npmjs.com/package/memize is being used as a replacement to the lodash memoize(attaching why in ref), can we explore this?
Ref:
Lint config of Gutenberg preventing lodash dependencies recommending memize: https://github.com/WordPress/gutenberg/blob/d9da1250c77d3dc083729b72b8143dfc7bc222ba/.eslintrc.js#L157
Why: WordPress/gutenberg#6686 (comment)
Removed unused imports Removed unused props
Added Memoized support for Sidebar
officiallygod
changed the title
This reverts commit 5a4655c.
Added a few improvements so as to reduce the number of rerenders in the commonly used pages throughout the Onboarding Module