Performance: find wallets first render optimizations #13139
Conversation
pettinarip
requested review from
wackerow,
corwintines,
minimalsm and
nhsz
as code owners
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -47,7 +43,6 @@ export const MobileFiltersMenu = ({ | |||
| onClose, | |||
| }: MobileFiltersMenuProps) => { | |||
| const { t } = useTranslation("page-wallets-find-wallet") | |||
| const { filteredWallets } = useWalletTable({ filters, t, walletData }) | |||
There was a problem hiding this comment.
Moved this hook to the parent (page component) to share the results and minimize the calls we do for it since its doing the main filtering.
| filteredWallets, | ||
| updateMoreInfo, | ||
| walletCardData, | ||
| } = useWalletTable({ filters, t, walletData }) |
There was a problem hiding this comment.
As mentioned before, moved this to the parent.
| @@ -232,6 +234,17 @@ const WalletTable = ({ | |||
| }) | |||
| } | |||
|
|
|||
| const showMoreInfo = (wallet) => { | |||
There was a problem hiding this comment.
Moved this function creation outside of the .map loop.
| @@ -120,20 +120,12 @@ export const useWalletTable = ({ | |||
| }, | |||
| ] | |||
|
|
|||
| const [walletCardData, setWalletData] = useState( | |||
| const [walletCardData, setWalletData] = useState(() => | |||
There was a problem hiding this comment.
Use the initialized function to run this map only once.
| return { ...wallet, moreInfo: false, key: wallet.name } | ||
| }) | ||
| ) | ||
| }, [walletData]) |
There was a problem hiding this comment.
Removed since walletData is not never going to change now that we build the list at build time. This reduces also one extra render that we had.
Lighthouse scores are calculated based on the latest audit results |
| }} | ||
| /> | ||
| {/* Use `Show` instead of `hideBelow` prop to avoid rendering the sidebar on mobile */} | ||
| <Show above="lg"> |
There was a problem hiding this comment.
This will add a bit of CLS on the table section on desktop versions. I think it is worth it considering that we are removing this from the render of slower mobile devices.
There was a problem hiding this comment.
Okay.. yeah, if we're preventing a large unnecessary chunk of data from loading then I think it's worth it.
Description
Refactor the find wallet logic to optimize the first render.
Current dev
Task 1: 550ms
Task 2: 2.2s - 2.7s
Task 3: 1.4s
With optimizations
Task 1: 500ms
Task 2: 1.8s
Task 3: 1.2s