fix(core): make QueryList implement Iterable in the type system (#33536)

Originally, QueryList implemented Iterable and provided a Symbol.iterator
on its prototype. This caused issues with tree-shaking, so QueryList was
refactored and the Symbol.iterator added in its constructor instead. As
part of this change, QueryList no longer implemented Iterable directly.

Unfortunately, this meant that QueryList was no longer assignable to
Iterable or, consequently, NgIterable. NgIterable is used for NgFor's input,
so this meant that QueryList was not usable (in a type sense) for NgFor
iteration. View Engine's template type checking would not catch this, but
Ivy's did.

As a fix, this commit adds the declaration (but not the implementation) of
the Symbol.iterator function back to QueryList. This has no runtime effect,
so it doesn't affect tree-shaking of QueryList, but it ensures that
QueryList is assignable to NgIterable and thus usable with NgFor.

Fixes #29842

PR Close #33536
This commit is contained in:
Alex Rickabaugh
2019-11-01 12:31:56 -07:00
parent 850aee2448
commit bb290cefae
6 changed files with 57 additions and 5 deletions

View File

@ -42,7 +42,7 @@ function symbolIterator<T>(this: QueryList<T>): Iterator<T> {
*
* @publicApi
*/
export class QueryList<T>/* implements Iterable<T> */ {
export class QueryList<T> implements Iterable<T> {
public readonly dirty = true;
private _results: Array<T> = [];
public readonly changes: Observable<any> = new EventEmitter();
@ -142,4 +142,11 @@ export class QueryList<T>/* implements Iterable<T> */ {
(this.changes as EventEmitter<any>).complete();
(this.changes as EventEmitter<any>).unsubscribe();
}
// The implementation of `Symbol.iterator` should be declared here, but this would cause
// tree-shaking issues with `QueryList. So instead, it's added in the constructor (see comments
// there) and this declaration is left here to ensure that TypeScript considers QueryList to
// implement the Iterable interface. This is required for template type-checking of NgFor loops
// over QueryLists to work correctly, since QueryList must be assignable to NgIterable.
[Symbol.iterator] !: () => Iterator<T>;
}

View File

@ -135,6 +135,22 @@ import {beforeEach, describe, expect, it} from '@angular/core/testing/src/testin
expect(queryList.some(item => item === 'four')).toEqual(false);
});
it('should be iterable', () => {
const data = ['one', 'two', 'three'];
queryList.reset([...data]);
// The type here is load-bearing: it asserts that queryList is considered assignable to
// Iterable<string> in TypeScript. This is important for template type-checking of *ngFor
// when looping over query results.
const queryListAsIterable: Iterable<string> = queryList;
// For loops use the iteration protocol.
for (const value of queryListAsIterable) {
expect(value).toBe(data.shift() !);
}
expect(data.length).toBe(0);
});
if (getDOM().supportsDOMEvents()) {
describe('simple observable interface', () => {
it('should fire callbacks on change', fakeAsync(() => {