Skip to content

createReducer does not infer action types #260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Glinkis opened this issue Nov 20, 2019 · 17 comments
Closed

createReducer does not infer action types #260

Glinkis opened this issue Nov 20, 2019 · 17 comments

Comments

@Glinkis
Copy link

Glinkis commented Nov 20, 2019

When I create a reducer, the action type does not get inferred from the action.
Example:

const myAction = createAction<boolean>('my/action')
const reducer = createReducer(false, {
   // action is of type "any" (and then so is the payload)
  [myAction.type](state, action) {
    return action.payload
  },
})

I am currently rolling my own setup, where I've got this type of inference working.
Though I'm using the payload rather than the whole action object as a parameter for my callbacks.

In my case, I have to type the action like this through an extra function invocation, though I'm not sure how necessary this is.

const action = createAction('my/action')<boolean>()

I don't pretend to understand how all of this works, but maybe someone can figure out a way to get the best of both worlds? What follows is pretty much my whole setup for this.

import { produce, Draft } from 'immer'
import { EmptyAction, PayloadAction,  createAction as createTypesafeAction } } from 'typesafe-actions'

export default function createAction<T extends string>(type: T) {
  return function<P extends any = undefined>() {
    return Object.assign(createTypesafeAction(type)<P>(), { type })
  }
}

export type Action<T extends string = string, P = never> = EmptyAction<T> | PayloadAction<T, P>

export type Producer<S, A extends Action> = A extends PayloadAction<any, infer P>
  ? (draft: Draft<S>, payload: P) => void | S
  : (draft: Draft<S>) => void | S

export type Producers<S, A extends Action> = A extends PayloadAction<any, any>
  ? { [T in A['type']]: Producer<S, A> }
  : { [T in A['type']]: Producer<S, A> }

export default function createReducer<S, A extends Action>(defaultState: S, producers: Producers<S, A>) {
  return function reducer(state = defaultState, action: A) {
    return produce(state, draft => {
      if (action.type in producers) {
          return producers[action.type](draft, action.payload)
      }
    })
  }
}
@markerikson
Copy link
Collaborator

I don't think createReducer is able to infer action types based solely on a name. There's no way for it to have any idea what the actual contents of the action might be from just a random string.

That's actually half the point of createSlice. You define a reducer, including the expected action type, and you get the action creator generated for free using the same action type value and TS type.

Is there a reason you're calling createAction and createReducer manually, instead of using createSlice?

@eddyw
Copy link

eddyw commented Nov 20, 2019

@Glinkis you could do this:

const increment = createAction('counter/increment', (n: number) => ({ payload: n }))
const decrement = createAction('counter/decrement', (n: number) => ({ payload: { n } }))

const counterReducer = createReducer(0, {
  [increment.type]: (state, action: ReturnType<typeof increment>) => state + action.payload,
  [decrement.type]: (state, action: ReturnType<typeof decrement>) => state - action.payload.n,
})

@markerikson
Copy link
Collaborator

Or, just use createSlice:

const counterSlice = createSlice({
    name: "counter",
    initialState: 0,
    reducers: {
        increment: (state, action: PayloadAction<number>) => state + action.payload
    }
})

@Glinkis
Copy link
Author

Glinkis commented Nov 20, 2019

I will probably end up using createSlice in the end, but it would be nice if the other utilities had full type inference too.

action.type is not just a general string but the exact string that is entered into createAction. This is what I use in my code, which does actually infer the types correctly.

@markerikson
Copy link
Collaborator

@phryneas , any thoughts?

@phryneas
Copy link
Member

phryneas commented Nov 20, 2019

@Glinkis You're essentially right, to get type inference for createAction, it would need to be the const action = createAction('my/action')<boolean>() syntax.
This is due to the fact that TS (to my knowledge) cannot mix specified and inferred generics in one definition. But while it would be nice to have that type inference, that syntax just won't do it - people would get irritated, as currying is something that just isn't widely accepted in JS, and type-currying is something I rarely see.

Building on the idea of @eddyw , you could do something like this though:

function withPayloadType<T>(){
  return (t:T) => ({payload: t})
}

const x = createAction('test', withPayloadType<string>())

Would that work for you?

@markerikson we could add this as an example of an alternative way to invoke createAction to the examples, what do you think?

@phryneas
Copy link
Member

phryneas commented Nov 20, 2019

Oh, and as far as createReducer goes: We can't do much more than just basic type guarding there. To make sure the caseReducer's action type (if an action is specified) matches the caseReducer's object key.

But that actually doesn't seem to be working - so I'll guess we'll have to at least fix that (@markerikson , your thoughts?)

const myReducer = createReducer(0, {
  // this is correct
  increment(state, action: {type: "increment"}){},
  // this should error, but doesn't
  decrement(state, action: {type: "foo"}){}
})

@phryneas
Copy link
Member

phryneas commented Nov 20, 2019

On second thought, this could be another application of the API I suggested here for extraReducers. That should do all that inference.
Hmm.. Opinions?

@eddyw
Copy link

eddyw commented Nov 21, 2019

On a similar thing, with createSlice and using prepare, the reducer's second argument (action) is still any:

const counterSlice = createSlice({
  initialState: 0,
  name: 'counter',
  reducers: {
    decrement: {
      prepare: (value: number) => ({ payload: value }),
      reducer: (state, action) => { }, << // action is "any" even though it has 'prepare'
    },
  }
})

@Glinkis

I will probably end up using createSlice in the end, but it would be nice if the other utilities had full type inference too.

action.type is not just a general string but the exact string that is entered into createAction. This is what I use in my code, which does actually infer the types correctly.

I don't think it'd be possible for createSlice to correctly infer the type because it's kind of dynamic, it needs name, so you can't actually do this in TS:

type InferActionType<N extends string, T extends string> = N + '/' + T

🙈

What I currently have with Redux is a type ActionTypes = InferTypes<typeof actions> so I'm also looking at how to achieve this using createSlice but so far, no idea.

I need this because there is a middleware function which looks like:

// action is -> action: ActionTypes
switch (action.type)
   case SOME_ACTION:
      action.payload <<< correctly inferred based on `action.type`
   ...

However, with createSlice, this is just string, so payload is an intersection of all payloads which is not exactly the desired output.

@Glinkis
Copy link
Author

Glinkis commented Nov 21, 2019

@eddyw

I don't think it'd be possible for createSlice to correctly infer the type because it's kind of dynamic, it needs name, so you can't actually do this in TS:

type InferActionType<N extends string, T extends string> = N + '/' + T

Yes, I've looked into this extensively a few times, and it is indeed currently impossible to concatenate string types.

For the slice, wouldn't it be possible to do something like

interface SliceAction<S extends string> extends Action {
  slice: S // the slice name
}

And then use the additional slice property as a type qualifier?

@Glinkis
Copy link
Author

Glinkis commented Nov 21, 2019

@phryneas

Would that work for you?

It might, as long as the rest of the type safety works from this.
It would then also be very easy to create something similar to typesafe-actions createAction.

function createPayloadAction<T extends string>(type: T) {
    // should also be able to take a "prepareAction" without much effort.
    return <P>() => createAction<P, T>(type) 
}
const action = createPayloadAction("action")<string>()

@eddyw
Copy link

eddyw commented Nov 21, 2019

Just a suggestion, don't know if it'd be the best way of doing it:
If createAction added an additional field (now there is payload, meta, error) called .. scope (or something better) and action looked like:

{
   type: 'counter/increment',
   payload: ...
   meta?: ...
   error?: ...
   scope: { name: 'counter', type: 'increment' }
}

This could help, maybe.

I also have a lazily created slice, for example, that'd look (more or less) like:

const createCounter = (name: string, initialState: any) => createSlice({
  name,
  initialState,
  reducers: { ... }
})

This is because I'd like to have composable reducers that are not aware of name and they could be used more than once:

const counter1 = createCounter('counter1')
const counter2 = createCounter('counter2')

If lazily creating reducer with createReducer and createAction, it causes the same issue, the type is lost as just string, so it becomes hard to do things like:

type ActionTypes = InferTypes<typeof allActions>

@phryneas
Copy link
Member

What I currently have with Redux is a type ActionTypes = InferTypes<typeof actions> so I'm also looking at how to achieve this using createSlice but so far, no idea.

I need this because there is a middleware function which looks like:

// action is -> action: ActionTypes
switch (action.type)
   case SOME_ACTION:
      action.payload <<< correctly inferred based on `action.type`
   ...

However, with createSlice, this is just string, so payload is an intersection of all payloads which is not exactly the desired output.

@eddyw you can do this using actionCreator.match see documentation and if-statements instead of a switch:

if (slice.actions.increment.match(action)){
  // payload is typed correctly in here
} else if (slice.actions.decrement.match(action)){
  // payload is typed correctly in here
}

@markerikson
Copy link
Collaborator

markerikson commented Nov 21, 2019

Given the weirdness of JS's switch statement, can you actually do that as a switch? Something like this:

switch(true) {
     case increment.match(action.type) {}
     case decrement.match(action.type) {}
}

@Glinkis
Copy link
Author

Glinkis commented Nov 21, 2019

Given the weirdness of JS's switch statement, can you actually do that as a switch? Something like this:

You can, but it might be a bit too clever. If/else ishould be more idiomatic in that case.

@phryneas
Copy link
Member

Okay, I just opened #262 - would that notation be an improvement for you?

@markerikson
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants