ngxs-labs / entity-state

⏱ WIP: Entity adapter
48 stars 12 forks source link

EntityState selectors recalculate when any part of root state was updated #206

Closed lomchik closed 4 years ago

lomchik commented 4 years ago

Instead of repro steps I added test

import { EntityState, EntityStateModel, defaultEntityState, IdStrategy } from '@ngxs-labs/entity-state';
import { State, NgxsModule, Store, Action, StateContext } from '@ngxs/store';
import { TestBed } from '@angular/core/testing';

fdescribe('EntityState selectors recalculate when not needed', () => {
  beforeEach(() => {
    TestBed.configureTestingModule({
      imports: [
        NgxsModule.forRoot([
          SomeEntityState,
          AnotherState
        ])
      ]
    });
  });

  it('get entities once when updating different state', () => {
    const spy = jasmine.createSpy('callback');
    const store = TestBed.get(Store) as Store;
    store.select(SomeEntityState.entities).subscribe(spy);
    store.dispatch(new UpdateAnotherState);
    store.dispatch(new UpdateAnotherState);
    expect(spy).toHaveBeenCalledTimes(1); // Expected spy callback to have been called once. It was called 3 times.
  });
});

@State<EntityStateModel<any>>({
  name: 'someEntity',
  defaults: defaultEntityState<any>()
})
class SomeEntityState extends EntityState<any> {
 constructor() {
  super(SomeEntityState, 'id', IdStrategy.EntityIdGenerator);
 }
}

class UpdateAnotherState {
  static type = 'UpdateAnotherState';
}

@State<number>({
  name: 'another',
  defaults: 999
})
class AnotherState {
 constructor() {
 }
 @Action(UpdateAnotherState)
 update(ctx: StateContext<number>) {
  ctx.setState(Math.random());
 }
}
JanMalch commented 4 years ago

I can confirm the behaviour. I'll see what I can do.

@markwhitfeld do you have any insight on how select works? If I debug I can see that the UpdateAnotherState action is dispatched and the returned function from TodoState.entities gets called. The that variable points to the correct class.


Opened branch issue-206.

kodze commented 4 years ago

Hello ! :)

Same issue for me ! Are there any plans to fix this issue?

Thanks a lot for this great lib !

JanMalch commented 4 years ago

I would like to fix it but I've no idea what might cause this. I don't know enough about the inner workings.

amollahi commented 4 years ago

I think it's about how the selectors are created in the test, just checking.

amollahi commented 4 years ago

There isn't a bug in the library, it's the test what doesn't work properly. I test it in a app, and it works properly, only emits once.

For the answer to the issue i put on the ngxs/store repository, it should be the same here. Probably what your getting the factory function, that's also a selector.

JanMalch commented 4 years ago

I refactored the selectors to use createSelector and it seems to be working. I will make a release within the next few days.

amollahi commented 4 years ago

👍 Nice @JanMalch , really interesting to have this ready to production.

I think is a must for all state management libraries(Akita have it by default), to be able to do basic operations with entities(create/update, delete...). And it's great and descriptive the way to define actions without the need to write more code:

new UpdateActive(TodoState, { done: true })

Probably will be easy to extend this to add more generic Actions for the needs of each App:

I probably include soon ngxs/store and ngxs/entity-state in a company project.