Skip to content
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

Creating Generic Slices #286

Closed
MarceloAlves opened this issue Dec 20, 2019 · 9 comments
Closed

Creating Generic Slices #286

MarceloAlves opened this issue Dec 20, 2019 · 9 comments

Comments

@MarceloAlves
Copy link
Contributor

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<PT, Action>;
    }, string, unknown, never, never>, string>

    This expression is not callable.
    Not all constituents of type 'void | WithTypeProperty<WithMatch<() => WithPayload<undefined, Action>, string, undefined, never, never>, string> | WithTypeProperty<WithMatch<{ (payload?: undefined): WithPayload<undefined, Action>; (payload?: PT | undefined): WithPayload<PT, Action>; }, 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
Copy link
Member

phryneas commented Dec 20, 2019

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
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Member

@MarceloAlves I've got a Draft PR over in #290 - if that lands, you could do it like this:
b755f9d

I guess that would solve your use case?

@MarceloAlves
Copy link
Contributor Author

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

@markerikson
Copy link
Collaborator

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

@MarceloAlves
Copy link
Contributor Author

Hi @markerikson

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

@markerikson
Copy link
Collaborator

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

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

3 participants