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

Selectors in MockStore not reset even with destroyAfterEach #4420

Open
2 tasks
sbarfurth opened this issue Jul 8, 2024 · 2 comments
Open
2 tasks

Selectors in MockStore not reset even with destroyAfterEach #4420

sbarfurth opened this issue Jul 8, 2024 · 2 comments

Comments

@sbarfurth
Copy link

sbarfurth commented Jul 8, 2024

Which @ngrx/* package(s) are the source of the bug?

store

Minimal reproduction of the bug/regression with instructions

Reproduction in sbarfurth/selector-reset-repro.

Run ng test to reproduce.

Expected behavior

All tests pass.

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

NgRx: v18.0.1
Angular: v18.0.7
Node: v20.15.0
Browser: Chrome v126
OS: macOS Sonoma 14.5

Other information

The automagic reset of the MockStore was removed with v13 as a breaking change. The migration guide states that this is a non-issue for tests using destroyAfterEach: https://ngrx.io/guide/migration/v13#testing-reset-mock-store

The overrideSelector method of MockStore sets the value on the memoized selector itself using setResult when not using a string. This value will leak between tests regardless of whether the TestBed is torn down or not since the memoized selector is only imported once per test suite.

import {someSelector} from './selectors';

// Test 1
mockStore.overrideSelector(someSelector, 10);
// calls someSelector.setResult(10)

// Reset TestBed
testBed.resetTestEnvironment();
// The imported memoized selector is not touched by this.

// Test 2
mockStore.select(someSelector);
// simply retrieves the result in selector (10)

This can be remedied by manually calling resetSelectors on the MockStore since that calls release and clearResult for every selector.

It seems the migration guide is incorrect and that this implication of overrideSelector is undocumented.

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

  • Yes
  • No
@timdeschryver
Copy link
Member

@sbarfurth maybe that the v13 migration guide isn't really clear/specific about this.

Destroying the TestBed has little to do with resetting the cached value of a selector.
To reset the selectors you must use resetSelectors.

getTestBed().inject(MockStore, null).resetSelectors();

The problem occurs when relying on Angular to teardown the Store.
While doing so, resetSelectors loses its context and cannot reset previously set selectors.

@sbarfurth
Copy link
Author

Yes, I realize this. However, the migration guide suggests that the behavior is somehow different or a non-issue when using destroyAfterEach. It seems to me that simply isn't true? As far as I can tell this has no bearing on whether or not you need to reset selectors. I'm only asking if it wouldn't make sense to update the migration guide since I think the current version is unclear at best. It might even be incorrect.

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

2 participants