Store: Lazy selectors reevaluating without memoizing the result

[malfusion]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => https://github.com/ngxs/store/blob/master/CONTRIBUTING.md
[ ] Other... Please describe:

Current behavior

I have a simple Zoo State with an animals array and a zooName attribute. I'm using a lazy selector to select animals from the animals array which contain a certain name, eg: "panda".
Everytime I change the zooName field, the lazy selector's subscribe method also seems to be firing (which shouldn't happen).

Expected behavior

If memoization of the lazy selector is working, the subscribe method must not be fired.
The lazy selector must evaluate the function, compare with the previous returned value and not call the subscribe function if there are no changes.

Minimal reproduction of the problem with instructions

1. Dispatching an action to change zooName every 1000 ms.
2. The lazy selector subscription gets called and writes to the HTML every 1 second, when it truly shouldn't get called at all.

https://stackblitz.com/edit/ngxs-lazyselector-issue

What is the motivation / use case for changing the behavior?

Lazy selectors getting reevaluated everytime causes lots of repetitive dependent computation in the components that are dependent on it.

Environment

Libs:
- @angular/core version: ~7.0.0 (and i tried 6 as well, same result)
- @ngxs/store version: 3.4.2


Browser:
- [x] Chrome (desktop) version XX
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX

[markwhitfeld]

See the fix win this stackblitz:
https://stackblitz.com/edit/ngxs-lazyselector-issue-fix?file=app%2Fapp.component.ts
It really boils down to how you constructed your selector.

  @Selector()
  static animalOfType(state: ZooStateModel) {
    return (type: string) => {
      return state.animals.filter(s => s.indexOf(type) > -1);
    };
  }

because that selector is dependent on the entire ZooStateModel it will return a new function on every change of the model.

In my solution I broke the selector down into two parts.
A selector that returns only the slice we are interested in:

  @Selector()
  static animals(state: ZooStateModel) {
    return state.animals;
  }

And a query class that has a selector that is only dependent in the selector above:

export class ZooStateQueries {
  @Selector([ZooState.animals])
  static animalOfType(animals: string[]) {
    return (type: string) => {
      return animals.filter(s => s.indexOf(type) > -1);
    };
  }
}

Note that this was done in a query class to avoid the container state being injected as the first parameter of the selector (this bug will be fixed in NGXS v4).

Please close if I have answered your question sufficiently.

[markwhitfeld]

PS. You will also see in my example that I demonstrated the alternative where you could use a dynamic selector... that could have been declared within the class and would not suffer the container state injection issue.

[malfusion]

Thank you so much! You kinda solved a dozen of my other issues in my app with that answer. Thanks for the super clear explanations.

[markwhitfeld]

It's a pleasure 👍