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

example of the selector used in multiple component instances #1515

Open
anvk opened this issue Jan 29, 2020 · 12 comments
Open

example of the selector used in multiple component instances #1515

anvk opened this issue Jan 29, 2020 · 12 comments
Labels

Comments

@anvk
Copy link

anvk commented Jan 29, 2020

I'm aware that this is a bug tracker and not a support system. But there is one place in the official documentation which puzzles me. I just wonder if react-redux team has decided to pick one implementation over another for a specific reason or else.

https://react-redux.js.org/next/api/hooks#useselector-examples

const makeNumOfTodosWithIsDoneSelector = () =>
  createSelector(
    state => state.todos,
    (_, isDone) => isDone,
    (todos, isDone) => todos.filter(todo => todo.isDone === isDone).length
  )

const selectNumOfTodosWithIsDone = useMemo(
    makeNumOfTodosWithIsDoneSelector,
    []
  )

  const numOfTodosWithIsDoneValue = useSelector(state =>
    selectNumOfTodosWithIsDone(state, isDone)
  )

Can I ask why the team decided to give this example instead of curried function and partial application?

const makeNumOfTodosWithIsDoneSelector = () => isDone =>
  createSelector(
    state => state.todos,
    todos => todos.filter(todo => todo.isDone === isDone).length
  )

const selectNumOfTodosWithIsDone = useMemo(
    makeNumOfTodosWithIsDoneSelector,
    []
  )

  const numOfTodosWithIsDoneValue = useSelector(selectNumOfTodosWithIsDone(isDone))
  )

Is there some performance/cache implications? Or it is just easier to read and understand for everyone who is reading this documentation?

@markerikson
Copy link
Contributor

markerikson commented Jan 29, 2020

The example was contributed by a user as we were putting together the docs for the hooks API.

I'd say it's largely because most folks are used to seeing selectors explicitly called with someSelector(state, arg1, arg2).

To be honest, I don't like this entire example at all. The naming is overly long, and it doesn't feel like there's enough real memoizing going on here to justify the complexity.

@anvk
Copy link
Author

anvk commented Jan 29, 2020

@markerikson thanks a lot for your quick reply.

I totally agree that a more relevant and clear example would be more beneficial for developers. Honestly I have a more complex use case when you have a big state object and you need to extract slice of data by the object key property which is uuid. And this selector is used by many component instances which need to get their data slices from this state.
Took me few times to re-read this "Todo done/not-done example" to get an idea on what I need.

So in my case I wanted to double check that I'm not missing any performance optimization or hidden redux cache functionality if you pass a dynamic property into createSelector (e.g. (_, isDone) => isDone,). But in the end it is just a personal preference.

Is react-redux team planning to update documentation?

@markerikson
Copy link
Contributor

I'd like to come up with a better example, but my current focus is on reworking the Redux core docs (per reduxjs/redux#3592 ).

For reference, I have an extensive post on selectors at Using Reselect Selectors for Performance and Encapsulation. I'd really like that post turned into a complete docs page, but never gotten around to it.

If you or someone else would like to try putting together a better example for the docs, I'd be very interested.

@anvk
Copy link
Author

anvk commented Jan 29, 2020

@markerikson thanks a lot again. I will most definitely read the article you provided.

I will close this thread since it is pretty much resolved.

Also about the example - I can totally give it a shot in the next few days or over the weekend. Or suggest at my current company if anyone has a great example in mind.

Do you have a specific example in mind or you want people to suggest theirs?
Where do you want people to post their examples for a discussion? Maybe gist?

@anvk anvk closed this as completed Jan 29, 2020
@markerikson
Copy link
Contributor

We might as well keep this issue open to cover that discussion.

@markerikson markerikson reopened this Jan 29, 2020
@anvk
Copy link
Author

anvk commented Jan 30, 2020

@markerikson I will ask another very silly question here which is possibly would be a good idea to mention in the documentation as well. It is the difference between:

const { user, isFetching } = useSelector(state => state.profilePage, shallowEqual) // profilePage consists of { user, isFetching } in the redux state

vs

const user = useSelector(state => state.profilePage.user)
const isFetching = useSelector(state => state.profilePage.isFetching)

My sample is heavily inspired by the conversation described here #1459 Just instead of generic names I thought it might be more useful to be slightly more specific in this example.

@markerikson what would be a more idiomatic Redux way to select data in this case?

I see that the second approach is definitely more welcome for useState react hooks examples. Definitely highlights potentially re-usable chunks which could be put in their own re-usable custom hooks. But then I can also imagine of a case of getting 50 properties from the same state object... which at least will blow up code in sizes and making useSelector trying to get properties of the same object again and again (potential performance issue when we talking about large production data?).

@markerikson
Copy link
Contributor

The main question is whether there are any other fields in state.profilePage.

If those are the only two fields, and you want to re-render when either of those change, then sure, select the entire object and destructure.

However, say there many more fields in state.profilePage, but your component only cares about those two. With state => state.profilePage, the component will re-render when the profilePage object itself is a new reference (ie, when any of the fields have been updated, assuming correct immutable updates). With state => state.profilePage.user, it will only re-render when the .user field has been updated.

As I said in the other thread, always try to select the minimal amount of data needed by this component, and that is true for both useSelector and mapState.

@anvk
Copy link
Author

anvk commented Jan 30, 2020

oh crap, my bad! I forgot shallowEqual in my example above. Well when you have shallowEqual then there are no extra re-renders. In that case which method is more advisable?

@markerikson
Copy link
Contributor

In that case the results should be basically equivalent.

@anvk
Copy link
Author

anvk commented Jan 30, 2020

@markerikson but is there a more idiomatic preferred option? I see that there is more and more state splitting when people use useState instead of keeping one big state how it was before introduction of react hooks. So wonder if useSelector should also be suggesting this approach more. It is a silly question.

@markerikson
Copy link
Contributor

Once the behavior is equivalent, no, it's really up to you, same as writing one large useState with an object vs multiple individual useStates vs a useReducer.

@timdorr timdorr added the docs label Jan 31, 2020
@anvk
Copy link
Author

anvk commented Feb 7, 2020

@markerikson before even starting to write any code I just wanted to propose an idea:
I thought that an example of the company HRAddressBook would be a good example. App state will consist of an array of employee information: Avatar, First Name, Last Name, Position, Salary, Home Address, etc.
An example could start with a basic form and one entry to modify or add.

Then we could add a re-usable reducer for a single employee as well as introduce a new visual component Employee Card which suppose to show only limited information about the person (Avatar, First Name and Last Name). This new component will explain use of selectors, selectors which require access to specific values instead of the whole object (performance note).
Then we would face a problem of using the same selector for multiple components which requires using a selector factory.

Another idea I had is something related to map pins and extra meta data associated with them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants