Sitelet https://github.com/angular/angular/commit/5f815c0
Skip to content

Commit 5f815c0

Browse files
ranma42alxhub
ranma42
authored and
alxhub
committed
fix(common): correct and simplify typing of AsyncPipe (#37447)
`AsyncPipe.transform` will never return `undefined`, even when passed `undefined` in input, in contrast with what was declared in the overloads. Additionally the "actual" method signature can be updated to match the most generic case, since the implementation does not rely on wrappers anymore. BREAKING CHANGE: The async pipe no longer claims to return `undefined` for an input that was typed as `undefined`. Note that the code actually returned `null` on `undefined` inputs. In the unlikely case you were relying on this, please fix the typing of the consumers of the pipe output. PR Close #37447
1 parent c7d5555 commit 5f815c0

4 files changed

Lines changed: 18 additions & 13 deletions

File tree

‎goldens/public-api/common/common.d.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@ export declare const APP_BASE_HREF: InjectionToken<string>;
33
export declare class AsyncPipe implements OnDestroy, PipeTransform {
44
constructor(_ref: ChangeDetectorRef);
55
ngOnDestroy(): void;
6-
transform<T>(obj: null): null;
7-
transform<T>(obj: undefined): undefined;
8-
transform<T>(obj: Observable<T> | null | undefined): T | null;
9-
transform<T>(obj: Promise<T> | null | undefined): T | null;
6+
transform<T>(obj: Observable<T> | Promise<T>): T | null;
7+
transform<T>(obj: null | undefined): null;
8+
transform<T>(obj: Observable<T> | Promise<T> | null | undefined): T | null;
109
}
1110

1211
export declare class CommonModule {

‎packages/common/src/pipes/async_pipe.ts‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,10 @@ export class AsyncPipe implements OnDestroy, PipeTransform {
9494
}
9595
}
9696

97-
transform<T>(obj: null): null;
98-
transform<T>(obj: undefined): undefined;
99-
transform<T>(obj: Observable<T>|null|undefined): T|null;
100-
transform<T>(obj: Promise<T>|null|undefined): T|null;
101-
transform(obj: Observable<any>|Promise<any>|null|undefined): any {
97+
transform<T>(obj: Observable<T>|Promise<T>): T|null;
98+
transform<T>(obj: null|undefined): null;
99+
transform<T>(obj: Observable<T>|Promise<T>|null|undefined): T|null;
100+
transform<T>(obj: Observable<T>|Promise<T>|null|undefined): T|null {
102101
if (!this._obj) {
103102
if (obj) {
104103
this._subscribe(obj);
@@ -108,7 +107,7 @@ export class AsyncPipe implements OnDestroy, PipeTransform {
108107

109108
if (obj !== this._obj) {
110109
this._dispose();
111-
return this.transform(obj as any);
110+
return this.transform(obj);
112111
}
113112

114113
return this._latestValue;

‎packages/common/test/pipes/async_pipe_spec.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ import {SpyChangeDetectorRef} from '../spies';
129129
reject = rej;
130130
});
131131
ref = new SpyChangeDetectorRef();
132-
pipe = new AsyncPipe(<any>ref);
132+
pipe = new AsyncPipe(ref as any);
133133
});
134134

135135
describe('transform', () => {
@@ -218,10 +218,17 @@ import {SpyChangeDetectorRef} from '../spies';
218218
});
219219
});
220220

221+
describe('undefined', () => {
222+
it('should return null when given undefined', () => {
223+
const pipe = new AsyncPipe(null as any);
224+
expect(pipe.transform(undefined)).toEqual(null);
225+
});
226+
});
227+
221228
describe('other types', () => {
222229
it('should throw when given an invalid object', () => {
223230
const pipe = new AsyncPipe(null as any);
224-
expect(() => pipe.transform(<any>'some bogus object')).toThrowError();
231+
expect(() => pipe.transform('some bogus object' as any)).toThrowError();
225232
});
226233
});
227234
});

‎packages/language-service/test/definitions_spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ describe('definitions', () => {
251251
expect(textSpan).toEqual(marker);
252252

253253
expect(definitions).toBeDefined();
254-
expect(definitions!.length).toBe(4);
254+
expect(definitions!.length).toBe(3);
255255

256256
const refFileName = '/node_modules/@angular/common/common.d.ts';
257257
for (const def of definitions!) {

0 commit comments

Comments
 (0)