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

Add overload support for StoreModule.forFeature({ name, reducer }) #2809

Closed
lacolaco opened this issue Dec 7, 2020 · 10 comments · Fixed by #2821 or #2885
Closed

Add overload support for StoreModule.forFeature({ name, reducer }) #2809

lacolaco opened this issue Dec 7, 2020 · 10 comments · Fixed by #2821 or #2885

Comments

@lacolaco
Copy link
Contributor

lacolaco commented Dec 7, 2020

Proposal

I propose a new overload signature of StoreModule.forFeature().

StoreModule.forFeature({ name, reducer });

Motivation

The interface {name, reducer} is compatible with the Slice type from Redux Toolkit package.

Redux Toolkit is a set of best practices for using Redux. I think it's better to have interoperability with that.
With Redux Toolkit, a typical small example will be like below:

import { createSlice } from '@reduxjs/tooklit';
import { StoreModule } from '@ngrx/store';

const counterSlice = createSlice({
  name: 'counter',
  initialState: { count: 0 },
  reducers: {
    increment: (state) => { state.count++; }
  }
});

@NgModule({
  imports: [
    StoreModule.forFeature(counterSlice.name, counterSlice.reducer)
  ]
})
export class CounterFeatureStoreModule {}

A slice has its name and reducer. Its concept is similar to NgRx's "feature". So I want to link them seamlessly. The following is an example when forFeature()supports {name, reducer} option.

import { createSlice } from '@reduxjs/tooklit';
import { StoreModule } from '@ngrx/store';

const counterSlice = createSlice({
  name: 'counter',
  initialState: { count: 0 },
  reducers: {
    increment: (state) => { state.count++; }
  }
});

@NgModule({
  imports: [
    StoreModule.forFeature(counterSlice)
  ]
})
export class CounterFeatureStoreModule {}

Describe any alternatives/workarounds you're currently using

Other information:

If accepted, I would be willing to submit a PR for this feature

[x] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

@timdeschryver
Copy link
Member

Just out of interest, are you using redux toolkit with NgRx @lacolaco ?

@lacolaco
Copy link
Contributor Author

lacolaco commented Dec 8, 2020

@timdeschryver Recently I'm experimenting that interop and I feel it works well more than I expected.

NgRx's Redux-compability (shape of actions and reducers) is great for integrate to Redux ecosystem.

Demo: https://codesandbox.io/s/ngrx-store-redux-toolkit-example-pg930?file=/src/app/counter/counter-slice.ts

@lacolaco
Copy link
Contributor Author

lacolaco commented Dec 9, 2020

When I use Redux Toolkit with NgRx Store, the role of NgRx is just bridging Angular's dependency injection system and Redux state management. Redux Toolkit can help developers to create state management logic with (almost) zero boilerplates but it doesn't have a lazy-loading mechanism. I think NgRx store can be thinner by delegating Redux-things to the Toolkit.

@markostanimirovic
Copy link
Member

markostanimirovic commented Dec 9, 2020

@lacolaco
Here's the library with similar functionality: https://github.com/markostanimirovic/ngrx-handlers

When I created it, I was not aware of createSlice function from Redux Toolkit. Anyway, in V11 NgRx Handlers will be adapted to good action hygiene, it will be in accordance with [source] event action style.

@markerikson
Copy link

Hey, just wanted to say that I love this idea! It's really neat to see interop back and forth between the Redux and NgRx ecosystems. We probably should have more interop than there is now, so it's great to see suggestions like this!

@brandonroberts
Copy link
Member

@lacolaco seems reasonable to me to have some interop here.

@nartc
Copy link

nartc commented Dec 10, 2020

@markerikson Glad to see you're here. If this lands, would it make sense to have createSlice to allow for customize the action.type to allow for Good Action Hygiene? Eg:

createSlice({
   name: 'counter',
   initialState: { count: 0 },
   reducers: {
      increment: state => state.count ++
   }
})

slice.actions.increment() returns { type: 'counter/increment' }. It would be nice if createSlice provides a custom function: (sliceName: string, actionName: string) => string to customize the type on the action creators. This would allow for slice.actions.increment() to return { type: '[counter] increment' } instead.
https://github.com/reduxjs/redux-toolkit/blob/d5f1b65541c8b2a7b99b7e7ba5d34c6c3e2ac896/src/createSlice.ts#L231

@markerikson
Copy link

I'm reallllly hesitant to do that, especially because our TS types are already stupidly complex, and more options are just going to make that more difficult to deal with.

Also, part of the point of RTK is to be opinionated and just handle that stuff for you.

If someone wants to take the time to figure out how that might be implemented and file a PR, I'm open to discussing it, but I'm going to need convincing this is a thing that is really worth adding to RTK.

@lacolaco
Copy link
Contributor Author

@brandonroberts Thanks. Does it mean could I start creating a PR?

@brandonroberts
Copy link
Member

@lacolaco sure

brandonroberts added a commit that referenced this issue Jan 17, 2021
…#2885)

* feat(store): add object-style StoreModule.forFeature overload (#2821)

Closes #2809

* fix(store): update StoreModule to be compliant with AOT

Co-authored-by: Suguru Inatomi <suguru.inatomi@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants