Bug (?) with "action.payload" type inference in AsyncThunkFulfilledActionCreator

[e965]

Situation - I have a promise that returns some data. This promise may return with an error, so it is wrapped in a try/catch (where the error is handled and I get the payload using the thunkAPI.rejectWithValue function).

The problem - on version 1.6.1, the value of the payload returned in the thunk.fulfilled case was the same as the value in the promise. As of version 1.6.2, an unknown type is returned. Without a try/catch wrapper, thunk works as expected and returns the correct type.

Demos:

• 1.6.1 - https://codesandbox.io/s/redux-tookit-demo-1-6-1-bqq95?file=/src/redux-tookit-demo.ts
• 1.6.2 - https://codesandbox.io/s/redux-tookit-demo-1-6-2-wutyl?file=/src/redux-tookit-demo.ts

[loursbourg]

This is actually interesting.
A quick bisect shows that this goes back to #e3bc1fe
For some reason the return statement is what makes the difference here (remove it and the type is immediately recognized).
Was this change intentional?

[markerikson]

Yes, per the commit message, the change there was supposed to fix another type inference bug that happened in 1.6.0:

#1156

[phryneas]

The problem is that with the addition of rejecting and fulfilling with additional meta, the types have gotten to a level of complexity where in some cases inference just breaks and you will need to specify the generics.

Since you should be using the generic arguments as soon as you are using rejectWithValue anyways (otherwise rejectedValue will stay unknown), I'm inclined to just accept that this is happening here.

But if you have the time and resources to experiment around and find a type hack that works in every situation, a PR would be very welcome :)

[e965]

@markerikson The way to handle errors with try/catch is described directly in the official documentation and, I dare to assume, I was not the only one who could break something when upgrading to 1.6.2.

I suggest to roll back this change for now, because it breaking (in fact), and leave it until the next minor version. Patches should not contain breaking changes.

[phryneas]

The documentation example you linked to is a JavaScript example, not a TypeScript example.
The way how to type createAsyncThunk correctly when rejected errors are involved is described in the createAsyncThunk paragraph of the "usage with TypeScript" guide. And that includes specifying the generic.

Also, this cannot simply be rolled back as it is a fix for a much more serious type bug.

Your use case was frankly just working for you purely accidentally and never an intended use case - we would never have assumed that someone would want to use rejectWithError in TypeScript and be happy with the error type being unknown, as that seems to defeat the purpose in many ways.
The assumption was that as soon as rejectWithError was used, people would start declaring the generics, as documented.

As I said, PRs to make this work (without breaking the serious #1156) are welcome.

[e965]

@phryneas ok, got it. Indeed, it is my mistake that I did not read everything carefully :)

Since this is probably not a bug, I close the issue. Thanks for the clarification!

[markerikson]

@e965 yeah, the other factors here are that:

• There's a distinct difference between "bugs" and "breaking changes"
• With TS, almost any change to the types can end up being a "breaking change". This is especially true when you're dealing with types as complex as RTK's
• TS itself doesn't even follow semver, largely because any release of the compiler could end up affecting compilation behavior in some way

If we did something like intentionally rewriting our types to remove public symbols or completely change generic arguments, then yeah, I'd classify that as a true "breaking change" worthy of a major version.

But in most cases, what we're doing is trying to improve behavior with small tweaks, and in some cases that may unfortunately end up causing compile issues. Frankly, this is a tradeoff that we all have to deal with as TS users - you accept the risk of additional complexity up front, in order to get better safety for the codebase as a whole.