Rxjs: Type signature review progress for v7 beta

Created on 10 Oct 2019 · 7Comments · Source: ReactiveX/rxjs

This issue is for the tracking of the review of the TypeScript signatures - that involve either an Observable or ObservableInput rest parameter or a callback that returns ObservableInput or have open, type-related issues, etc. - as per the discussion in today's core-team meeting.

Deprecations for the scheduler-related changes are not in this list. My understanding is that those deprecations will remain in place, with the breaking changes being made in version 8.

@benlesh As I've review individual source files, I'll check them off in this list if they're okay and will link to a comment (or issue) if changes are required or attention is warranted.

In addition to what's shown below, I think we should look at how null and undefined are used to ensure that passing either- to indicate the absence of an argument - is supported.

[x] AsyncSubject.ts
[x] BehaviorSubject.ts
[x] Notification.ts - see #5478, #5031
[x] Observable.ts - see https://github.com/ReactiveX/rxjs/issues/3890, https://github.com/ReactiveX/rxjs/issues/4159, https://github.com/ReactiveX/rxjs/issues/4415 and https://github.com/ReactiveX/rxjs/pull/5072
[x] Observer.ts
[x] Operator.ts
[x] ReplaySubject.ts
[x] Scheduler.ts
[x] Subject.ts - see this comment
[x] Subscriber.ts
[x] Subscription.ts
[ ] types.ts - see https://github.com/ReactiveX/rxjs/issues/4532 and this comment and this comment
[ ] observable/bindCallback.ts - needs N-args signature for values? Should fall back to unknown instead of any. Also, see this comment.
[ ] observable/bindNodeCallback.ts - needs N-args signature for values? Should fall back to unknown instead of any. Also, see this comment.
[x] observable/combineLatest.ts - needs N-args signature for observable inputs? See also #5209
[x] observable/concat.ts
[x] observable/defer.ts
[x] observable/dom/AjaxObservable.ts
[ ] observable/dom/WebSocketSubject.ts
[x] observable/dom/ajax.ts
[x] observable/dom/animationFrames.ts
[x] observable/dom/fetch.ts - see https://github.com/ReactiveX/rxjs/issues/4744
[ ] observable/dom/webSocket.ts
[x] observable/empty.ts
[x] observable/forkJoin.ts - needs N-args signature for observable inputs? See also #5209
[x] observable/from.ts - see https://github.com/ReactiveX/rxjs/issues/4532
[x] observable/generate.ts
[x] observable/iif.ts
[x] observable/interval.ts
[x] observable/merge.ts - needs N-args signature for observable inputs?
[x] observable/never.ts
[x] observable/of.ts - see https://github.com/ReactiveX/rxjs/pull/5027
[ ] observable/onErrorResumeNext.ts - needs N-args signature for observable inputs?
[x] observable/pairs.ts - see https://github.com/ReactiveX/rxjs/issues/4468
[x] observable/partition.ts
[ ] observable/race.ts - needs N-args signature for observable inputs?
[x] observable/range.ts
[x] observable/throwError.ts
[x] observable/timer.ts
[ ] observable/using.ts - needs observable input factory handled?
[ ] observable/zip.ts - needs N-args signature for observable inputs? Also see #5209
[x] operators/audit.ts
[x] operators/auditTime.ts
[x] operators/buffer.ts
[x] operators/bufferCount.ts
[x] operators/bufferTime.ts
[x] operators/bufferToggle.ts
[x] operators/bufferWhen.ts
[x] operators/catchError.ts
[x] operators/combineAll.ts
[ ] operators/combineLatest.ts - see https://github.com/ReactiveX/rxjs/issues/3927 and needs N-args signature for observable inputs?
[ ] operators/concat.ts - see https://github.com/ReactiveX/rxjs/issues/3927 and needs N-args signature for observable inputs?
[x] operators/concatAll.ts
[x] operators/concatMap.ts
[x] operators/concatMapTo.ts
[x] operators/count.ts
[x] operators/debounce.ts
[x] operators/debounceTime.ts
[x] operators/defaultIfEmpty.ts
[x] operators/delay.ts
[x] operators/delayWhen.ts
[ ] operators/dematerialize.ts (more work here #5478)
[x] operators/distinct.ts
[x] operators/distinctUntilChanged.ts
[x] operators/distinctUntilKeyChanged.ts
[x] operators/elementAt.ts
[x] operators/endWith.ts - needs N-args signature for values?
[x] operators/every.ts
[x] operators/exhaust.ts
[x] operators/exhaustMap.ts
[ ] operators/expand.ts - needs observable input factory handled?
[x] operators/filter.ts
[x] operators/finalize.ts
[x] operators/find.ts
[x] operators/findIndex.ts
[x] operators/first.ts
[x] operators/groupBy.ts
[x] operators/ignoreElements.ts
[x] operators/isEmpty.ts
[x] operators/last.ts
[x] operators/map.ts
[ ] operators/mapTo.ts - see https://github.com/ReactiveX/rxjs/issues/5090
[x] operators/materialize.ts
[x] operators/max.ts
[ ] operators/merge.ts - see https://github.com/ReactiveX/rxjs/issues/3927 and needs N-args signature for observable inputs?
[x] operators/mergeAll.ts
[x] operators/mergeMap.ts
[x] operators/mergeMapTo.ts
[ ] operators/mergeScan.ts
[x] operators/min.ts
[x] operators/multicast.ts
[x] operators/observeOn.ts
[ ] operators/onErrorResumeNext.ts - needs N-args signature for observable inputs?
[x] operators/pairwise.ts
[x] operators/pluck.ts
[x] operators/publish.ts
[ ] operators/publishBehavior.ts - see https://github.com/ReactiveX/rxjs/issues/2845
[x] operators/publishLast.ts
[ ] operators/publishReplay.ts - see https://github.com/ReactiveX/rxjs/issues/2845
[ ] operators/race.ts - see https://github.com/ReactiveX/rxjs/issues/3927 and needs N-args signature for observable inputs?
[ ] operators/reduce.ts
[x] operators/refCount.ts
[x] operators/repeat.ts
[x] operators/repeatWhen.ts
[x] operators/retry.ts
[x] operators/retryWhen.ts
[x] operators/sample.ts
[x] operators/sampleTime.ts
[ ] operators/scan.ts
[x] operators/sequenceEqual.ts
[x] operators/share.ts
[x] operators/shareReplay.ts
[x] operators/single.ts
[x] operators/skip.ts
[x] operators/skipLast.ts
[x] operators/skipUntil.ts
[x] operators/skipWhile.ts
[x] operators/startWith.ts - needs N-args signature for observable inputs?
[x] operators/subscribeOn.ts
[x] operators/switchAll.ts
[x] operators/switchMap.ts
[x] operators/switchMapTo.ts
[x] operators/take.ts
[x] operators/takeLast.ts
[x] operators/takeUntil.ts
[x] operators/takeWhile.ts
[ ] operators/tap.ts - see https://github.com/ReactiveX/rxjs/issues/4159
[x] operators/throttle.ts
[x] operators/throttleTime.ts
[x] operators/throwIfEmpty.ts
[x] operators/timeInterval.ts
[x] operators/timeout.ts
[x] operators/timeoutWith.ts
[x] operators/timestamp.ts
[x] operators/toArray.ts
[x] operators/window.ts
[x] operators/windowCount.ts
[x] operators/windowTime.ts
[x] operators/windowToggle.ts
[x] operators/windowWhen.ts
[ ] operators/withLatestFrom.ts - needs N-args signature for observable inputs?
[ ] operators/zip.ts - see https://github.com/ReactiveX/rxjs/issues/3927 and needs N-args signature for observable inputs?
[x] operators/zipAll.ts
[x] scheduled/scheduled.ts

TS types

Source

cartant

Most helpful comment

Whether or not anything can be done with this to make things safer, IDK, but we should have another look at it.

Yes, please have another look at it.

The last comment on that issue says it all: You can just replace value?: T by value: T. When T is void, then the argument becomes optional.

const s1 = new Subject<void>();
s1.next(); // OK
s1.next(undefined); // OK

const s2 = new Subject<number>();
s2.next(); // error

The only problem is when someone does this:

const sAny = new Subject(); // No type parameter so defaults to `any`
sAny.next(); // Error because `any` is not mapped to an optional parameter

I don't know what people expect when they create a Subject without an explicit type. The default of Subject<any> is itself dangerous, and could be changed to void:

export declare class Subject<T = void> { /* ... */ }

const sVoid = new Subject(); // Defaults to Subject<void>.
sVoid.next(); // Works fine

If you like to have bugs in your code, then you can still create a subject of any with:

const sAny = new Subject<any>;

hmil on 3 Feb 2020

👍4 🚀1 ❤1 🎉1

All 7 comments

There is an issue with the Subject typings so that next can be called without an argument for Subject<void>, but the implementation makes Subject unsafe - see https://github.com/ReactiveX/rxjs/issues/2852. Whether or not anything can be done with this to make things safer, IDK, but we should have another look at it.

cartant on 10 Oct 2019

👍1

IMO, at some stage we should change signatures that involve errors to use unknown instead of any:

export interface NextObserver<T> {
  closed?: boolean;
  next: (value: T) => void;
  error?: (err: unknown) => void;
  complete?: () => void;
}

See https://github.com/ReactiveX/rxjs/issues/2646

However, this is likely to be an inconvenient breaking change. And it would be reasonable to argue that whilst the TypeScript typings for Promise use any for errors/rejection reasons, this library's typings should continue to use any.

Related Twitter conversation.

cartant on 10 Oct 2019

👍5

startWith and endWith are taken care of here: #5247 #5246

benlesh on 21 Jan 2020

❤1 🎉1

Whether or not anything can be done with this to make things safer, IDK, but we should have another look at it.

Yes, please have another look at it.

The last comment on that issue says it all: You can just replace value?: T by value: T. When T is void, then the argument becomes optional.

const s1 = new Subject<void>();
s1.next(); // OK
s1.next(undefined); // OK

const s2 = new Subject<number>();
s2.next(); // error

The only problem is when someone does this:

const sAny = new Subject(); // No type parameter so defaults to `any`
sAny.next(); // Error because `any` is not mapped to an optional parameter

I don't know what people expect when they create a Subject without an explicit type. The default of Subject<any> is itself dangerous, and could be changed to void:

export declare class Subject<T = void> { /* ... */ }

const sVoid = new Subject(); // Defaults to Subject<void>.
sVoid.next(); // Works fine

If you like to have bugs in your code, then you can still create a subject of any with:

const sAny = new Subject<any>;

hmil on 3 Feb 2020

👍4 🚀1 ❤1 🎉1

OMG, I received a DM question about interop observables and I was writing up some notes that I could turn into an article. The ergonomics of actually using the Subscribable type are horrible. The problem is the number of overloads that are in the type and - when used with from - RxJS always passes an observer - never separate callbacks - so including callbacks in the type is unnecessarily painful.

When https://github.com/ReactiveX/rxjs/issues/4532 is also taken into consideration, pretty much all aspects of this interop are :-1:

cartant on 15 Feb 2020

@cartant I replied to you in https://github.com/ReactiveX/rxjs/issues/4532#issuecomment-619343464 :)