fix(table): schematic generates code that throws an exception

[Splaktar]

fix exception when instantiating MatPaginator before dataSource defined
fix ExpressionChangedAfterItHasBeenCheckedError caused by above

• first initialize the template w/o dataSource & w/ a sort and paginator
• then get the references to the template's MatSort and MatPaginator
• then set the data source's sort and paginator
• then set the table's data source from the component

Fixes #15788

Not the same issue, but this is also related to #15791.

[jelbourn]

@andrewseguin PTAL

[eugene-milostivenko]

Any fixes? The problem is still relevant.

[jelbourn]

@Splaktar can you rebase?

@andrewseguin can you review?

[Splaktar]

Rebased ✅

[andrewseguin]

It seems it would be easier to just add [length]="dataSource?.data.length" as the fix here and we can have another PR that changes the data source structure if we want to improve how it flows.

I'm not a fan of providing the data source as an input to the table in the code rather than the template, and the data source code in general is confusing because it tries to be a mix of client-side and server-side data.

Also, this is particular is confusing to read and doesn't seem to be the best way to show users how to use the datasource:

return merge(...dataMutations).pipe(map(() => {
      return this.getPagedData(this.getSortedData([...this.data]));
    }));

[Splaktar]

@andrewseguin I tried to explain it a bit in the commit, but it may be too brief. I had discussed this a good deal with Paul while working on it.

Just making the [length]="dataSource?.data.length" change causes an ExpressionChangedAfterItHasBeenCheckedError to be thrown. That can be worked around by using ChangeDetectorRef.detectChanges() but that didn't seem like a good practice to be using in a schematic.

[jelbourn]

Yeah, people should extremely rarely call detectChanges in real code

[Splaktar]

@andrewseguin I agree that the snippet of RxJS that you posted is not an easily readable piece of code as it uses a lot of concepts that may not be well known by every developer. However, that wasn't introduced or changed in this PR. I had considered doing so, but I didn't want to widen the scope of the PR, so I tried my best to keep that intact.

[Splaktar]

@andrewseguin on the topic of client vs server side paging, I agree that this is confusing to developers who just want to try out our schematic and may only be considering one of those options. I commented in #15791 (comment) that we should probably have an option to generate the table with client or server side paging. I also mentioned that we should probably have an option to generate a table w/o paging or sorting. Should I open issues to track that?

[Splaktar]

@crisbeto is also working on a related change to "Rename MatTableDataSource to ClientArrayTableDataSource" (#14378) since as Jeremy noted in the issue, it causes a lot of confusion. The PR is #15432.

[andrewseguin]

What about just removing [length]="dataSource.data.length" and changing the lifecycle hook from ngAfterViewInit to ngOnInit?

[Splaktar]

@andrewseguin if I make only those changes, then the original error from #15788 remains: TypeError: Cannot read property 'page' of undefined due to

components/src/material/schematics/ng-generate/table/files/__path__/__name@dasherize@if-flat__/__name@dasherize__-datasource.ts

Line 60 in dd7f4ea

this.paginator.page,

When the MatPaginator's length is not defined, it's readonly page: EventEmitter<PageEvent> is also undefined.

[andrewseguin]

Hm, well hopefully we could figure out some better syntax to the whole schematics. For now this looks fine if the other solutions aren't panning out

[felikf]

How to find out which version contains this fix?
I have this one: "@angular/material": "7.3.7"

[Splaktar]

@felikf it's in the commit linked above: 301371a.
8.0.0-rc.2 is the first version with this fix. 8.0.0 is the first stable version with the fix.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}