Platform: Allow Selector to accept arguments besides state

Created on 25 Jun 2018  路  13Comments  路  Source: ngrx/platform

Thanks for this awesome library!

With Reselect, we can call a selector with arguments like below (almost copied from Reselect's README):

import { createSelector } from 'reselect'

const getVisibilityFilter = (state, props) =>
  state.todoLists[props.listId].visibilityFilter

const getTodos = (state, props) =>
  state.todoLists[props.listId].todos

const getVisibleTodos = createSelector(
  [ getVisibilityFilter, getTodos ],
  (visibilityFilter, todos) => {
    switch (visibilityFilter) {
      case 'SHOW_COMPLETED':
        return todos.filter(todo => todo.completed)
      case 'SHOW_ACTIVE':
        return todos.filter(todo => !todo.completed)
      default:
        return todos
    }
  }
)

const todos = getVisibleTodos(state, props)

but ngrx's selector does not seem to have such a feature. It will be helpful for me if it's possible.

Other information:

In the above example of Reselect, the props is not used in createSelector, but actually it also can be used like below:

import { createSelector } from 'reselect'

const shootingById = createSelector(
  [shootings,(state, shootingId) => shootingId],
  (shootings, shootingId) => head(shootings.filter(s => s._id === shootingId))
);

const shootings = shootingById(state, someId)

ref: https://github.com/reduxjs/reselect/issues/272#issuecomment-316754662

If accepted, I would be willing to submit a PR for this feature

[x] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

Store enhancement

All 13 comments

@brandonroberts
The point I wanted to say is not passing multiple selectors to createSelector, but passing arguments besides State (props in the example) to a selector created by createSelector.

const getUser = (state, id) => {
  return state.userLists[id]
}

const user = getUser(state, someId);

in this example, the selector function getUser cannot be created with ngrx's createSelector.

@brandonroberts
Hi, I would be grateful if you could reply.

What's your proposed change?

@brandonroberts
I've assumed these.
https://github.com/reduxjs/reselect/blob/master/test/test_selector.js#L141-L153
https://github.com/reduxjs/reselect/blob/master/test/test_selector.js#L201-L219

and update Selector interface like below:

interface Selector<T, V, P = {}> {
  (state: T, props?: P): V;
}

If this is something we want, I can provide a PR.

Seems reasonable to me. Let @tsugitta put in the PR if they still want to work this change.

thanks, let me try to submit the PR.

I got one ready tho 馃槄
@tsugitta you can checkout the commit if this is what you want you can let me know and I'll open a PR. If not you can submit a PR.

@timdeschryver
seems great, please open a PR.

Does documentation of the selectors reflect this change ? I tried to check there but couldn't find it out.

It's documented at https://ngrx.io/guide/store/selectors#using-selectors-with-props @Abhay-Joshi-Git . Are the docs missing something?

ok, thanks for sharing this.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

doender picture doender  路  3Comments

ghost picture ghost  路  3Comments

axmad22 picture axmad22  路  3Comments

gperdomor picture gperdomor  路  3Comments

sandangel picture sandangel  路  3Comments