.await has wrong type?

[jasonkuhrt]

Summary

I think the TypeScript type for Most.await is problematic. Consider this code:

  return Most
    .periodic(queueSettings.WaitTimeSeconds / 1000)
    .map(() => pull(settings.queue, queueSettings))
    .await() // should be awaitPromises but TypeScript definition doesn't accept that yet...
    .concatMap((data: AWS.SQS.ReceiveMessageResult) =>
      Most.from(data.messages.map(process)),
    )
    .multicast()

Expected result

It should return a Stream<Change>.

Actual Result

It instead returns a Stream<{}>. This is because of await which has a return value of Stream<{}>.

Versions

• most.js: ^1.4.0

I will try to come back with a PR or failing that a simplified repro.

[briancavalier]

@jasonkuhrt I'm not really sure a correct type definition can be written for the method form of .await(). See this note. AFAIK, there's no way in TS to express "this method can only be called on a this whose value type is Promise<B>".

I don't know why TS would infer a type of {} for B in this case, though. Maybe @TylorS has some fresh insight on it.

[briancavalier]

Ah I just saw your comment about awaitPromises being missing from the TS defs, too! Good catch. We should def fix that as well.

[jasonkuhrt]

for the method form of .await()

Oh but function form yes?

// Note: Without higher-kinded types, this type cannot be written properly

What would higher-kinded types look like in this case? FlowType has the same limitation?

[jasonkuhrt]

No idea if this would help: https://medium.com/@gcanti/higher-kinded-types-in-typescript-static-and-fantasy-land-d41c361d0dbe

[briancavalier]

Yep, it's possible for the function form. The type def shows how await "removes" the Promise "wrapper" from every event in the stream.

What would higher-kinded types look like in this case?

In this case, there would need to be a way to express that the .await method is only valid for a this which is Stream<Promise<A>>. Maybe HKTs aren't strictly necessary for that, but I don't know of any way to express it in TS.

FlowType has the same limitation?

It does indeed. I'm not aware of any way to do it Flow, either.

@gcanti has done some pretty amazing work in simulating HKTs and more recently, representing them using property types (I think that's what they're called) in TS. It seems like applying his patterns may require a fairly significant restructuring of most.js, but I really have no idea right now.

[jasonkuhrt]

@briancavalier I've never used compse with most before but the API doesn't appear to be curried which does hinder its practicality as a replacement for method chains IMHO.

[gcanti]

there's no way in TS to express "this method can only be called on a this whose value type is Promise<B>"

Maybe this can be helpful

declare class Stream<A> {
  map<B>(f: (a: A) => B): Stream<B>
  // you can define a type annotation for this
  await<B>(this: Stream<Promise<B>>): Stream<B>
  // etc...
}

new Stream<number>().await() // error
new Stream<Promise<number>>().await() // ok
new Stream<string>().map(s => Promise.resolve(s.length)).await() // ok

[briancavalier]

Thanks, @gcanti. I didn't know type constraints on this existed as a feature in TS! Seems like a great solution. Do either of you know how new that feature is? i.e. what version of TS? If it is "too new", we may need to wait a bit until that version of TypeScript is in wide use.

Looks like there's at least some discussion about adding an equivalent feature to Flow.

[briancavalier]

the API doesn't appear to be curried

Yeah, in 1.x it isn't. In @most/core, it is, and it will be in most 2.0, since it'll be based on @most/core.

[gcanti]

@briancavalier seems a pretty old feature (v2.0.0) https://github.com/Microsoft/TypeScript/wiki/What's-new-in-TypeScript#specifying-the-type-of-this-for-functions

[briancavalier]

@gcanti Great, seems safe to require it then. Thanks for the help :+1:

@jasonkuhrt If you're still up for a PR, I'd certainly welcome one that applies a this constraint for .await() and adds awaitPromises alias for function and method.

[jasonkuhrt]

@briancavalier I'll give it a go! I'll try to do it this week.

[briancavalier]

@jasonkuhrt this just landed in most 1.6.1. Thanks again for raising the issue, and thanks @gcanti for the solution.