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

feat(store): add createFeature #3033

Merged

Conversation

markostanimirovic
Copy link
Member

@markostanimirovic markostanimirovic commented May 25, 2021

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Closes #2974

What is the behavior?

Example 1:

// products.reducer.ts
interface State {
  products: Product[];
  activeProductId: number | null;
  filter: string;
}

const initialState: State = {
  products: [],
  activeProductId: null,
  filter: '',
};

export const productsFeature = createFeature({
  name: 'products',
  reducer: createReducer(
    initialState,
    on(ProductsPageActions.enter, /* ... */),
    on(ProductsApiActions.loadSuccess, /* ... */),
  ),
});

// products.selectors.ts
import { productsFeature } from './products.reducer';

export const {
  selectProductsState,
  selectProducts,
  selectActiveProductId,
  selectFilter,
} = productsFeature;

export const selectProductsPageViewModel = createSelector(
  selectProducts,
  // ...
);

// products.module.ts
import { productsFeature } from './products.reducer';

StoreModule.forFeature(productsFeature);

Example 2: (with application state)

interface AppState {
  products: ProductsState;
}

interface ProductsState {
  products: Product[];
  activeProductId: number | null;
  filter: string;
}

const initialState: ProductsState = {
  products: [],
  activeProductId: null,
  filter: '',
};

export const productsFeature = createFeature<AppState>({
  name: 'products',
  reducer: createReducer(
    initialState,
    // ...
  ),
});

Does this PR introduce a breaking change?

[ ] Yes
[x] No

@ngrxbot
Copy link
Collaborator

ngrxbot commented May 25, 2021

Preview docs changes for ac54fb0 at https://previews.ngrx.io/pr3033-ac54fb03/

@wesleygrimes
Copy link
Contributor

I love this 👏

@nartc
Copy link

nartc commented May 27, 2021

This is great! Now if createFeature also returns a collection of dispatchable actions, that'd be 💋

@wesleygrimes
Copy link
Contributor

This is great! Now if createFeature also returns a collection of dispatchable actions, that'd be 💋

Yep! Then no need for facades haha

@nartc
Copy link

nartc commented May 27, 2021

This is great! Now if createFeature also returns a collection of dispatchable actions, that'd be 💋

Yep! Then no need for facades haha

I'm curious. Going to try something out from this branch

@markerikson
Copy link

Yep, love seeing the cross inspiration here!

@GregOnNet
Copy link
Contributor

GregOnNet commented May 28, 2021

This is great! Now if createFeature also returns a collection of dispatchable actions, that'd be 💋

NgRx Ducks provides self-dispatching Actions: https://co-it.gitbook.io/ngrx-ducks/ngrx-ducks-core/architecture/createduck

It also extracts plain action creators:
https://co-it.gitbook.io/ngrx-ducks/ngrx-ducks-core/architecture/getactions

Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great, here and there I left some comments and questions.
We should also add a test to make sure that we can use createFeature to create feature here.

modules/store/src/feature_creator.ts Show resolved Hide resolved
modules/store/src/feature_creator.ts Show resolved Hide resolved
modules/store/src/feature_creator.ts Outdated Show resolved Hide resolved
modules/store/src/feature_creator.ts Outdated Show resolved Hide resolved
modules/store/src/models.ts Outdated Show resolved Hide resolved
modules/store/src/index.ts Outdated Show resolved Hide resolved
modules/store/src/helpers.ts Outdated Show resolved Hide resolved
modules/store/src/feature_creator.ts Outdated Show resolved Hide resolved
Copy link
Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👏👏

Copy link
Member

@brandonroberts brandonroberts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@brandonroberts brandonroberts merged commit 5fd1c7b into ngrx:master Jun 9, 2021
@brandonroberts
Copy link
Member

Thanks @markostanimirovic!

@markgoho
Copy link

@brandonroberts
image
🤔

@brandonroberts
Copy link
Member

😅 The docs weren't published. I'll push them up tomorrow

@FritzHerbers
Copy link

I am missing some best practice guidance for createFuture.

How are "sub" states to be handled, similar https://ultimatecourses.com/blog/ngrx-store-understanding-state-selectors, see Feature state selectors, e.g. products -> pizzas -> entities.
Also when using multiple reducers with combineReducers.

  • Should the "sub" states implemented at is own (flatten the states) ? e.g. StoreModule.forFeature(pizzas), StoreModule.forFeature(entities) ?
  • Or, having one feature 'products' and one reducer, with all actions for the "sub" features inside, and selecting the sub state somehow (at on()) ? how?

@FritzHerbers
Copy link

@markostanimirovic
Thanks for your reply at https://dev.to/markostanimirovic/comment/216ob

For feature states with nested reducers, I recommend using the "traditional" way without createFeature.

Maybe your comment should be added to the documentation.

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

Successfully merging this pull request may close these issues.

NgRx Store: Add createChildSelectors function
10 participants