Merge pull request #2464 from reduxjs/bugfix/entity-adapter-sorting

Author: markerikson

packages/toolkit/src/entities/sorted_state_adapter.ts

@@ -71,33 +71,30 @@ export function createSortedStateAdapter<T>(
     return updateManyMutably([update], state)
   }
 
-  // eslint-disable-next-line @typescript-eslint/prefer-readonly-parameter-types
-  function takeUpdatedModel(models: T[], update: Update<T>, 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<Update<T>>,
     state: R
   ): void {
-    const models: T[] = []
+    let appliedUpdates = false
+
+    for (let update of updates) {
+      const entity = state.entities[update.id]
+      if (!entity) {
+        continue
+      }
 
-    updates.forEach((update) => takeUpdatedModel(models, update, state))
+      appliedUpdates = true
 
-    if (models.length !== 0) {
-      merge(models, state)
+      Object.assign(entity, update.changes)
+      const newId = selectId(entity)
+      if (update.id !== newId) {
+        delete state.entities[update.id]
+        state.entities[newId] = entity
+      }
+    }
+
+    if (appliedUpdates) {
+      resortEntities(state)
     }
   }
 
@@ -139,6 +136,10 @@ export function createSortedStateAdapter<T>(
       state.entities[selectId(model)] = model
     })
 
+    resortEntities(state)
+  }
+
+  function resortEntities(state: R) {
     const allEntities = Object.values(state.entities) as T[]
     allEntities.sort(sort)
 

packages/toolkit/src/entities/tests/sorted_state_adapter.test.ts

@@ -343,6 +343,34 @@ describe('Sorted State Adapter', () => {
     })
   })
 
+  it('should maintain a stable sorting order when updating items', () => {
+    interface OrderedEntity {
+      id: string
+      order: number
+      ts: number
+    }
+    const sortedItemsAdapter = createEntityAdapter<OrderedEntity>({
+      sortComparer: (a, b) => a.order - b.order,
+    })
+    const withInitialItems = sortedItemsAdapter.setAll(
+      sortedItemsAdapter.getInitialState(),
+      [
+        { id: 'A', order: 1, ts: 0 },
+        { id: 'B', order: 2, ts: 0 },
+        { id: 'C', order: 3, ts: 0 },
+        { id: 'D', order: 3, ts: 0 },
+        { id: 'E', order: 3, ts: 0 },
+      ]
+    )
+
+    const updated = sortedItemsAdapter.updateOne(withInitialItems, {
+      id: 'C',
+      changes: { ts: 5 },
+    })
+
+    expect(updated.ids).toEqual(['A', 'B', 'C', 'D', 'E'])
+  })
+
   it('should let you update many entities by id in the state', () => {
     const firstChange = { title: 'Zack' }
     const secondChange = { title: 'Aaron' }
@@ -652,12 +680,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 +702,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",
           ],