feat(groupBy): Add typeguards support for groupBy

[kolodny]

A couple of notes here. I couldn't figure out how to test this since it doesn't work in oldish versions of TS like 2.8 which is where the dtslint tests run. If there was some way to enable/disable for TS versions that would be one solution.

Note that in the example in #5439 you need to test key === true in order to get the type checking to work for both the if and else blocks. This may be related to microsoft/TypeScript#26852

Another item and this is probably a TS issue is that while all 4 of the following usages work, the 5th one, using an inline type guard with no explicit arg type, doesn't work. I'm guessing there's some TS weirdness going on that's probably a bug but I can't explain what is making it behave that way

function isNumberFromAny(input: any): input is number {
  return typeof input === 'number';
}

function isNumberFromStringOrNumber(input: string | number): input is number {
  return typeof input === 'number';
}

// Works!
of('a', 1, 'b', 2).pipe(
  groupBy((x: any): x is number => typeof x === 'number'),
  mergeMap(group => {
    if (group.key === true) {
        group.subscribe(x => x.toExponential(1)); // x is number here
    } else {
        group.subscribe(x => x.toLowerCase()); // x is string here
    }
    return group;
  })
);

// Works!
of('a', 1, 'b', 2).pipe(
  groupBy((x: string | number): x is number => typeof x === 'number'),
  mergeMap(group => {
    if (group.key === true) {
        group.subscribe(x => x.toExponential(1)); // x is number here
    } else {
        group.subscribe(x => x.toLowerCase()); // x is string here
    }
    return group;
  })
);

// Works!
of('a', 1, 'b', 2).pipe(
  groupBy(isNumberFromAny),
  mergeMap(group => {
    if (group.key === true) {
        group.subscribe(x => x.toExponential(1)); // x is number here
    } else {
        group.subscribe(x => x.toLowerCase()); // x is string here
    }
    return group;
  })
);


// Works!
of('a', 1, 'b', 2).pipe(
  groupBy(isNumberFromStringOrNumber),
  mergeMap(group => {
    if (group.key === true) {
        group.subscribe(x => x.toExponential(1)); // x is number here
    } else {
        group.subscribe(x => x.toLowerCase()); // x is string here
    }
    return group;
  })
);

// THIS IS BUSTED!
of('a', 1, 'b', 2).pipe(
  groupBy((x): x is number => typeof x === 'number'),
  mergeMap(group => {
    if (group.key === true) {
        // group is GroupedObservable<true, string | number>
        group.subscribe(x => x.toExponential(1))
    } else {
        // group is GroupedObservable<false, never>
        group.subscribe(x => x.toLowerCase())
    }
    return group;
  })
);

[cartant]

The dtslint tests should be using the version of TS that's installed in node_modules. If that's not the case, the config is broken. FWIW, you need to run it using npm run dtslint.

IDK what happened when I looked quickly at this, on Saturday morning - before I noticed that it has been reassigned - but this now works, for me, in VS Code. That is, hovering over the statements within the if and else show the expected types. It does not seem to need group.key === true and the inline user-defined type guard seems fine.

import { of, OperatorFunction } from "rxjs";
import { groupBy, mergeMap } from "rxjs/operators";

function isNumber(input: any): input is number {
  return typeof input === "number";
}

declare module "rxjs/internal/operators/groupBy" {
  export function groupBy<T, K extends T>(
    keySelector: (value: T) => value is K
  ): OperatorFunction<
    T,
    GroupedObservable<true, K> | GroupedObservable<false, Exclude<T, K>>
  >;
}

of("a", 1, "b", 2).pipe(
  groupBy(isNumber),
  mergeMap((group) => {
    if (group.key) {
      return group; // GroupedObservable<true, number>
    } else {
      return group; // GroupedObservable<false, any>
    }
  })
);

of("a", 1, "b", 2).pipe(
  groupBy((value): value is number => typeof value === "number"),
  mergeMap((group) => {
    if (group.key) {
      return group; // GroupedObservable<true, number>
    } else {
      return group; // GroupedObservable<false, string>
    }
  })
);

PS. the Exclude is nice! ❤️

[cartant]

The problem was that the signature used K for the value type in the user-defined type guard where it should have used T. I've pushed commits to fix this and to add dtslint tests.

Again, the use of Exclude is really nice! ❤️

[benlesh]

Nice work, guys!