fix(publish): fix selector typings#2891
Conversation
Generated by 🚫 dangerJS |
-+32.9KB-red.svg?style=flat-square){kind=icon}
-+2.3KB-red.svg?style=flat-square){kind=icon}
|
With the changes in this PR, the |
|
I recall @benlesh intentionally brought |
src/operators/publish.ts
Outdated
| * @owner Observable | ||
| */ | ||
| export function publish<T>(selector?: MonoTypeOperatorFunction<T>): MonoTypeOperatorFunction<T> { | ||
| export function publish<T, R>(selector?: any): any { |
There was a problem hiding this comment.
This should be publish<T, R>(selector?: OperatorFunction<T, R>): OperatorFunction<T, R>
There was a problem hiding this comment.
Using OperatorFunction<T, R> instead of any effects this error:
25 return selector ?
~~~~~~~~~~
26 multicast(() => new Subject<T>(), selector) :
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
27 multicast(new Subject<T>());
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
dist/src/operators/publish.ts(25,10): error TS2322: Type '((source: Observable<T>) => Observable<R>) | ((source: Observable<T>) => Observable<T>)' is not assignable to type '(source: Observable<T>) => Observable<R>'.
Type '(source: Observable<T>) => Observable<T>' is not assignable to type '(source: Observable<T>) => Observable<R>'.
Type 'Observable<T>' is not assignable to type 'Observable<R>'.
Type 'T' is not assignable to type 'R'.
The problem is the returning of multicast(new Subject<T>()). That call sees the return type inferred as OperatorFunction<T, T> - hence the error.
The problem can be fixed by using any here, instead of in the signature:
return selector ?
multicast(() => new Subject<T>(), selector) :
multicast(new Subject<T>()) as any; // <-- here
Would you prefer that?
|
@kwonoj ... this is mostly right. The problem I encountered at work (now that I'm using RxJS with TypeScript all the time, haha) was |
|
@cartant if you can try to get this in today I'll try to get it out in the next beta. Because I've had to code around it, and it's annoying. haha |
Fixes the typings for publish and multicast, so that selectors that change the observable's type are supported. Closes ReactiveX#2889
|
@benlesh After a couple of iterations - each preceded by a cup of coffee - there are now no |
| */ | ||
| export function publish<T>(selector?: MonoTypeOperatorFunction<T>): MonoTypeOperatorFunction<T> { | ||
| export function publish<T, R>(selector?: OperatorFunction<T, R>): MonoTypeOperatorFunction<T> | OperatorFunction<T, R> { | ||
| return selector ? |
There was a problem hiding this comment.
I think you can go with just OperatorFunction<T, R> as the return type.
There was a problem hiding this comment.
Nope. You can't. That will effect the error I mentioned in response to your initial change request.
There was a problem hiding this comment.
It's a necessity. I don't see it as a problem, as TypeScript uses only the overload signatures when matching methods calls. It won't match calls against the implementation signature - which is why I have the habit of whacking implementation signatures with the any hammer. That is, the implementation signature does not form part of the method's public contract; it only enforces the internal contract.
There was a problem hiding this comment.
Yup. I see why now... This works. Sorry for any confusion.
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
5 participants
Description:
Fixes the typings for
publishandmulticast, so that selectors that change the observable's type are supported.Adds some typings tests for
publishandmulticast.Related issue (if exists): #2889