From 5f815c05650c15e414fe72cddaa2b690a7962273 Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Fri, 27 Mar 2020 11:27:54 +0100 Subject: [PATCH] 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 --- goldens/public-api/common/common.d.ts | 7 +++---- packages/common/src/pipes/async_pipe.ts | 11 +++++------ packages/common/test/pipes/async_pipe_spec.ts | 11 +++++++++-- packages/language-service/test/definitions_spec.ts | 2 +- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/goldens/public-api/common/common.d.ts b/goldens/public-api/common/common.d.ts index 55856b6199..f0daa116c8 100644 --- a/goldens/public-api/common/common.d.ts +++ b/goldens/public-api/common/common.d.ts @@ -3,10 +3,9 @@ export declare const APP_BASE_HREF: InjectionToken; export declare class AsyncPipe implements OnDestroy, PipeTransform { constructor(_ref: ChangeDetectorRef); ngOnDestroy(): void; - transform(obj: null): null; - transform(obj: undefined): undefined; - transform(obj: Observable | null | undefined): T | null; - transform(obj: Promise | null | undefined): T | null; + transform(obj: Observable | Promise): T | null; + transform(obj: null | undefined): null; + transform(obj: Observable | Promise | null | undefined): T | null; } export declare class CommonModule { diff --git a/packages/common/src/pipes/async_pipe.ts b/packages/common/src/pipes/async_pipe.ts index 3716d7db42..3921e1c018 100644 --- a/packages/common/src/pipes/async_pipe.ts +++ b/packages/common/src/pipes/async_pipe.ts @@ -94,11 +94,10 @@ export class AsyncPipe implements OnDestroy, PipeTransform { } } - transform(obj: null): null; - transform(obj: undefined): undefined; - transform(obj: Observable|null|undefined): T|null; - transform(obj: Promise|null|undefined): T|null; - transform(obj: Observable|Promise|null|undefined): any { + transform(obj: Observable|Promise): T|null; + transform(obj: null|undefined): null; + transform(obj: Observable|Promise|null|undefined): T|null; + transform(obj: Observable|Promise|null|undefined): T|null { if (!this._obj) { if (obj) { this._subscribe(obj); @@ -108,7 +107,7 @@ export class AsyncPipe implements OnDestroy, PipeTransform { if (obj !== this._obj) { this._dispose(); - return this.transform(obj as any); + return this.transform(obj); } return this._latestValue; diff --git a/packages/common/test/pipes/async_pipe_spec.ts b/packages/common/test/pipes/async_pipe_spec.ts index 42414897ea..00a2de5064 100644 --- a/packages/common/test/pipes/async_pipe_spec.ts +++ b/packages/common/test/pipes/async_pipe_spec.ts @@ -129,7 +129,7 @@ import {SpyChangeDetectorRef} from '../spies'; reject = rej; }); ref = new SpyChangeDetectorRef(); - pipe = new AsyncPipe(ref); + pipe = new AsyncPipe(ref as any); }); describe('transform', () => { @@ -218,10 +218,17 @@ import {SpyChangeDetectorRef} from '../spies'; }); }); + describe('undefined', () => { + it('should return null when given undefined', () => { + const pipe = new AsyncPipe(null as any); + expect(pipe.transform(undefined)).toEqual(null); + }); + }); + describe('other types', () => { it('should throw when given an invalid object', () => { const pipe = new AsyncPipe(null as any); - expect(() => pipe.transform('some bogus object')).toThrowError(); + expect(() => pipe.transform('some bogus object' as any)).toThrowError(); }); }); }); diff --git a/packages/language-service/test/definitions_spec.ts b/packages/language-service/test/definitions_spec.ts index 0a3bed10e8..954736b8ce 100644 --- a/packages/language-service/test/definitions_spec.ts +++ b/packages/language-service/test/definitions_spec.ts @@ -251,7 +251,7 @@ describe('definitions', () => { expect(textSpan).toEqual(marker); expect(definitions).toBeDefined(); - expect(definitions!.length).toBe(4); + expect(definitions!.length).toBe(3); const refFileName = '/node_modules/@angular/common/common.d.ts'; for (const def of definitions!) {