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

Issue with typings in store module #149

Closed
karptonite opened this issue Jul 21, 2017 · 2 comments
Closed

Issue with typings in store module #149

karptonite opened this issue Jul 21, 2017 · 2 comments

Comments

@karptonite
Copy link
Contributor

While looking into #141, I noticed that there is an issue with the typings in the forFeature method of StoreModule.
https://github.com/ngrx/platform/blob/master/modules/store/src/store_module.ts#L124-L133

static forFeature<T, V extends Action = Action>(
    featureName: string,
    reducers: ActionReducerMap<T, V>,
    config?: StoreConfig<T, V>
  ): ModuleWithProviders;
  static forFeature<T, V extends Action = Action>(
    featureName: string,
    reducer: ActionReducer<T, V>,
    config?: StoreConfig<T, V>
  ): ModuleWithProviders;

Note that in the second definition, the second parameter is called reducer, a variable that is not used in the function at all. I'm not sure what the intention is here, so I'm not sure how to fix it.

If that is intended to be reducers, allowing reducers to be an ActionReducer, I think the type of reducers in the first section could be amended to ActionReducerMap<T, V> | ActionReducer<T, V>, and the second section could be omitted. Or, if it is intended that reducers should be an ActionReducerMap only, the second section could also be omitted.

@brandonroberts
Copy link
Member

You can provide a single reducer instead of a map of reducers for the feature. See #34

StoreModule.forFeature('feature', featureReducer);

or

StoreModule.forFeature('feature', featureReducerMap);

@karptonite
Copy link
Contributor Author

@brandonroberts Thanks for the clarification! My confusion was that I didn't understand that the names of parameters in overloaded functions didn't have to be the same as the names in the actual function. It might have been slightly clearer (but more awkward) if the second parameter of the actual function declaration for forFeature were reducerOrReducers, since, as you say, is can be either.

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

2 participants