Skip to content

Commit

Permalink
fix(cdk/scrolling): expand type for "cdkVirtualForOf" input to… (#17421)
Browse files Browse the repository at this point in the history
Currently the `cdkVirtualForOf` input accepts `null` or `undefined` as valid
values. Although when using strict template input type checking
(which will be supported by `ngtsc`), passing `null` or `undefined`
with strict null checks enabled causes a type check failure because
the type definition of the input becomes too strict with "--strictNullChecks" enabled.

For extra content: by default if strict null checks are not enabled, `null` or `undefined`
are assignable to _every_ type. The current type definiton of the `cdkVirtualForOf` input
is incorrectly making the assumption that `null` or `undefined` are always assignable.

The type of the input needs to be expanded to explicitly accept `null` or
`undefined` to behave consistently regardless of the `strictNullChecks` flag.

A common scenario where this breaks is the use of `cdkVirtualFor` in combination
with the async pipe from `@angular/common`. This is because the async pipe returns
`null` until a value has been emitted. This means that with strict null checks, the type
checking will fail because `null` is not allowed.

This is similar to what we did for `ngFor` in framework: angular/angular@c1bb886

Fixes #17411
  • Loading branch information
devversion authored and mmalerba committed Oct 16, 2019
1 parent 5c752c2 commit 3e5e9db
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 6 deletions.
10 changes: 6 additions & 4 deletions src/cdk/scrolling/virtual-for-of.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,18 +83,18 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy

/** The DataSource to display. */
@Input()
get cdkVirtualForOf(): DataSource<T> | Observable<T[]> | NgIterable<T> {
get cdkVirtualForOf(): DataSource<T> | Observable<T[]> | NgIterable<T> | null | undefined {
return this._cdkVirtualForOf;
}
set cdkVirtualForOf(value: DataSource<T> | Observable<T[]> | NgIterable<T>) {
set cdkVirtualForOf(value: DataSource<T> | Observable<T[]> | NgIterable<T> | null | undefined) {
this._cdkVirtualForOf = value;
const ds = isDataSource(value) ? value :
// Slice the value if its an NgIterable to ensure we're working with an array.
new ArrayDataSource<T>(
value instanceof Observable ? value : Array.prototype.slice.call(value || []));
this._dataSourceChanges.next(ds);
}
_cdkVirtualForOf: DataSource<T> | Observable<T[]> | NgIterable<T>;
_cdkVirtualForOf: DataSource<T> | Observable<T[]> | NgIterable<T> | null | undefined;

/**
* The `TrackByFunction` to use for tracking changes. The `TrackByFunction` takes the index and
Expand Down Expand Up @@ -363,7 +363,9 @@ export class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy
// comment node which can throw off the move when it's being repeated for all items.
return this._viewContainerRef.createEmbeddedView(this._template, {
$implicit: null!,
cdkVirtualForOf: this._cdkVirtualForOf,
// It's guaranteed that the iterable is not "undefined" or "null" because we only
// generate views for elements if the "cdkVirtualForOf" iterable has elements.
cdkVirtualForOf: this._cdkVirtualForOf!,
index: -1,
count: -1,
first: false,
Expand Down
4 changes: 2 additions & 2 deletions tools/public_api_guard/cdk/scrolling.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ export declare class CdkScrollable implements OnInit, OnDestroy {
}

export declare class CdkVirtualForOf<T> implements CollectionViewer, DoCheck, OnDestroy {
_cdkVirtualForOf: DataSource<T> | Observable<T[]> | NgIterable<T>;
cdkVirtualForOf: DataSource<T> | Observable<T[]> | NgIterable<T>;
_cdkVirtualForOf: DataSource<T> | Observable<T[]> | NgIterable<T> | null | undefined;
cdkVirtualForOf: DataSource<T> | Observable<T[]> | NgIterable<T> | null | undefined;
cdkVirtualForTemplate: TemplateRef<CdkVirtualForOfContext<T>>;
cdkVirtualForTemplateCacheSize: number;
cdkVirtualForTrackBy: TrackByFunction<T> | undefined;
Expand Down

0 comments on commit 3e5e9db

Please sign in to comment.