Update to TypeScript 3.5.3 #4985

benlesh · 2019-08-27T00:07:01Z

Updates places that were inferring Observable<{}> to infer Observable<unknown> instead.
Updates to proper types for TS 3.5.3 for certain dtslint tests
Updates types for of.
- BREAKING CHANGE: generic signature of of has changed. Users should allow this type to infer properly or use as. There is deprecated handling for the common cases of of<T>() and of<T>(x), but multiple specified generic arguments (like of<A, B>(a, b)) will yield incorrect types, possibly build errors, and should not be used.
- BREAKING CHANGE: uses of of with >9 arguments punctuated with a SchedulerLike will no longer properly infer, and will need to be cast. Scheduled of is deprecated anyhow, and developers should be using scheduled from rxjs. (Before it would infer Observable<{}>, which is no longer possible, and now it will infer Observable<A | B | C | D | E | F | G | H | I>, which isn't correct. Again, use scheduled instead.

benlesh · 2019-08-27T00:07:56Z

Replaces #4976

cartant

LGTM. Just a couple of nits/questions.

cartant · 2019-08-27T12:00:35Z

src/internal/types.ts

@@ -101,3 +101,5 @@ export interface SchedulerAction<T> extends Subscription {
 export type ObservedValueOf<O> = O extends ObservableInput<infer T> ? T : never;

 export type ObservedValuesFromArray<X> = X extends Array<ObservableInput<infer T>> ? T : never;
+
+export type ArrayValueOf<A> = A extends Array<infer T> ? T : never;


Given that the type above this one is named ObservedValuesFromArray and extracts the observed values from an array and that this one just extracts the value from an array, should it be called ValueFromArray?

cartant · 2019-08-27T12:02:31Z

src/internal/observable/concat.ts

@@ -139,5 +139,5 @@ export function concat<R>(...observables: (ObservableInput<any> | SchedulerLike)
 * @owner Observable
 */
 export function concat<O extends ObservableInput<any>, R>(...observables: Array<O | SchedulerLike>): Observable<ObservedValueOf<O> | R> {


Do we really need this R here and in the signatures above it? It seems that it's only present so that folk can explicitly provide it as a type parameter and do we really want to encourage that? IIRC, when discussing this sort of thing with Rado, the preference was to return Observable<unknown> - or, at the time, Observable<{}> - and require the caller to use an as assertion.

I was looking at that and thought the same thing, honestly.

Okay, I updated the typings for concat. Note the possible breaking change for people that are explicitly passing generics.

cartant · 2019-08-27T12:04:03Z

spec-dtslint/operators/scan-spec.ts

-  // the below could be a number or a string.
-  const a = of(1, 2, 3).pipe(scan((a: any, v) => '' + v)); // $ExpectType Observable<string | number>
+  // Starting in TS 3.5, the return type is inferred from the accumulator's type if it's provided without a seed.
+  const a = of(1, 2, 3).pipe(scan((a: any, v) => '' + v)); // $ExpectType Observable<any>


Maybe type the accumulator as a string and expect Observable<string>? Is there anything to be gained by using any here?

The problem is if you do a: string, TypeScript will infer an operator like OperatorFunction<string, string>, which is incorrect here, with a source of Observable<number>

I see. Do you think it would be worth including another test that explicitly types the value - to make sure that works, too? I assume that value will also be any here - for the reason you gave above? FWIW, in application code I'd type it like this:

(scan((a: string, v: number) => '' + v))

So maybe we need a test to make sure this is okay?

cartant · 2019-08-27T12:04:43Z

spec-dtslint/operators/reduce-spec.ts

-  // the below could be a number or a string.
-  const a = of(1, 2, 3).pipe(reduce((a: any, v) => '' + v)); // $ExpectType Observable<string | number>
+  // Starting in TS 3.5, the return type is inferred from the accumulator's type if it's provided without a seed.
+  const a = of(1, 2, 3).pipe(reduce((a: any, v) => '' + v)); // $ExpectType Observable<any>


As for scan, maybe use string as the accumulator type?

cartant

LGTM. A nit about a test description and an unused import, that's all.

cartant · 2019-08-29T00:21:53Z

spec-dtslint/observables/concat-spec.ts

@@ -28,8 +28,8 @@ it('should accept more than 6 params', () => {
  const o = concat(of(1), of(2), of(3), of(4), of(5), of(6), of(7), of(8), of(9)); // $ExpectType Observable<number>
 });

-it('should return Observable<{}> for more than 6 different types of params', () => {
-  const o = concat(of(1), of('a'), of(2), of(true), of(3), of([1, 2, 3]), of(4)); // $ExpectType Observable<{}>
+it('should return Observable<unknown> for more than 6 different types of params', () => {


This description no longer matches the test's expectation.

cartant · 2019-08-29T00:28:16Z

src/internal/observable/of.ts

 import { isScheduler } from '../util/isScheduler';
 import { fromArray } from './fromArray';
 import { Observable } from '../Observable';
 import { scheduleArray } from '../scheduled/scheduleArray';
+import { never } from './never';


This isn't needed. It looks like VS Code's auto-import has added this.

- Uses `unknown` type instead of `{}` per TS 3 - Updates dtslint tests where certain things are now working better BREAKING CHANGE: RxJS requires TS 3.5

- `of()` now properly infers `Observable<never>` - `of()` can handle any number of arguments gracefully BREAKING CHANGE: Generic signature changed, do not specify generics, allow them to be inferred or use `as` BREAKING CHANGE: Use with more than 9 arguments, where the last argument is a `SchedulerLike` may result in the wrong type which includes the `SchedulerLike`, even though the run time implementation does not support that. Developers should be using `scheduled` instead

- Removes superfluous return typing - Updates deprecation messages BREAKING CHANGE: `concat` generic signature changed. Recommend not explicitly passing generics, just let inference do its job. If you must, cast with `as`.

…oolean is the predicate

Updates bad types in test-helpers and removes dependency on symbol-observable

…gs for us

benlesh mentioned this pull request Aug 27, 2019

fix(of): Fix compilation failure in TS 3.5+ for of() #4976

Closed

benlesh force-pushed the update-ts-3.5.3 branch 3 times, most recently from 2948f98 to e641af8 Compare August 27, 2019 00:34

benlesh requested a review from cartant August 27, 2019 00:37

benlesh mentioned this pull request Aug 27, 2019

Using Boolean as predicates. #4986

Closed

benlesh force-pushed the update-ts-3.5.3 branch from e641af8 to 944c745 Compare August 27, 2019 04:23

cartant reviewed Aug 27, 2019

View reviewed changes

benlesh force-pushed the update-ts-3.5.3 branch from 944c745 to cdb9218 Compare August 28, 2019 00:31

cartant approved these changes Aug 29, 2019

View reviewed changes

benlesh added 9 commits September 3, 2019 16:10

feat(TS): Update to TypeScript 3.5.3

741a136

- Uses `unknown` type instead of `{}` per TS 3 - Updates dtslint tests where certain things are now working better BREAKING CHANGE: RxJS requires TS 3.5

fixup! feat(of): Update of typings

b8cb3a9

feat(concat): can infer N types

6c0cbc4

- Removes superfluous return typing - Updates deprecation messages BREAKING CHANGE: `concat` generic signature changed. Recommend not explicitly passing generics, just let inference do its job. If you must, cast with `as`.

fix(filter): Resolve TS build failures for certain situations where B…

77c7dfd

…oolean is the predicate

fixup! feat(of): Update of typings

73f2150

chore: remove symbol-observable dependency

97ee666

Updates bad types in test-helpers and removes dependency on symbol-observable

chore: remove explicit of generics

7b88a27

chore(ci): Switch CI to TypeScript 3.5.*, because 3.6 has breaking bu…

8a78923

…gs for us

benlesh force-pushed the update-ts-3.5.3 branch from cf14990 to 8a78923 Compare September 3, 2019 21:11

fixup! chore: remove symbol-observable dependency

9e3bb85

benlesh merged commit 56cbd22 into ReactiveX:master Sep 16, 2019

benlesh deleted the update-ts-3.5.3 branch September 16, 2019 19:49

lock bot locked as resolved and limited conversation to collaborators Oct 16, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to TypeScript 3.5.3 #4985

Update to TypeScript 3.5.3 #4985

benlesh commented Aug 27, 2019 •

edited

Loading

benlesh commented Aug 27, 2019

cartant left a comment

cartant Aug 27, 2019 •

edited

Loading

cartant Aug 27, 2019 •

edited

Loading

benlesh Aug 27, 2019

benlesh Aug 28, 2019

cartant Aug 27, 2019

benlesh Aug 27, 2019

cartant Aug 28, 2019

cartant Aug 27, 2019

cartant left a comment

cartant Aug 29, 2019

cartant Aug 29, 2019

Update to TypeScript 3.5.3 #4985

Update to TypeScript 3.5.3 #4985

Conversation

benlesh commented Aug 27, 2019 • edited Loading

benlesh commented Aug 27, 2019

cartant left a comment

Choose a reason for hiding this comment

cartant Aug 27, 2019 • edited Loading

Choose a reason for hiding this comment

cartant Aug 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cartant left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benlesh commented Aug 27, 2019 •

edited

Loading

cartant Aug 27, 2019 •

edited

Loading

cartant Aug 27, 2019 •

edited

Loading