ngrx
Factory selector won't change using overrideselector and setResult on tests
Minimal reproduction of the bug/regression with instructions
https://stackblitz.com/edit/angular-bbsczu-qiwqy3?file=src%2Fapp%2Fapp.component.spec.ts
Minimal reproduction of the bug/regression with instructions
Using factory selectors, we expect to be able to use overrideSelector and setResult to change the result of the selector. For example to update a template in test context.
As you can see on reproduction, based on test template given on ngrx doc website, we are able to change a standard selector result without any problem. The UI reacts properly to the change.
But when it comes to factory selectors, it seems some kind of memoization occurs and the result cannot be changed on the fly. The repro is kind of dumb cause it's very unlikely you set the factory parameter as we did in repro but having it more dynamically, for example inside an effect rxjs pipeline produces the same errors at test levels.
Scenario
We define a factory selector:
export const selectBookById = (id) => createSelector(
selectBooks,
(books) => {
return books.find((book) => book.id === id);
}
);
In component we use a simple factory selector:
favoriteBook$ = this.store.pipe(select(selectBookById('thirdId')), map(book => book.id));
amazingBook$ = this.store.pipe(select(selectBookById('firstId')), map(book => book.id));
<h2>My Favorite Book by Id</h2>
<div class="my-favorite">
{{ favoriteBook$ | async }}
</div>
<h2>The amazing Book by Id</h2>
<div class="amazing">
{{ amazingBook$ | async }}
</div>
In tests we used initial overrideSelectors to set the value:
mockFavoriteBookSelector = store.overrideSelector(
selectBookById('thirdId'),
{
id: 'thirdId',
volumeInfo: {
title: 'Third Title',
authors: ['Third Author'],
},
}
);
mockAmazingBookSelector = store.overrideSelector(
selectBookById('firstId'),
{
id: 'firstId',
volumeInfo: {
title: 'First Title',
authors: ['First Author'],
},
}
);
Then we assert against values, changes the selector results, and assert again. Value doesn't change.
expect(
fixture.debugElement.query(By.css('.my-favorite')).nativeElement
.textContent
).toContain('thirdId');
expect(
fixture.debugElement.query(By.css('.amazing')).nativeElement.textContent
).toContain('firstId');
mockFavoriteBookSelector.setResult({
id: 'secondId',
volumeInfo: {
title: 'Second Title',
authors: ['Second Author'],
},
});
mockAmazingBookSelector.setResult({
id: 'fourthId',
volumeInfo: {
title: 'Fourth Title',
authors: ['Fourth Author'],
},
});
store.refreshState();
fixture.detectChanges();
expect(
fixture.debugElement.query(By.css('.my-favorite')).nativeElement
.textContent
).toContain('secondId');
expect(
fixture.debugElement.query(By.css('.amazing')).nativeElement.textContent
).toContain('fourthId');
Result:
Error: Expected 'thirdId ' to contain 'secondId'.
Expected 'firstId ' to contain 'fourthId'.
Please not not using refreshState produces the same results.
Versions of NgRx, Angular, Node, affected browser(s) and operating system(s)
"@angular/animations": "^13.1.1",
"@angular/cdk": "^13.1.1",
"@angular/common": "^13.1.1",
"@angular/compiler": "^13.1.1",
"@angular/core": "^13.1.1",
"@angular/forms": "^13.1.1",
"@angular/material": "^13.1.1",
"@angular/platform-browser": "^13.1.1",
"@angular/platform-browser-dynamic": "^13.1.1",
"@angular/router": "^13.1.1",
"@ngrx/component-store": "^13.0.2",
"@ngrx/effects": "^13.0.2",
"@ngrx/entity": "^13.0.2",
"@ngrx/router-store": "^13.0.2",
"@ngrx/store": "^13.0.2",
"rxjs": "~7.4.0",
"tslib": "^2.0.0",
"zone.js": "~0.11.3"
...
"typescript": "4.5.4"
node: >=16.14.2 test engine: jasmine or jest produces the same error os: all
Other information
It worked very well in selectors with props, as overrideSelector didn't have to be aware of param, and result would be mocked as we wish.
Please note the doc doesn't mention the new factory syntax anywhere in testing sections.
I would be willing to submit a PR to fix this issue
- [ ] Yes
- [X] No
Every time you call your factory selector, a new selector is created with a new reference.
Therefor, you cannot use overrideSelector to override the result of the factor selector, because it will have a different reference.
Here's an answer I provided to a similar thread: https://github.com/ngrx/platform/discussions/3279#discussioncomment-1920144. There's also other information + suggestions to check out in that discussion.
In the reply to this question https://github.com/ngrx/platform/discussions/3279#discussioncomment-2175881 I wrote a more detailed explanation of the problem.
If we cannot use overrideSelector, we would need a reliable and documented way to achieve this behavior in a reproductible way. Using initial mockStore argument is not something that will work in most cases where things changes a lot within a component as an example.
As this is the newly recommended way of proceeding, not being able to test and change results along tests seems more like a bug than a case for discussion to us.
As mentioned in the discussion, you can mock the factory method to return a selector (or selector like) that returns the expected result.
You're right that this should be in the docs, if you want you can create a PR for this.
Any way NGRX can produce some testing tooling similar to overrideSelector to allow having something that work in all cases and not test framework dependant?
Yeah, we'll look into this on our side and hope to come with an exportable solution.
Given that we can use createSelector to create memoized functions, the we can create memoized factories:
it('should memoize the result', () => {
const selectCollection = createSelector(() => undefined, _ => ({ '1': { name: 'David' }, '2': { name: 'Craig' } } as Record<string, { name: string }>));
// Using a function that returns a new selector each time means we can't override it
const selectById = (id: string) => createSelector(selectCollection, collection => collection[id]);
expect(selectById('1')).not.toEqual(selectById('1'));
// But we can memoize the factory so that it returns the same selector given the same input
const selectByIdMemoized = createSelector((id: string) => id, id => createSelector(selectCollection, collection => collection[id]));
expect(selectByIdMemoized('1')).toEqual(selectByIdMemoized('1'));
});
If the selector in the issue was declared as so:
export const selectBookById = createSelector(
id => id,
id => createSelector(
selectBooks,
(books) => {
return books.find((book) => book.id === id);
}
);
Then this would since selectBookById('thirdId') would always return the same selector:
store.overrideSelector(
selectBookById('thirdId'),
{
id: 'thirdId',
volumeInfo: {
title: 'Third Title',
authors: ['Third Author'],
},
}
);
This solution seems to work as intended. Is there any drawback writing factory selectors this way?
https://stackblitz.com/edit/angular-bbsczu-qiwqy3?file=src%2Fapp%2Fapp.component.spec.ts,src%2Fapp%2Fstate%2Fbooks.selectors.ts
- Is more verbose
- Is a little bit of an abuse of
createSelectorsince we are using it to create a memoized function that returns a selector - The input must be a single parameter, so you cannot create a factory with multiple parameters (instead, you would have to pass in multiple properties in a single object to the factory)
Idk if the approach should be recommended. But, given some mechanism is used to memoize the selector returned by the factory, then testing would be made simpler.
If you're already using NgRx in your project, then using createSelector for memoization could be OK.
Maybe NgRx should expose the memoization function it uses under-the-hood inside selectors for use in creating factories. Or maybe NgRx should publish some function distinct from the old "props" selector api that makes factories easier.