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

Issue with MatTable using MatSort #568

Closed
Dji75 opened this issue May 18, 2021 · 8 comments · Fixed by #582
Closed

Issue with MatTable using MatSort #568

Dji75 opened this issue May 18, 2021 · 8 comments · Fixed by #582

Comments

@Dji75
Copy link
Contributor

Dji75 commented May 18, 2021

I have a problem with ngMocks, MatTableModule and MatSortModule.

If I have the mat-table sort initialization in the ngOnInit (or ngAfterViewInit, like Angular Material documents), I've got the following error:

  TypeError: You provided 'undefined' where a stream was expected. You can provide an Observable, Promise, Array, or Iterable.

Component's code:

  ...
  datasource = new MatTableDataSource<object>([]);
  @ViewChild(MatSort, { static: true }) mySort: MatSort = new MatSort();

  ngOnInit(): void {
    this.datasource.sort = this.mySort;
  }
  ...

If I move the sort initialization in the constructor, it works.
I can use my init in the constructor but on my point of view, it would also work if initialization is present in ngOnInit, right ?

@satanTime
Copy link
Member

satanTime commented May 18, 2021

Hi @Dji75,

thanks for the report.

I would assume that in ngOnInit or ngAfterViewInit, @ViewChild has an affect, whereas in ctor new MatSort();.
Looks like MatSort is mocked, therefore ViewChild gets it.
Because all mocked things don't return streams - the test fails.

Do you know which property is undefined?

I would suggest to use MockInstance or ngMocks.defaultMock here:

it('asadfsadf', () => {
  MockInstance(MatSort, 'streamPropWeNeedToStub$', EMPTY);
  const fixture = MockRender(CompoenntWithMatSort);
});

or in test.ts, then it will be EMPTY for all tests by default (it can be still overwritten with MockInstance in a test):

ngMocks.defaultMock(MatSort, () => ({
  streamPropWeNeedToStub: EMPTY,
}));

@Dji75
Copy link
Contributor Author

Dji75 commented May 20, 2021

It was initialized, with this code it works:

      ngMocks.defaultMock(MatSort, () => ({
        initialized: EMPTY,
      }));

@Dji75
Copy link
Contributor Author

Dji75 commented May 20, 2021

It seems all material components which implements HasInitialized would have same issue with default mock.

@satanTime
Copy link
Member

satanTime commented May 20, 2021

Right, this is the biggest downside of mocks in my option.
Currently it is impossible to detect which properties / functions should return observables / promises, in other words to detect their return type at runtime.

Even more, it is not always possible to detect all the properties a js class has. Thanks God, that a class prototype has all methods at least.

That's why ngMocks.defaultMock was invented. The idea was that devs would define in a single place all observable / promise types of mock properties / methods they need to handle.

Maybe we can do a kind of preset for well known libraries and their declarations so MatSort.initialized would be EMPTY by default, but I'm not sure how to collect all the data for it.

@satanTime
Copy link
Member

I think for now I could extend ngMocks.defaultMock in order to accept an array of classes. So there wouldn't be a need to copypaste the same code again and again.

ngMocks.defaultMock([MatSort, MatResort, MatCurort], () => ({
  initialized: EMPTY,
}));

Would it help? what are you thoughts on that?

@Dji75
Copy link
Contributor Author

Dji75 commented May 21, 2021

It's always a good idea to mutualize code wherever it make sense, and this is the case here. Looks good for me ! 👍

@satanTime
Copy link
Member

Hi @Dji75,

thanks for the feedback. I'm releasing these changes, they will be published the next hour.

The next step for me is to solve the issue you mentioned in another ticket, regarding store. Would be nice if you could find time to provide an example there.

Thank you in advance and happy coding!

@satanTime
Copy link
Member

v12.0.1 has been released and contains a fix for the issue. Feel free to reopen the issue or to submit a new one if you meet any problems.

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