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

RFC: Add reducer factory to Store #1724

Closed
brandonroberts opened this issue Apr 10, 2019 · 3 comments
Closed

RFC: Add reducer factory to Store #1724

brandonroberts opened this issue Apr 10, 2019 · 3 comments

Comments

@brandonroberts
Copy link
Member

brandonroberts commented Apr 10, 2019

We've discussed this in other places, and was mentioned here with the introduction of action creators. https://blog.angularindepth.com/ngrx-action-creators-redesigned-d396960e46da

This is to open up the discussion about rationale. We've introduced two functions recently: createAction and createEffect, both with the purpose of preserving the type-safety of the things they create, while making them easier to use and consume. This lines up with what we already have in createSelector and createEntityAdapter from Store and Entity respectively. Naturally, next up would be createReducer that might make handling creating a reducer to handle state changes somewhat less verbose.

What we promote today:

export function reducer(state, action) {
  switch(action.type) {
    case 'someAction1':
      return newState;
    case 'someAction2':
      return newState;
    default:
      return state;
  }
}

All current proposals include generating a reducer function using a map, array, etc. This does not include or make use of decorators to create them. Examples

https://github.com/reduxjs/redux-starter-kit/blob/master/src/createReducer.ts
https://github.com/cartant/ts-action/blob/master/source/reducer.ts
https://redux.js.org/recipes/reducing-boilerplate#generating-reducers

I still like using the switch statement. Its less magical, somewhat easier to glance at. That being said, at this point I think we're at least looking at having a createReducer function. Upfront I'll say that using it with AOT is gonna be a pain because you can't use generated functions within NgModule metadata without workarounds. I've tested this using ts-action and the createReducer method. You end up having to wrap the generated reducer in an exported function to be compliant.

As mentioned, there are AOT workarounds to that approach, but it would add complexity to registering state with generated reducers. Ideally, we want the best of both worlds. Easy to use APIs that provide the same level of type safety and flexibility.

Feel free to comment with other proposals/suggestions.

@brandonroberts brandonroberts pinned this issue Apr 10, 2019
@timdeschryver
Copy link
Member

To play the devils advocate here, what benefits would createReducer bring us, because the benefits of createAction and createEffect were known before we started the implementation.

This is my list:

  • No default return case needed, resulting in no more "frustrations" when one would forget to handle it
  • We could add a check that every (intended) action is being handled within the reducer
  • Have the same naming with createAction, createSelector and createEffect (as described in the issue)

@alex-okrushko
Copy link
Member

alex-okrushko commented Apr 10, 2019

my list would include the ability to drop Action unions entirely 🎉

We could add a check that every (intended) action is being handled within the reducer

We discussed this during Firebase's design review and came to the conclusion that it is not the problem that needs fixing. It would cause a lot more friction with little benefit. On top of that, with action union dropped there would be no 'intended' actions :)

@damienwebdev
Copy link

To tack on here, re: #1734 it may be nice for createReducer to optionally accept a prefix and createAction to also optionally accept a prefix so that you can reuse reducers for multiple instances of a state. E.g. multiple instances of a sidebar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants