angular / components

Component infrastructure and Material Design components for Angular
https://material.angular.io
MIT License
24.32k stars 6.73k forks source link

feat(table): typescript/strong typing of cell templates #22290

Open andreialecu opened 3 years ago

andreialecu commented 3 years ago

Feature Description

Currently, *matCellDef types the resulting variable as any:

Screenshot 2021-03-19 at 11 29 32

This issue is an attempt to bring https://github.com/angular/components/issues/16273 back in discussion, which has been closed as Ivy wasn't yet available.

There has been some progress on several fronts:

It appears that the approach described in this comment https://github.com/angular/angular/issues/28731#issuecomment-782806178 might allow this to be implemented.

Use Case

As we've been converting more code to use Material Data Tables, the lack of strong typing has been really problematic, and it feels like a downgrade in developer experience compared to regular tables with *ngFor.

Further motivation for this has been described in:

https://github.com/angular/components/issues/16273 https://github.com/angular/angular/issues/28731

andreialecu commented 3 years ago

Very simple repro:

<table mat-table [dataSource]="[{ hello: 'world' }]">
  <tr mat-cell *matCellDef="let element">
    {{
      element.test
    }}
  </tr>
</table>

Generates the following typecheck block with Ivy:

import * as i0 from 'some.component';
import * as i1 from '@angular/material/table';

const _ctor1: <T = any>(init: Pick<i1.MatTable<T>, "trackBy" | "dataSource"> & {
    multiTemplateDataRows: typeof i1.MatTable.ngAcceptInputType_multiTemplateDataRows;
    fixedLayout: typeof i1.MatTable.ngAcceptInputType_fixedLayout;
}) => i1.MatTable<T> = (null!);

/*tcb1*/
function _tcb1(ctx: i0.SomeComponent) { if (true) {
    ctx. /*D:ignore*/ /*T:COMPCOMP*/;
    var _t1 = document.createElement("table") /*96,149*/;
    var _t2 /*T:DIR*/ /*96,149*/ = _ctor1({ "dataSource": ([{
                "hello": "world" /*137,144*/
            } /*128,146*/] /*127,147*/) /*113,148*/, "trackBy": (null as any), "multiTemplateDataRows": (null as any), "fixedLayout": (null as any) }) /*D:ignore*/;
    _t2.dataSource /*114,124*/ = ([{
            "hello": "world" /*137,144*/
        } /*128,146*/] /*127,147*/) /*113,148*/;
    var _t3: i1.MatCellDef /*T:DIR*/ /*152,191*/ = (null!);
    var _t4: any = (null!);
    {
        var _t5 /*182,189*/ = _t4.$implicit /*178,189*/;
        var _t6 = document.createElement("tr") /*152,191*/;
        "" + (((_t5 /*205,212*/).test /*213,218*/) /*205,218*/);
    }
} }

export const IS_A_MODULE = true;

The correct type is available as part of the _t2 variable.

If _t4 was able to use it instead of any, it seems like this would be achievable.

andreialecu commented 3 years ago

Here's a working workaround for this. Most of the credits for it go to @nartc 🏆 , on the Angular Discord.

import { CdkCellDef } from '@angular/cdk/table';
import { Directive, Input } from '@angular/core';
import { MatCellDef } from '@angular/material/table';
import { Observable } from 'rxjs';

@Directive({
  selector: '[matCellDef]', // same selector as MatCellDef
  providers: [{ provide: CdkCellDef, useExisting: TypeSafeMatCellDef }],
})
export class TypeSafeMatCellDef<T> extends MatCellDef {
  @Input() matCellDefDataSource: T[] | Observable<T[]> | MatTableDataSource<T>;

  static ngTemplateContextGuard<T>(
    dir: TypeSafeMatCellDef<T>,
    ctx: any,
  ): ctx is { $implicit: T; index: number } {
    return true;
  }
}

You can add this directive to your providers and then use it like:

<table mat-table [dataSource]="data">
...
 <td mat-cell *matCellDef="let element; dataSource: data">
    {{ element.hello }}
 </td>

And element will no longer be any: image

There's a bit of redundancy, with needing to specify the data source in every *matCellDef that you want type checked, but it's optional. If it isn't defined, it falls back to any just like the original directive.

KingDarBoja commented 3 years ago

Do not forget to follow the steps at Under the hood of the language service to properly enable this workaround.

aceArt-GmbH commented 1 year ago

Is there a solution which would only require 1 change per table instead of 1 change per column?

wongk commented 1 year ago

+1

DmitryEfimenko commented 1 year ago

I feel like this issue belongs under @angular/core instead of @angular/components. The fix should allow providing type through a parent directive.

Maybe it would work if ngTemplateContextGuard accepted 3 arguments?

static ngTemplateContextGuard<T, P>(
  thisdirective: ThisDirective<T>,
  parentDirective: ParentDirective<P>
  ctx: unknown
): ctx is { $implicit: T, parent: P } {
  return true;
}
Simon-GHI commented 8 months ago

Still waiting for this feature to be integrated in Angular as loosing types in table is still a big disadvantage.

maartentibau commented 2 months ago

Would love to get this one as well... 🙏

aceArt-GmbH commented 2 months ago

For simply using the table reference version you can search-replace with following regex

\*matCellDef="let (\w*)"
↓
*matCellDef="let $1; table: table"

And mat-table -> mat-table #table.

Finds lots of real type errors. But also shows many limitations of angular templates. Like optional chaining working different from typescript, numeric index signature ([key: `prop${number}`]) and type narrowing