cargdev/angular
1
0
Fork 0
Code Issues 17 Pull Requests Actions Packages Projects Releases Wiki Activity

feat(common): stricter types for number pipes (#37447)

Browse Source 
Make typing of number pipes stricter to catch some misuses (such as
passing an Observable or an array) at compile time.

BREAKING CHANGE:
The signatures of the number pipes now explicitly state which types are
accepted. This should only cause issues in corner cases, as any other
values would result in runtime exceptions.

PR Close #37447
This commit is contained in:
Andrea Canciani
2020-03-27 11:41:50 +01:00
committed by Alex Rickabaugh
parent daf8b7f100
commit 7b2aac97df
3 changed files with 76 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

36
packages/common/src/pipes/number_pipe.ts
View File

@ -67,8 +67,12 @@ export class DecimalPipe implements PipeTransform {
* When not supplied, uses the value of `LOCALE_ID`, which is `en-US` by default.
* See [Setting your app locale](guide/i18n#setting-up-the-locale-of-your-app).
*/
transform(value: any, digitsInfo?: string, locale?: string): string|null {
if (isEmpty(value)) return null;
transform(value: number|string, digitsInfo?: string, locale?: string): string|null;
transform(value: null|undefined, digitsInfo?: string, locale?: string): null;
transform(value: number|string|null|undefined, digitsInfo?: string, locale?: string): string|null;
transform(value: number|string|null|undefined, digitsInfo?: string, locale?: string): string
|null {
if (!isValue(value)) return null;
locale = locale || this._locale;
@ -121,8 +125,12 @@ export class PercentPipe implements PipeTransform {
* When not supplied, uses the value of `LOCALE_ID`, which is `en-US` by default.
* See [Setting your app locale](guide/i18n#setting-up-the-locale-of-your-app).
*/
transform(value: any, digitsInfo?: string, locale?: string): string|null {
if (isEmpty(value)) return null;
transform(value: number|string, digitsInfo?: string, locale?: string): string|null;
transform(value: null|undefined, digitsInfo?: string, locale?: string): null;
transform(value: number|string|null|undefined, digitsInfo?: string, locale?: string): string|null;
transform(value: number|string|null|undefined, digitsInfo?: string, locale?: string): string
|null {
if (!isValue(value)) return null;
locale = locale || this._locale;
try {
const num = strToNumber(value);
@ -213,10 +221,22 @@ export class CurrencyPipe implements PipeTransform {
* See [Setting your app locale](guide/i18n#setting-up-the-locale-of-your-app).
*/
transform(
value: any, currencyCode?: string,
value: number|string, currencyCode?: string,
display?: 'code'|'symbol'|'symbol-narrow'|string|boolean, digitsInfo?: string,
locale?: string): string|null;
transform(
value: null|undefined, currencyCode?: string,
display?: 'code'|'symbol'|'symbol-narrow'|string|boolean, digitsInfo?: string,
locale?: string): null;
transform(
value: number|string|null|undefined, currencyCode?: string,
display?: 'code'|'symbol'|'symbol-narrow'|string|boolean, digitsInfo?: string,
locale?: string): string|null;
transform(
value: number|string|null|undefined, currencyCode?: string,
display: 'code'|'symbol'|'symbol-narrow'|string|boolean = 'symbol', digitsInfo?: string,
locale?: string): string|null {
if (isEmpty(value)) return null;
if (!isValue(value)) return null;
locale = locale || this._locale;
@ -246,8 +266,8 @@ export class CurrencyPipe implements PipeTransform {
}
}
function isEmpty(value: any): boolean {
return value == null || value === '' || value !== value;
function isValue(value: number|string|null|undefined): value is number|string {
return !(value == null || value === '' || value !== value);
}
/**

42
packages/common/test/pipes/number_pipe_spec.ts
View File

@ -50,8 +50,20 @@ import {beforeEach, describe, expect, it} from '@angular/core/testing/src/testin
expect(pipe.transform('1.1234')).toEqual('1.123');
});
it('should return null for NaN', () => {
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 not support other objects', () => {
expect(() => pipe.transform({}))
expect(() => pipe.transform({} as any))
.toThrowError(
`InvalidPipeArgument: '[object Object] is not a number' for pipe 'DecimalPipe'`);
expect(() => pipe.transform('123abc'))
@ -82,8 +94,20 @@ import {beforeEach, describe, expect, it} from '@angular/core/testing/src/testin
expect(pipe.transform(12.3456, '0.0-10')).toEqual('1,234.56%');
});
it('should return null for NaN', () => {
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 not support other objects', () => {
expect(() => pipe.transform({}))
expect(() => pipe.transform({} as any))
.toThrowError(
`InvalidPipeArgument: '[object Object] is not a number' for pipe 'PercentPipe'`);
});
@ -125,8 +149,20 @@ import {beforeEach, describe, expect, it} from '@angular/core/testing/src/testin
expect(pipe.transform(5.1234, 'USD', 'Custom name')).toEqual('Custom name5.12');
});
it('should return null for NaN', () => {
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 not support other objects', () => {
expect(() => pipe.transform({}))
expect(() => pipe.transform({} as any))
.toThrowError(
`InvalidPipeArgument: '[object Object] is not a number' for pipe 'CurrencyPipe'`);
});