mobx-state-tree: [TypeScript 3.6+] flow() | Inferred Generator type is not assignable to IterableIterator

Feature request

Is your feature request related to a problem? Please describe. Prior to TS 3.6 flow() yield return types would be typed as any due to typescript limitations. Those limitations are being addressed in TS 3.6 release as described in this iteration plan

Describe the solution you’d like Full support for yield return types in flow()

Describe alternatives you’ve considered If the full inference is not possible due to some TS limitations (I haven’t completely dived into new generators typings) I wouldn’t mind having to manually declare generic parameters in flow()

Additional context I’ve seen some mentions in this PR, but I have not seen the actual issue for tracking this particular case

Are you willing to (attempt) a PR?

  • Yes
  • No

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 10
  • Comments: 22 (13 by maintainers)

Commits related to this issue

Most upvoted comments

Sorry for slacking on this. For now I’ve come up with this temporary solution:

import { flow as _flow } from 'mobx-state-tree'
import { FlowReturnType } from 'mobx-state-tree/dist/core/flow'

export type Flow = <T extends Promise<any>, R, Args extends any[]>(
    generator: (...args: Args) => Generator<T, R, T extends Promise<infer Y> ? Y : never>
) => (...args: Args) => Promise<FlowReturnType<R>>

export const flow = _flow as Flow

Good parts:

  • Works with inferred Generator type
  • TS now requires Promise to be yielded, which is required by flow() but wasn’t type checked before
  • TS now can infer something for each yielded “awaited” Promise

Bad part is that whatever TS infers has to be compatible with every single one of the yield calls. In practice it turns into something like this:

flow(function*() {
    const x = yield Promise.resolve(42)
    const y = yield Promise.resolve('42')
    const z = yield Promise.resolve(true)
    return null
})

The return type is inferred correctly, but type for x, y and z is number | string | boolean.

Bottom line is, in case of having only one yield call TS works as expected, otherwise we get a union type, which is in my opinion still better than having any, since it’s at least partially type checks.

v3.15.0 released, please give it a try 😃

How to use it:

    const M = types
        .model({
            title: types.string
        })
        .actions(self => ({
            setTitleAsync: flow(function* (newTitle: string) {
                yield delay(10)
                self.title = newTitle
                return 23
            }))
        })

const m = M.create({})
const n = await m.setTitleAsync("new title") // n is correctly inferred to be a number

The only thing that is still not quite working is getting the proper return type from yields, so those will still return any for now and should be type casted. e.g.

const whatever: string = yield somethingThatReturnsPromiseString()

actionAsync is an alternative to mobx flows, not mobx state tree ones. To make them work with mobx state tree it would need its own particular implementation (same than there’s a different implementation for flows in mobx).

That being said I’ll cut a new version of mst with the fix for flow typings, but it will make ts 3.6 the new minimum version when using flows

@xaviergonz I’m now able to remove the icky typing and I can confirm it works. Thank you!

you guys should check out mobx-utils actionAsync thanks to the great work of @xaviergonz! I could rewrite all of my flows using that pretty easily.

As for flow() post TS3.6, I think instead of Generator<Promise<any>, void, any> I would use Generator<Promise<unknown>, void, unknown> so that Typescript will force me to cast type.

Upon further research, new TS typings are indeed not flexible enough to provide strongly typed next(), since it would require to define relationships between yield’ed invocations, which is not possible (at least as of yet).

However, as @fruitraccoon mentioned we now get compile errors, since the new inferred Generator<…> type is not assignable to IterableIterator<…>, because of the ability to distinguish between yield and return types.

I believe this issue should be about compatibility with TS 3.6+, but I am not quite sure that there’s a backward-compatible change to be made, so that’s a real problem

I see that the linked mobx issue mentioned above has been closed as the TS typing was deemed not powerful enough. However, I get compile errors when attempting to use TS 3.6.2 with MST flow because of the new types.

The code below shows me a typescript error (sorry, I couldn’t find anywhere online to create a demo):

import { types, flow, applySnapshot } from 'mobx-state-tree';

function getItems() {
  return new Promise<string[]>(resolve => resolve(['one', 'two']));
}

const ItemsRepo = types
  .model('ItemsRepo', {
    items: types.array(types.string),
  })
  .actions(self => {
    const load = flow(function*() { // <-- error here under the `function` keyword
      const content: string[] = yield getItems();
      applySnapshot(self, {
        items: content,
      });
    });

    return { load };
  });

The error itself is:

(local function)(): Generator<Promise<string[]>, void, string[]>
Argument of type '() => Generator<Promise<string[]>, void, string[]>' is not assignable to parameter of type '() => IterableIterator<Promise<string[]>>'.
  Type 'Generator<Promise<string[]>, void, string[]>' is not assignable to type 'IterableIterator<Promise<string[]>>'.
    Types of property 'next' are incompatible.
      Type '(...args: [] | [string[]]) => IteratorResult<Promise<string[]>, void>' is not assignable to type '(...args: [undefined] | []) => IteratorResult<Promise<string[]>, any>'.
        Types of parameters 'args' and 'args' are incompatible.
          Type '[undefined] | []' is not assignable to type '[] | [string[]]'.
            Type '[undefined]' is not assignable to type '[] | [string[]]'.
              Type '[undefined]' is not assignable to type '[string[]]'.
                Type 'undefined' is not assignable to type 'string[]'.ts(2345)

The best workaround that I’ve found looks like this:

.actions(self => {
    const load = flow(function*(): Generator<Promise<any>, void, any> {
      const content: string[] = yield getItems();
      applySnapshot(self, {
        items: content,
      });
    });

This explicitly types the returned generator. The any as the third generic parameter fixes/hides the issue.

Perhaps there’s a way to update flow so that this explicit typing isn’t required? (ideally in a backward-compatible way)

I would argue, that falling back to Promise<any> is a shaky solution. It is quite unsound, so you might forget to specify a type or specify a wrong type and TS would not complain about it.

Inferred union type is more verbose(you’re forced to use “useless” type guard or as cast), but it is also more sound. Consider this: (playground link)

type A = string | number
const x: A = /* ... */
const y: string = x as string // TS knows it could be done
const z: boolean = x as boolean // TS would complain here

In this example as cast is pretty safe, because starting at some version TS would complain about casting non-overlapping types.

Downside is you can still choose an incorrect type from a union, and TS would fail to notice: (playground link)

function* f(): Generator<string | number, void, string | number> {
    const x = (yield '42') as string // Safe, but ugly and a little awkward
    const y = (yield '42') as number // Unsound, ERROR at runtime, but OK in terms of casting
    const z = (yield '42') as boolean // Sound, error at compile time
}

For my case it is a trade-off I’m willing to take in favour of having any