From fbd6a66ac7aaf73c5293ee5e5299313e4b0f7d96 Mon Sep 17 00:00:00 2001 From: Mike Ryan Date: Thu, 17 Aug 2017 19:50:19 -0500 Subject: [PATCH] fix(Entity): Return a referentially equal state if state did not change --- .../entity/spec/sorted_state_adapter.spec.ts | 4 +- .../spec/unsorted_state_adapter.spec.ts | 4 +- modules/entity/src/sorted_state_adapter.ts | 43 ++++++++++------ modules/entity/src/state_adapter.ts | 10 ++-- modules/entity/src/unsorted_state_adapter.ts | 49 +++++++++++++------ 5 files changed, 73 insertions(+), 37 deletions(-) diff --git a/modules/entity/spec/sorted_state_adapter.spec.ts b/modules/entity/spec/sorted_state_adapter.spec.ts index eaef6dc797..f8b8d7ce5d 100644 --- 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 index 2a9d1176a8..941ab87088 100644 --- 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 index 44a4afd269..13ece9f8e5 100644 --- a/modules/entity/src/sorted_state_adapter.ts +++ b/modules/entity/src/sorted_state_adapter.ts @@ -19,26 +19,31 @@ export function createSortedStateAdapter( 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( state.entities[id] = model; return id; }); - } - function updateOneMutably(update: Update, state: R): void { - const index = state.ids.indexOf(update.id); + return true; + } - if (index === -1) { - return; + function updateOneMutably(update: Update, state: R): boolean { + if (!(update.id in state.entities)) { + return false; } const original = state.entities[update.id]; @@ -64,14 +69,16 @@ export function createSortedStateAdapter( 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( } state.entities[updatedKey] = updated; + + return true; } - function updateManyMutably(updates: Update[], state: R): void { + function updateManyMutably(updates: Update[], 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 index 6a6b24f744..9a0d6c68be 100644 --- a/modules/entity/src/state_adapter.ts +++ b/modules/entity/src/state_adapter.ts @@ -1,7 +1,7 @@ import { EntityState, EntityStateAdapter } from './models'; export function createStateOperator( - mutator: (arg: R, state: EntityState) => void + mutator: (arg: R, state: EntityState) => boolean ) { return function operation>(arg: R, state: S): S { const clonedEntityState: EntityState = { @@ -9,8 +9,12 @@ export function createStateOperator( 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 index afc6980055..3f1cad07e7 100644 --- a/modules/entity/src/unsorted_state_adapter.ts +++ b/modules/entity/src/unsorted_state_adapter.ts @@ -6,46 +6,59 @@ export function createUnsortedStateAdapter( ): EntityStateAdapter { type R = EntityState; - 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(state: S): S { @@ -55,11 +68,11 @@ export function createUnsortedStateAdapter( }); } - function updateOneMutably(update: Update, state: R): void { + function updateOneMutably(update: Update, 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( } state.entities[newKey] = updated; + + return true; } - function updateManyMutably(updates: Update[], state: R): void { + function updateManyMutably(updates: Update[], state: R): boolean { + let didMutate = false; + for (let index in updates) { - updateOneMutably(updates[index], state); + didMutate = updateOneMutably(updates[index], state) || didMutate; } + + return didMutate; } return {