fix(Entity): Return a referentially equal state if state did not change

ngrx · Aug 18, 2017 · fbd6a66 · fbd6a66
1 parent 335d255
commit fbd6a66
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 37 deletions.
diff --git a/modules/entity/spec/sorted_state_adapter.spec.ts b/modules/entity/spec/sorted_state_adapter.spec.ts
@@ -36,7 +36,7 @@ describe('Sorted State Adapter', () => {
 
     const readded = adapter.addOne(TheGreatGatsby, withOneEntity);
 
-    expect(readded).toEqual(withOneEntity);
+    expect(readded).toBe(withOneEntity);
   });
 
   it('should let you add many entities to the state', () => {
@@ -150,7 +150,7 @@ describe('Sorted State Adapter', () => {
       state
     );
 
-    expect(withUpdates).toEqual(state);
+    expect(withUpdates).toBe(state);
   });
 
   it('should let you update the id of entity', () => {

diff --git a/modules/entity/spec/unsorted_state_adapter.spec.ts b/modules/entity/spec/unsorted_state_adapter.spec.ts
@@ -35,7 +35,7 @@ describe('Unsorted State Adapter', () => {
 
     const readded = adapter.addOne(TheGreatGatsby, withOneEntity);
 
-    expect(readded).toEqual(withOneEntity);
+    expect(readded).toBe(withOneEntity);
   });
 
   it('should let you add many entities to the state', () => {
@@ -149,7 +149,7 @@ describe('Unsorted State Adapter', () => {
       state
     );
 
-    expect(withUpdates).toEqual(state);
+    expect(withUpdates).toBe(state);
   });
 
   it('should let you update the id of entity', () => {

diff --git a/modules/entity/src/sorted_state_adapter.ts b/modules/entity/src/sorted_state_adapter.ts
@@ -19,26 +19,31 @@ export function createSortedStateAdapter<T>(
     selectId
   );
 
-  function addOneMutably(entity: T, state: R): void {
+  function addOneMutably(entity: T, state: R): boolean {
     const key = selectId(entity);
-    const index = state.ids.indexOf(key);
 
-    if (index !== -1) {
-      return;
+    if (key in state.entities) {
+      return false;
     }
 
     const insertAt = findTargetIndex(state, entity);
     state.ids.splice(insertAt, 0, key);
     state.entities[key] = entity;
+
+    return true;
   }
 
-  function addManyMutably(newModels: T[], state: R): void {
+  function addManyMutably(newModels: T[], state: R): boolean {
+    let didMutate = false;
+
     for (let index in newModels) {
-      addOneMutably(newModels[index], state);
+      didMutate = addOneMutably(newModels[index], state) || didMutate;
     }
+
+    return didMutate;
   }
 
-  function addAllMutably(models: T[], state: R): void {
+  function addAllMutably(models: T[], state: R): boolean {
     const sortedModels = models.sort(sort);
 
     state.entities = {};
@@ -47,13 +52,13 @@ export function createSortedStateAdapter<T>(
       state.entities[id] = model;
       return id;
     });
-  }
 
-  function updateOneMutably(update: Update<T>, state: R): void {
-    const index = state.ids.indexOf(update.id);
+    return true;
+  }
 
-    if (index === -1) {
-      return;
+  function updateOneMutably(update: Update<T>, state: R): boolean {
+    if (!(update.id in state.entities)) {
+      return false;
     }
 
     const original = state.entities[update.id];
@@ -64,14 +69,16 @@ export function createSortedStateAdapter<T>(
     if (result === 0) {
       if (updatedKey !== update.id) {
         delete state.entities[update.id];
+        const index = state.ids.indexOf(update.id);
         state.ids[index] = updatedKey;
       }
 
       state.entities[updatedKey] = updated;
 
-      return;
+      return true;
     }
 
+    const index = state.ids.indexOf(update.id);
     state.ids.splice(index, 1);
     state.ids.splice(findTargetIndex(state, updated), 0, updatedKey);
 
@@ -80,12 +87,18 @@ export function createSortedStateAdapter<T>(
     }
 
     state.entities[updatedKey] = updated;
+
+    return true;
   }
 
-  function updateManyMutably(updates: Update<T>[], state: R): void {
+  function updateManyMutably(updates: Update<T>[], state: R): boolean {
+    let didMutate = false;
+
     for (let index in updates) {
-      updateOneMutably(updates[index], state);
+      didMutate = updateOneMutably(updates[index], state) || didMutate;
     }
+
+    return didMutate;
   }
 
   function findTargetIndex(state: R, model: T) {

diff --git a/modules/entity/src/state_adapter.ts b/modules/entity/src/state_adapter.ts
@@ -1,16 +1,20 @@
 import { EntityState, EntityStateAdapter } from './models';
 
 export function createStateOperator<V, R>(
-  mutator: (arg: R, state: EntityState<V>) => void
+  mutator: (arg: R, state: EntityState<V>) => boolean
 ) {
   return function operation<S extends EntityState<V>>(arg: R, state: S): S {
     const clonedEntityState: EntityState<V> = {
       ids: [...state.ids],
       entities: { ...state.entities },
     };
 
-    mutator(arg, clonedEntityState);
+    const didMutate = mutator(arg, clonedEntityState);
 
-    return Object.assign({}, state, clonedEntityState);
+    if (didMutate) {
+      return Object.assign({}, state, clonedEntityState);
+    }
+
+    return state;
   };
 }
diff --git a/modules/entity/src/unsorted_state_adapter.ts b/modules/entity/src/unsorted_state_adapter.ts
@@ -6,46 +6,59 @@ export function createUnsortedStateAdapter<T>(
 ): EntityStateAdapter<T> {
   type R = EntityState<T>;
 
-  function addOneMutably(entity: T, state: R): void {
+  function addOneMutably(entity: T, state: R): boolean {
     const key = selectId(entity);
-    const index = state.ids.indexOf(key);
 
-    if (index !== -1) {
-      return;
+    if (key in state.entities) {
+      return false;
     }
 
     state.ids.push(key);
     state.entities[key] = entity;
+
+    return true;
   }
 
-  function addManyMutably(entities: T[], state: R): void {
+  function addManyMutably(entities: T[], state: R): boolean {
+    let didMutate = false;
+
     for (let index in entities) {
-      addOneMutably(entities[index], state);
+      didMutate = addOneMutably(entities[index], state) || didMutate;
     }
+
+    return didMutate;
   }
 
-  function addAllMutably(entities: T[], state: R): void {
+  function addAllMutably(entities: T[], state: R): boolean {
     state.ids = [];
     state.entities = {};
 
     addManyMutably(entities, state);
+
+    return true;
   }
 
-  function removeOneMutably(key: string, state: R): void {
+  function removeOneMutably(key: string, state: R): boolean {
     const index = state.ids.indexOf(key);
 
     if (index === -1) {
-      return;
+      return false;
     }
 
     state.ids.splice(index, 1);
     delete state.entities[key];
+
+    return true;
   }
 
-  function removeManyMutably(keys: string[], state: R): void {
+  function removeManyMutably(keys: string[], state: R): boolean {
+    let didMutate = false;
+
     for (let index in keys) {
-      removeOneMutably(keys[index], state);
+      didMutate = removeOneMutably(keys[index], state) || didMutate;
     }
+
+    return didMutate;
   }
 
   function removeAll<S extends R>(state: S): S {
@@ -55,11 +68,11 @@ export function createUnsortedStateAdapter<T>(
     });
   }
 
-  function updateOneMutably(update: Update<T>, state: R): void {
+  function updateOneMutably(update: Update<T>, state: R): boolean {
     const index = state.ids.indexOf(update.id);
 
     if (index === -1) {
-      return;
+      return false;
     }
 
     const original = state.entities[update.id];
@@ -72,12 +85,18 @@ export function createUnsortedStateAdapter<T>(
     }
 
     state.entities[newKey] = updated;
+
+    return true;
   }
 
-  function updateManyMutably(updates: Update<T>[], state: R): void {
+  function updateManyMutably(updates: Update<T>[], state: R): boolean {
+    let didMutate = false;
+
     for (let index in updates) {
-      updateOneMutably(updates[index], state);
+      didMutate = updateOneMutably(updates[index], state) || didMutate;
     }
+
+    return didMutate;
   }
 
   return {