diff --git a/lib/change_detection/dirty_checking_change_detector.dart b/lib/change_detection/dirty_checking_change_detector.dart index d59ebb7e3..666f9daa7 100644 --- a/lib/change_detection/dirty_checking_change_detector.dart +++ b/lib/change_detection/dirty_checking_change_detector.dart @@ -514,19 +514,10 @@ class DirtyCheckingRecord implements Record, WatchRecord { } var last = currentValue; - if (!identical(last, current)) { - if (last is String && current is String && - last == current) { - // This is false change in strings we need to recover, and pretend it - // is the same. We save the value so that next time identity will pass - currentValue = current; - } else if (last is num && last.isNaN && current is num && current.isNaN) { - // we need this for the compiled JavaScript since in JS NaN !== NaN. - } else { - previousValue = last; - currentValue = current; - return true; - } + if (!_looseIdentical(last, current)) { + previousValue = currentValue; + currentValue = current; + return true; } return false; } @@ -620,14 +611,10 @@ class _MapChangeRecord implements MapChangeRecord { var newSeqRecord; if (oldSeqRecord != null && key == oldSeqRecord.key) { newSeqRecord = oldSeqRecord; - if (!identical(value, oldSeqRecord._currentValue)) { + if (!_looseIdentical(value, oldSeqRecord._currentValue)) { var prev = oldSeqRecord._previousValue = oldSeqRecord._currentValue; oldSeqRecord._currentValue = value; - if (!((value is String && prev is String && value == prev) || - (value is num && value.isNaN && prev is num && prev.isNaN))) { - // Check string by value rather than reference - _addToChanges(oldSeqRecord); - } + _addToChanges(oldSeqRecord); } } else { seqChanged = true; @@ -955,7 +942,7 @@ class _CollectionChangeRecord implements CollectionChangeRecord { _length = list.length; for (int index = 0; index < _length; index++) { var item = list[index]; - if (record == null || !identical(item, record.item)) { + if (record == null || !_looseIdentical(record.item, item)) { record = mismatch(record, item, index); maybeDirty = true; } else if (maybeDirty) { @@ -967,7 +954,7 @@ class _CollectionChangeRecord implements CollectionChangeRecord { } else { int index = 0; for (var item in collection) { - if (record == null || !identical(item, record.item)) { + if (record == null || !_looseIdentical(record.item, item)) { record = mismatch(record, item, index); maybeDirty = true; } else if (maybeDirty) { @@ -1037,19 +1024,6 @@ class _CollectionChangeRecord implements CollectionChangeRecord { * - [index] is the position of the item in the collection */ ItemRecord mismatch(ItemRecord record, item, int index) { - if (record != null) { - if (item is String && record.item is String && record.item == item) { - // this is false change in strings we need to recover, and pretend it is - // the same. We save the value so that next time identity can pass - return record..item = item; - } - - if (item is num && (item as num).isNaN && record.item is num && (record.item as num).isNaN) { - // we need this for JavaScript since in JS NaN !== NaN. - return record; - } - } - // The previous record after which we will append the current one. ItemRecord previousRecord; @@ -1343,7 +1317,8 @@ class _DuplicateItemRecordList { record._prevDup = null; } else { // adding a duplicate [ItemRecord] to the list - assert(record.item == _head.item); + assert(record.item == _head.item || + record.item is num && record.item.isNaN && _head.item is num && _head.item.isNaN); if (insertBefore == null) { _tail._nextDup = record; record._prevDup = _tail; @@ -1370,10 +1345,10 @@ class _DuplicateItemRecordList { ItemRecord record; for (record = _head; record != null; record = record._nextDup) { if ((afterIndex == null || afterIndex < record.currentIndex) && - identical(record.item, item)) { - return record; + _looseIdentical(record.item, item)) { + return record; + } } - } return null; } @@ -1414,10 +1389,12 @@ class _DuplicateItemRecordList { * The list of duplicates is implemented by [_DuplicateItemRecordList]. */ class DuplicateMap { + static final nanKey = const Object(); final map = new HashMap(); void put(ItemRecord record, [ItemRecord insertBefore = null]) { - map.putIfAbsent(record.item, () => new _DuplicateItemRecordList()).add(record, insertBefore); + var key = _getKey(record.item); + map.putIfAbsent(key, () => new _DuplicateItemRecordList()).add(record, insertBefore); } /** @@ -1427,9 +1404,10 @@ class DuplicateMap { * Use case: `[a, b, c, a, a]` if we are at index `3` which is the second `a` then asking if we * have any more `a`s needs to return the last `a` not the first or second. */ - ItemRecord get(key, [int afterIndex]) { + ItemRecord get(value, [int afterIndex]) { + var key = _getKey(value); _DuplicateItemRecordList recordList = map[key]; - return recordList == null ? null : recordList.get(key, afterIndex); + return recordList == null ? null : recordList.get(value, afterIndex); } /** @@ -1438,10 +1416,11 @@ class DuplicateMap { * The list of duplicates also is removed from the map if it gets empty. */ ItemRecord remove(ItemRecord record) { - assert(map.containsKey(record.item)); - _DuplicateItemRecordList recordList = map[record.item]; + var key = _getKey(record.item); + assert(map.containsKey(key)); + _DuplicateItemRecordList recordList = map[key]; // Remove the list of duplicates when it gets empty - if (recordList.remove(record)) map.remove(record.item); + if (recordList.remove(record)) map.remove(key); return record; } @@ -1451,5 +1430,32 @@ class DuplicateMap { map.clear(); } + /// Required to handle num.NAN as a Map value + dynamic _getKey(value) => value is num && value.isNaN ? nanKey : value; + String toString() => "DuplicateMap($map)"; } + +/** + * Returns whether the [dst] and [src] are loosely identical: + * * true when the value are identical, + * * true when both values are equal strings, + * * true when both values are NaN + * + * If both values are equal string, src is assigned to dst. + */ +bool _looseIdentical(dst, src) { + if (identical(dst, src)) return true; + + if (dst is String && src is String && dst == src) { + // this is false change in strings we need to recover, and pretend it is the same. We save the + // value so that next time identity can pass + dst = src; + return true; + } + + // we need this for JavaScript since in JS NaN !== NaN. + if (dst is num && (dst as num).isNaN && src is num && (src as num).isNaN) return true; + + return false; +} diff --git a/test/change_detection/dirty_checking_change_detector_spec.dart b/test/change_detection/dirty_checking_change_detector_spec.dart index e8b3fb285..656eea566 100644 --- a/test/change_detection/dirty_checking_change_detector_spec.dart +++ b/test/change_detection/dirty_checking_change_detector_spec.dart @@ -390,6 +390,21 @@ void testWithGetterFactory(FieldGetterFactory getterFactory) { expect(detector.collectChanges().moveNext()).toEqual(false); }); + it('should detect [NaN] moves', () { + var list = [double.NAN, double.NAN]; + detector..watch(list, null, null)..collectChanges(); + + list..clear()..addAll(['foo', double.NAN, double.NAN]); + var iterator = detector.collectChanges(); + expect(iterator.moveNext()).toEqual(true); + expect(iterator.current.currentValue, toEqualCollectionRecord( + collection: ['foo[null -> 0]', 'NaN[0 -> 1]', 'NaN[1 -> 2]'], + previous: ['NaN[0 -> 1]', 'NaN[1 -> 2]'], + additions: ['foo[null -> 0]'], + moves: ['NaN[0 -> 1]', 'NaN[1 -> 2]'], + removals: [])); + }); + it('should remove and add same item', () { var list = ['a', 'b', 'c']; var record = detector.watch(list, null, 'handler');