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(data): add entity config in App module declaration #2133

Merged
merged 4 commits into from
Oct 31, 2019

Conversation

norato
Copy link
Contributor

@norato norato commented Sep 28, 2019

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?

On ng-add @ngrx/data we do not create the entity-config file

Closes #

What is the new behavior?

Create and declare entity-config file to AppModule.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@norato norato changed the title feat(data): Add entity config in App module declaration feat(data): add entity config in App module declaration Sep 28, 2019
@ngrxbot
Copy link
Collaborator

ngrxbot commented Sep 28, 2019

Preview docs changes for f42c7c4 at https://previews.ngrx.io/pr2133-f42c7c4/

@ngrxbot
Copy link
Collaborator

ngrxbot commented Sep 28, 2019

Preview docs changes for ee1d059 at https://previews.ngrx.io/pr2133-ee1d059/

@brandonroberts
Copy link
Member

Thanks for the PR @norato! Isn't this pretty much a required step when adding @ngrx/data? I don't think it needs to be behind a flag.

@norato
Copy link
Contributor Author

norato commented Oct 2, 2019

Hello, @brandonroberts.
The entity-config is a required configuration to @ngrx/data works, as at the documentation we need to declare it to include in the store.

The ng-add only includes the EntityDataModule or EntityDataModuleWithouEffects at AppModule, but do not declare the entityConfig.

@timdeschryver
Copy link
Member

As the config is required I would make this the default and not provide a flag for it, as @brandonroberts mentioned.

@ngrxbot
Copy link
Collaborator

ngrxbot commented Oct 9, 2019

Preview docs changes for 6b245b1 at https://previews.ngrx.io/pr2133-6b245b1/

@wardbell wardbell self-assigned this Oct 18, 2019
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 LGTM.
Can you confirm that what this schematic is doing is correct @wardbell ?

It will create an empty config file:

import { EntityMetadataMap, EntityDataModuleConfig } from '@ngrx/data';

const entityMetadata: EntityMetadataMap = {};

const pluralNames = {  };

export const entityConfig: EntityDataModuleConfig = {
  entityMetadata,
  pluralNames
};

And it will import, and pass is to the DataModule:

EntityDataModule.forRoot(entityConfig)

@brandonroberts brandonroberts merged commit 6ca3056 into ngrx:master Oct 31, 2019
jordanpowell88 pushed a commit to jordanpowell88/platform that referenced this pull request Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants