Platform: Selector function runs multiple times even though arguments have not changed

Created on 30 Oct 2018  路  2Comments  路  Source: ngrx/platform

Minimal reproduction of the bug/regression with instructions:

https://stackblitz.com/edit/ngrx-seed-vnhhsk?file=src%2Fapp%2Freducers%2Findex.ts

Setup:

  • The example contains of a store with a state that constains two properties. One entity object that contains objects with their id as key and a counter property.
  • There is a factory function to create a selector that returns an object from the store with the provided id. The entity selector does some computations, sleeps for 1 second in this example and we would therefore want it to be memoized so that the computation doesn't have to run too often.
  • the app.component is calling the factory function to get a selector for the entity with id 1.

Steps to reproduce:
1 Click on the + button to increase the counter. This will be quick and not trigger the getEntity selector
2 Click on the "Update 2" button. This will update the entity with id 2 and the selector function will be executed.
3 Click on the + button again. This time it will not be quick since the getEntity selector was triggered again. However, it shouldn't have because the entity object was not updated.

Expected behavior:

When clicking on the + button in step 3, the created getEntity selector should not be called since the entity object has not been changed.

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s):

6.1.0

I would be willing to submit a PR to fix this issue

I think the error is caused by the memoize function here: https://github.com/ngrx/platform/blob/master/modules/store/src/selector.ts#L62. If the arguments were changed but not the result, the lastArguments will never be reassigned. This means that the selector will be called the next time the memoize function is called since it thinks that the arguments has been changed. This can be prevented by adding the following line
lastArguments = arguments;
in the if(isResultEqual...

[x ] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

Store bug

Most helpful comment

Sorry I didn't notice you came to the same conclusion as I did, after I created the PR 馃槄 .
But it rang a bell from a previous commit so I went ahead and created the PR.

All 2 comments

Same behaviour here. All our selectors are created via createSelectorFactory with a custom memoization function to bypass the result check. But sometime the defaultMemoize is called anyway.

Sorry I didn't notice you came to the same conclusion as I did, after I created the PR 馃槄 .
But it rang a bell from a previous commit so I went ahead and created the PR.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

hccampos picture hccampos  路  3Comments

dmytro-gokun picture dmytro-gokun  路  3Comments

mappedinn picture mappedinn  路  3Comments

brandonroberts picture brandonroberts  路  3Comments

dollyshah-02-zz picture dollyshah-02-zz  路  3Comments