Sitelet https://github.com/angular/angular/commit/7b2aac9
Skip to content

Commit 7b2aac9

Browse files
ranma42alxhub
ranma42
authored and
alxhub
committed
feat(common): stricter types for number pipes (#37447)
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
1 parent daf8b7f commit 7b2aac9

3 files changed

Lines changed: 76 additions & 14 deletions

File tree

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ export declare class CommonModule {
1313

1414
export declare class CurrencyPipe implements PipeTransform {
1515
constructor(_locale: string, _defaultCurrencyCode?: string);
16-
transform(value: any, currencyCode?: string, display?: 'code' | 'symbol' | 'symbol-narrow' | string | boolean, digitsInfo?: string, locale?: string): string | null;
16+
transform(value: number | string, currencyCode?: string, display?: 'code' | 'symbol' | 'symbol-narrow' | string | boolean, digitsInfo?: string, locale?: string): string | null;
17+
transform(value: null | undefined, currencyCode?: string, display?: 'code' | 'symbol' | 'symbol-narrow' | string | boolean, digitsInfo?: string, locale?: string): null;
18+
transform(value: number | string | null | undefined, currencyCode?: string, display?: 'code' | 'symbol' | 'symbol-narrow' | string | boolean, digitsInfo?: string, locale?: string): string | null;
1719
}
1820

1921
export declare class DatePipe implements PipeTransform {
@@ -25,7 +27,9 @@ export declare class DatePipe implements PipeTransform {
2527

2628
export declare class DecimalPipe implements PipeTransform {
2729
constructor(_locale: string);
28-
transform(value: any, digitsInfo?: string, locale?: string): string | null;
30+
transform(value: number | string, digitsInfo?: string, locale?: string): string | null;
31+
transform(value: null | undefined, digitsInfo?: string, locale?: string): null;
32+
transform(value: number | string | null | undefined, digitsInfo?: string, locale?: string): string | null;
2933
}
3034

3135
export declare const DOCUMENT: InjectionToken<Document>;
@@ -342,7 +346,9 @@ export declare class PathLocationStrategy extends LocationStrategy {
342346

343347
export declare class PercentPipe implements PipeTransform {
344348
constructor(_locale: string);
345-
transform(value: any, digitsInfo?: string, locale?: string): string | null;
349+
transform(value: number | string, digitsInfo?: string, locale?: string): string | null;
350+
transform(value: null | undefined, digitsInfo?: string, locale?: string): null;
351+
transform(value: number | string | null | undefined, digitsInfo?: string, locale?: string): string | null;
346352
}
347353

348354
export declare abstract class PlatformLocation {

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

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,12 @@ export class DecimalPipe implements PipeTransform {
6767
* When not supplied, uses the value of `LOCALE_ID`, which is `en-US` by default.
6868
* See [Setting your app locale](guide/i18n#setting-up-the-locale-of-your-app).
6969
*/
70-
transform(value: any, digitsInfo?: string, locale?: string): string|null {
71-
if (isEmpty(value)) return null;
70+
transform(value: number|string, digitsInfo?: string, locale?: string): string|null;
71+
transform(value: null|undefined, digitsInfo?: string, locale?: string): null;
72+
transform(value: number|string|null|undefined, digitsInfo?: string, locale?: string): string|null;
73+
transform(value: number|string|null|undefined, digitsInfo?: string, locale?: string): string
74+
|null {
75+
if (!isValue(value)) return null;
7276

7377
locale = locale || this._locale;
7478

@@ -121,8 +125,12 @@ export class PercentPipe implements PipeTransform {
121125
* When not supplied, uses the value of `LOCALE_ID`, which is `en-US` by default.
122126
* See [Setting your app locale](guide/i18n#setting-up-the-locale-of-your-app).
123127
*/
124-
transform(value: any, digitsInfo?: string, locale?: string): string|null {
125-
if (isEmpty(value)) return null;
128+
transform(value: number|string, digitsInfo?: string, locale?: string): string|null;
129+
transform(value: null|undefined, digitsInfo?: string, locale?: string): null;
130+
transform(value: number|string|null|undefined, digitsInfo?: string, locale?: string): string|null;
131+
transform(value: number|string|null|undefined, digitsInfo?: string, locale?: string): string
132+
|null {
133+
if (!isValue(value)) return null;
126134
locale = locale || this._locale;
127135
try {
128136
const num = strToNumber(value);
@@ -213,10 +221,22 @@ export class CurrencyPipe implements PipeTransform {
213221
* See [Setting your app locale](guide/i18n#setting-up-the-locale-of-your-app).
214222
*/
215223
transform(
216-
value: any, currencyCode?: string,
224+
value: number|string, currencyCode?: string,
225+
display?: 'code'|'symbol'|'symbol-narrow'|string|boolean, digitsInfo?: string,
226+
locale?: string): string|null;
227+
transform(
228+
value: null|undefined, currencyCode?: string,
229+
display?: 'code'|'symbol'|'symbol-narrow'|string|boolean, digitsInfo?: string,
230+
locale?: string): null;
231+
transform(
232+
value: number|string|null|undefined, currencyCode?: string,
233+
display?: 'code'|'symbol'|'symbol-narrow'|string|boolean, digitsInfo?: string,
234+
locale?: string): string|null;
235+
transform(
236+
value: number|string|null|undefined, currencyCode?: string,
217237
display: 'code'|'symbol'|'symbol-narrow'|string|boolean = 'symbol', digitsInfo?: string,
218238
locale?: string): string|null {
219-
if (isEmpty(value)) return null;
239+
if (!isValue(value)) return null;
220240

221241
locale = locale || this._locale;
222242

@@ -246,8 +266,8 @@ export class CurrencyPipe implements PipeTransform {
246266
}
247267
}
248268

249-
function isEmpty(value: any): boolean {
250-
return value == null || value === '' || value !== value;
269+
function isValue(value: number|string|null|undefined): value is number|string {
270+
return !(value == null || value === '' || value !== value);
251271
}
252272

253273
/**

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

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,20 @@ import {beforeEach, describe, expect, it} from '@angular/core/testing/src/testin
5050
expect(pipe.transform('1.1234')).toEqual('1.123');
5151
});
5252

53+
it('should return null for NaN', () => {
54+
expect(pipe.transform(Number.NaN)).toEqual(null);
55+
});
56+
57+
it('should return null for null', () => {
58+
expect(pipe.transform(null)).toEqual(null);
59+
});
60+
61+
it('should return null for undefined', () => {
62+
expect(pipe.transform(undefined)).toEqual(null);
63+
});
64+
5365
it('should not support other objects', () => {
54-
expect(() => pipe.transform({}))
66+
expect(() => pipe.transform({} as any))
5567
.toThrowError(
5668
`InvalidPipeArgument: '[object Object] is not a number' for pipe 'DecimalPipe'`);
5769
expect(() => pipe.transform('123abc'))
@@ -82,8 +94,20 @@ import {beforeEach, describe, expect, it} from '@angular/core/testing/src/testin
8294
expect(pipe.transform(12.3456, '0.0-10')).toEqual('1,234.56%');
8395
});
8496

97+
it('should return null for NaN', () => {
98+
expect(pipe.transform(Number.NaN)).toEqual(null);
99+
});
100+
101+
it('should return null for null', () => {
102+
expect(pipe.transform(null)).toEqual(null);
103+
});
104+
105+
it('should return null for undefined', () => {
106+
expect(pipe.transform(undefined)).toEqual(null);
107+
});
108+
85109
it('should not support other objects', () => {
86-
expect(() => pipe.transform({}))
110+
expect(() => pipe.transform({} as any))
87111
.toThrowError(
88112
`InvalidPipeArgument: '[object Object] is not a number' for pipe 'PercentPipe'`);
89113
});
@@ -125,8 +149,20 @@ import {beforeEach, describe, expect, it} from '@angular/core/testing/src/testin
125149
expect(pipe.transform(5.1234, 'USD', 'Custom name')).toEqual('Custom name5.12');
126150
});
127151

152+
it('should return null for NaN', () => {
153+
expect(pipe.transform(Number.NaN)).toEqual(null);
154+
});
155+
156+
it('should return null for null', () => {
157+
expect(pipe.transform(null)).toEqual(null);
158+
});
159+
160+
it('should return null for undefined', () => {
161+
expect(pipe.transform(undefined)).toEqual(null);
162+
});
163+
128164
it('should not support other objects', () => {
129-
expect(() => pipe.transform({}))
165+
expect(() => pipe.transform({} as any))
130166
.toThrowError(
131167
`InvalidPipeArgument: '[object Object] is not a number' for pipe 'CurrencyPipe'`);
132168
});

0 commit comments

Comments
 (0)