ngneat/query

beforeEach with waitForAsync fails when trying to compile a component

cuddlecake opened this issue · 3 comments

Which @ngneat/query-* package(s) are the source of the bug?

query

Is this a regression?

No

Description

When trying to create a component and initially calling detectChanges as would be usually done:

beforeEach(waitForAsync(() => {
  TestBed.configureTestingModule({ declarations: [AppComponent] });
  TestBed.compileComponents();
  const component = TestBed.createComponent(AppComponent);
  component.detectChanges();
}));

All tests in the file will fail due to jest timeout.
It also doesn't matter if I attempt to detect changes outside of the beforeEach hook, because then only the first test will succeed, while all subsequent tests will fail.

Please provide a link to a minimal reproduction of the bug

https://github.com/cuddlecake/query/blob/main/packages/ng-query/src/lib/tests/component.spec.ts

Please provide the exception or error you saw

No response

Please provide the environment you discovered this bug in

No response

Anything else?

My minimal reproduction is as minimal as it gets, but I first stumbled on this when I tried to introduce @ngneat/query in my app with a proper use case, involving an HTTP Request.

Do you want to create a pull request?

No

I've revisited this and I can't remember why, but I've misunderstood why my tests fail.

It's still because of the waitForAll in beforeEach, but it happens when multiple tests run a mutation, and mutation.result$ is subscribed to:

@Component({ template: `{{ mutation.result$ | async | json }}` })
class AppComponent implements OnInit {
  mutation = inject(UseMutation)(() => of('hello world'));

  ngOnInit() {
    this.mutation.mutate('id')
  }
}

beforeEach(waitForAsync(() => {
  TestBed.configureTestingModule({declarations: [AppComponent] });
  TestBed.compileComponents();
}));

it('should work 1', () => {
  const component = TestBed.createComponent(AppComponent);
  component.detectChanges()
});

it('should work 2', () => {
  const component = TestBed.createComponent(AppComponent);
  component.detectChanges()
});

Which is why I think it might be similar to #82

luii commented

@cuddlecake did you found a fix in the meantime?

So what is happening here exactly? From what i can see is, that @tanstack/query is caching/memoizing the mutation under the hood and is returning the same instance you had in the should work 1 test. Since the result$ observable is saved on the MutationObserver (L44) it gets also saved/memoized/cached by @tanstack/query under the hood.

The logical reason then would be to clear all caches (with QueryClientService.clear()) and tearing down the entire TestBed to get a clean new MutationProvider for every test case. But when you run your Angular Test cases with ng test (the Angular CLI) then you dont need to add the waitForAsync and .compileComponents() to beforeEach(...), this is just boilerplate Angular generated for you so you could run your tests with the jest runner (or any other) for example. I would remove the waitForAsync and .compileComponents() boilerplate, it runs fine without it, but only if you dont need to do a custom buildup for your app.

BTW, i tried to apply the "fix" i used in #82 by adding shareReplay(1) which confirmed that its kinda similiar, if not the same issue. But this shouldnt be used as a "fix" since everytime you'd rebuild the TestBed for every test case, you would also chain a new shareReplay(1) to the result$ operator (looking like this then: result$.shareReplay(1).shareReplay(1).... and so on). This would work, but in the long run maybe slow down you pipe.

I didn't find or search for a fix, since I couldn't introduce this library at work yet.
Also, waitForAsync and .compileComponents() is difficult to omit, since we make heavy use of ngneat/spectator here, and it involves both of these functions

And I don't feel like explaining the nuances of shareReplay as a workaround to my colleagues 😄 I might try again at some point, to introduce this lib at work, but not allow usage of useMutation.