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

Default selectId function for EntityAdapter #405

Merged
merged 5 commits into from
Sep 20, 2017

Conversation

phillipzada
Copy link
Contributor

See #378

What is the current behavior?

I have several entities, and I use always id property for the primary key. So, to create the EntityAdater I must repeat the code like:

export const myEntityAdapter: EntityAdapter<MyEntity> = createEntityAdapter<MyEntity>({
  selectId: (myEntity: MyEntity) => myEntity.id
});

Expected behavior:

This could be greate if createEntityAdapter uses a default selectId method which will take id property of object as a primary key. So I just need call this function

export const myEntityAdapter: EntityAdapter<MyEntity> = createEntityAdapter<MyEntity>();

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 91.264% when pulling 3219623 on PhillipZada:master into c31573f on ngrx:master.

@@ -10,10 +10,12 @@ import { createSelectorsFactory } from './state_selectors';
import { createSortedStateAdapter } from './sorted_state_adapter';
import { createUnsortedStateAdapter } from './unsorted_state_adapter';

export function createEntityAdapter<T>(options: {
export function createEntityAdapter<T>(options?: {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of making options optional, make selectId optional and provide a default below

selectId: IdSelector<T>;
sortComparer?: false | Comparer<T>;
}): EntityAdapter<T> {
options = options || { selectId: (instance: any) => instance.id };

const { selectId, sortComparer }: EntityDefinition<T> = {
sortComparer: false,
Copy link
Member

Choose a reason for hiding this comment

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

Added a default selectId function here under the sortComparer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -149,12 +149,16 @@ export class StoreModule {
): ModuleWithProviders;
Copy link
Member

Choose a reason for hiding this comment

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

Do these changes in a separate commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey Brandon, these changes are happening automatically as per the precommit script. Any suggestions on how to stop it.

Copy link
Member

Choose a reason for hiding this comment

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

Don't worry about it then. Must have merged another change that didn't get formatted.

@phillipzada
Copy link
Contributor Author

Hey @brandonroberts,

The original request was to make it possible to create an adaptor using this

export const myEntityAdapter: EntityAdapter<MyEntity> = createEntityAdapter<MyEntity>();

I removed the optional options flag and replaced it with a default value if not provided. Hope this meets requirements.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.06%) to 91.264% when pulling 5249ac5 on PhillipZada:master into c31573f on ngrx:master.

@brandonroberts brandonroberts merged commit 2afb792 into ngrx:master Sep 20, 2017
@brandonroberts
Copy link
Member

Thanks!

sharikovvladislav added a commit to sharikovvladislav/platform that referenced this pull request Sep 28, 2017
* 'master' of github.com:ngrx/platform: (35 commits)
  chore(Example): Add login info to example app. Fix sidenav issue with IE (ngrx#436)
  chore(docs): Fix OnRunEffects example (ngrx#430)
  chore(docs): Add docs on usage of custom router state serializer with store freeze (ngrx#426)
  chore(docs): Add missing from in metareducer example. (ngrx#418)
  docs(Entity): Fix examples for @ngrx/entity usage (ngrx#415)
  feat(Entity): Add default selectId function for EntityAdapter (ngrx#405)
  fix(Example): Add missing import for catch operator (ngrx#409)
  docs(effects): Remove usage of deprecated `toPayload` util in example (ngrx#407)
  chore(Example): Add Jest as test runner for example tests (ngrx#371)
  fix(Store): Refactor parameter initialization in combineReducers for Closure
  feat(Entity): Rename 'sort' to 'sortComparer'
  chore(docs): Minor fixes to entity interfaces documentation (ngrx#390)
  chore(docs): Change providing injected reducers by factory instead of value (ngrx#387)
  fix(Store): Fix typing for feature to accept InjectionToken (ngrx#375)
  chore(docs): Update @ngrx/entity documentation examples and usage (ngrx#369)
  docs(entity): Fix typo in url (ngrx#365)
  chore(Example): Documented @ngrx/entity and added to example app (ngrx#328)
  fix(RouterStore): Stringify error from navigation error event (ngrx#357)
  chore(docs): Fix action interface example (ngrx#360)
  chore(Example): Added ngrx-store-freeze meta-reducer to example application (ngrx#343)
  ...
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.

3 participants