fix(store): memoize selector arguments (#1393)

The arguments weren't being memoized when the result of the selector was equal to its previous result. This fix moves the memoization of the arguments before the check if the results are equal. Closes #1389
ngrx · Nov 1, 2018 · 7cc9702 · 7cc9702
1 parent 009c3bc
commit 7cc9702
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 1 deletion.
diff --git a/modules/store/spec/selector.spec.ts b/modules/store/spec/selector.spec.ts
@@ -90,6 +90,7 @@ describe('Selectors', () => {
       selector(firstState);
       selector(firstState);
       selector(secondState);
+      selector(secondState);
 
       expect(incrementOne).toHaveBeenCalledTimes(2);
       expect(incrementTwo).toHaveBeenCalledTimes(2);
@@ -201,6 +202,7 @@ describe('Selectors', () => {
       selector(firstState, props);
       selector(firstState, props);
       selector(secondState, props);
+      selector(secondState, props);
 
       expect(counter).toBe(2);
       expect(projectFn).toHaveBeenCalledTimes(2);
@@ -322,6 +324,7 @@ describe('Selectors', () => {
       selector(firstState);
       selector(firstState);
       selector(secondState);
+      selector(secondState);
 
       expect(incrementOne).toHaveBeenCalledTimes(2);
       expect(incrementTwo).toHaveBeenCalledTimes(2);
@@ -433,6 +436,7 @@ describe('Selectors', () => {
       selector(firstState, props);
       selector(firstState, props);
       selector(secondState, props);
+      selector(secondState, props);
 
       expect(counter).toBe(2);
       expect(projectFn).toHaveBeenCalledTimes(2);

diff --git a/modules/store/src/selector.ts b/modules/store/src/selector.ts
@@ -70,13 +70,14 @@ export function defaultMemoize(
       return lastResult;
     }
 
+    lastArguments = arguments;
+
     const newResult = projectionFn.apply(null, arguments);
     if (isResultEqual(lastResult, newResult)) {
       return lastResult;
     }
 
     lastResult = newResult;
-    lastArguments = arguments;
 
     return newResult;
   }