Skip to content
Permalink
Browse files

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

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
  • Loading branch information...
cexbrayat authored and jasonaden committed Apr 26, 2019
1 parent 124d1ab commit 722b2fa6ed77f3afff29a3525e56b6af3e9670af
@@ -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;

@@ -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]);
@@ -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 = {

0 comments on commit 722b2fa

Please sign in to comment.
You can’t perform that action at this time.