Skip to content
This repository has been archived by the owner on Jun 2, 2022. It is now read-only.

[FSSS-172] Fix/Adjust inappropriate rerenders #304

Merged
merged 2 commits into from
Feb 7, 2022

Conversation

ArthurTriis1
Copy link
Contributor

@ArthurTriis1 ArthurTriis1 commented Feb 4, 2022

What's the purpose of this pull request?

This PR fixes some inappropriate rerenders with memo function and removing peer dependencies from hooks. To observe this rerenders I used the React Developer Tools with the option "Highlight updates when components render" checked.

Screen Shot 2022-02-04 at 11 05 10

Before After
PLP plp-old plp
HOME home-old home
PDP pdp-old pdp

@ArthurTriis1 ArthurTriis1 force-pushed the feat/revisit-rerender branch 2 times, most recently from 368ea18 to 9388674 Compare February 4, 2022 13:39
@vtex-sites
Copy link

vtex-sites bot commented Feb 4, 2022

Preview is ready

This pull request generated a Preview

👀   Preview: https://preview-304--base.preview.vtex.app
🔬   Go deeper by inspecting the Build Logs
📝   based on commit 4f3e1c6

@ArthurTriis1 ArthurTriis1 changed the title feat: add memo in some componentes [FSSS-172] Add memo in some components Feb 4, 2022
@ArthurTriis1 ArthurTriis1 marked this pull request as ready for review February 4, 2022 14:59
@ArthurTriis1 ArthurTriis1 changed the title [FSSS-172] Add memo in some components [FSSS-172] Adjust inappropriate rerenders Feb 4, 2022
@ArthurTriis1 ArthurTriis1 changed the title [FSSS-172] Adjust inappropriate rerenders [FSSS-172] Fix/Adjust inappropriate rerenders Feb 4, 2022
Copy link
Contributor

@filipewl filipewl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Memo all the things

src/pages/{StoreProduct.slug}/p.tsx Show resolved Hide resolved
src/pages/{StoreCollection.slug}.tsx Show resolved Hide resolved
src/pages/{StoreCollection.slug}.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@danzanzini danzanzini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 👏 👏 Made it looks easy. Great job 🚀

@hellofanny
Copy link
Contributor

VERYY NICEE! 🥳 🥳 Well done!! 🚀

@filipewl
Copy link
Contributor

filipewl commented Feb 4, 2022

I also didn't know about that option for the React extension! Today I Learned.

@ArthurTriis1 ArthurTriis1 force-pushed the feat/revisit-rerender branch from 9388674 to c58d1b2 Compare February 4, 2022 18:13
Copy link
Member

@eduardoformiga eduardoformiga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 👏 👏 Heyyy, Amazing changes!
Very nice extractions into new hooks, good use of destructuring, etc.
The code looks more legible and better now! 🤩

I wonder if should we memo small components since maybe keeping the react algorithm rerender can be better for performance in this case. It's more recommendable memo medium to large components.
ref: https://dmitripavlutin.com/use-react-memo-wisely/#2-when-to-use-reactmemo

Signed-off-by: Arthur Andrade <arthurfelandrade@gmail.com>
Signed-off-by: Arthur Andrade <arthurfelandrade@gmail.com>
@ArthurTriis1 ArthurTriis1 force-pushed the feat/revisit-rerender branch from c58d1b2 to 4f3e1c6 Compare February 7, 2022 13:24
@ArthurTriis1 ArthurTriis1 merged commit bc7b751 into develop Feb 7, 2022
@ArthurTriis1 ArthurTriis1 deleted the feat/revisit-rerender branch February 7, 2022 13:39
@renatamottam renatamottam mentioned this pull request Feb 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants