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

Mockstore selector override with undefined value #2244

Closed
peterreisz opened this issue Nov 12, 2019 · 2 comments · Fixed by #2270
Closed

Mockstore selector override with undefined value #2244

peterreisz opened this issue Nov 12, 2019 · 2 comments · Fixed by #2270

Comments

@peterreisz
Copy link
Contributor

Minimal reproduction of the bug/regression with instructions:

interface MyFeatureState {
  myProp?: string;
}

const selectMyFeature = createFeatureSelector<MyFeatureState>('my-feature');

const selectMyProp = createSelector(selectMyFeature,
  (state: MyFeatureState) => state.myProp);

TestBed.configureTestingModule({
  providers: [provideMockStore()],
}).compileComponents();

const store: MockStore<any> = TestBed.get(Store);
const mockSelector = store.overrideSelector(selectMyProp, undefined);

mockSelector.setResult(undefined);
store.refreshState();

store.select(selectMyProp).subscribe(val => expect(val).toBe(undefined))

Expected behavior:

User should be able to override selector result with undefined too, at this moment undefined value is indicating that the selector result is not overwritten, thus mockstore will try to run all the selector logic.

This would be useful in case of a deep state contains an optional value and the user do not want to define the whole state tree in the parameter of the provideMockStore method. As a workaround setResult can be called with null as any, but projects can ban null keyword via a tslint rule, thus it might requires additional lint ignore comment too.

NgRx should use a different variable for indicating is the selector value overwritten.

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

NgRx: 8.4.0

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

@Maximaximum
Copy link
Contributor

Wow, I had to spend a couple of hours in order to find this issue and realize that overrideSelector() does not properly handle the undefined value. Indeed, this is a huge problem and has to be fixed

@alex-okrushko
Copy link
Member

Yeah, I knew about this issue, and still spent a few hours debugging... 🤦‍♂️

brandonroberts pushed a commit that referenced this issue Jan 8, 2020
Closes #2244 

BREAKING CHANGE:

BEFORE:

Using `mockSelector.setResult(undefined)` resulted in clearing the
return value.

AFTER:

Using `mockSelector.setResult(undefined)` will set the return value of
the selector to `undefined`.
To reset the mock selector, use `mockSelector.clearResult()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants