From fdc845dec57dae12461a3f6847ce382d44297da5 Mon Sep 17 00:00:00 2001 From: Jake Boone Date: Tue, 21 Dec 2021 17:28:31 -0700 Subject: [PATCH] Fix #1853 --- .../src/entities/sorted_state_adapter.ts | 48 +++++++++++-------- .../tests/sorted_state_adapter.test.ts | 26 +++++++--- .../src/entities/unsorted_state_adapter.ts | 37 +++++--------- 3 files changed, 60 insertions(+), 51 deletions(-) diff --git a/packages/toolkit/src/entities/sorted_state_adapter.ts b/packages/toolkit/src/entities/sorted_state_adapter.ts index 4812d8e575..01f7d87951 100644 --- a/packages/toolkit/src/entities/sorted_state_adapter.ts +++ b/packages/toolkit/src/entities/sorted_state_adapter.ts @@ -71,32 +71,40 @@ export function createSortedStateAdapter( return updateManyMutably([update], state) } - // eslint-disable-next-line @typescript-eslint/prefer-readonly-parameter-types - function takeUpdatedModel(models: T[], update: Update, state: R): boolean { - if (!(update.id in state.entities)) { - return false - } - - const original = state.entities[update.id] - const updated = Object.assign({}, original, update.changes) - const newKey = selectIdValue(updated, selectId) - - delete state.entities[update.id] - - models.push(updated) - - return newKey !== update.id - } - function updateManyMutably( updates: ReadonlyArray>, state: R ): void { - const models: T[] = [] + const changedKeys: EntityId[] = [] + + const updatesPerEntity: { [id: string]: Update } = {} + + updates.forEach((update) => { + if (update.id in state.entities) { + updatesPerEntity[update.id] = { + id: update.id, + changes: { + ...(updatesPerEntity[update.id] + ? updatesPerEntity[update.id].changes + : null), + ...update.changes, + }, + } + } + const newId = selectId(update.changes as T) + if (newId !== undefined && update.id !== newId) { + changedKeys.push(update.id) + } + }) - updates.forEach((update) => takeUpdatedModel(models, update, state)) + updates = Object.values(updatesPerEntity) - if (models.length !== 0) { + if (updates.length > 0) { + const models = updates.map( + (update) => + Object.assign({}, state.entities[update.id], update.changes) as T + ) + changedKeys.forEach((key) => delete state.entities[key]) merge(models, state) } } diff --git a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts index 4a018803f5..504415b913 100644 --- a/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts +++ b/packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts @@ -30,9 +30,7 @@ describe('Sorted State Adapter', () => { beforeEach(() => { adapter = createEntityAdapter({ selectId: (book: BookModel) => book.id, - sortComparer: (a, b) => { - return a.title.localeCompare(b.title) - }, + sortComparer: (a, b) => a.title.localeCompare(b.title), }) state = { ids: [], entities: {} } @@ -652,12 +650,20 @@ describe('Sorted State Adapter', () => { test('updateMany', () => { const firstChange = { title: 'First Change' } const secondChange = { title: 'Second Change' } - const withMany = adapter.setAll(state, [TheGreatGatsby, AClockworkOrange]) + const thirdChange = { title: 'Third Change' } + const fourthChange = { author: 'Fourth Change' } + const withMany = adapter.setAll(state, [ + TheGreatGatsby, + AClockworkOrange, + TheHobbit, + ]) const result = createNextState(withMany, (draft) => { adapter.updateMany(draft, [ - { id: TheGreatGatsby.id, changes: firstChange }, - { id: AClockworkOrange.id, changes: secondChange }, + { id: TheHobbit.id, changes: firstChange }, + { id: TheGreatGatsby.id, changes: secondChange }, + { id: AClockworkOrange.id, changes: thirdChange }, + { id: TheHobbit.id, changes: fourthChange }, ]) }) @@ -666,14 +672,20 @@ describe('Sorted State Adapter', () => { "entities": Object { "aco": Object { "id": "aco", - "title": "Second Change", + "title": "Third Change", }, "tgg": Object { "id": "tgg", + "title": "Second Change", + }, + "th": Object { + "author": "Fourth Change", + "id": "th", "title": "First Change", }, }, "ids": Array [ + "th", "tgg", "aco", ], diff --git a/packages/toolkit/src/entities/unsorted_state_adapter.ts b/packages/toolkit/src/entities/unsorted_state_adapter.ts index ffa104a74e..5702f2ae15 100644 --- a/packages/toolkit/src/entities/unsorted_state_adapter.ts +++ b/packages/toolkit/src/entities/unsorted_state_adapter.ts @@ -98,26 +98,6 @@ export function createUnsortedStateAdapter( }) } - function takeNewKey( - keys: { [id: string]: EntityId }, - update: Update, - state: R - ): boolean { - const original = state.entities[update.id] - const updated: T = Object.assign({}, original, update.changes) - const newKey = selectIdValue(updated, selectId) - const hasNewKey = newKey !== update.id - - if (hasNewKey) { - keys[update.id] = newKey - delete state.entities[update.id] - } - - state.entities[newKey] = updated - - return hasNewKey - } - function updateOneMutably(update: Update, state: R): void { return updateManyMutably([update], state) } @@ -127,7 +107,6 @@ export function createUnsortedStateAdapter( state: R ): void { const newKeys: { [id: string]: EntityId } = {} - const updatesPerEntity: { [id: string]: Update } = {} updates.forEach((update) => { @@ -146,6 +125,10 @@ export function createUnsortedStateAdapter( }, } } + const newId = selectId(update.changes as T) + if (newId !== undefined && update.id !== newId) { + newKeys[update.id] = newId; + } }) updates = Object.values(updatesPerEntity) @@ -153,9 +136,15 @@ export function createUnsortedStateAdapter( const didMutateEntities = updates.length > 0 if (didMutateEntities) { - const didMutateIds = - updates.filter((update) => takeNewKey(newKeys, update, state)).length > - 0 + const changedKeys = Object.keys(newKeys); + const didMutateIds = changedKeys.length > 0; + + updates.forEach((update) => { + const newEntity = Object.assign({}, state.entities[update.id], update.changes) + state.entities[newKeys[update.id] || update.id] = newEntity + }) + + changedKeys.forEach(key => delete state.entities[key]) if (didMutateIds) { state.ids = state.ids.map((id) => newKeys[id] || id)