From c7d5555dfb14f5deca4a2d947fe82a68453e5f09 Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Fri, 27 Mar 2020 11:22:08 +0100 Subject: [PATCH] fix(common): let case conversion pipes accept type unions with `null` (#36259) (#37447) 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 reflects the behaviour on `null` and `undefined`. Fixes #36259 BREAKING CHANGE: The case conversion pipes no longer let falsy values through. They now map both `null` and `undefined` to `null` and raise an exception on invalid input (`0`, `false`, `NaN`) just like most "common pipes". If your code required falsy values to pass through, you need to handle them explicitly. PR Close #37447 --- goldens/public-api/common/common.d.ts | 6 ++++ .../common/src/pipes/case_conversion_pipes.ts | 21 +++++++---- .../test/pipes/case_conversion_pipes_spec.ts | 36 +++++++++++++++++-- 3 files changed, 54 insertions(+), 9 deletions(-) diff --git a/goldens/public-api/common/common.d.ts b/goldens/public-api/common/common.d.ts index d7f8438c85..55856b6199 100644 --- a/goldens/public-api/common/common.d.ts +++ b/goldens/public-api/common/common.d.ts @@ -194,6 +194,8 @@ export declare abstract class LocationStrategy { export declare class LowerCasePipe implements PipeTransform { transform(value: string): string; + transform(value: null | undefined): null; + transform(value: string | null | undefined): string | null; } export declare class NgClass implements DoCheck { @@ -392,6 +394,8 @@ export declare type Time = { export declare class TitleCasePipe implements PipeTransform { transform(value: string): string; + transform(value: null | undefined): null; + transform(value: string | null | undefined): string | null; } export declare enum TranslationWidth { @@ -403,6 +407,8 @@ export declare enum TranslationWidth { export declare class UpperCasePipe implements PipeTransform { transform(value: string): string; + transform(value: null | undefined): null; + transform(value: string | null | undefined): string | null; } export declare const VERSION: Version; diff --git a/packages/common/src/pipes/case_conversion_pipes.ts b/packages/common/src/pipes/case_conversion_pipes.ts index 556c7cc1c1..fea96ec2d2 100644 --- a/packages/common/src/pipes/case_conversion_pipes.ts +++ b/packages/common/src/pipes/case_conversion_pipes.ts @@ -29,8 +29,11 @@ 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): string; + transform(value: null|undefined): null; + transform(value: string|null|undefined): string|null; + transform(value: string|null|undefined): string|null { + if (value == null) return null; if (typeof value !== 'string') { throw invalidPipeArgumentError(LowerCasePipe, value); } @@ -72,8 +75,11 @@ 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): string; + transform(value: null|undefined): null; + transform(value: string|null|undefined): string|null; + transform(value: string|null|undefined): string|null { + if (value == null) return null; if (typeof value !== 'string') { throw invalidPipeArgumentError(TitleCasePipe, value); } @@ -96,8 +102,11 @@ 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): string; + transform(value: null|undefined): null; + transform(value: string|null|undefined): string|null; + transform(value: string|null|undefined): string|null { + if (value == null) return null; if (typeof value !== 'string') { throw invalidPipeArgumentError(UpperCasePipe, value); } diff --git a/packages/common/test/pipes/case_conversion_pipes_spec.ts b/packages/common/test/pipes/case_conversion_pipes_spec.ts index 0fde495f33..ca1bc289ce 100644 --- a/packages/common/test/pipes/case_conversion_pipes_spec.ts +++ b/packages/common/test/pipes/case_conversion_pipes_spec.ts @@ -25,8 +25,18 @@ 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(0 as any)).toThrowError(); + }); it('should not support other objects', () => { - expect(() => pipe.transform({})).toThrowError(); + expect(() => pipe.transform({} as any)).toThrowError(); }); }); @@ -80,8 +90,18 @@ 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(0 as any)).toThrowError(); + }); it('should not support other objects', () => { - expect(() => pipe.transform({})).toThrowError(); + expect(() => pipe.transform({} as any)).toThrowError(); }); }); @@ -101,8 +121,18 @@ 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(0 as any)).toThrowError(); + }); it('should not support other objects', () => { - expect(() => pipe.transform({})).toThrowError(); + expect(() => pipe.transform({} as any)).toThrowError(); }); }); }