tp / tsaga

Typesafe and lightweight way to write Redux-connected functions with asynchronous side-effects that are easily testable.
Apache License 2.0
8 stars 1 forks source link

`take` type inference broken #44

Closed tp closed 5 years ago

tp commented 5 years ago

Just saw this in OMS with TypeScript 3.4.1.

Broken: take(storeCampaign);

Argument of type 'ActionCreator<Campaign | null>' is not assignable to parameter of type '{ (payload: null, meta?: { [key: string]: any; } | null | undefined): Action<null>; type: string; match: (action: AnyAction) => action is Action<null>; }'.
  Types of property 'match' are incompatible.
    Type '(action: AnyAction) => action is Action<Campaign | null>' is not assignable to type '(action: AnyAction) => action is Action<null>'.
      Type predicate 'action is Action<Campaign | null>' is not assignable to 'action is Action<null>'.
        Type 'Action<Campaign | null>' is not assignable to type 'Action<null>'.
          Types of property 'payload' are incompatible.
            Type 'Campaign | null' is not assignable to type 'null'.
              Type 'Campaign' is not assignable to type 'null'.

24     take(storeCampaign),

Ok take<Campaign | null>(storeCampaign)

Especially annoying, since we aren't even interested in / using the result, but just want to wait for the message,

Possibly related:

const watchForSetBasketNotification = forEvery<{state: any}>(
  Actions.setBasketNotification,
  handleBasketNotification
);

Does also not work without the type assertion on the forEvery.

HenriBeck commented 5 years ago

The actual issue is that we have two versions of typescript-fsa here. Once 2.5.0 and once 3.0.0-beta-2 which leads to the conflict. The changed the type of the ActionCreator. If the payload extends void which leads to this issue I believe. We could downgrade this version here, but I also pushed a fix which would solve the issue in OMS without upgrading the packages

tp commented 5 years ago

I think we could also just update OMS, no?

tp commented 5 years ago

Ok, we can't do that without further changes. I'll try to downgrade here, the other one is still in beta and hasn't progressed in a while.

tp commented 5 years ago

Thanks for the investigation, I think downgrading is our best choice for now. Published tsaga 2.2.0 with TS FSA 2.5.