Improve type inference of case reducers

[denisw]

Previously, the TypeScript compiler would reject case reducer maps (such as the ones passed to createReducers and createSlice) with different incompatible PayloadAction types. I restructured the types a bit to improve type inference in these cases.

Fixes #131

[netlify]

Deploy preview for redux-starter-kit-docs ready!

Built with commit d1ae4fe

https://deploy-preview-133--redux-starter-kit-docs.netlify.com

[denisw]

I added a commit that upgrades to TypeScript 3.4.3, which I needed to do to reproduce the problem in #131. As a consequence, I also had to upgrade the ESLint TypeScript parser.

[wldcordeiro]

@denisw I had an overlapping dependency bump but this will solve an error case for me!

[tydira]

Thank you for your work on this, @denisw. It's exactly what I had hoped to find.

After testing, I have to report an unexpected (to me) and unfortunate consequence of the changes. I'm creating a reducer with an action type of PayloadAction<T | T[]> and the action creator returned by createSlice() wants a payload of the type T & T[].

I've scrutinized the changes and I'm not exactly sure what would cause this. After reading about union to intersection conversion, I'm guessing it has something to do with the conditionals in CaseReducerActionPayloads. This problem is a bit beyond my understanding for the moment but I'm happy to help any way I can.

[tydira]

Apologies for multiple notifications; I got ahead of myself when I first thought I had a solution.

The behavior I described comes from how TypeScript performs distributive conditional inference on a union of contra-variant types. By wrapping each side of the match of P in SliceActionCreator<P> with a single-element tuple, it prevents the distributive behavior and preserves a union type when inferred in CaseReducerActionPayloads. I have an example at https://github.com/kroogs/redux-starter-kit/commit/27c45cadd82310a4658eda3992c51e5f882b72ab that works as I describe and passes the full test suite. I'd like to submit a PR unless there's a better solution.

[markerikson]

@denisw : does this supersede #110 , or do we need both of them?

[denisw]

@markerikson Pretty sure we only need this PR. I closed the other one.

[markerikson]

@denisw : great, that simplifies things :)

[markerikson]

Awright, green lights, and I don't know what I'm doing types-wise anyway :)

[markerikson]

@kroogs : not quite sure I'm following what you're saying there. If you think there's still typings issues after this PR was merged, please go ahead and file a separate issue / PR.

[markerikson]

Hmm. @denisw : looking at the diffs for this PR and #110 , the slice action types do look different.

// #110
export type SliceActionCreator<P, T extends string = string> = P extends void
  ? () => Action<T>
  : (payload: P) => PayloadAction<P, T>

// #133
export type SliceActionCreator<P> = P extends void
  ? () => PayloadAction<void>
  : (payload: P) => PayloadAction<P>

Looks like 110 is returning a plain Action, while 133 is returning a PayloadAction. Does that make any meaningful difference?

[tydira]

@markerikson The typing issue I encountered with PayloadAction remains. I'll create a new PR tomorrow with a detailed description and a simple fix.

My PR can be found here: #138.

[farah]

@tydira hi there, I know this is a very old pr, but was curious with your fix in #138 about the distributed union behaviour. I was unable to replicate the type errors you mentioned in your comment.

I'm creating a reducer with an action type of PayloadAction<T | T[]> and the action creator returned by createSlice() wants a payload of the type T & T[].

I was unable to reproduce this error v3.2 & 3.4 at he commits prior to pr 138. The test cases you added also would have passed prior to your change. I'm not exactly sure whats happening here, any ideas?

[tydira]

@farah That's odd. Is it possible you're using a non-local TS install? As evident in surrounding comments, it was an issue that a few people were able to confirm at the time.

Edit: I'm guessing the behavior of distributed conditionals hasn't changed in TS, so that's probably not it. Other than that, I wonder if you've somehow got the wrong commit. Not sure, sorry.