reduer.case should mutate reducer

[erin-noe-payne]

Currently reducers are effectively immutable, with reducer.case returning an a new reducer object

This means that to build up my reducer object I must either:

build up a list of named reducer functions and combine them later:

export const GetMeetupListRequest = actionCreator('GET_MEETUP_LIST_REQUEST');
const GetMeetupListResponse = actionCreator<MeetupListPayload | Error>('GET_MEETUP_LIST_RESPONSE');
function getMeetupListRequestReducer(state: MeetupsState): MeetupsState {
    return {...state} //etc...
}
function getMeetupListResponseReducer(state: MeetupsState, payload: MeetupListPayload | Error): MeetupsState {
    return {...state} //etc...
}

// and then later...

export const meetupReducer = reducerWithInitialState(initialState)
    .case(CreateMeetupRequest, createMeetupRequestReducer)
    .case(CreateMeetupResponse, createMeetupResponseReducer)
    .case(GetMeetupListRequest, getMeetupListRequestReducer)
    .case(GetMeetupListResponse, getMeetupListResponseReducer)
    .case(SetFilterString, setFilterStringReducer);

This becomes annoying because I end up with a number of variables with almost the same name.

If .case mutated the reducer object, then instead I could inline the reducer definitions adjacent to the Action definitions - which is how we typically think about our actions / reducers anyway:

export const meetupReducer = reducerWithInitialState(initialState);

// Create new meetup
export const CreateMeetupRequest = actionCreator<MeetupDetailsPayload>('CREATE_MEETUP_REQUEST');
const CreateMeetupResponse = actionCreator<MeetupDetailsPayload | Error>('CREATE_MEETUP_RESPONSE');
meetupReducer.case(CreateMeetupRequest, (state: MeetupsState) => {
    return {...state} //etc...
});
meetupReducer.case(CreateMeetupResponse, (state: MeetupsState, payload: MeetupDetailsPayload | Error) => {
    return {...state} //etc...
});

I may be missing the use case where you want your reducers to be immutable, but I haven't encountered the need yet.

Again, if you agree with the approach I'm happy to submit a PR.

[dphilipson]

Hi again!

This makes a lot of sense to me- your sample code is indeed cleaner.

I think some users would prefer not to export mutable reducers under the reasoning that they could get confusing errors if somebody imported the reducer and added additional cases to it. If we were to make the reducers mutable, I would want to also be able to think of it as a builder and have a "build" method which returns a plain, immutable function, e.g. write code like

export const myReducer = reducerWithInitialState(initialState)
    .case(SomeAction, someActionReducer)
    .case(SomeOtherAction, someOtherActionReducer)
    .build();

Other possible names: .toPlainReducer(), .asPlainReducer(), etc.

The .build() call would probably be optional, so you could continue to use the chained builder as the reducer itself if you don't care about the immutability.

I really appreciate the input. Going to think about this a bit more.

[erin-noe-payne]

👍 I think the build() approach makes sense

[dphilipson]

Change is added to v0.4.0.