ngxs / store

🚀 NGXS - State Management for Angular
http://ngxs.io
MIT License
3.54k stars 404 forks source link

🐞[BUG]: dynamic selectors data is not being memoized #1406

Closed yadue closed 5 years ago

yadue commented 5 years ago

Affected Package

@ngxs/store

Description

When using dynamic selector using createSelector, data returned is not being memoized if filtered. When updating state if data returned by createSelector shouldn't emit new data.

🔬 Minimal Reproduction

https://stackblitz.com/edit/ngxs-searching-different-component-f8gdvm?file=src%2Fapp%2Fapp.component.ts Please check the console.

Environment


Libs:
- @angular/core version: 8.2.2
- @ngxs/store version: 3.5.1


Browser:
- [x] Chrome (desktop) version 77            
splincode commented 5 years ago

The selector works as intended! The state changes according to the changed state (immutable) Use child states to avoid constant re-computations See: https://stackblitz.com/edit/ngxs-memoize

yadue commented 5 years ago

Creating new store is not a solution in my case. Just compare different results from provided stackblitz.

In my case store is being dynamically generated during init phase and each one has unique key:

{
  a1: {
     someField: 'test',
     items: [1,2,3]
  },
  a2: {
     someField: 'test',
     items: [1,2,3]
  }
}

each object is generated per component instance, if you edit anything in component a1 it also gets notified in component a2 even if the a2.items were not changed. im using dynamic selectors just to get the data.

when returning

    return createSelector([AppState], (state: AppStateModel) => {
      return state.people.filter(item => item.age === 50);
    }); 

even if the result has not been changed it emits "new" data where returning

     return createSelector([AppState], (state: AppStateModel) => {
      return state.people;
    });

does exactly as expected. I can't simply create new state since the main "app" state should be unique per component. the stackblitz I prepared is just a minimal reproduction.

markwhitfeld commented 5 years ago

@yadue I modified your stackblitz example to showcase how you could handle multiple dynamic nested states: https://stackblitz.com/edit/ngxs-searching-different-component-mqhanw?file=src/app/app.state.ts

The key thing to note here is how I decomposed the selectors into smaller slices of the state because a selector will recalculate when the slice it receives as input changes. If you make that slice to be only the data that the selector depends on then there will be less recalculations.

splincode commented 5 years ago

I think we need to improve examples with documentation

markwhitfeld commented 5 years ago

@splincode We could include a more detailed example of the dynamic selector as a recipe

yadue commented 5 years ago

@markwhitfeld @splincode thanks for your help guys. That solution works perfectly!

markwhitfeld commented 5 years ago

@yadue would you be willing to submit a PR with a new recipe for your scenario (in the Recipes section in the docs)? Create the page that you wish was there when you were wanting to solve the issue. Maybe the recipe could be called "Mapped Child States" or something like that?

yadue commented 5 years ago

https://github.com/ngxs/store/pull/1422 feel free to edit whatever you need