Skip to content

Various adjustments to hooks docs #1293

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

Merged
merged 3 commits into from
May 25, 2019
Merged

Conversation

MrWolfZ
Copy link
Contributor

@MrWolfZ MrWolfZ commented May 25, 2019

Reading through the docs one more time, I noticed some things. Have a look at the line comments for more details on some of the adjustments.

@netlify
Copy link

netlify bot commented May 25, 2019

Deploy preview for react-redux-docs ready!

Built with commit 156a15c

https://deploy-preview-1293--react-redux-docs.netlify.com


If further performance optimizations are necessary, you may consider either wrapping your function component in `React.memo(MyFunctionComponent)`, or using `useMemo()` to memoize the render output of your component:
If further performance optimizations are necessary, you may consider wrapping your function component in `React.memo()`:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the useMemo suggestion since my performance tests during implementation of the hooks showed that the gains from useMemo are negligible.

Copy link
Contributor

Choose a reason for hiding this comment

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

The useMemo() aspect is almost like a parent component pseudo-wrapping its child in React.memo() or having the child implement shouldComponentUpdate. If React sees the exact same child element references in the same place, it will skip updating that subtree.

The performance benefits are entirely dependent on the size of that subtree.


> **Note**: For a longer description of this issue, see [this chat log that describes the problems in more detail](https://gist.github.com/markerikson/faac6ae4aca7b82a058e13216a7888ec), as well as [issue #1179](https://github.com/reduxjs/react-redux/issues/1179).

### Performance

As mentioned earlier, `useSelector()` will do basic shallow comparisons of return values when running the selector function after an action is dispatched. However, unlike `connect()`, `useSelector()` does not do anything to prevent your own function component from completing a re-render if the derived state has changed.
As mentioned earlier, by default `useSelector()` will do a reference equality comparison of the selected value when running the selector function after an action is dispatched, and will only cause the component to re-render if the selected value changed. However, unlike `connect()`, `useSelector()` does not prevent the component from re-rendering due to its parent re-rendering, even if the component's props did not change.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't really understand what this paragraph was trying to say, so I rephrased it.

@@ -401,7 +383,7 @@ export function useActions(actions, deps) {
return actions.map(a => bindActionCreators(a, dispatch))
}
return bindActionCreators(actions, dispatch)
}, deps)
}, deps ? [dispatch, ...deps] : deps)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bugfix for an issue I noticed with my original useActions implementation.

@@ -340,45 +340,27 @@ If you prefer to deal with this issue yourself, here are some possible options f

- Don't rely on props in your selector function for extracting data
- In cases where you do rely on props in your selector function _and_ those props may change over time, _or_ the data you're extracting may be based on items that can be deleted, try writing the selector functions defensively. Don't just reach straight into `state.todos[props.id].name` - read `state.todos[props.id]` first, and verify that it exists before trying to read `todo.name`.
- Because `connect` adds the necessary `Subscription` to the context provider, wrapping the component with `connect` (without any arguments, i.e. `connect()(MyComponent)` will keep those issues from occurring.
- Because `connect` adds the necessary `Subscription` to the context provider and delays evaluating child subscriptions until the connected component has re-rendered, putting a connected component in the component tree just above the component using `useSelector` will prevent these issues as long as the connected component gets re-rendered due to the same store update as the hooks component.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wrote a test for the original phrasing of this, and that version did not work at all. Therefore, I adjusted this and made it much more explicit about what is required for it to work.

@timdorr
Copy link
Member

timdorr commented May 25, 2019

LGTM. Thanks!

@timdorr timdorr merged commit 0640501 into reduxjs:master May 25, 2019
albertodev7 pushed a commit to albertodev7/react-redux that referenced this pull request Dec 8, 2022
* fix bug in useActions recipe in the hooks docs

* various adjustments to hooks docs

* correct a statement about preventing stale props
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.

3 participants