Skip to content

Ability to change sortComparer in the ngrx/entity #898

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

Closed
keeshii opened this issue Mar 8, 2018 · 15 comments
Closed

Ability to change sortComparer in the ngrx/entity #898

keeshii opened this issue Mar 8, 2018 · 15 comments

Comments

@keeshii
Copy link

keeshii commented Mar 8, 2018

I would like to have a ability to change sortComparer without recreating the whole collection.

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[X] Feature request
[ ] Documentation issue or request

What is the current behavior?

sortComparer can be defined only once, while calling the createEntityAdapter. Right now we have just a read-only access to this function.

Expected behavior:

A new method defined in the EntityState<T>, let's call it setSortComparer. This function should return a new state with collection sorted using different comparer.

@MattiJarvinen
Copy link

MattiJarvinen commented Apr 9, 2018

Isn't the sort a calculated property? Shouldn't it be then implemented in a selector?

Pseudo:

// State
{
  selectedSort: {
    name: 'byName',
    order: 'asc'
  }
}

const getSortedKeysAsArray = createSelector( 
    getAll, 
    getSelectedSort, 
    ( entityArray, sortProps ) => {
         // point compareFn to sort function generator taking order as a parameter
         switch( sortProps.name ) {
         //...
         }
         // don't mess with actual entity array
         return entityArray.slice()
         // sort
         .sort( compareFn(sortProps.order) )
         // collect only ids in order
         .map( entity => entity.id );
});

const getSortedArray = createSelector( 
    getEntities, 
    getSortedKeysAsArray, 
    ( entities, sortedKeys ) => {
        return sortProps.map( id => entities[id]);
})

@keeshii
Copy link
Author

keeshii commented Apr 9, 2018

Um… I think we shouldn’t use selectors this way. Selectors are executed multiple times per each action. And we are returning new instances of the array, so all subscriptions will be executed even if no values has been changed.

Of course there is a better work-around for my problem. We can recreate the EntityAdapter in the reducer whenever the sorting method is changed. In my opinion there should be an easier solution for that, and it doesn't look hard to implement.

@MattiJarvinen
Copy link

A selector is not recomputed unless one of its arguments changes.

You have accidental mutable state if one action leads to multiple calls to one selector.

@keeshii
Copy link
Author

keeshii commented Apr 9, 2018

I don’t like putting logic into selectors. It feels just wrong. And sorting the entire table after each change doesn’t look performance friendly.

@dereklin
Copy link

Guys, I am starting to learn about entities and wonder about the sortComparer feature. In the example, I see

export const adapter: EntityAdapter<Book> = createEntityAdapter<Book>({
  selectId: (book: Book) => book.id,
  sortComparer: false,
});

In my app, I want to sort my entities dynamically by a dropdown in the UI: Time Created ASC/DESC, Entity Name A-Z, and Entity Name Z-A.

Can the sortComparer function be an observable that it will react to my dropdown change and then automatically reorder the entities for me?

If not, I think I will try out what @MattiJarvinen-BA has suggested

@brandonroberts
Copy link
Member

@dereklin selectors are meant to be pure and synchronous, so if you need to do some advanced sorting that reacts to some other change, you would have a selector that listens for a dropdown change value, combine it with your entities and return a newly sorted list.

There's nothing wrong with doing this sorting at the component level also before consuming it also.

@dereklin
Copy link

@brandonroberts Thanks for the reply. For now, I think I have to implement a custom selector. It seems like the sortComparer function only takes in two arguments: entityA and entityB. It will be nice if it has the store snapshot as the third argument. Then I can implement a more comprehensive sorting algorithm. This will allow me to just use the selectAll selector provided by ngrx/entity, without having to create a custom selector.

@timdeschryver
Copy link
Member

@dereklin what are you exactly trying to do?
Because I think you could (should?) use @MattiJarvinen-BA 's snippet.
You select the entities state, select the sorting state, and use these 2 to return sorted entities.

@dereklin
Copy link

dereklin commented May 14, 2018

@tdeschryver @MattiJarvinen-BA 's snippet is a custom selector, right?

I think that will work.

I guess what I am saying is that if the sortComparer function has the state snapshot as a param, then I don't need to implement a custom selector. Instead, I can change of the order of my entities based on other properties that are outside of the entities but are inside of the state.

@skushagra3107
Copy link

I guess what I am saying is that if the sortComparer function has the state snapshot as a param, then I don't need to implement a custom selector. Instead, I can change of the order of my entities based on other properties that are outside of the entities but are inside of the state.

exactly what I was looking for and I think it's a very common use case.

@dereklin
Copy link

dereklin commented Dec 26, 2018

Was reading what akita is about and found this: https://netbasal.gitbook.io/akita/entity-store/entity-query/sort-by

const sortBy = (a, b, state) => (state.sortyByPrice ? sortByPrice(a, b) : sortById(a, b));

Seems like they have thought about this.

From what I can tell, Akita uses services heavily and basically move actions and reducers inside services; which reminds me of what ngrx-data does for entities. With the integration between ngrx-data and nrgx, will we see a bigger role for services inside ngrx, thus reducing the need to write actions and reducers?

@timdeschryver
Copy link
Member

@dereklin if you want to use services with NgRx, take a look at the Facades pattern.
Thomas Burleson wrote NgRx + Facades: Better State Management about this.

@MikeRyanDev
Copy link
Member

@dereklin Something to keep in mind about NgRx Entity's sortComparer interface is that by having it be immutable we can optimize operations like addOne and updateOne to change the target entity's position in the sorted list without having to resort the entire collection.

If we passed state as a third parameter and the sorting method changed based on the state then it would be difficult for NgRx Entity to know when to invalidate the current sorted set. We would have to change NgRx Entity to resort the entire collection on every addOne or updateOne. This would be a pretty big de-optimization.

Using selectors on the other hand means that your application will only change the sort method when the target property in your state changes. Selectors plus an unsorted state adapter would be the best optimization strategy for your application and wouldn't require us to de-optimize parts of the sorted state adapter.

As for your last question there will be no increased emphasis on services in NgRx in the future. Pure functions will always be our default. 👍

@aSmakov
Copy link

aSmakov commented Sep 17, 2019

Hello! I am using NGRX with Entity. Now I have to sort my entities, but I can not find a single example. Can you give a working example to do this?

@zohar1000
Copy link

I used a different approach. I created an object in the reducer.ts file which holds the sort fields, those fields are being updated whenever the user changes the sort field/order. This object is being imported in the entity-metadata.ts file and is being used by the sortComparer function.

The example below is for user entity, the object is usersSelectedSort and it holds key and order fields which are used for sort by the sortComparer.

user.reducer.ts:

import { createReducer, on } from '@ngrx/store';
import { UserState } from '../../shared/models/user-state.model';
import { UserSortAction } from './user.actions';

export const initialState: UserState = {
  sort: {
    key: 'userId',
    order: 1
  }
};

export const userSort = {
  key: initialState.sort.key,
  order: initialState.sort.order
};

const _userReducer = createReducer(
  initialState,
  on(UserSortAction, (state, data) => {
    userSort.key = data.key;
    userSort.order = data.order;
    return { ...state, sort: { key: data.key, order: data.order } };
  })
);

export function userReducer(state, action) {
  return _userReducer(state, action);
}

app.entity-metadata.ts:

import { EntityMetadataMap } from '@ngrx/data';
import { User } from '../../shared/models/user.model';
import { userSort } from '../user/user.reducer';

function usersSortFn(a, b) {
  const val1 = a[userSort.key];
  const val2 = b[userSort.key];
  return val1 < val2 ? -1 * userSort.order : val1 > val2 ? userSort.order : 0;
}

export const entityMetadata: EntityMetadataMap = {
  User: {
    selectId: (user: User) => user.userId,
    sortComparer: usersSortFn
  }
};

export const pluralNames = {
  User: 'user' // the plural of User
};

export const entityConfig = {
  entityMetadata,
  pluralNames
};

for listening to sort event I placed an event handler in the template via matSortChange.

user.component.html:

<table mat-table [dataSource]="dataSource" matSort (matSortChange)="onSortChange($event)" class="mat-elevation-z8">

and in my component I dispatch the sort action and afterwards refresh the view. unfortunately I haven't found a good way to refresh the view other then replacing the items with addAllToCache.

user.component.ts:

export class UsersComponent implements OnInit {
  readonly COLUMNS = ['userId', 'firstName', 'lastName', 'status', 'email', 'role', 'edit', 'delete'];
  loading$: Observable<boolean>;
  users$: Observable<User[]>;
  users: User[];
  dataSource;

  constructor(private userService: UserService,
              private toastr: ToastrService) {
    this.users$ = this.userService.entities$.pipe(tap(users => this.users = users));
    this.loading$ = this.userService.loading$;
    this.dataSource = new UserDataSource(this.userService);
  }

  ngOnInit() {
    this.getUsers();
  }

  onSortChange(e) {
    if (e.direction === '') return;
    const order = SortDirToOrder[e.direction];
    this.userService.dispatch(UserSortAction({ key: e.active, order }));
    this.userService.addAllToCache(this.users);
  }

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

9 participants