angular / components

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

feat(cdk/scrolling): improve template type checking for CdkVirtualForOf Directive #26609

Open windmichael opened 1 year ago

windmichael commented 1 year ago

Feature Description

Feature Description

The CdkVirtualForOf Directive in the "@angular/cdk/scrolling" module does not implement the ng-template context guard, as it is done at the ngFor Structual Directive in the Angular Common package. Thus, when using the cdkVirtualFor Structual Directive in an HTML Template, the type of the item is "any".

Expample:

<cdk-virtual-scroll-viewport itemSize="50" class="example-viewport">
  <div *cdkVirtualFor="let item of items" class="example-item">{{ item }}</div>
</cdk-virtual-scroll-viewport>

Although the type of the property "items" is "string[]", the type of "item" is "any".

Expected behavior

The type of the property "item" should be "string" when the type of "items" is "string[]".

Suggested solution

Implement the static method "ngTemplateContextGuard" for the CdkVirtualForOf Directive.

public static ngTemplateContextGuard<T>(
    directive: CdkVirtualForOf<T>,
    context: unknown,
  ): context is CdkVirtualForOfContext<T> {
    return true;
  }

https://angular.io/guide/structural-directives#typing-the-directives-context

If you want, I can create a PR therefore, because I tried this solution already.

Use Case

No response

distante commented 8 months ago

Still an Issue

mstawick commented 7 months ago

If anyone wants a workaround, I was suggested one here: https://youtrack.jetbrains.com/issue/WEB-64040/Angular-template-broken-type-inference-for-cdkVirtualFor-2023.3-Beta

basically add: static ngTemplateContextGuard<T>(dir: CdkVirtualForOf<T>, ctx: any): ctx is CdkVirtualForOfContext<T>;

in the @angular/cdk/scrolling/index.d.ts file for the:

export declare class CdkVirtualForOf<T> implements CdkVirtualScrollRepeater<T>, CollectionViewer, DoCheck, OnDestroy

declaration.

medchat-layton commented 6 months ago

@mstawick but this just gets completely blown away whenever we update packages right? Fixes like this just drop through the cracks and can't be considered safe in any way, as far as I can tell :(

PhOder commented 6 months ago

@medchat-layton I know this might not be ideal but for workarounds like this I like to use https://github.com/ds300/patch-package to create a diff of such workarounds that you can add to your repository.

On npm install these patches will be automatically applied to the node_modules folder.

mstawick commented 6 months ago

@medchat-layton I completely agree with you, that's why I used the word "workaround". Given current state, not having proper type inference in templates destroys my productivity, so I'd rather apply the workaround after update if I have to, rather then have no workaround at all.

jakubgosk commented 5 months ago

I'm trying to figure it out in a way that would not require changing library files. In the past I managed to achive such thing for matCellDef with directive, so I tried that method here as well and came up with this:

@Directive({
  selector: '[cdkVirtualFor]',
  standalone: true,
})
export class TypeSafeCdkVirtualForDirective<T> {
  @Input() cdkVirtualForDataSource: T[];

  static ngTemplateContextGuard<T>(
    _dir: TypeSafeCdkVirtualForDirective<T>,
    ctx: unknown,
  ): ctx is CdkVirtualForOfContext<T> {
    return true;
  }
}

and then you simply change cdkVirtualFor to: *cdkVirtualFor="let customer of data; dataSource: data"

It's not perfect but for me it gets job done.