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.

Sign up for GitHub

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

Jump to bottom

EntityAdapter.getSelectors(selectState) has wrong typing #2751

Closed
bryanforbes opened this issue Oct 20, 2020 · 5 comments · Fixed by #4074
Closed

EntityAdapter.getSelectors(selectState) has wrong typing #2751

bryanforbes opened this issue Oct 20, 2020 · 5 comments · Fixed by #4074
Labels
Project: Entity

Comments

@bryanforbes
Copy link

Minimal reproduction of the bug/regression with instructions:

https://stackblitz.com/edit/ngrx-seed-wyr8ke?file=src%2Fapp%2Freducers%2Fusers.reducer.ts

At line 22 of app/reducers/users.reducer.ts of the above StackBlitz, the typing of the users selector is (state: UsersAppState) => Users[]. This isn't a big issue with applications, but is most notable in unit tests because MockStore.overrideSelector() requires a MemoizedSelector.

Expected behavior:

The typing of the users selector should be MemoizedSelector<UsersAppState, User[], DefaultProjectorFn<User[]>> when passing a selectState selector.

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s):

NgRx: 10.0.0
Angular: n/a

I would be willing to submit a PR to fix this issue

[x] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No
bryanforbes added a commit to bryanforbes/ngrx-platform that referenced this issue Oct 20, 2020
@bryanforbes
fix(entity): Improve typing of EntityAdapter.getSelectors()
2f19c12 
Fixes ngrx#2751
@bryanforbes bryanforbes mentioned this issue Oct 20, 2020
Closed
3 tasks
@bbaia
Copy link
Contributor

bbaia commented Dec 15, 2020

Same pb here when trying to use the MemoizedSelector.projector in tests.

@SanthoshBonala SanthoshBonala mentioned this issue Dec 31, 2020
Closed
@Lakston
Copy link

Lakston commented Jan 25, 2021

Is there anything we can do to help get this PR merged ? It would be nice to finally have those types fixed.

@twittwer
Copy link

Last time the ngrx team aimed to allow the overrideSelector to accept non memoized selector as well instead of adjusting the getSelectors. #2502

I hope someone can get it done this time - fingers crossed :)

@bdirito bdirito mentioned this issue Mar 17, 2021
Closed
@aymeric-duchein
Copy link

aymeric-duchein commented Apr 23, 2021 •
edited
Loading

Hi, as mentioned in #2978 the docs don't link to the correct interface.
I'll gladly update this PR with some update to the docs, but I can't find how to update it. Could someone point me in the good direction ? 😄

@twittwer twittwer mentioned this issue Oct 18, 2021
Closed
@cl-parsons
Copy link

Hi, as mentioned in #2978 the docs don't link to the correct interface.
I'll gladly update this PR with some update to the docs, but I can't find how to update it. Could someone point me in the good direction ? smile

Bump this issue for the documentation error.

markostanimirovic added a commit that referenced this issue Oct 18, 2023
@markostanimirovic
fix(entity): set correct return type for getSelectors signature with …
dfb89a9 
…parent selector

Closes #2751, #3172

BREAKING CHANGES:

Selectors returned by the `adapter.getSelectors` signature that accepts a parent selector are strongly typed.

BEFORE:

```ts
const {
  selectIds, // type: (state: object) => string[] | number[]
  selectEntities, // type: (state: object) => Dictionary<Book>
  selectAll, // type: (state: object) => Book[]
  selectTotal, // type: (state: object) => number
} = adapter.getSelectors(selectBooksState);
```

AFTER:

```ts
const {
  selectIds, // type: MemoizedSelector<object, string[] | number[]>
  selectEntities, // type: MemoizedSelector<object, Dictionary<Book>>
  selectAll, // type: MemoizedSelector<object, Book[]>
  selectTotal, // type: MemoizedSelector<object, number>
} = adapter.getSelectors(selectBooksState);
```
markostanimirovic added a commit that referenced this issue Oct 18, 2023
@markostanimirovic
fix(entity): set correct return type for getSelectors signature with …
b1ec6f2 
…parent selector

Closes #2751, #3172

BREAKING CHANGES:

Selectors returned by the `adapter.getSelectors` signature that accepts a parent selector are strongly typed.

BEFORE:

```ts
const {
  selectIds, // type: (state: object) => string[] | number[]
  selectEntities, // type: (state: object) => Dictionary<Book>
  selectAll, // type: (state: object) => Book[]
  selectTotal, // type: (state: object) => number
} = adapter.getSelectors(selectBooksState);
```

AFTER:

```ts
const {
  selectIds, // type: MemoizedSelector<object, string[] | number[]>
  selectEntities, // type: MemoizedSelector<object, Dictionary<Book>>
  selectAll, // type: MemoizedSelector<object, Book[]>
  selectTotal, // type: MemoizedSelector<object, number>
} = adapter.getSelectors(selectBooksState);
```
markostanimirovic added a commit that referenced this issue Oct 18, 2023
@markostanimirovic
fix(entity): set correct return type for getSelectors signature with …
3abe14a 
…parent selector

Closes #2751, #3172

BREAKING CHANGES:

Selectors returned by the `adapter.getSelectors` signature that accepts a parent selector are strongly typed.

BEFORE:

const {
  selectIds, // type: (state: object) => string[] | number[]
  selectEntities, // type: (state: object) => Dictionary<Book>
  selectAll, // type: (state: object) => Book[]
  selectTotal, // type: (state: object) => number
} = adapter.getSelectors(selectBooksState);

AFTER:

const {
  selectIds, // type: MemoizedSelector<object, string[] | number[]>
  selectEntities, // type: MemoizedSelector<object, Dictionary<Book>>
  selectAll, // type: MemoizedSelector<object, Book[]>
  selectTotal, // type: MemoizedSelector<object, number>
} = adapter.getSelectors(selectBooksState);
@markostanimirovic markostanimirovic mentioned this issue Oct 18, 2023
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Project: Entity
Projects
None yet
Milestone
No milestone
7 participants
@bryanforbes @bbaia @twittwer @Lakston @timdeschryver @aymeric-duchein @cl-parsons