Skip to content

Commit 8357961

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

File tree

2 files changed

+36
-10
lines changed

2 files changed

+36
-10
lines changed

lib/change_detection/dirty_checking_change_detector.dart

+21-10
Original file line numberDiff line numberDiff line change
@@ -1343,7 +1343,8 @@ class _DuplicateItemRecordList {
13431343
record._prevDup = null;
13441344
} else {
13451345
// adding a duplicate [ItemRecord] to the list
1346-
assert(record.item == _head.item);
1346+
assert(record.item == _head.item ||
1347+
record.item is num && record.item.isNaN && _head.item is num && _head.item.isNaN);
13471348
if (insertBefore == null) {
13481349
_tail._nextDup = record;
13491350
record._prevDup = _tail;
@@ -1369,9 +1370,12 @@ class _DuplicateItemRecordList {
13691370
ItemRecord get(item, int afterIndex) {
13701371
ItemRecord record;
13711372
for (record = _head; record != null; record = record._nextDup) {
1372-
if ((afterIndex == null || afterIndex < record.currentIndex) &&
1373-
identical(record.item, item)) {
1374-
return record;
1373+
if (afterIndex == null || afterIndex < record.currentIndex) {
1374+
if (identical(record.item, item) ||
1375+
record.item is String && item is String && record.item == item ||
1376+
record.item is num && record.item.isNaN && item is num && item.isNaN) {
1377+
return record;
1378+
}
13751379
}
13761380
}
13771381
return null;
@@ -1414,10 +1418,12 @@ class _DuplicateItemRecordList {
14141418
* The list of duplicates is implemented by [_DuplicateItemRecordList].
14151419
*/
14161420
class DuplicateMap {
1421+
static final nanKey = const Object();
14171422
final map = new HashMap<dynamic, _DuplicateItemRecordList>();
14181423

14191424
void put(ItemRecord record, [ItemRecord insertBefore = null]) {
1420-
map.putIfAbsent(record.item, () => new _DuplicateItemRecordList()).add(record, insertBefore);
1425+
var key = _getKey(record.item);
1426+
map.putIfAbsent(key, () => new _DuplicateItemRecordList()).add(record, insertBefore);
14211427
}
14221428

14231429
/**
@@ -1427,9 +1433,10 @@ class DuplicateMap {
14271433
* Use case: `[a, b, c, a, a]` if we are at index `3` which is the second `a` then asking if we
14281434
* have any more `a`s needs to return the last `a` not the first or second.
14291435
*/
1430-
ItemRecord get(key, [int afterIndex]) {
1436+
ItemRecord get(value, [int afterIndex]) {
1437+
var key = _getKey(value);
14311438
_DuplicateItemRecordList recordList = map[key];
1432-
return recordList == null ? null : recordList.get(key, afterIndex);
1439+
return recordList == null ? null : recordList.get(value, afterIndex);
14331440
}
14341441

14351442
/**
@@ -1438,10 +1445,11 @@ class DuplicateMap {
14381445
* The list of duplicates also is removed from the map if it gets empty.
14391446
*/
14401447
ItemRecord remove(ItemRecord record) {
1441-
assert(map.containsKey(record.item));
1442-
_DuplicateItemRecordList recordList = map[record.item];
1448+
var key = _getKey(record.item);
1449+
assert(map.containsKey(key));
1450+
_DuplicateItemRecordList recordList = map[key];
14431451
// Remove the list of duplicates when it gets empty
1444-
if (recordList.remove(record)) map.remove(record.item);
1452+
if (recordList.remove(record)) map.remove(key);
14451453
return record;
14461454
}
14471455

@@ -1451,5 +1459,8 @@ class DuplicateMap {
14511459
map.clear();
14521460
}
14531461

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

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+
it('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)