components icon indicating copy to clipboard operation
components copied to clipboard

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

Open windmichael opened this issue 2 years ago • 6 comments

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

windmichael avatar Feb 12 '23 19:02 windmichael

Still an Issue

distante avatar Oct 30 '23 07:10 distante

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.

mstawick avatar Dec 01 '23 12:12 mstawick

@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 :(

medchat-layton avatar Dec 30 '23 02:12 medchat-layton

@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.

PhOder avatar Dec 30 '23 10:12 PhOder

@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.

mstawick avatar Dec 30 '23 12:12 mstawick

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.

jakubgosk avatar Feb 01 '24 21:02 jakubgosk