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

Store: is metaReducers signature correct? #264

Closed
GiuseppePiscopo opened this issue Aug 11, 2017 · 0 comments
Closed

Store: is metaReducers signature correct? #264

GiuseppePiscopo opened this issue Aug 11, 2017 · 0 comments

Comments

@GiuseppePiscopo
Copy link

GiuseppePiscopo commented Aug 11, 2017

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request

What is the current behavior?

In StoreModule.forRoot(), in its StoreConfig<T,V> parameter, its metaReducers property is typed as an (optional) array of ActionReducer<T,V>.

But then in meta reducers docs provided example (which is correct, I think), does not adhere to that type:

function debug(reducer) {
  return function(state, action) {
    // ... log things ...
    return reducer(state, action);
  }
}

which is like (adding fictional generic types):

function debug(reducer: ActionReducer<T,V>): 
  (red: ActionReducer<T,V>) => ActionReducer<T,V> {

  return function wrapperReducer(state: T, action: V): ActionReducer<T,V> {
    // ... log things ...
    return reducer(state, action);
  }

}

Infact, being a meta-reducer, debug is not a reducer itself: is a function taking a reducer and giving another (transformed, wrapped, decorated ...) reducer.

So it seems to me that metaReducers cannot be an array of ActionReducer<T,V>. As a matter of fact, when I add (non generic) types to my debug function, code does not compile because of metareducers issue with types:

[...]
Types of property 'metaReducers' are incompatible. [...]
Type '(reducer: ActionReducer<PublicAppState, Action>) => ActionReducer<PublicAppState, Action>' is not assignable to type 'ActionReducer<PublicAppState, Action>'.

Expected behavior:

metaReducers property should be typed as an (optional) array of reducer-transformers, each item like: (red: ActionReducer<T,V>) => ActionReducer<T,V>

Minimal reproduction of the problem with instructions:

Version of affected browser(s),operating system(s), npm, node and ngrx:

Other information:

brandonroberts added a commit that referenced this issue Aug 15, 2017
brandonroberts added a commit that referenced this issue Aug 15, 2017
brandonroberts added a commit that referenced this issue Aug 15, 2017
brandonroberts added a commit that referenced this issue Aug 15, 2017
ItamarGronich pushed a commit to hasadna/open_pension that referenced this issue Aug 24, 2017
- Versions 4.0.0 - 4.0.3 have a bug related to setting up metaReducers
  (ngrx/platform#264).
- We should include a package-lock.json top prevent those kinds of
  things
- package-lock.json will make sure we all have the same versions of
  everything.
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

1 participant