From daf8b7f10038c7ecf7ea78bc6fe39766c85224a6 Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Fri, 27 Mar 2020 11:35:08 +0100 Subject: [PATCH] feat(common): stricter types for DatePipe (#37447) Make typing of DatePipe stricter to catch some misuses (such as passing an Observable or an array) at compile time. BREAKING CHANGE: The signature of the `date` pipe now explicitly states which types are accepted. This should only cause issues in corner cases, as any other values would result in runtime exceptions. PR Close #37447 --- goldens/public-api/common/common.d.ts | 4 +++- packages/common/src/pipes/date_pipe.ts | 10 +++++++++- packages/common/test/pipes/date_pipe_spec.ts | 10 +++++++++- packages/language-service/test/definitions_spec.ts | 2 +- packages/language-service/test/hover_spec.ts | 2 +- 5 files changed, 23 insertions(+), 5 deletions(-) diff --git a/goldens/public-api/common/common.d.ts b/goldens/public-api/common/common.d.ts index f0daa116c8..418fecafd2 100644 --- a/goldens/public-api/common/common.d.ts +++ b/goldens/public-api/common/common.d.ts @@ -18,7 +18,9 @@ export declare class CurrencyPipe implements PipeTransform { export declare class DatePipe implements PipeTransform { constructor(locale: string); - transform(value: any, format?: string, timezone?: string, locale?: string): string | null; + transform(value: Date | string | number, format?: string, timezone?: string, locale?: string): string | null; + transform(value: null | undefined, format?: string, timezone?: string, locale?: string): null; + transform(value: Date | string | number | null | undefined, format?: string, timezone?: string, locale?: string): string | null; } export declare class DecimalPipe implements PipeTransform { diff --git a/packages/common/src/pipes/date_pipe.ts b/packages/common/src/pipes/date_pipe.ts index bd9236c865..e4b8d4d104 100644 --- a/packages/common/src/pipes/date_pipe.ts +++ b/packages/common/src/pipes/date_pipe.ts @@ -171,7 +171,15 @@ export class DatePipe implements PipeTransform { * See [Setting your app locale](guide/i18n#setting-up-the-locale-of-your-app). * @returns A date string in the desired format. */ - transform(value: any, format = 'mediumDate', timezone?: string, locale?: string): string|null { + transform(value: Date|string|number, format?: string, timezone?: string, locale?: string): string + |null; + transform(value: null|undefined, format?: string, timezone?: string, locale?: string): null; + transform( + value: Date|string|number|null|undefined, format?: string, timezone?: string, + locale?: string): string|null; + transform( + value: Date|string|number|null|undefined, format = 'mediumDate', timezone?: string, + locale?: string): string|null { if (value == null || value === '' || value !== value) return null; try { diff --git a/packages/common/test/pipes/date_pipe_spec.ts b/packages/common/test/pipes/date_pipe_spec.ts index 21cf55ab7b..c054061188 100644 --- a/packages/common/test/pipes/date_pipe_spec.ts +++ b/packages/common/test/pipes/date_pipe_spec.ts @@ -59,12 +59,20 @@ import {JitReflector} from '@angular/platform-browser-dynamic/src/compiler_refle expect(pipe.transform(Number.NaN)).toEqual(null); }); + it('should return null for null', () => { + expect(pipe.transform(null)).toEqual(null); + }); + + it('should return null for undefined', () => { + expect(pipe.transform(undefined)).toEqual(null); + }); + it('should support ISO string without time', () => { expect(() => pipe.transform(isoStringWithoutTime)).not.toThrow(); }); it('should not support other objects', () => { - expect(() => pipe.transform({})).toThrowError(/InvalidPipeArgument/); + expect(() => pipe.transform({} as any)).toThrowError(/InvalidPipeArgument/); }); }); diff --git a/packages/language-service/test/definitions_spec.ts b/packages/language-service/test/definitions_spec.ts index 954736b8ce..2bb0dcd5f8 100644 --- a/packages/language-service/test/definitions_spec.ts +++ b/packages/language-service/test/definitions_spec.ts @@ -274,7 +274,7 @@ describe('definitions', () => { expect(textSpan).toEqual(marker); expect(definitions).toBeDefined(); - expect(definitions!.length).toBe(1); + expect(definitions!.length).toBe(3); const refFileName = '/node_modules/@angular/common/common.d.ts'; for (const def of definitions!) { diff --git a/packages/language-service/test/hover_spec.ts b/packages/language-service/test/hover_spec.ts index ce2147c924..5116c42f09 100644 --- a/packages/language-service/test/hover_spec.ts +++ b/packages/language-service/test/hover_spec.ts @@ -211,7 +211,7 @@ describe('hover', () => { expect(textSpan).toEqual(marker); expect(toText(displayParts)) .toBe( - '(pipe) date: (value: any, format?: string | undefined, timezone?: string | undefined, locale?: string | undefined) => string | null'); + '(pipe) date: { (value: string | number | Date, format?: string | undefined, timezone?: string | undefined, locale?: string | undefined): string | null; (value: null | undefined, format?: string | undefined, timezone?: string | undefined, locale?: string | undefined): null; (value: string | ... 3 more ... | undefined, format?: st...'); }); it('should work for the $any() cast function', () => {