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

Type payload-less slice action creators to take no arguments #110

Closed
wants to merge 1 commit into from
Closed

Type payload-less slice action creators to take no arguments #110

wants to merge 1 commit into from

Conversation

denisw
Copy link
Contributor

@denisw denisw commented Feb 5, 2019

Previously, it was not possible to assign a slice action creator without payload to () => any because the type was inferred as (payload: void) => any. Now we use a conditional type for SliceActionCreator that special-cases void.

Fixes #108.

@netlify
Copy link

netlify bot commented Feb 5, 2019

Deploy preview for redux-starter-kit-docs ready!

Built with commit 3d1772d

https://deploy-preview-110--redux-starter-kit-docs.netlify.com

Previously, it was not possible to assign a slice action creator
without payload to `() => any` because the type was inferred as
`(payload: void) => any`. Now we use a conditional type for
`SliceActionCreator` that special-cases `void`.
@denisw denisw mentioned this pull request Feb 14, 2019
9 tasks
@phryneas
Copy link
Member

Works for me nicely!

I could go from

export const slice = createSlice<
    State,
    any,
    {
        increment: CaseReducer<State, PayloadAction<number>>;
        decrement: CaseReducer<State, PayloadAction<number>>;
        navigation: CaseReducer<State, NavigationAction<{ counter: string }>>;
    }
>({
    slice: 'home',
    reducers: {
        increment: (state, action) => {
            state.counter += action.payload;
        },
        decrement: (state, action) => {
            state.counter -= action.payload;
        },
        navigation: (state, action) => {
            state.counter = Number.parseInt(action.payload.routeParams.counter);
        }
    },
    initialState: { counter: 0 }
});

to

export const slice = createSlice({
    slice: 'home',
    reducers: {
        increment: (state, action: PayloadAction<number>) => {
            state.counter += action.payload;
        },
        decrement: (state, action: PayloadAction<number>) => {
            state.counter -= action.payload;
        },
        navigation: (state, action: NavigationAction<{ counter: string }>) => {
            state.counter = Number.parseInt(action.payload.routeParams.counter);
        }
    },
    initialState: { counter: 0 } as State
});

The only thing that is not working is defining the State type as a Generic like this (but this is only a nice-to-have):

export const slice = createSlice<State>({
    slice: 'home',
    reducers: {
        increment: (state, action: PayloadAction<number>) => {
            state.counter += action.payload;
        },
        decrement: (state, action: PayloadAction<number>) => {
            state.counter -= action.payload;
        },
        navigation: (state, action: NavigationAction<{ counter: string }>) => {
            state.counter = Number.parseInt(action.payload.routeParams.counter);
        }
    },
    initialState: { counter: 0 } 
});

@phryneas
Copy link
Member

phryneas commented Feb 15, 2019

Actually I am noticing right now that example B seems to be working with the current version without this patch, too, so I might have never had this specific problem & been the wrong one to test this o_O

Sorry!

@pat-son
Copy link

pat-son commented Mar 25, 2019

Is there an ETA on this getting merged? When can we expect a new version of RSK?

@markerikson
Copy link
Collaborator

markerikson commented Mar 25, 2019

Woops, forgot this was here - sorry! I've been completely occupied with work on React-Redux v7 the last couple months. I'll try to swing back and examine this in more detail this week.

@denisw : is there any chance we can make a no-param action creator actually not try to assign payload at all? Right now, if I do something like onClick={increment}, the React event object always gets passed through as the payload, and that creates problems (React doesn't like the event object being kept around, and RSK's "no non-serializable values" middleware yells at you).

@denisw
Copy link
Contributor Author

denisw commented Mar 25, 2019

@markerikson I guess we could add a function which explicitly creates an action creator taking no parameters. Perhaps there could be separate createAction and createPayloadAction functions.

And alternative approach is to keep a single createAction function, but require a chained call to a method (perhaps called withPayload) if you want the action creator to take a payload.

const increment = createAction('increment')
// increment takes no payload

const multiply = createAction('multiply').withPayload()
// multiply does take a payload

In TypeScript, you would then be able to specify the payload type with the withPayload call:

const multiply = createAction('multiply').withPayload<number>()

@denisw
Copy link
Contributor Author

denisw commented Apr 28, 2019

Superseded by #133.

@denisw denisw closed this Apr 28, 2019
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

Successfully merging this pull request may close these issues.

Type issue with ActionCreators from createSlice with no payload
4 participants