Skip to content

Commit

Permalink
fix(common): let case conversion pipes accept type unions with null (
Browse files Browse the repository at this point in the history
…angular#36259)

The old implementation of case conversion types can handle several
values which are not strings, but the signature did not reflect this.

The new one reports errors when falsy non-string inputs are given to
the pipe (such as `false` or `0`) and has a new signature which
instead reflcts the behaviour on `null` and `undefined`.

Fixes angular#36259
  • Loading branch information
ranma42 committed Jun 3, 2020
1 parent e648a0c commit 3f7c772
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 6 deletions.
3 changes: 3 additions & 0 deletions goldens/public-api/common/common.d.ts
Expand Up @@ -193,6 +193,7 @@ export declare abstract class LocationStrategy {
}

export declare class LowerCasePipe implements PipeTransform {
transform(value: string | null | undefined): string | null;
transform(value: string): string;
}

Expand Down Expand Up @@ -391,6 +392,7 @@ export declare type Time = {
};

export declare class TitleCasePipe implements PipeTransform {
transform(value: string | null | undefined): string | null;
transform(value: string): string;
}

Expand All @@ -402,6 +404,7 @@ export declare enum TranslationWidth {
}

export declare class UpperCasePipe implements PipeTransform {
transform(value: string | null | undefined): string | null;
transform(value: string): string;
}

Expand Down
18 changes: 12 additions & 6 deletions packages/common/src/pipes/case_conversion_pipes.ts
Expand Up @@ -29,8 +29,10 @@ export class LowerCasePipe implements PipeTransform {
/**
* @param value The string to transform to lower case.
*/
transform(value: string): string {
if (!value) return value;
transform(value: string|null|undefined): string|null;
transform(value: string): string;
transform(value: string|null|undefined): string|null {
if (value == null) return null;
if (typeof value !== 'string') {
throw invalidPipeArgumentError(LowerCasePipe, value);
}
Expand Down Expand Up @@ -72,8 +74,10 @@ export class TitleCasePipe implements PipeTransform {
/**
* @param value The string to transform to title case.
*/
transform(value: string): string {
if (!value) return value;
transform(value: string|null|undefined): string|null;
transform(value: string): string;
transform(value: string|null|undefined): string|null {
if (value == null) return null;
if (typeof value !== 'string') {
throw invalidPipeArgumentError(TitleCasePipe, value);
}
Expand All @@ -96,8 +100,10 @@ export class UpperCasePipe implements PipeTransform {
/**
* @param value The string to transform to upper case.
*/
transform(value: string): string {
if (!value) return value;
transform(value: string|null|undefined): string|null;
transform(value: string): string;
transform(value: string|null|undefined): string|null {
if (value == null) return null;
if (typeof value !== 'string') {
throw invalidPipeArgumentError(UpperCasePipe, value);
}
Expand Down
30 changes: 30 additions & 0 deletions packages/common/test/pipes/case_conversion_pipes_spec.ts
Expand Up @@ -25,6 +25,16 @@ import {LowerCasePipe, TitleCasePipe, UpperCasePipe} from '@angular/common';
expect(pipe.transform('BAr')).toEqual('bar');
});

it('should map null to null', () => {
expect(pipe.transform(null)).toEqual(null);
});
it('should map undefined to null', () => {
expect(pipe.transform(undefined)).toEqual(null);
});

it('should not support numbers', () => {
expect(() => pipe.transform(<any>0)).toThrowError();
});
it('should not support other objects', () => {
expect(() => pipe.transform(<any>{})).toThrowError();
});
Expand Down Expand Up @@ -80,6 +90,16 @@ import {LowerCasePipe, TitleCasePipe, UpperCasePipe} from '@angular/common';
expect(pipe.transform('éric')).toEqual('Éric');
});

it('should map null to null', () => {
expect(pipe.transform(null)).toEqual(null);
});
it('should map undefined to null', () => {
expect(pipe.transform(undefined)).toEqual(null);
});

it('should not support numbers', () => {
expect(() => pipe.transform(<any>0)).toThrowError();
});
it('should not support other objects', () => {
expect(() => pipe.transform(<any>{})).toThrowError();
});
Expand All @@ -101,6 +121,16 @@ import {LowerCasePipe, TitleCasePipe, UpperCasePipe} from '@angular/common';
expect(pipe.transform('bar')).toEqual('BAR');
});

it('should map null to null', () => {
expect(pipe.transform(null)).toEqual(null);
});
it('should map undefined to null', () => {
expect(pipe.transform(undefined)).toEqual(null);
});

it('should not support numbers', () => {
expect(() => pipe.transform(<any>0)).toThrowError();
});
it('should not support other objects', () => {
expect(() => pipe.transform(<any>{})).toThrowError();
});
Expand Down

0 comments on commit 3f7c772

Please sign in to comment.