reduxjs / redux-toolkit

The official, opinionated, batteries-included toolset for efficient Redux development
https://redux-toolkit.js.org
MIT License
10.71k stars 1.17k forks source link

Creating Generic Slices #286

Closed MarceloAlves closed 4 years ago

MarceloAlves commented 4 years ago

Stemming from this thread

For better or worse a lot of our state has the same structure and so I'm trying to create helper where we can pass a name, the initialState and end up with a slice that contains most of everything we need.

I'm running into some issues with how to type it correctly. I'm pretty green to more advanced types so this could all be me not understanding how to type it. An example of what I'm trying to achieve:

export interface StateSlice<T = any> {
  data: T
  isFetching: boolean
  receivedAt: number | null
  error: boolean
  errorMessage: string | null
  errorCode: number | null
}

const requestStart: CaseReducer<StateSlice, PayloadAction> = (state: StateSlice) => {
  state.isFetching = true
}

const requestSuccess: CaseReducer<StateSlice, PayloadAction> = (state, { payload }) => {
  state.isFetching = false
  state.receivedAt = Date.now()
  state.error = false
  state.errorMessage = null
  state.errorCode = null
  state.data = payload
}

export const createSlice = ({
  name = '',
  initialState,
  reducers,
  extraReducers = {}
}: {
  name: string
  initialState: StateSlice
  reducers?: Record<string, CaseReducer<StateSlice, AnyAction>> // I've tried many different types here. Not final
}): Slice<StateSlice, AnyAction> => {
  return createSliceToolkit({
    name,
    initialState,
    reducers: {
      requestSuccess,
      requestStart,
      ...reducers
    }
  })
}

The issues I end up with:

  1. When using this new createSlice and trying to pass it more reducers:

    const slice = createSlice({
      name: 'generic/name',
      initialState,
      reducers: {
        updateSuccess: (state: StateSlice<Item[]>, { payload }: PayloadAction<Item>) => {}
      }
    })

    This error happens:

    Type '{ updateSuccess: (state: StateSlice<Item[]>, { payload }: WithPayload<Item, Action>) => void; }' is not assignable to type 'Record<string, CaseReducer<StateSlice, AnyAction>>'. Property 'updateSuccess' is incompatible with index signature. Type '(state: StateSlice<Item[]>, { payload }: WithPayload<Item, Action>) => void' is not assignable to type 'CaseReducer<StateSlice, AnyAction>'. Types of parameters '__1' and 'action' are incompatible. Type 'AnyAction' is not assignable to type 'WithPayload<Item, Action>'. Property 'payload' is missing in type 'AnyAction' but required in type '{ payload: FeedPen; }'

  2. When using actions:

    const { requestStart, requestSuccess, updateSuccess } = bunkScoringSlice.actions
    
    dispatch(requestStart())

    This error occurs:

    const requestStart: void | WithTypeProperty<WithMatch<() => WithPayload<undefined, Action>, string, undefined, never, never>, string> | WithTypeProperty<WithMatch<{ (payload?: undefined): WithPayload<undefined, Action>;

    (payload?: PT | undefined): WithPayload>; }, string, unknown, never, never>, string> This expression is not callable. Not all constituents of type 'void | WithTypeProperty WithPayload>, string, undefined, never, never>, string> | WithTypeProperty>; (payload?: PT | undefined): WithPayload>; }, string, unknown, never, never>, string>' are callable. Type 'void' has no call signatures.ts(2349)

All this to say, I'm stuck. Not sure if I'm missing some pretty obvious things but any help would be greatly appreciated!

phryneas commented 4 years ago

At the moment, I'm sorry to say this is simply impossible without copy-pasting code from the type definitions and mimicking the current generics construct.

I've spent the whole day yesterday trying to find a general simpler alternative to that generics construct, but to no avail. I believe there just is none.

I'll try to find something that will at least satisfy those restrictions you're running into sometime this weekend.

If I get this to work you'll be able to do it like in https://codesandbox.io/s/dreamy-elion-5dnr8 (this is just a mock-up, I need to figure out what works as ValidReducers there)

markerikson commented 4 years ago

As an alternative, I've had success defining a base type for the data and reusable reducers that work with that type, and then "manually" calling createSlice() instead of wrapping it in a higher-order function. Not too far off from what you're doing, but I don't bother with the CaseReducer typing - just const commonReducerA = (state: SomeState, action: PayloadAction<whatever>) => {}.

MarceloAlves commented 4 years ago

At the moment, I'm sorry to say this is simply impossible without copy-pasting code from the type definitions and mimicking the current generics construct.

I was afraid of this.

As an alternative, I've had success defining a base type for the data and reusable reducers that work with that type, and then "manually" calling createSlice() instead of wrapping it in a higher-order function.

This was actually my first approach (but I used the CaseReducer type) when the team asked if we could create the generator. It sounds like this might be the best way forward for now. At least it'll greatly simplify the reducer part of each slice.

I'll try to find something that will at least satisfy those restrictions you're running into sometime this weekend.

No need to spend any more time on this! It's the holidays after all :)

We can stick with generic reducers and maybe revisit this in the future. Converting our older pieces of the store to createSlice and cleaning up where we can has already made a huge difference.

Thanks for your time!

phryneas commented 4 years ago

No need to spend any more time on this! It's the holidays after all :)

I won't guarantee I'll spend too much on it but this bugs me enough that I want to give it at least another try ^^

phryneas commented 4 years ago

@MarceloAlves I've got a Draft PR over in #290 - if that lands, you could do it like this: https://github.com/reduxjs/redux-toolkit/pull/290/commits/b755f9d4ed6744a46ca8f05333d4731f7536196a

I guess that would solve your use case?

MarceloAlves commented 4 years ago

I think this will work perfectly! I'll give it a shot. Thanks so much for taking the time!!

markerikson commented 4 years ago

Yep, and that's out in https://github.com/reduxjs/redux-toolkit/releases/tag/v1.2.0 .

MarceloAlves commented 4 years ago

Hi @markerikson

Back to work, got everything updated and it looks to be working out great. Should I close this issue?

markerikson commented 4 years ago

Yeah, looks like this has been resolved. Glad this is working!