Skip to content

Commit

Permalink
feat(common): stricter types for SlicePipe (#30156)
Browse files Browse the repository at this point in the history
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 May 17, 2019
1 parent 124d1ab commit 722b2fa
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 2 deletions.
4 changes: 4 additions & 0 deletions packages/common/src/pipes/slice_pipe.ts
Expand Up @@ -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;

Expand Down
9 changes: 8 additions & 1 deletion packages/common/test/pipes/slice_pipe_spec.ts
Expand Up @@ -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]);
Expand Down
5 changes: 4 additions & 1 deletion tools/public_api_guard/common/common.d.ts
Expand Up @@ -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 = {
Expand Down

0 comments on commit 722b2fa

Please sign in to comment.