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

Make it possible to use MockStore's overrideSelector method against selectors given by entityAdapter.getSelectors() #3172

Closed
dil-btarkovacs opened this issue Oct 10, 2021 · 3 comments

Comments

@dil-btarkovacs
Copy link

dil-btarkovacs commented Oct 10, 2021

Currently if a selector (booksSelector in this example) is created in the following way (using entityAdapter's getSelectors method):

export const booksAdapter = createEntityAdapter<Book>();
const { selectAll, selectTotal } = booksAdapter.getSelectors((state: BooksFeature) => state.books);
export const booksSelector = selectAll;

then it's impossible to use the overrideSelector method in a unit test, eg.:

let store = spectator.inject(MockStore);
let mockBooksSelector = store.overrideSelector(booksSelector, testBooks);
//...
mockBooksSelector.setResult(otherTestBooks)

Because the overrideSelector expects a Selector as its first parameter.

I think this makes us writing unnecessary workarounds in unit tests.
3 solutions seem possible:
1: Extend the the acceptable types of the first parameter of MockStore's overrideSelector.
2: Extend the MockStore with an overrideTypelessSelector method that is similar to the original overrideSelector but it accepts these kind of selectors as first param.
3: Modify the EntityAdapter's getSelectors method to return with selectors that implement Selector or MemoizedSelector.

Describe any alternatives/workarounds you're currently using

Currently I can't test the connection only between a state selector and a linked variable.
One of the (unwanted) workarounds is that we can change the state through the MockStore (or the entityAdapter) to trigger a change in the desired selector's value.
Another solution is to wrap the selectAll into a MemoizedSelector with createSelector.
Aaaaand the best workaround is:

export const booksSelector = createSelector(
    selectBooksState,
    (books) => Object.values(books.entities)
);

:D

Other information:

Same problem is detailed on stackoverflow: https://stackoverflow.com/questions/69519000/in-unit-test-how-to-override-an-ngrx-selector-which-is-created-by-entityadapter

If accepted, I would be willing to submit a PR for this feature
(I mean I can try)

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

@smashthestate
Copy link

smashthestate commented Oct 14, 2021

I'm no expert in the inner workings of ngrx, but solution 3 seems to make the most sense - in fact, I kind of expected it to be the case.

@twittwer
Copy link

relates #2751

markostanimirovic added a commit that referenced this issue Oct 18, 2023
…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
…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
…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
Copy link
Member

closed by #4074

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

No branches or pull requests

4 participants