umbrella icon indicating copy to clipboard operation
umbrella copied to clipboard

[rstream] Subscription<A, B> doesn't implement ISubscribable<B> as expected

Open earthlyreason opened this issue 5 years ago • 5 comments

Symptom

I have often run into the following, in one form or another:

import * as rs from "@thi.ng/rstream";

const get_sub = <T>(arg: rs.Subscription<any, T>): rs.ISubscribable<T> => arg;
const get_isub = <T>(arg: rs.ISubscribable<T>): rs.ISubscribable<T> => arg;

const foo = get_sub(rs.subscription<any, number>()); // rs.ISubscribable<number>
const ifoo = get_isub(rs.subscription<any, number>()); // rs.ISubscribable<unknown>

(Note that the same applies to Stream<T>, which is how I usually encounter this in practice.)

Because Subscription<A, B> implements ISubscribable<B>, you would expect both types to be the same. And in fact, assignability works as you would expect:

type ThisIsTrue = rs.Subscription<any, number> extends rs.ISubscribable<number>
  ? true
  : false;

However, the inference of T in ISubscribable does not work as expected when the source type is a Subscription, as seen in the call sites above.

Cause

The reason traces back to the way that Subscription<A, B> fulfills the overloads of ISubscribable<T>.subscribe().

Here is a standalone extract of the relevant declarations:

interface ISubscriber<T> {}
interface Transducer<T, U> {}

export interface ISubscribable<T> {
  subscribe<C>(sub: Partial<ISubscriber<T>>, xform: Transducer<T, C>, id?: string): Subscription<T, C>;
  subscribe<C>(sub: Subscription<T, C>): Subscription<T, C>;
  subscribe<C>(xform: Transducer<T, C>, id?: string): Subscription<T, C>;
  subscribe(sub: Partial<ISubscriber<T>>, id?: string): Subscription<T, T>;
}
export declare class Subscription<A, B> implements ISubscriber<A>, ISubscribable<B> {
  subscribe<C>(sub: Partial<ISubscriber<C>>, xform: Transducer<B, C>, id?: string): Subscription<B, C>;
  subscribe<C>(sub: Subscription<B, C>): Subscription<B, C>;
  subscribe<C>(xform: Transducer<B, C>, id?: string): Subscription<B, C>;
  subscribe(sub: Partial<ISubscriber<B>>, id?: string): Subscription<B, B>;
}

type NumSub = Subscription<any, number>;
export type Number = NumSub extends Subscription<any, infer S> ? S : never;
export type Unknown = NumSub extends ISubscribable<infer S> ? S : never;

This reproduces the problem that the second type is unknown instead of number.

However, those overloads may not be resolving as intended. If you remove all but the first, for example:

export interface ISubscribable<T> {
  subscribe<C>(sub: Partial<ISubscriber<T>>, xform: Transducer<T, C>, id?: string): Subscription<T, C>;
}
export declare class Subscription<A, B> implements ISubscriber<A>, ISubscribable<B> {
  subscribe<C>(sub: Partial<ISubscriber<C>>, xform: Transducer<B, C>, id?: string): Subscription<B, C>;
}

TypeScript gives the following error:

Property 'subscribe' in type 'Subscription<A, B>' is not assignable to the same property in base type 'ISubscribable<B>'.
  Type '<C>(sub: Partial<ISubscriber<C>>, xform: Transducer<B, C>, id?: string) => Subscription<B, C>' is not assignable to type '<C>(sub: Partial<ISubscriber<B>>, xform: Transducer<B, C>, id?: string) => Subscription<B, C>'.
    Types of parameters 'xform' and 'xform' are incompatible.
      Types of parameters 'rfn' and 'rfn' are incompatible.
        Type 'Reducer<any, B>' is not assignable to type 'Reducer<any, C>'.
          Type 'B' is not assignable to type 'C'.
            'B' is assignable to the constraint of type 'C', but 'C' could be instantiated with a different subtype of constraint '{}'. [2416]

So in fact, the Subscription class is only able to compile because some other overload is satisfying that signature. You end up with something like the following—which TypeScript allows.

export interface IThing<T> {
  thing(arg: Thing<T>): Thing<T>;
}

export declare class Thing<T> implements IThing<T> {
  thing<C>(arg: Thing<C>): Thing<C>;
}

export type Number = Thing<number> extends Thing<infer S> ? S : never;
export type Unknown = Thing<number> extends IThing<infer S> ? S : never;

When an overload matches but C cannot be inferred, the result is unknown.

(Note that TypeScript cares about the actual implementation, not the declared one. In other words, just because class X<T> implements Y<T>, TypeScript does not automatically substitute for T in Y when it is known in X. The declaration only serves to ensure that X includes members assignable to the corresponding properties. TIL.)

Resolution

It appears that this overload

    subscribe<C>(
        sub: Partial<ISubscriber<B>>,
        xform: Transducer<B, C>,
        id?: string
    ): Subscription<B, C>;

should instead say

        sub: Partial<ISubscriber<C>>,

In order to match the interface. This causes the overloads to line up as intended and resolves the unknown issue that I was seeing.

When I make that change on the latest master, however, this causes a type error in subs/transduce.ts at line 39:

                    const _acc = rfn[2](acc, x);
                                             ^
Argument of type 'A' is not assignable to parameter of type 'B'.
  'A' is assignable to the constraint of type 'B', but 'B' could be instantiated with a different subtype of constraint '{}'. [2345]

Before I go into this further, does this make sense so far?

Thanks.

earthlyreason avatar Oct 25 '19 21:10 earthlyreason

Hey @gavinpc-mindgrub - thank you so much for this (once again!) detailed analysis. I've struggled with these Subscription generics quite a bit in the beginning once I made transducers play a more central role. I think this change makes sense, but I'd ask you to not make any changes to any rstream package on any other branch than feature/rstream-opts, since all these packages have already undergone some other refactoring (related to #92, #91, #81, #74), but I haven't had a chance to merge them back into develop (or even master) yet... maybe applying this change to that branch also doesn't cause that subs/transduce to break?

postspectacular avatar Oct 25 '19 21:10 postspectacular

Just wanted to say thanks @postspectacular for the (as usual!) quick reply. I have not had a chance to revisit this and am currently using annotations as a workaround. But I will proceed with your suggestion (testing change on feature/rstream-opts) early next week.

earthlyreason avatar Oct 29 '19 14:10 earthlyreason

No worries, @gavinpc-mindgrub - I also still owe you a proper reply to the meta discussion thread! I'm aiming to push out new releases of everything new on the develop branch later this week, but there're a few other rstream related pieces I want to revisit, so this has no time pressure...

postspectacular avatar Oct 29 '19 23:10 postspectacular

Hi @gavinpc-mindgrub - I'm currently working on the various rstream updates and just been looking at this issue here. It seems I've already addressed this (albeit unreleased so far) in a commit to the feature/rstream-opts branch in August (https://github.com/thi-ng/umbrella/commit/da52b9872b5063b617bb217a7c54743d427aa219), where I also introduced a new ITransformable interface. Would be keen to hear your thoughts on these changes...

postspectacular avatar Nov 23 '19 20:11 postspectacular

Thanks, @postspectacular. I am on vacation at the moment but am also keen to check out the branch you mentioned. I'll try it and report back at next opportunity.

earthlyreason avatar Nov 27 '19 04:11 earthlyreason