feat(common): stricter types for SlicePipe (#30156)

cexbrayat · jasonaden · commit 722b2fa6ed77 · 2019-05-17T14:21:37.000-07:00
Adds overloads to the `transform` methods of `SlicePipe`, to have better types than `any` for `value` and `any` as a return. With this commit, using `slice` in an `ngFor` still allow to type-check the content of the `ngFor` with `fullTemplateTypeCheck` enabled in Ivy: <div *ngFor="let user of users | slice:0:2">{{ user.typo }}</div> | `typo` does not exist on type `UserModel` whereas it is currently not catched (as the return of `slice` is `any`) neither in VE nor in Ivy. BREAKING CHANGE `SlicePipe` now only accepts an array of values, a string, null or undefined. This was already the case in practice, and it still throws at runtime if another type is given. But it is now a compilation error to try to call it with an unsupported type. PR Close #30156
diff --git a/packages/common/src/pipes/slice_pipe.ts b/packages/common/src/pipes/slice_pipe.ts
@@ -62,6 +62,10 @@ export class SlicePipe implements PipeTransform {
    *   - **if positive**: return all items before `end` index of the list or string.
    *   - **if negative**: return all items before `end` index from the end of the list or string.
    */
+  transform<T>(value: ReadonlyArray<T>, start: number, end?: number): Array<T>;
+  transform(value: string, start: number, end?: number): string;
+  transform(value: null, start: number, end?: number): null;
+  transform(value: undefined, start: number, end?: number): undefined;
   transform(value: any, start: number, end?: number): any {
     if (value == null) return value;
 
diff --git a/packages/common/test/pipes/slice_pipe_spec.ts b/packages/common/test/pipes/slice_pipe_spec.ts
@@ -26,16 +26,23 @@ import {expect} from '@angular/platform-browser/testing/src/matchers';
     describe('supports', () => {
       it('should support strings', () => { expect(() => pipe.transform(str, 0)).not.toThrow(); });
       it('should support lists', () => { expect(() => pipe.transform(list, 0)).not.toThrow(); });
+      it('should support readonly lists',
+         () => { expect(() => pipe.transform(list as ReadonlyArray<number>, 0)).not.toThrow(); });
 
       it('should not support other objects',
-         () => { expect(() => pipe.transform({}, 0)).toThrow(); });
+         // this would not compile
+         // so we cast as `any` to check that it throws for unsupported objects
+         () => { expect(() => pipe.transform({} as any, 0)).toThrow(); });
     });
 
     describe('transform', () => {
 
       it('should return null if the value is null',
          () => { expect(pipe.transform(null, 1)).toBe(null); });
 
+      it('should return undefined if the value is undefined',
+         () => { expect(pipe.transform(undefined, 1)).toBe(undefined); });
+
       it('should return all items after START index when START is positive and END is omitted',
          () => {
            expect(pipe.transform(list, 3)).toEqual([4, 5]);
diff --git a/tools/public_api_guard/common/common.d.ts b/tools/public_api_guard/common/common.d.ts
@@ -410,7 +410,10 @@ export interface PopStateEvent {
 export declare function registerLocaleData(data: any, localeId?: string | any, extraData?: any): void;
 
 export declare class SlicePipe implements PipeTransform {
-    transform(value: any, start: number, end?: number): any;
+    transform<T>(value: ReadonlyArray<T>, start: number, end?: number): Array<T>;
+    transform(value: string, start: number, end?: number): string;
+    transform(value: null, start: number, end?: number): null;
+    transform(value: undefined, start: number, end?: number): undefined;
 }
 
 export declare type Time = {
