Skip to content

Useless (?) type property in createSlice #2623

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
Volper212 opened this issue Aug 22, 2022 · 3 comments
Closed

Useless (?) type property in createSlice #2623

Volper212 opened this issue Aug 22, 2022 · 3 comments

Comments

@Volper212
Copy link

When you create a slice, the reducers field is automatically split into all the action types so I don't see a reason to later include that property inside a function. Code could be a lot simpler if you could just have the second argument be the payload, not an object with both the action and the payload:

reducers: {
    remove: (state, id: ID) => adapter.removeOne(state, id),
}

instead of

reducers: {
    remove: (state, { payload: id }: PayloadAction<ID>) => adapter.removeOne(state, id),
}

If, in that rare case, you need the type in the action, why not just pass it as the third argument?

Redux Toolkit states on its website that it's simple, however it could be simpler (as in the first code block). To make this work in my project, I have this wrapper:

import {
    CaseReducer,
    createSlice,
    Draft,
    PayloadAction,
    ValidateSliceCaseReducers,
} from "@reduxjs/toolkit";

export function makeSlice<Name extends string, State, Reducers extends SliceReducers<State>>({
    name,
    initialState,
    reducers,
}: {
    name: Name;
    initialState: State;
    reducers: Reducers;
}) {
    return createSlice({
        name,
        initialState,
        reducers: Object.fromEntries(
            Object.entries(reducers).map(([name, reducer]) => [
                name,
                (state: Draft<State>, { payload }: PayloadAction<Payload>) =>
                    reducer(state, payload),
            ])
        ) as ValidateSliceCaseReducers<State, OutputReducers>,
    });

    type OutputReducers = {
        [K in keyof Reducers]: CaseReducer<State, PayloadAction<Parameters<Reducers[K]>[1]>>;
    };
}

type SliceReducers<State> = Record<string, Reducer<State>>;
type Reducer<State> = (state: Draft<State>, payload: Payload) => void | State | Draft<State>;

// eslint-disable-next-line @typescript-eslint/no-explicit-any
type Payload = any;

So users have a choice - write something weird like this or repetitively write PayloadAction in their reducers and take out the payload field from the object. Does it have to be like that? If you don't want to change this in Redux Toolkit because the field reducers would not fit the definition of a reducer anymore (that accepts state as one argument and the action as the second), you could call this actionHandlers or just handlers or something and have it be an opt-in alternative for reducers (just another optional field while keeping the old one).

@markerikson
Copy link
Collaborator

This is a topic we specifically discussed in the early alpha design phase:

#17 (comment)

We always pass the entire action object into the reducer, because there may be additional properties on it such as .meta and .error that your reducer logic may care about.

If you only care about .payload, you can destructure it in the declaration, but we even have code in RTK itself that specifically references action.meta, such as here:

const { queryCacheKey } = action.meta.arg

@markerikson markerikson closed this as not planned Won't fix, can't repro, duplicate, stale Aug 22, 2022
@Volper212
Copy link
Author

Can't you have them as the third and fourth argument?

@markerikson
Copy link
Collaborator

markerikson commented Aug 22, 2022

No. For one thing, it doesn't make any sense conceptually to take a single action object parameter, and suddenly start passing it as (state, payload, meta, error). Second, it would be a massive breaking change to the entire ecosystem, and to all of the very carefully constructed RTK TS types, and for no real benefit.

Like I said, if all you care about is payload, then just destructure it.

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

2 participants