xstream icon indicating copy to clipboard operation
xstream copied to clipboard

Type empty() with Stream<never> instead of Stream<any> ?

Open ryota-ka opened this issue 5 years ago • 2 comments

Currently (as of v11.11.0) empty() returns Stream<any>.

https://github.com/staltz/xstream/blob/3181720fc714be8a712a371462f6b17003c3ea5f/src/index.ts#L1305-L1310

But IMO Stream<never> is more suitable here.

For example, I ran into the following situation in Cycle.js.

function higherOrderComponent<So extends ..., Si extends ...>(main: Component<So, Si>): ... {
    const { foo = Stream.empty() , ...sinks } = main(sources);
    // `foo` is typed as Stream<any> here
    // Its original type has been lost!
}

If Stream.empty() was typed Stream<never>, foo would retain its original type.

concerns

  • This change requires newer TypeScript version
    • but never type was introduced already in TypeScript v2.0 (almost 3 years ago)
    • Which version of TypeScript does xstream support?
  • This change may introduce some breaking changes to ill-typed codebases.

ryota-ka avatar May 27 '19 03:05 ryota-ka

Hey @ryota-ka thanks for this thoughtful issue and the PR too. The reason I didn't answer yet was because this issue is not so obvious what's the right solution.

I'm aware of never, but changing empty() and never() from Stream<any> to Stream<never> has to support the same use cases as before. From the top of my head, here are some important use cases:

  • xs.merge(a$, xs.never()) should be allowed
  • xs.merge(a$, xs.empty()) should be allowed
  • concat(a$, xs.never()) should be allowed
  • concat(a$, xs.empty()) should be allowed
  • Passing xs.never() or xs.empty() to a function f: Stream<T> => B should be allowed, e.g. f(xs.empty())

The problem with Stream<never> is that it doesn't support this last use case. Not in the cases I tested. For instance, if you change xstream's source code according to your PR, and try to build the Cycle.js codebase (the monorepo), it will break in a couple of those cases.

Actually the xs.merge(a$, xs.never()) use case seems well supported by Stream<never> and produces even more meaningful types in the end. But with this change we gain on one side and lose on another side.

staltz avatar Dec 19 '19 17:12 staltz

So are you talking about the variance of the generic parameter T? Is it related to https://github.com/staltz/xstream/issues/286?

BTW, I came up with another typing, does it possibly work...?

- static never(): Stream<any> {
-    return new Stream<any>({ _start: noop, _stop: noop });
+ static never<T = never>(): Stream<T> {
+    return new Stream<T>({ _start: noop, _stop: noop });


ryota-ka avatar Jan 01 '20 14:01 ryota-ka