Comments: fix memoization of selectors #7227

Closed

infinite-persistence wants to merge 3 commits from ip/memo into master

infinite-persistence commented

2021-10-05 10:15:19 +02:00

(Migrated from github.com)

Jessop, this serves as a one-time review for all upcoming PRs that are similar, so I don't need to bug you every time.

Pretty much what we've discussed in Slack: general idea is to ensure we don't re-calculate heavy items and return new references:

Ensure selectors uses stashed value from createSelector.
For selectors where the input differs between instances (e.g. props.uri passed in), use the suggested factory method to create a unique selector per instance.
At the React side, we can remove any deep-compare work done in React.memo or shouldComponentUpdate to skip renders, since the default shallow compare would suffice.

See commit messages, especially the last one, regarding a quick solution to avoid repetition in CommentReactions.

For this specific example, the Comment component renders 4-5 times because it thinks the othersReacts object is different, when it actually only updated once. This is just 1 comment alone.

After the changes, the number of renders is cut down to 2 -- one for initial render (can't avoid), and the next one when reactions are fetched:

Jessop, this serves as a one-time review for all upcoming PRs that are similar, so I don't need to bug you every time. Pretty much what we've discussed in Slack: general idea is to ensure we don't re-calculate heavy items and return new references: 1. Ensure selectors uses stashed value from `createSelector`. 2. For selectors where the input differs between instances (e.g. `props.uri` passed in), use the suggested factory method to create a unique selector per instance. 3. At the React side, we can remove any deep-compare work done in `React.memo` or `shouldComponentUpdate` to skip renders, since the default shallow compare would suffice. ---- _See commit messages, especially the last one, regarding a quick solution to avoid repetition in `CommentReactions`._ For this specific example, the `Comment` component renders 4-5 times because it thinks the `othersReacts` object is different, when it actually only updated once. This is just 1 comment alone. <img src="https://user-images.githubusercontent.com/64950861/135984637-7fc620d5-24da-4d77-9836-d6f2d571a6c1.png" width="500"> After the changes, the number of renders is cut down to 2 -- one for initial render (can't avoid), and the next one when reactions are fetched: <img src="https://user-images.githubusercontent.com/64950861/135984802-22260ae2-8bf4-42d5-8902-40e1ec17fdaa.png" width="500">

Comments: fix memoization of selectors #7227

Close

Pull request closed