Update RxJS in user test suite

[DanielRosenwasser]

Sounds like something slipped by us this release.

https://twitter.com/BenLesh/status/1166838448688381953

[DanielRosenwasser]

@benlesh got any details you can share on what went wrong?

[benlesh]

I'm currently trying to update master to 3.6.2, and just running into interesting new typing issues. Probably a dozen or so in total. Most of them are probably a correctness thing, I'm sure.

You can try it though:

1. Pull RxJS from this sha: ReactiveX/rxjs@c311d5b
2. npm i -D [email protected]
3. npm run build_all

:)

I'm not complaining, but I swear that 90% of RxJS work is just updating typings and dealing with the fallout of type updates, new inference patterns, and changes to strictness. :) It's just the job now, I guess. haha

[benlesh]

Some of it seems to stem from some interesting trickery we're doing to type things like switchMap, such that they're able to properly infer types from union returns.

Prior to doing some trickery (a while ago) with conditional types, this didn't work:

import { of } from 'rxjs';
import { switchMap } from 'rxjs';

of(Math.random()).pipe(switchMap(n => n > 0.5 ? of('test') : of(123))) // $ExpectType Observable<string | number>

See ObservedValueOf<O> in src/internal/types.ts and the type overloads in src/internal/operators/switchMap.ts to see said trickery, btw.

[weswigham]

I think the Rx in our user suite is just the published npm package, not their input source build. We'd probably need to add it properly as a built item.

[benlesh]

Yeah, there's some incompatibility with @types/node (v9 and v10) it looks like:

node_modules/@types/node/index.d.ts:74:11 - error TS2300: Duplicate identifier 'IteratorResult'.

74 interface IteratorResult<T> { }
             ~~~~~~~~~~~~~~

  node_modules/typescript/lib/lib.es2015.iterable.d.ts:41:6
    41 type IteratorResult<T, TReturn = any> = IteratorYieldResult<T> | IteratorReturnResult<TReturn>;
            ~~~~~~~~~~~~~~
    'IteratorResult' was also declared here.

node_modules/typescript/lib/lib.es2015.iterable.d.ts:41:6 - error TS2300: Duplicate identifier 'IteratorResult'.

41 type IteratorResult<T, TReturn = any> = IteratorYieldResult<T> | IteratorReturnResult<TReturn>;
        ~~~~~~~~~~~~~~

  node_modules/@types/node/index.d.ts:74:11
    74 interface IteratorResult<T> { }
                 ~~~~~~~~~~~~~~
    'IteratorResult' was also declared here.``

Updating to the latest version of @types/node resolves that.

[OliverJAsh]

We just tried to upgrade unsplash.com from TS 3.5.2 to 3.6.2 and ran into 2 regressions, which I've filed:

• 3.6 regression: error TS2321: Excessive stack depth comparing types 'Function<?>' and 'Function<?>'. #33132
• 3.6 regression: Error: Debug Failure. No error for last overload signature #33133

I'd really love for Unsplash to be part of the TS user test suite as well, so we don't have to find these issues ourselves each time we try to upgrade (as we have been doing).

[felixfbecker]

Related: #33131

[benlesh]

It looks like the biggest problems right now are failures around a solution we were using to get types to infer properly from some of our functions. Here you can find a minimal reproduction of what we were doing

Basically this:

/** Problem */

function toIterator<T>(iterable: Iterable<T>): Iterator<T> {
    return iterable[Symbol.iterator]();
}


const a = toIterator('test');
const b = toIterator([1, 2, 3]);
const c = toIterator(Math.random() ? 'test' : [1, 2, 3]); // ERROR HERE

/** Solution */

type IteratedValueOf<I> = I extends Iterable<infer T> ? T : never;

function toIteratorFixed<T extends Iterable<any>>(iterable: T): Iterator<IteratedValueOf<T>> {
    return iterable[Symbol.iterator]();
}

const d = toIteratorFixed('test');
const e = toIteratorFixed([1, 2, 3]);
const f = toIteratorFixed(Math.random() ? 'test' : [1, 2, 3]); // FIXED HERE

[weswigham]

cc @ahejlsberg the above example is connected with the union type matching behavior we added into inference.

[weswigham]

Although probably just a dupe of #33131 at this point (which I opened #33154 for) but I'll link the two issues anyway for context.