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

Error: 1 periodic timer(s) still in the queue when testing a store with fakeAsync #3573

Closed
1 of 2 tasks
amakhrov opened this issue Sep 16, 2022 · 2 comments · Fixed by #3580
Closed
1 of 2 tasks

Error: 1 periodic timer(s) still in the queue when testing a store with fakeAsync #3573

amakhrov opened this issue Sep 16, 2022 · 2 comments · Fixed by #3580

Comments

@amakhrov
Copy link
Contributor

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

component-store

Minimal reproduction of the bug/regression with instructions

it("doesn't work", fakeAsync(() => {
  new ComponentStore<{}>();
  expect(1).toEqual(1);
}));

Running this unit test produces

Error: 1 periodic timer(s) still in the queue.

Minimal reproduction of the bug/regression with instructions

The test above should pass.

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

NgRx: 14.0.2
Angular: 14.2

Other information

I believe it's a regression introduced with the lifecycle hooks. In particular, the run-time check in this PR: https://github.com/ngrx/platform/pull/3432/files#diff-dd156bfdaf294eb2eb0c9c87e1403ddd40190b88afa6ed8b443fb7cb1cd008f6R322

This check uses rxjs asyncScheduler.schedule(), which internally uses setInterval - a periodic task that offends fakeAsync :)
https://github.com/ReactiveX/rxjs/blob/6.x/src/internal/scheduler/AsyncAction.ts#L50

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

  • Yes
  • No
@markostanimirovic
Copy link
Member

Hi @amakhrov,

Replacing asyncScheduler with asapScheduler here (scheduling lifecycle hooks check to the microtask queue instead of macrotask) should solve this issue. Do you want to open the PR?

@amakhrov
Copy link
Contributor Author

Yep, here is the PR: #3580

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.

2 participants