Skip to content

Commit e514da9

Browse files
committed
fix(change_detector): Fix for detecting NanN moves in collections
Closes dart-archive#1136
1 parent 9651c87 commit e514da9

File tree

2 files changed

+37
-10
lines changed

2 files changed

+37
-10
lines changed

lib/change_detection/dirty_checking_change_detector.dart

+22-10
Original file line numberDiff line numberDiff line change
@@ -1343,7 +1343,9 @@ class _DuplicateItemRecordList {
13431343
record._prevDup = null;
13441344
} else {
13451345
// adding a duplicate [ItemRecord] to the list
1346-
assert(record.item == _head.item);
1346+
assert(identical(record.item, _head.item) ||
1347+
record.item is String && _head.item is String && record.item == _head.item ||
1348+
record.item is num && record.item.isNaN && _head.item is num && _head.item.isNan);
13471349
if (insertBefore == null) {
13481350
_tail._nextDup = record;
13491351
record._prevDup = _tail;
@@ -1369,9 +1371,12 @@ class _DuplicateItemRecordList {
13691371
ItemRecord get(item, int afterIndex) {
13701372
ItemRecord record;
13711373
for (record = _head; record != null; record = record._nextDup) {
1372-
if ((afterIndex == null || afterIndex < record.currentIndex) &&
1373-
identical(record.item, item)) {
1374-
return record;
1374+
if (afterIndex == null || afterIndex < record.currentIndex) {
1375+
if (identical(record.item, item) ||
1376+
record.item is String && item is String && record.item == item ||
1377+
record.item is num && record.item.isNaN && item is num && item.isNan) {
1378+
return record;
1379+
}
13751380
}
13761381
}
13771382
return null;
@@ -1414,10 +1419,12 @@ class _DuplicateItemRecordList {
14141419
* The list of duplicates is implemented by [_DuplicateItemRecordList].
14151420
*/
14161421
class DuplicateMap {
1422+
static final nanKey = const Object();
14171423
final map = new HashMap<dynamic, _DuplicateItemRecordList>();
14181424

14191425
void put(ItemRecord record, [ItemRecord insertBefore = null]) {
1420-
map.putIfAbsent(record.item, () => new _DuplicateItemRecordList()).add(record, insertBefore);
1426+
var key = _getKey(record.item);
1427+
map.putIfAbsent(key, () => new _DuplicateItemRecordList()).add(record, insertBefore);
14211428
}
14221429

14231430
/**
@@ -1427,9 +1434,10 @@ class DuplicateMap {
14271434
* Use case: `[a, b, c, a, a]` if we are at index `3` which is the second `a` then asking if we
14281435
* have any more `a`s needs to return the last `a` not the first or second.
14291436
*/
1430-
ItemRecord get(key, [int afterIndex]) {
1437+
ItemRecord get(value, [int afterIndex]) {
1438+
var key = _getKey(value);
14311439
_DuplicateItemRecordList recordList = map[key];
1432-
return recordList == null ? null : recordList.get(key, afterIndex);
1440+
return recordList == null ? null : recordList.get(value, afterIndex);
14331441
}
14341442

14351443
/**
@@ -1438,10 +1446,11 @@ class DuplicateMap {
14381446
* The list of duplicates also is removed from the map if it gets empty.
14391447
*/
14401448
ItemRecord remove(ItemRecord record) {
1441-
assert(map.containsKey(record.item));
1442-
_DuplicateItemRecordList recordList = map[record.item];
1449+
var key = _getKey(record.item);
1450+
assert(map.containsKey(key));
1451+
_DuplicateItemRecordList recordList = map[key];
14431452
// Remove the list of duplicates when it gets empty
1444-
if (recordList.remove(record)) map.remove(record.item);
1453+
if (recordList.remove(record)) map.remove(key);
14451454
return record;
14461455
}
14471456

@@ -1451,5 +1460,8 @@ class DuplicateMap {
14511460
map.clear();
14521461
}
14531462

1463+
/// Required to handle num.NAN as a Map value
1464+
dynamic _getKey(value) => value is num && value.isNaN ? nanKey : value;
1465+
14541466
String toString() => "DuplicateMap($map)";
14551467
}

test/change_detection/dirty_checking_change_detector_spec.dart

+15
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,21 @@ void testWithGetterFactory(FieldGetterFactory getterFactory) {
390390
expect(detector.collectChanges().moveNext()).toEqual(false);
391391
});
392392

393+
iit('should detect [NaN] moves', () {
394+
var list = [double.NAN, double.NAN];
395+
detector..watch(list, null, null)..collectChanges();
396+
397+
list..clear()..addAll(['foo', double.NAN, double.NAN]);
398+
var iterator = detector.collectChanges();
399+
expect(iterator.moveNext()).toEqual(true);
400+
expect(iterator.current.currentValue, toEqualCollectionRecord(
401+
collection: ['foo[null -> 0]', 'NaN[0 -> 1]', 'NaN[1 -> 2]'],
402+
previous: ['NaN[0 -> 1]', 'NaN[1 -> 2]'],
403+
additions: ['foo[null -> 0]'],
404+
moves: ['NaN[0 -> 1]', 'NaN[1 -> 2]'],
405+
removals: []));
406+
});
407+
393408
it('should remove and add same item', () {
394409
var list = ['a', 'b', 'c'];
395410
var record = detector.watch(list, null, 'handler');

0 commit comments

Comments
 (0)