cujojs / most

Ultra-high performance reactive programming
MIT License
3.49k stars 231 forks source link

Streams became covariant #521

Open bigslycat opened 5 years ago

bigslycat commented 5 years ago

I want to downgrade a type sometimes.

declare var string$: Stream<string>;
const stringNumber$ = (string$: Stream<number | string>).startWith(0);

Streams are read-only, so there is no need to leave them invariant.

briancavalier commented 5 years ago

Hey @bigslycat. Thanks for the PR. Can you help me understand the real world use cases where someone might want to do this? Are there any cases for it outside of startWith?

bigslycat commented 5 years ago

@briancavalier

declare var string$: Stream<string>;
declare var number$: Stream<number>;

const stringNumber1$ = (string$: Stream<number | string>).startWith(0);
const stringNumber2$ = (string$: Stream<number | string>).merge(number$);
const stringNumber3$ = merge(string$, number$);
const stringNumber4$ = (string$: Stream<number | string>).concat(number$);
const stringNumber5$ = (string$: Stream<number | string>).recoverWith(() => of(0));
const stringNumber6$ = (string$: Stream<number | string>).continueWith(() => of(0));

Invariance makes streams not usable. Types of read-only entities can be covariant.

briancavalier commented 5 years ago

@bigslycat What we're looking for is real world use cases. I want to understand the benefits to developers for allowing covariance at the highest level like this.

There are benefits to invariance, and precedent. For example, lists in many statically typed languages are invariant. Since startWith is similar to list Cons, there is precedent for invariance.

I'm not saying I'm against this. Rather, I just want to understand the tradeoffs and real world use cases of allowing covariance so broadly. For example, @most/core has already allowed merge to produce a union type, which is similar to what you're proposing, though more narrow in scope. You can read the summary of why we decided it was safe to do so.

Perhaps similarly, it would be better to identify particular operations where this may be useful, and explicitly allow them to return unions?

Would like to hear thoughts on that from @TylorS, @davidchase, and @Frikki as well.

If we do this, I think it makes sense to do it over in most/core, which is the future of mostjs.

evilsoft commented 5 years ago

Not very familiar with T$, so I have more of a commenty-question then any real feed back or anything.

Does this concept of covariant as a pre-order (I am assuming it is a pre-order, but it probably is a partial order) on some hierarchy, apply for what you want? To me it sounds like you really just want just a union of javascript types (I am not talking a tagged Union Type like Either or Maybe or something, but a union as from Set Theory, basically a Type that is String + Number). Is there a way in T$ to create a type like Number | String that can be that A. That way A is the type union and will work with Stream (String + Number).

Please pardon my ignorance in the subject, it just seems like a type union (if you can do that with T$ that is) could be treated as the type A and that nothing needs to be changed in most.

EDIT: You can also "degrade" by using the Functor portion of the Stream you just need a function that goes from String + Number -> Number then you can lift that using map. of course you will need to have a function that can collapse the String values to a Number.

But this will take the Stream String+Number to a Stream Number...I use things like this (without T$ mind you) for things like query strings on a url and what not when I may have a Numeric value but it hits my server as a String. In that case, my morphisim is just parseInt or her cousins. which is a valid morphisim from String + Number -> Number.

Additional EDIT: Sorry I kept saying T$, when I meant flow. So sorry about that.

briancavalier commented 5 years ago

Hey @evilsoft, good to hear from you.

To me it sounds like you really just want just a union of javascript types

This matches my current thinking. We did it for merge in @most/core (comment here), and so I think we can just extend it to other operations as well.

As an example using @most/core's API: merge(now(x), s) is extremely similar to startWith(x, s).

it probably is a partial order

It seems like partial order (it's antisymmetric?), but you've gone beyond my current type theory knowledge :). I'd love to know the answer.

evilsoft commented 5 years ago

So from what I gather is from this article the is-a hierarchy defines a partial order with:

SanFran -> City -> Noun (covarience)

and of course the Dual (or Opposite or just Op) being: SanFran <- City <- Noun (contravarience)

So while the other two are obvious Partial orders, when I draw it out bivariance, it seems to be a pre-order, where SanFran is lte City AND City is lte SanFran.

So from an order perspective they are all essentially the same things as there is an isomorphisim between all members. So they can all be the same thing as far as the object is 🌽cerned.

But I may be super wrong about that, I am a JS fellow and not really a set/category theorist.

But either way. I do not know if this should be at the heart, it seems the consumer of the Stream should be responsible for managing their types. This could get dangerously close to imposing some structure on the underlying data and breaking the Functor portion. And if Functor ever gets invalidated, then most other operations become invalid. (it may not though, I have not drawn it up to see how imposing an order on the underlying type would effect the underlying structure of JavaScript).

EDIT: I drew up a couple diagrams and yes it commutes if and only if the methods on the subtypes do not override to return/accept different types. Again though, while it does commute, it commutes with constraints and cannot be generalized. I still believe that this order should not be imposed on the underlying type but should be managed by the consumer of Stream. That way they have control over the morphisms.

Just my opinion and thoughts though. Please take with a grain of salt.

briancavalier commented 5 years ago

With apologies to @bigslycat for coopting this thread with slightly tangential discussion :)

Ah, yes, thanks. I was definitely making it too complicated in my head. It's just subtyping, which, as far as I know in Flow (and most other places) is a preorder. I'm far less sure about that when things get into weird Flow/TS territory like width subtyping, function subtypes, etc.

This could get dangerously close to imposing some structure on the underlying data and breaking the Functor portion

Hmm, possibly. It does make me a bit nervous, but is Stream<+A> really somehow "less parametric"?

Or does it mean a Functor in this world can be: (A => +B) => Stream<+A> => Stream<+B>? That seems ok. However, this certainly seems bad: map: (+A => +B) => Stream<+A> => Stream<+B>, due to the covariant function argument, i.e. it'd allow: map: (Dog => Int) => Stream<Animal> => Stream<Int>

briancavalier commented 5 years ago

@bigslycat Please let us know your thoughts about going with union types as per the similar approach in @most/core merge. Thanks!

briancavalier commented 5 years ago

Ping @bigslycat

bigslycat commented 5 years ago

@briancavalier pong. Sorry for delay. I will answer today.

bigslycat commented 5 years ago

@briancavalier

/* @flow */

type Cat = 'cat';
type Dog = 'dog';
type Animal = Cat | Dog;

declare class Stream<+T> {}

declare var cats: Stream<Cat>;
declare var dogs: Stream<Dog>;

// We have two steams and we can to downgrade this types to supertype:

const anumals1: Stream<Animal> = cats;
const anumals2: Stream<Animal> = dogs;

// But how you can to send a Cat to anumals2?
// And how you can to send a Dog to anumals1?
// Observables are readonly. This is safe. If not, show me this in real code plz. =)
// For negative example arrays are writable and in this case you really
// can to send a Cat to anumals2 or can to send a Dog to anumals1.

This code on Flow repl.

bigslycat commented 5 years ago

I can to show you a use case:

import { of, from, Stream } from 'most';
import { mapPropsStream } from 'react-honeycombs';

declare var requestFail$: Stream<Error>;

export BlaBlaComponent: React$ComponentType<Props> = mapPropsStream(
  (props$: Observable<Props>) =>
    from(props$).combine(
      (props, error) => ({ ...props, errror }),
      (requestFail$: Stream<?Error>).startWith(null),
    ),
)(
  ...
);

Look to (requestFail$: Stream<?Error>).startWith(null). Ok if this looks bad, this can be replaced by (requestFail$: Stream<?Error>).merge(of(null)). Anyway, I can't to do anything when Stream is invariant. This is not usable. This is make problems but unnecessary.

briancavalier commented 5 years ago

Thanks for this real use case, @bigslycat. I see what you're saying: you'd like "B is a subtype of A" to imply "Stream<B> is a subtype of Stream<A>".

Here are my main concerns:

  1. The usage of Typescript will be different. It requires explicitly specifying the type, since TS doesn't have co/contravariant annotations (afaik). [Here's an example](https://www.typescriptlang.org/play/#src=type%20Disposable%20%3D%20%7B%20dispose%20()%3A%20void%20%7D%0D%0Atype%20Scheduler%20%3D%20%7B%7D%0D%0A%0D%0Atype%20Sink%3CA%3E%20%3D%20%7B%20event%20(t%3A%20number%2C%20a%3A%20A)%3A%20void%20%7D%0D%0A%0D%0Atype%20Stream%3CA%3E%20%3D%20%7B%20run%20(sink%3A%20Sink%3CA%3E%2C%20s%3A%20Scheduler)%3A%20Disposable%20%7D%0D%0A%0D%0Aclass%20At%3CA%3E%20%7B%0D%0A%20%20time%3A%20number%0D%0A%20%20value%3A%20A%0D%0A%20%20constructor%20(t%3A%20number%2C%20x%3A%20A)%20%7B%0D%0A%20%20%20%20this.time%20%3D%20t%0D%0A%20%20%20%20this.value%20%3D%20x%0D%0A%20%20%7D%0D%0A%0D%0A%20%20run%20(sink%3A%20Sink%3CA%3E%2C%20scheduler%3A%20Scheduler)%3A%20Disposable%20%7B%0D%0A%20%20%20%20return%20%7B%20dispose()%3A%20void%20%7B%7D%20%7D%0D%0A%20%20%7D%0D%0A%7D%0D%0A%0D%0Adeclare%20function%20merge%20%3CA%3E%20(s1%3A%20Stream%3CA%3E%2C%20s2%3A%20Stream%3CA%3E)%3A%20Stream%3CA%3E%3B%0D%0A%0D%0A%2F%2F%20Both%20kinds%20of%20supertype%20need%20to%20work%0D%0Aclass%20Animal%20%7B%7D%0D%0A%2F%2Ftype%20Animal%20%3D%20Cat%20%7C%20Dog%0D%0A%0D%0Aclass%20Cat%20extends%20Animal%20%7B%20meow()%20%7B%7D%20%7D%0D%0Aclass%20Dog%20extends%20Animal%20%7B%20bark()%20%7B%7D%20%7D%0D%0A%0D%0Aconst%20cats%3A%20Stream%3CCat%3E%20%3D%20new%20At(0%2C%20new%20Cat())%0D%0Aconst%20dogs%3A%20Stream%3CDog%3E%20%3D%20new%20At(0%2C%20new%20Dog())%0D%0A%0D%0Aconst%20animals%20%3D%20merge%3CAnimal%3E(cats%2C%20dogs)). The last line has call merge explicitly in the context of Animal.
  2. Flow may infer an unexpected (although still reasonable) type, which could be confusing for users. See this example where Flow infers Cat | Dog on the last line, and some users will expect it to infer Animal.
  3. We need to consider implications to the mostjs ecosystem, especially things like most-subject, which allows operations similar to mutable Array push(). I wish we didn't need it, but it's been necessary at a practical level for integrating with other libs like virtual DOM implementations.

It may be ok, though, since most-subject already has two type parameters: one implication is that mostjs Sink must be contravariant, Sink<-A>, and then most-subject would need to change its Subject type to Subject<-A, +B>.

@bigslycat I'd like to hear your thoughts on those 3 concerns. Thanks!

briancavalier commented 5 years ago

@bigslycat Your most recent example seems also to work with invariant union types, as has been proposed already in this thread. It just requires startWith to allow a union return type.

Could you please respond about whether that would work for you. I realize it may not be the solution you'd prefer, but we have to consider the big picture, so it would be helpful to know if this is viable for you. If it's not, please explain and give an example of why it's not.

Thanks!