From e5551a68477dcd9d80557d545c90bbe53296e960 Mon Sep 17 00:00:00 2001 From: Matchu Date: Mon, 24 May 2021 17:50:31 -0700 Subject: [PATCH] Fix performance regression in SpeciesColorPicker Oops, my cute API idea for `speciesPickerProps` breaks `React.memo`, of course! We could fix this by having the caller memoize the `speciesPickerProps` object, but that's too unusual and error-prone. We could also fix this by writing a custom function for `React.memo` to determine whether props match, but that seems like overkill when there's only one actual prop we're using here in practice. So yeah, I've updated `SpeciesColorPicker` to just accept `speciesTestId` and `colorTestId` props instead! Note that actually this component _is_ still re-rendering too often, because of a Chakra bug I just discovered and reported! So this change won't immediately improve performance, but it should stop re-rendering too often once we _also_ upgrade Chakra after this bug is fixed. https://github.com/chakra-ui/chakra-ui/issues/4080 --- src/app/WardrobePage/OutfitControls.js | 24 +++++++++--------------- src/app/components/SpeciesColorPicker.js | 8 ++++---- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/src/app/WardrobePage/OutfitControls.js b/src/app/WardrobePage/OutfitControls.js index 75fe9c9..83667f9 100644 --- a/src/app/WardrobePage/OutfitControls.js +++ b/src/app/WardrobePage/OutfitControls.js @@ -191,21 +191,15 @@ function OutfitControls({ - { - - } + diff --git a/src/app/components/SpeciesColorPicker.js b/src/app/components/SpeciesColorPicker.js index 3cd4c1b..79fd548 100644 --- a/src/app/components/SpeciesColorPicker.js +++ b/src/app/components/SpeciesColorPicker.js @@ -27,8 +27,8 @@ function SpeciesColorPicker({ isDisabled = false, speciesIsDisabled = false, size = "md", - speciesPickerProps = {}, - colorPickerProps = {}, + speciesTestId = null, + colorTestId = null, onChange, }) { const { loading: loadingMeta, error: errorMeta, data: meta } = useQuery(gql` @@ -190,7 +190,7 @@ function SpeciesColorPicker({ valids={valids} speciesId={speciesId} colorId={colorId} - {...colorPickerProps} + data-test-id={colorTestId} > { // If the selected color isn't in the set we have here, show the @@ -234,7 +234,7 @@ function SpeciesColorPicker({ valids={valids} speciesId={speciesId} colorId={colorId} - {...speciesPickerProps} + data-test-id={speciesTestId} > { // If the selected species isn't in the set we have here, show the