Skip to content

Commit 83bfab7

Browse files
anna-shakhovaanna-shakhova
authored
TabPanel: log error if duplicated item keys detected (DevExpress#30543)
1 parent 6009ecf commit 83bfab7

File tree

2 files changed

+107
-74
lines changed

2 files changed

+107
-74
lines changed

‎packages/devextreme/js/__internal/core/utils/m_array_compare.ts‎

Lines changed: 88 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
import { isObject } from '@js/core/utils/type';
1+
import { logger } from '@js/core/utils/console';
2+
import { isDefined, isObject } from '@js/core/utils/type';
3+
import errors from '@js/ui/widget/ui.errors';
24

35
const getKeyWrapper = function (item, getKey) {
46
const key = getKey(item);
@@ -31,96 +33,108 @@ export const isKeysEqual = function (oldKeys, newKeys) {
3133
return true;
3234
};
3335

36+
const mapIndexByKey = function (items, getKey) {
37+
const indexByKey = {};
38+
39+
items.forEach((item, index) => {
40+
const key = getKeyWrapper(item, getKey);
41+
42+
if (isDefined(indexByKey[String(key)])) {
43+
throw errors.Error('E1040', key);
44+
}
45+
46+
indexByKey[key] = index;
47+
});
48+
49+
return indexByKey;
50+
};
51+
3452
export const findChanges = function ({
3553
oldItems,
3654
newItems,
3755
getKey,
3856
isItemEquals,
3957
detectReorders = false,
4058
}) {
41-
const oldIndexByKey = {};
42-
const newIndexByKey = {};
43-
let addedCount = 0;
44-
let removeCount = 0;
45-
const result: any[] = [];
59+
try {
60+
const oldIndexByKey = mapIndexByKey(oldItems, getKey);
61+
const newIndexByKey = mapIndexByKey(newItems, getKey);
62+
let addedCount = 0;
63+
let removeCount = 0;
64+
const result: any[] = [];
4665

47-
oldItems.forEach(function (item, index) {
48-
const key = getKeyWrapper(item, getKey);
49-
oldIndexByKey[key] = index;
50-
});
51-
52-
newItems.forEach(function (item, index) {
53-
const key = getKeyWrapper(item, getKey);
54-
newIndexByKey[key] = index;
55-
});
66+
const itemCount = Math.max(oldItems.length, newItems.length);
67+
for (let index = 0; index < itemCount + addedCount; index += 1) {
68+
const newItem = newItems[index];
69+
const oldNextIndex = index - addedCount + removeCount;
70+
const nextOldItem = oldItems[oldNextIndex];
71+
const isRemoved = !newItem || (nextOldItem && !getSameNewByOld(nextOldItem, newItems, newIndexByKey, getKey));
5672

57-
const itemCount = Math.max(oldItems.length, newItems.length);
58-
for (let index = 0; index < itemCount + addedCount; index++) {
59-
const newItem = newItems[index];
60-
const oldNextIndex = index - addedCount + removeCount;
61-
const nextOldItem = oldItems[oldNextIndex];
62-
const isRemoved = !newItem || (nextOldItem && !getSameNewByOld(nextOldItem, newItems, newIndexByKey, getKey));
63-
64-
if (isRemoved) {
65-
if (nextOldItem) {
66-
result.push({
67-
type: 'remove',
68-
key: getKey(nextOldItem),
69-
index,
70-
oldItem: nextOldItem,
71-
});
72-
removeCount++;
73-
index--;
74-
}
75-
} else {
76-
const key = getKeyWrapper(newItem, getKey);
77-
const oldIndex = oldIndexByKey[key];
78-
const oldItem = oldItems[oldIndex];
79-
if (!oldItem) {
80-
addedCount++;
81-
result.push({
82-
type: 'insert',
83-
data: newItem,
84-
index,
85-
});
86-
} else if (oldIndex === oldNextIndex) {
87-
if (!isItemEquals(oldItem, newItem)) {
73+
if (isRemoved) {
74+
if (nextOldItem) {
8875
result.push({
89-
type: 'update',
90-
data: newItem,
91-
key: getKey(newItem),
76+
type: 'remove',
77+
key: getKey(nextOldItem),
9278
index,
93-
oldItem,
79+
oldItem: nextOldItem,
9480
});
81+
removeCount++;
82+
index--;
9583
}
9684
} else {
97-
if (!detectReorders) {
98-
return;
99-
}
85+
const key = getKeyWrapper(newItem, getKey);
86+
const oldIndex = oldIndexByKey[key];
87+
const oldItem = oldItems[oldIndex];
88+
if (!oldItem) {
89+
addedCount++;
90+
result.push({
91+
type: 'insert',
92+
data: newItem,
93+
index,
94+
});
95+
} else if (oldIndex === oldNextIndex) {
96+
if (!isItemEquals(oldItem, newItem)) {
97+
result.push({
98+
type: 'update',
99+
data: newItem,
100+
key: getKey(newItem),
101+
index,
102+
oldItem,
103+
});
104+
}
105+
} else {
106+
if (!detectReorders) {
107+
return;
108+
}
100109

101-
result.push({
102-
type: 'remove',
103-
key: getKey(oldItem),
104-
index: oldIndex,
105-
oldItem,
106-
});
107-
result.push({
108-
type: 'insert',
109-
data: newItem,
110-
index,
111-
});
112-
addedCount++;
113-
removeCount++;
110+
result.push({
111+
type: 'remove',
112+
key: getKey(oldItem),
113+
index: oldIndex,
114+
oldItem,
115+
});
116+
result.push({
117+
type: 'insert',
118+
data: newItem,
119+
index,
120+
});
121+
addedCount++;
122+
removeCount++;
123+
}
114124
}
115125
}
116-
}
117126

118-
if (detectReorders) {
119-
const removes = result.filter((r) => r.type === 'remove').sort((a, b) => b.index - a.index);
120-
const inserts = result.filter((i) => i.type === 'insert').sort((a, b) => a.index - b.index);
121-
const updates = result.filter((u) => u.type === 'update');
122-
return [...removes, ...inserts, ...updates];
123-
}
127+
if (detectReorders) {
128+
const removes = result.filter((r) => r.type === 'remove').sort((a, b) => b.index - a.index);
129+
const inserts = result.filter((i) => i.type === 'insert').sort((a, b) => a.index - b.index);
130+
const updates = result.filter((u) => u.type === 'update');
131+
return [...removes, ...inserts, ...updates];
132+
}
124133

125-
return result;
134+
return result;
135+
} catch (e) {
136+
logger.error(e);
137+
138+
return undefined;
139+
}
126140
};

‎packages/devextreme/testing/tests/DevExpress.core/utils.array_compare.tests.js‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { findChanges } from 'core/utils/array_compare';
22
import { extend } from 'core/utils/extend';
33
import { applyBatch } from 'common/data/array_utils';
4+
import { logger } from '__internal/core/utils/m_console';
5+
import errors from 'ui/widget/ui.errors';
46

57
const ITEMS_ARRAY_LENGTH = 4;
68
const createItems = (length = ITEMS_ARRAY_LENGTH) => Array.from({ length }, (_, i) => ({ a: `Item ${i}`, id: i }));
@@ -134,4 +136,21 @@ QUnit.module('findChanges', {
134136

135137
assert.strictEqual(result, undefined);
136138
});
139+
140+
QUnit.test('should return undefined and log error if detect items with duplicated keys', function(assert) {
141+
sinon.spy(logger, 'error');
142+
sinon.spy(errors, 'Error');
143+
144+
this.newItems.push({ a: 'Item 4', id: 3 });
145+
const changes = this.findChanges();
146+
147+
assert.strictEqual(changes, undefined, 'changes are undefined');
148+
assert.strictEqual(errors.Error.callCount, 1, 'throws 1 error');
149+
assert.strictEqual(errors.Error.lastCall.args[0], 'E1040', 'error code id E1040');
150+
assert.strictEqual(errors.Error.lastCall.args[1], 3, 'error argument is duplicated key');
151+
assert.deepEqual(logger.error.getCall(0).args[0], errors.Error('E1040', 3));
152+
153+
logger.error.restore();
154+
errors.Error.restore();
155+
});
137156
});

0 commit comments

Comments
 (0)