esamattis / immer-reducer

Type-safe and terse reducers with Typescript for React Hooks and Redux
http://npm.im/immer-reducer
MIT License
225 stars 15 forks source link

RFD: Supporting passing of defaults in action payload #30

Closed Secretmapper closed 5 years ago

Secretmapper commented 5 years ago

Due to the design, it would not be possible to have default payload data passed in the action.

In plain redux, an action creator like this creates an action that has a default 'id':

addActionWithId () {
  return {
    type: 'action',
    payload: { id: uuid() }
  }
}

In immer-reducer, the closes way to achieve something like this is:

addActionWithId (id: number = uuid()) {
}

Unfortunately, this has a big downside - namely that the id will NOT be in the payload if the 'default parameter' is used. This means the id cannot be used in side effect libraries/middlewares like redux-saga, and furthermore, the reducer will not be pure/deterministic.

Another problem with this is that it can be 'overwritten' (i.e. onClick={myActionCreator}), which is slightly related to https://github.com/epeli/immer-reducer/issues/29

I'm not sure how to fix this. Looking at the code, it seems like the action generated is obtained from the arguments of the function call, but the function parameters in the method definitions are only used in the generated reducer function. Perhaps we can have an option to pass 'custom action creators' or some sort of mapping, maybe if the property in the class is an object/not a function?

MyImmerReducer {
  addActionWithId: {
    creator: () => ({
      type: 'action',
      payload: { id: uuid() }
    }),
    // though here I'm not sure how we can get the generated action type from above
    reducer: (id: string) => {
    }
  }
}

Another idea is to use something similar to react's defaultProps. The downside though is the API is perhaps ugly and hard to read:

  constructor () {
    this.addActionWithId.defaults = () => [uuid()]
  },

  addActionWithId: {
    creator: () => ({
      type: 'action',
      payload: { id: uuid() }
    })
esamattis commented 5 years ago

Yeah, I've struggled a bit with this too but I have not come up with anything for immer-reducer that I'm ok with. For example your .defaults proposal does not work that well with TypeScript: It is impossible to infer types automatically for defaults function.

I'm hoping to solve this with decorators once they get sorted out in the standardization process.

For example something like could be possible(?)

@defaults(() => ({id: uuid()})) // Merges with the params
addAction(params: {id?: string, thing: string}) {}

For now I've solved this issue by wrapping the generated action creators:

function addActionWithId (thing: string) {
  return ActionCreators.addAction({id: uuid(), thing});
}

This works really well with TypeScript types and is straightforward to understand - no need known any immer-reducer specific apis.

Secretmapper commented 5 years ago

Ah neat! I just ended up using the same pattern in my code yesterday:

const immerActions = {
  ...createActionCreators(AssignmentSessionReducer),
  ...createActionCreators(AssignmentSessionPlayableMediaReducer)
}

export const actions = {
  ...immerActions,
  record: (taskID: string, assetID: string = uuid()) =>
    immerActions.record(taskID, assetID)
}

Initially, I thought it would be a limitation of the library, but after reading the source, I realised what was happening with the various methods and such a way would be possible. The libraries wrapping is honestly magical (great work!) so I thought it wouldn't be.

The only downside I found is that you lose on the niceties provided by the immer actions, such as the action type i.e. action.record.type, though it's minor (I guess a small utility wrapper would work? but it feels too little to be justified)

I think it is fine as it is, just added a PR that notes this 'pattern' in the readme: https://github.com/epeli/immer-reducer/pull/32

Cheers!