Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce unnecessary calls to useSelector selector #1803

Conversation

sufian-slack
Copy link
Contributor

This change is intended to fix #1801

By checking if the store state has differed prior to recalculating a selector, we can avoid unnecessary selector recalculation in most cases for components on mount.

Currently, useSelector(selector) will call the passed selector twice on
mount when it only needs to be called once on mount. This is
unnecessary, and in the case of expensive selectors, can be a
performance concern.
By checking if the store state has differed prior to recalculating a
selector, we can avoid unnecessary selector recalculation in most cases
for components on mount.
@netlify
Copy link

netlify bot commented Aug 24, 2021

✔️ Deploy Preview for react-redux-docs ready!

🔨 Explore the source changes: 2ac8bfc

🔍 Inspect the deploy log: https://app.netlify.com/sites/react-redux-docs/deploys/61266edee810b800076e113d

😎 Browse the preview: https://deploy-preview-1803--react-redux-docs.netlify.app

@@ -246,6 +246,39 @@ describe('React', () => {

expect(renderedItems.length).toBe(1)
})

it('calls selector exactly once on mount and on update', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

✋ can you also add a test that does force the selector to run a second time? This should be doable by rendering a child component inside of a parent, and dispatching inside of a useLayoutEffect in the child. They mount in the same pass, and the child's useLayoutEffect should run before the parent's does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, added a test here and in the other PR with a useLayoutEffect that verifies the selector is re-run when state changes during initial mount

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, I'm about to head out on holiday and won't be checking back on this until Sep 7th, so don't be alarmed/think this is abandoned if I don't get back to any additional follow-ups. Thank you for all the help!

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, thanks!

@markerikson
Copy link
Contributor

Going in as well.

@markerikson markerikson merged commit 099e104 into reduxjs:7.x Sep 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants