From e5ce4a4a429009c34476b02677b82ab6698e644c Mon Sep 17 00:00:00 2001 From: Matchu Date: Thu, 24 Sep 2020 07:07:05 -0700 Subject: [PATCH] improve perf on item add/remove MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We did this a while back too, but I guess something changed in Apollo: I guess it used to return identical item objects from the cache on its own, and now it returns brand new item objects. So we gotta do the object caching hacks ourselves! This speeds up add/remove item state updates from 500ms to 100ms on my Mac, because we stop re-rendering all the Item components and their complex Chakra children. This is especially worth doing now, because animations make long updates much more noticeable! (It interrupts the animation 😅) --- src/app/WardrobePage/Item.js | 2 +- src/app/WardrobePage/ItemsPanel.js | 11 ++++++--- src/app/WardrobePage/useOutfitState.js | 34 +++++++++++++++++++++++++- 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/src/app/WardrobePage/Item.js b/src/app/WardrobePage/Item.js index 4289cfe..64b2b67 100644 --- a/src/app/WardrobePage/Item.js +++ b/src/app/WardrobePage/Item.js @@ -86,7 +86,7 @@ function Item({ icon={} label="Remove" onClick={(e) => { - onRemove(); + onRemove(item.id); e.preventDefault(); }} /> diff --git a/src/app/WardrobePage/ItemsPanel.js b/src/app/WardrobePage/ItemsPanel.js index 3a1c265..488e528 100644 --- a/src/app/WardrobePage/ItemsPanel.js +++ b/src/app/WardrobePage/ItemsPanel.js @@ -117,6 +117,13 @@ function ItemZoneGroup({ } }; + const onRemove = React.useCallback( + (itemId) => { + dispatchToOutfit({ type: "removeItem", itemId }); + }, + [dispatchToOutfit] + ); + return ( @@ -136,9 +143,7 @@ function ItemZoneGroup({ !isDisabled && outfitState.wornItemIds.includes(item.id) } isInOutfit={outfitState.allItemIds.includes(item.id)} - onRemove={() => - dispatchToOutfit({ type: "removeItem", itemId: item.id }) - } + onRemove={onRemove} isDisabled={isDisabled} /> ); diff --git a/src/app/WardrobePage/useOutfitState.js b/src/app/WardrobePage/useOutfitState.js index ab56616..3a6b196 100644 --- a/src/app/WardrobePage/useOutfitState.js +++ b/src/app/WardrobePage/useOutfitState.js @@ -72,7 +72,39 @@ function useOutfitState() { } ); - const items = data?.items || []; + const resultItems = data?.items || []; + + // Okay, time for some big perf hacks! Lower down in the app, we use + // React.memo to avoid re-rendering Item components if the items haven't + // updated. In simpler cases, we just make the component take the individual + // item fields as props... but items are complex and that makes it annoying + // :p Instead, we do these tricks to reuse physical item objects if they're + // still deep-equal to the previous version. This is because React.memo uses + // object identity to compare its props, so now when it checks whether + // `oldItem === newItem`, the answer will be `true`, unless the item really + // _did_ change! + const [cachedItemObjects, setCachedItemObjects] = React.useState([]); + let items = resultItems.map((item) => { + const cachedItemObject = cachedItemObjects.find((i) => i.id === item.id); + if ( + cachedItemObject && + JSON.stringify(cachedItemObject) === JSON.stringify(item) + ) { + return cachedItemObject; + } + return item; + }); + if ( + items.length === cachedItemObjects.length && + items.every((_, index) => items[index] === cachedItemObjects[index]) + ) { + // Even reuse the entire array if none of the items changed! + items = cachedItemObjects; + } + React.useEffect(() => { + setCachedItemObjects(items); + }, [items, setCachedItemObjects]); + const itemsById = {}; for (const item of items) { itemsById[item.id] = item;