Do not process animation if items keys are set incorrectly

surveyjs · May 12, 2024 · 508ace0 · 508ace0
1 parent 6493086
commit 508ace0
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 13 deletions.
diff --git a/src/utils/animation.ts b/src/utils/animation.ts
@@ -218,11 +218,11 @@ export class AnimationGroupUtils<T> extends AnimationUtils {
         removedItems.forEach((_, i) => {
           this.runAnimation(removedHtmlElements[i], leaveOptions[i], onAnimationEndCallback);
         });
-        reorderedHtmlElements.forEach((_, i) => {
+        reorderedItems.forEach((_, i) => {
           this.runAnimation(reorderedHtmlElements[i], reorderedOptions[i], onAnimationEndCallback);
         });
       },
-      () => addedItems.length == 0 || addedItems.some(el => !!options.getAnimatedElement(el)));
+      () => (addedItems.length == 0 || addedItems.some(el => !!options.getAnimatedElement(el))) && (reorderedItems.length == 0 || reorderedItems.some(el => !!options.getAnimatedElement(el.item))));
   }
 }
 
@@ -271,13 +271,17 @@ export class AnimationGroup<T> extends AnimationProperty<Array<T>, IAnimationGro
   protected animation: AnimationGroupUtils<T> = new AnimationGroupUtils();
   protected _sync (newValue: Array<T>): void {
     const oldValue = this.getCurrentValue();
-    const { addedItems, deletedItems, reorderedItems, mergedItems } = compareArrays(oldValue, newValue, this.animationOptions.getKey ?? ((item: T) => item));
-    if(deletedItems.length <= 0 || reorderedItems.length > 0 || addedItems.length > 0) this.update(mergedItems);
-    this.animation.runGroupAnimation(this.animationOptions, addedItems, deletedItems, reorderedItems, () => {
-      if(deletedItems.length > 0) {
-        this.update(newValue);
-      }
-    });
+    try {
+      const { addedItems, deletedItems, reorderedItems, mergedItems } = compareArrays(oldValue, newValue, this.animationOptions.getKey ?? ((item: T) => item));
+      if(deletedItems.length <= 0 || reorderedItems.length > 0 || addedItems.length > 0) this.update(mergedItems);
+      this.animation.runGroupAnimation(this.animationOptions, addedItems, deletedItems, reorderedItems, () => {
+        if(deletedItems.length > 0) {
+          this.update(newValue);
+        }
+      });
+    } catch {
+      this.update(newValue);
+    }
   }
 }
 export class AnimationTab<T> extends AnimationProperty<Array<T>, IAnimationGroupConsumer<T>> {

diff --git a/src/utils/utils.ts b/src/utils/utils.ts
@@ -474,11 +474,23 @@ export function compareArrays<T>(oldValue: Array<T>, newValue: Array<T>, getKey:
   const newItemsMap = new Map<any, T>();
   const commonItemsInNewMap = new Map<any, number>();
   const commonItemsInOldMap = new Map<any, number>();
-  oldValue.forEach((item, index) => {
-    oldItemsMap.set(getKey(item), item);
+  oldValue.forEach((item) => {
+    const itemKey = getKey(item);
+    if(!oldItemsMap.has(itemKey)) {
+      oldItemsMap.set(getKey(item), item);
+    } else {
+      //if keys are set incorrectly do not process comparing
+      throw new Error("keys must be unique");
+    }
   });
-  newValue.forEach((item, index) => {
-    newItemsMap.set(getKey(item), item);
+  newValue.forEach((item) => {
+    const itemKey = getKey(item);
+    if(!newItemsMap.has(itemKey)) {
+      newItemsMap.set(itemKey, item);
+    } else {
+      //if keys are set incorrectly do not process comparing
+      throw new Error("keys must be unique");
+    }
   });
   const addedItems: Array<T> = [];
   const deletedItems: Array<T> = [];

diff --git a/tests/utilstests.ts b/tests/utilstests.ts
@@ -912,4 +912,7 @@ QUnit.test("test compareArrays function", (assert) => {
   assert.deepEqual(res.addedItems, [{ value: 5 }, { value: 3 }, { value: 6 }, { value: 1 }, { value: 7 }]);
   assert.deepEqual(res.reorderedItems, []);
   assert.deepEqual(res.mergedItems, [{ value: 5 }, { value: 3 }, { value: 6 }, { value: 1 }, { value: 7 }, { value: 0 }, { value: 1 }, { value: 2 }, { value: 3 }, { value: 4 }]);
+
+  assert.throws(() => compareArrays([{ value: 0 }, { value: 0 }], [], (item) => item.value), new Error("keys must be unique"));
+  assert.throws(() => compareArrays([], [{ value: 1 }, { value: 1 }], (item) => item.value), new Error("keys must be unique"));
 });