redux icon indicating copy to clipboard operation
redux copied to clipboard

[bug: typescript] ActionCreator lose it's arguments type

Open dabuside opened this issue 6 years ago • 11 comments

What is the current behavior?

https://github.com/reduxjs/redux/blob/beb1fc29ca6ebe45226caa3a064476072cd9ed26/test/typescript/actionCreators.ts#L13-L18

We declare the addTodo function that accepts one string argument text, but we could pass any argument to it.

const addTodoAction: AddTodoAction = addTodo(false, 0, null); 👆👆👆 //no ts error!

There may be a problem (...args: any[]) https://github.com/reduxjs/redux/blob/c676a25525bd7e4f2bf407cf1a135f355a2c1d79/index.d.ts#L440-L442

What is the expected behavior?

Raise a type error

dabuside avatar Jun 21 '19 05:06 dabuside

We can likely add a second, defaulted type argument to define what the input should be.

timdorr avatar Jun 21 '19 13:06 timdorr

@dabuside that's a natural behavior. The ActionCreator type only looks at the returned type and because the arguments are typed as any it doesn't complain about the provided type.

It's a result of bad typing on reduxes side and that they want to allow different paradigms there. If they'd use unknown instead for the arguments, and you'd have enabled strictFunctionTypes - part of strict- in TypeScript compiler options then you'd see the problem, but as it is you're essentially casting with that type declaration.

If you want to only use this style of action creators where you don't build complicated actions from the arguments of the action creator - which I'd not recommend - then you might want to look at this https://www.npmjs.com/package/typed-redux-actions-reducer or this https://www.npmjs.com/package/typesafe-actions.

9SMTM6 avatar Jun 23 '19 13:06 9SMTM6

so, there is a way to fix this. It requires adding a new, simple type:

export type ArgsTypedActionCreator<T, A> = T extends (...args: any[]) => any
  ? (ReturnType<T> extends A ? T : never)
  : never

Used as in this example:

const creator: ArgsTypedActionCreator<(name: string, age?: number) => PersonAction, PersonAction>
  = (name: string, age?: number) => ({ type: 'person', name, age })

This can also be used in the definition for ActionCreatorsMapObject:

export interface ActionCreatorsMapObject<A = any, T = any> {
  [key: string]: T extends ArgsTypedActionCreator<T, A> ? T : never
}

By using the conditional, this forces the action creators to fit the definition, without making them un-typesafe.

If this seems reasonable, I'll whip up a PR (have a branch sitting here I used to experiment with)

cellog avatar Aug 24 '19 02:08 cellog

@dabuside check out #3525 for a possible solution that may work for you

cellog avatar Aug 25 '19 00:08 cellog

OK, so after 2 weeks of reflection, I am not convinced that there is a good solution with the existing typing.

We probably should just use unknown instead of any to prevent direct usage of it.

Or, we could revamp the action creator definition to make strict typing easier, but all of the solutions I have seen are awkward from a pure JS perspective, and so unlikely to age well.

cellog avatar Sep 08 '19 13:09 cellog

A very sad issue, in fact, a frustrating one.

The easiest way to fix this: just do not type the action creator itself and do not use this particular Redux typing, but instead type function's returning value directly with your Action type.

So instead:

export const doSomething: ActionCreator<AppAction> = (value: number) => {
    return {
        type: TYPE.DO_SOMETHING,
        value
    };
};

Do this:

export const doSomething = (value: number): AppAction => {
    return {
        type: TYPE.DO_SOMETHING,
        value
    };
};

For redux-thunk the same approach, but it may differ based on how you actually type thunks in your app.

In my case it was instead this:

export const doSomething: ThunkActionCreator = (value: number) => {
    return dispatch => {
        dispatch(anotherActionCreator());
    };
};

Do this:

export const doSomething = (value: number): ThunkActionType => {
    return dispatch => {
        dispatch(anotherActionCreator());
    };
};

Where ThunkActionType is this:

(dispatch: ThunkDispatch<AppState, any, AppAction>, getState?: () => AppState) => void

antmelnyk avatar Apr 22 '20 08:04 antmelnyk

I would recommend the same as @pixelgoo, It is possible to fix the ActionCreator type but it may not be worth to do it in my opinion.

Simpler solution as @pixelgoo indicated is probably the way to go.

I would recommend to check this library, it helps a lot for generating different kind of action: https://github.com/piotrwitek/typesafe-actions

chaabaj avatar Aug 06 '20 14:08 chaabaj

@chaabaj, @pixelgoo : we specifically recommend using our official Redux Toolkit package, which A) is already written in TS, and B) will auto-generate action creators for you:

https://redux-toolkit.js.org

markerikson avatar Aug 06 '20 14:08 markerikson

@markerikson Not that easy. Redux Toolkit is a bit too opinionated as for our project and we can't just move on to it.

  1. Performance concerns. We have a custom canvas UI layer and custom Redux bindings to it, performance is very important for us. The less code from dispatch call to the UI subscription handler, the better.
  2. Increased complexity of the API (especially with TypeScript usage). We have some huge reducers with a lot of composable functions, and some more complicated action handling then switch-case. It's really much easier to use pure TypeScript and rely on Redux core instead of using things like builder.addMatcher or prepare callback.
  3. Hidden "magic" with Immer.js. Some part of the team is not that experienced with Redux and this would only cause more confusion.

The simplicity and "just JavaScript" part of Redux really is good in our case, while additional API, and things like Immer.js just add additional abstractions.

Nevertheless, thank you for your work with the community and popularization of the Redux with the toolkit and new docs 👍 For next projects we totally will try it.

antmelnyk avatar Aug 08 '20 09:08 antmelnyk

@pixelgoo The pattern I use for Action Creators these days is:

const someAction = (someTypedData: string) => ({
  type: "SOME_ACTION" as const, 
  payload: { someTypedData }, 
});

export const actionsForSpecificReducer={
  someAction, 
  //... 
} 

export type ActionsForSpecificReducer = ExtractActionTypes<typeof actionsForSpecificReducer>;

Take note of the as const, which causes the string literal to not widen to the type string. We also need:

import {ValuesType} from "type-utils";

type ActionCreatorObject={
  [key in string]: (...args: any[]) => {type:string} 
} 

export type ExtractActionTypes<Actions extends ActionCreatorObject>= ValuesType<{
  [key in keyof Actions]: ReturnType<Actions[key]>;
}>

You can then use the ActionsForSpecificReducer as type for the Actions part of the reducer, which can be written in regular switch case structure and which WILL be able to correctly identify the right payload type depending on the type field. The cases will be simple strings, no enums, but typechecked. Honestly with the Union type and string literals theres little use case for enums in typescript as far as I'm concerned.

Dunno how much if something at all of that is usable for you, but it's closer to traditional usage so I thought I'd mention it.

9SMTM6 avatar Aug 08 '20 10:08 9SMTM6

@pixelgoo FWIW:

  • RTK is UI-agnostic, like the Redux core, and you can use it with any UI layer. Also, reducers are rarely perf bottlenecks - it's the cost of updating the UI that's typically more expensive.
  • Use of configureStore and createSlice/createReducer will eliminate the potential for any accidental mutations, as well as making the rest of your reducer logic shorter
  • Even if you aren't using them (and you should), createAction eliminates the need to write action creators by hand, and produces well-typed action creators
  • createAsyncThunk and createEntityAdapter will also simplify those use cases
  • Agree that use of Immer takes some explaining, but it's absolutely worth it in the long run. See the new "Redux Essentials" tutorial and how it explains immutability and Immer.

I'd be curious to see what your "more complex reducer logic" looks like. Could you open up an issue over in the RTK docs and show some examples? It would at least give us some ideas for cases that RTK might not currently handle that would be worth considering. (For that matter, I'm also curious to see what your "custom canvas UI layer" looks like, particularly in comparison to React-Redux.)

markerikson avatar Aug 08 '20 15:08 markerikson