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

feat(Store): createSelector pass props to projector function #1210

Merged
merged 1 commit into from
Jul 30, 2018

Conversation

timdeschryver
Copy link
Member

This is a follow up of #1194.

I was writing the docs and found it a bit awkward I had to create a selector to only pass the props

const getTodosById = createSelector(
        getTodos,
        (state: TodoAppSchema, { id }: { id: number }) => id,
        (todos, id) => todos.find(todo => todo.id === id)
);

Therefor this PR, it also passes the props to the projector function.
Meaning the props will be passed through all the selectors + the projection function.

const getTodosById = createSelector(
        getTodosState,
        (todos: Todo[], { id }: { id: number }) =>
          todos.find(todo => todo.id === id)
 );

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 88.378% when pulling d8e8bac on timdeschryver:pr/projector-props into 35a4848 on ngrx:master.

props: any,
memoizedProjector: MemoizedProjection
): any {
const args = selectors.map(fn => fn(state, props));
return memoizedProjector.memoized.apply(null, args);
if (props === undefined) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This check is added because it would break some tests otherwise.
The tests that would fail are the ones that check what the projector is receiving.
I don't know if these are heavily tested, hence the check (I don't want to break any tests on existing code bases).

@brandonroberts brandonroberts merged commit b1f9b34 into ngrx:master Jul 30, 2018
@timdeschryver timdeschryver deleted the pr/projector-props branch August 1, 2018 20:08
@jpduckwo
Copy link

jpduckwo commented Aug 6, 2018

Any chance you could provide an example of how to pass props when using a selector? It's not in the docs...

e.g this.store.pipe(select(getToDosById ???))

@timdeschryver
Copy link
Member Author

@jpduckwo We will, most likely I'll open up a PR later this week.

@timdeschryver
Copy link
Member Author

@jpduckwo see #1233 feel free to give feedback.

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.

4 participants