Can we consider not removing selector with props?

[yzhe554]

Dear NGRX maintainers,

I have seen the recent changes for deprecating props from selector. #2980
The very first question is: do we need to write a factory selector when we want an argument? In another word, is props the same to arguments?
My understanding is No.
https://github.com/reduxjs/reselect/tree/v4.0.0#accessing-react-props-in-selectors
If you only display one single instance in the UI, you won't need a factory selector. It's adding a lot of complexities when you need to use it across multiple selectors/components. This is where props make things so easier.
If you will need to display multiple instances, like the example from reselect, yes, we cannot just use props here. This is where factory selectors come.
But it's really common in redux community that a better solution(rereselect) has been created.
https://redux.js.org/usage/deriving-data-selectors#re-reselect

I really suggest we reconsider this change.

[yzhe554]

This will also break backwards compatibility to all projects have used reselect and re-reselect with NGRX.
As an open source project, we probably should reconsider the change.
But if this has been deeply discussed within NGRX team, can we please make this publicly announced or documented into roadmap. @brandonroberts @MikeRyanDev @timdeschryver

[timdeschryver]

Hi @yzhe554 I didn't think of reselect and re-reselect.
As someone who doesn't use both libraries, am I wrong to assume that you don't use createSelector from the NgRx package, but that you're the createSelector method from reselect?

If that's the case, then how does this deprecation affect this?
If it's not the case, then I'm curious how you integrate both libraries with NgRx selectors.

[yzhe554]

@timdeschryver thanks for replying. I didn’t use ngrx/createSelector because reselect/createSelector had wider features
at Day one IIRC. You don’t need to do anything but just use createSelector from reselect lib. I’m sure ngrx createSelector is inspired from reselect. They key thing is select function can purely just expect a memoized selector and it doesn’t need to know its implementation details or it’s decoupled from createSelector function. At least it was setup like this in day one or from the day it started to support props. Deprecating/Removing props will break backwards compatibility(ngrx/select throws warnings/errors when you use props) and we probably should consider the suggestions from redux. I know it will have a learning curve but if that won’t be an issue for react devs who are using redux, maybe it can be resolved by better documentations and examples in Angular community.

[timdeschryver]

I see, thanks for the elaboration.
If I understand correctly, it will "just" be the types for the NgRx select method that will be broken.

[yzhe554]

Yes, the only other thing is, you don't have to always use selector factory. For example, I have a todo list and in the page, I have header and content both only display the the todo with id 1.

const makeGetTodo= id=> {
  return createSelector(
    getTodos,
    todos=> todos.byId[id]
  );
}

Firstly, if we run it twice, one in header and one in content components, we are holding two references while it's only one for props. Secondly, if I need to compose selectors, this will make all selectors to be selector factory, which will add boilerplate.

As you have mentioned in #2980, we only need to use a selector factory/re-reselect when we are displaying more than one item in the view. i.e

const getTodo = createSelector(
  getTodos,
  (state, props) => props.id,
  (todos, id) => todos.byId[id]
);

// running both at the same time, then we need a select factory or re-reselect
const todo1 = this.store.pipe(select(getTodo, {id: 1}));
const todo2 = this.store.pipe(select(getTodo, {id: 2}));

[Denhai]

#2104 ?

[yzhe554]

@Denhai yeh. We can also see people mentioned re-reselect in that issue.

[timdeschryver]

We're still in favor of removing selectors with props.
In the meantime, I'm going to convert this issue to a discussion.