Typescript: createAsyncThunk type infer not working correctly in 1.6.0 with multiple enum

[243083df]

In 1.5.1 work fine, in 1.6.0 not.
If explicitly typed, everything ok.

Argument of type '() => Promise<Example>' is not assignable to parameter of type 'AsyncThunkPayloadCreator<Example.A, void, {}>'.
  Type 'Promise<Example>' is not assignable to type 'MaybePromise<Example.A | RejectWithValue<unknown, unknown>>'.
    Type 'Promise<Example>' is not assignable to type 'Promise<Example.A | RejectWithValue<unknown, unknown>>'.
      Type 'Example' is not assignable to type 'Example.A | RejectWithValue<unknown, unknown>'.
        Type 'Example.B' is not assignable to type 'Example.A | RejectWithValue<unknown, unknown>'.ts(2345)

export enum Example{
  A = 0,
  B = 1
}

export const bug = createAsyncThunk("foo/bar", async () => {
  if (Math.random() > 0.5) {
    return Example.A;
  } else {
    return Example.B;
  }
});

[phryneas]

Yes, it's possible that the behaviour changed here slightly.

It should work if you add a return value like:

- export const bug = createAsyncThunk("foo/bar", async () => {
+ export const bug = createAsyncThunk("foo/bar", async (): Example => {
  if (Math.random() > 0.5) {
    return Example.A;
  } else {
    return Example.B;
  }
});

Does that sound like an acceptable solution for you?

[243083df]

Its acceptable to me, but i just pointed breaking change.
It acceptable to you?

[gtestault]

1.6.1 broke one of my apps too. Type inference was not working anymore with an asyncthunk function returning a boolean

[gtestault]

Currently even annotating the async function doesn't work in my case. I'm stuck with 1.5.1 at the moment

[phryneas]

@gtestault seems to be a different problem then. Can you share code or even a reproduction?

[gtestault]

export const fetchIsAdmin = createAsyncThunk(
    'session/isAdmin',
    async () => {
        const response = await api.getInstance().checkAdminRights()
        return response.data
    }
)

throws this:

Argument of type '() => Promise<boolean>' is not assignable to parameter of type 'AsyncThunkPayloadCreator<false, void, {}>'.
  Type 'Promise<boolean>' is not assignable to type 'AsyncThunkPayloadCreatorReturnValue<false, {}>'.
    Type 'Promise<boolean>' is not assignable to type 'Promise<false | RejectWithValue<unknown, unknown>>'.
      Type 'boolean' is not assignable to type 'false | RejectWithValue<unknown, unknown>'.  TS2345

    17 | export const fetchIsAdmin = createAsyncThunk(
    18 |     'session/isAdmin',
  > 19 |     async () => {
       |     ^
    20 |         const response = await api.getInstance().checkAdminRights()
    21 |         return response.data
    22 |     }

[phryneas]

I have no idea what TypeScript is doing there, going from boolean to a strict false - I can reproduce it.
I'll look into that.

Meanwhile,

export const fetchIsAdmin = createAsyncThunk<boolean, void>(
    'session/isAdmin',
    async () => {
        const response = await checkAdminRights()
        return response.data
    }
)

should do the trick.

[bfricka]

Passing generic types into the function or having to define the return type is, to me, the crux of this issue and I consider 1.6 a breaking change for sure. Many of our AsyncThunks are now errors, where in 1.5 the correct return type was inferred.

Additionally, we use a factory to bind the state and some other types into createAsyncThunk and we define a default condition check. One of the points of this is that it gives us the types of of State and Extra in our AsyncThunks without having to define any generic params. We instead only have to have a stable return type and define the ThunkArg. E.g.

export const fetchFoo = createAsyncThunk(
  'foo',
  async ({ id, optionalProp = 'defaultProp' }: { id: string, optionalProp?: string }, { dispatch, extra: { http } }) => {
    const data = await dispatch(http.get<SomeReturn>('/foo', {id, optionalProp}))

    return data
  }

In this example, the return type is hinted to http generic. http and dispatch are fully typed because this createAsyncThunk is created from a factory w/ the types for those bound to our defaults. And ThunkArg is obviously typed as you'd expect. This means that in calling const a = await dispatch(fetchFoo({id: 'a'})).then(unwrapResult), a is SomeReturn and the ThunkArg is type checked.

I'm seeing a bunch of issues with return types in 1.6 when nothing in our code has changed. For example, union types are failing and it's showing the same sorts of errors as OP. Anything like:

enum AorBProp {
  A = 'A',
  B = 'B',
}
type A = { prop: AorBProp.A }
type B = { prop: AorBProp.B }
type AorB = A | B

Results in the error:

Type 'Promise<AorB>' is not assignable to type 'Promise<RejectWithValue<unknown, unknown> | A>'.
  Type 'AorB' is not assignable to type 'RejectWithValue<unknown, unknown> | A'.
    Type 'B' is not assignable to type 'RejectWithValue<unknown, unknown> | A'.
      Type 'B' is not assignable to type 'A'.
        Types of property 'prop' are incompatible.
          Type 'AorBProp.B' is not assignable to type 'AorBProp.A'.

In this case, even giving the return type explicitly does nothing to fix the situation.

I'm happy with some of the improvements in 1.6, but this sort of breaking change should not have made it into a feature release.

[bfricka]

Here's a follow up repro:

enum Foo {
  A = 'A',
  B = 'B',
}

type A = { prop: Foo.A }
type B = { prop: Foo.B }
type AorB = A | B

// Error
const a = createAsyncThunk('foo', () => Promise.resolve<AorB>({ prop: Foo.A }))

Also note that rejectWithValue type can only be defined through generics and can't be inferred. This is true of AsyncThunkConfig. E.g. we can't do something like:

createAsyncThunk('foo', (_: void, { rejectWithValue }: { rejectWithValue: RejectWithValueFn<AorB> }) => ({}))

It has to be defined through generics. That means we have to explicitly define Returned, ThunkArg, and ThunkApiConfig, just to provide types for rejectWithValue. The complexity of these types is why we bind State and Extra.

[markerikson]

@bfricka Dealing with types is, frankly, complex. It gets even more so when you as an end user are trying to wrap and reuse types from a library.

We do our best to avoid anything that might accidentally break user code, but honestly, sometimes these things happen. We can't anticipate every way that users will use our APIs, or every possible interaction. That's the nature of software development.

We have a ton of tests for both runtime and types behavior, and we put out several alphas and betas asking for feedback. No bugs were reported around this behavior.

What would help at the moment is reproductions that demonstrate code that worked in 1.5 and breaks in 1.6, so that we can look at those and try to figure out options for improving behavior (and add tests to ensure this doesn't break again in the future). You've given a couple of those, and if you could provide any more as CodeSandboxes or repos (especially with some of these more complex use cases) that would be helpful.

[bfricka]

You're entirely correct. I apologize for coming off as ranty, and I was actually thinking this as I was writing earlier, but I didn't convey it for whatever reason. The types for this project are very complex, but that let's them do some wonderfully powerful things. I'm definitely aware of that, and I'm super clear that there's no way you can model out all the ways in which people could use the types. I will do my best to try to tease out the root of these issues. I've found what I think is the problem area, but it might take me some time to find a solution that will preserve the desired type behavior of all of the existing use cases. Along the way, if I run into any issues, I'll post codesandbox reproductions and describe where I'm at. Thank you

…

On Fri, Aug 27, 2021, 16:46 Mark Erikson ***@***.***> wrote: @bfricka <https://github.com/bfricka> Dealing with types is, frankly, complex. It gets even more so when you as an end user are trying to wrap and reuse types from a library. We do our best to avoid anything that might accidentally break user code, but honestly, sometimes these things happen. We can't anticipate every way that users will use our APIs, or every possible interaction. That's the nature of software development. We have a ton of tests for both runtime and types behavior, and we put out several alphas and betas asking for feedback. No bugs were reported around this behavior. What would help at the moment is reproductions that demonstrate code that worked in 1.5 and breaks in 1.6, so that we can look at those and try to figure out options for improving behavior (and add tests to ensure this doesn't break again in the future). You've given a couple of those, and if you could provide any more as CodeSandboxes or repos (especially with some of these more complex use cases) that would be helpful. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1156 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFIJHYYUVBSZ4E6ARDAP2LT7APVPANCNFSM46MDCPYQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[phryneas]

TypeScript itself has breaking changes to all users of TypeScript in every minor version, similarly we also sometimes cannot avoid them on type level.
We are trying our best, but even though we have a full suite of type tests running against the last 5 versions of TS, things slip through.
Also, you might have noticed that we only do releases every few months, so if things slip through it might take some time to get back to it.
We are few people and doing this in our free time and there is much more to this project than just writing code - there is a lot of community work involved as well that is just as important.
Given that we are such a small team, real life has to have priority for us, as an Open Source project of this size otherwise can easily burn out the contributors. There have been enough examples of that over the last few years, so please keep that in mind and please in general keep in mind what message you are sending. This whole issue has not been very positive.

That said, I have looked into this and believe I have found a fix in #1449. Please try out the CodeSandbox CI build and report back.

[bfricka]

Unfortunately, while this may fix the boolean issue it doesn't appear to fix the enum + type union issue that I described above. I applied the MaybePromise fix and pasted my above example, and I get the same error. I'm now reading the conditional types article you linked (which is awesome, thank you), to see if I can figure out how to fix this.

Edit: I take it back. This was an IDE issue. While there are still some type issues I'll need to work through with the new rejectWithValue types, the enum + union issues are fixed by that conditional voodoo.

[phryneas]

As I said - best install the CodeSandbox build - the link above has install instructions, it's really just npm install <someUrl>. Copy-pasting types always leads to some problems 😅

[bfricka]

Sorry about that. I'm just unfamiliar with that workflow and took a shortcut, which obviously didn't pay off. Thank you for finding a fix for this so quickly.