From 708c237b900ac0f1dde1850717c8aaca1e8c5665 Mon Sep 17 00:00:00 2001 From: Victor Date: Tue, 18 Feb 2014 11:50:05 +0100 Subject: [PATCH 1/2] fix(change-detector): handle double.NAN for collections (in JS) --- .../dirty_checking_change_detector.dart | 16 +++++++++++----- .../dirty_checking_change_detector_spec.dart | 10 ++++++++-- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/lib/change_detection/dirty_checking_change_detector.dart b/lib/change_detection/dirty_checking_change_detector.dart index 24a568341..d04f2bf11 100644 --- a/lib/change_detection/dirty_checking_change_detector.dart +++ b/lib/change_detection/dirty_checking_change_detector.dart @@ -793,11 +793,17 @@ class _CollectionChangeRecord implements CollectionChangeRecord { */ ItemRecord mismatch(ItemRecord record, item, int index) { // Guard against bogus String changes - if (record != null && 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 will pass - return record..item = item; + 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.isNaN && record.item is num && record.item.isNaN){ + // we need this for JavaScript since in JS NaN !== NaN. + return record; + } } // find the previous record so that we know where to insert after. diff --git a/test/change_detection/dirty_checking_change_detector_spec.dart b/test/change_detection/dirty_checking_change_detector_spec.dart index 367a2fd96..7a6c2b0fc 100644 --- a/test/change_detection/dirty_checking_change_detector_spec.dart +++ b/test/change_detection/dirty_checking_change_detector_spec.dart @@ -245,14 +245,20 @@ main() => describe('DirtyCheckingChangeDetector', () { it('should test string by value rather than by reference', () { var list = ['a', 'boo']; - var record = detector.watch(list, null, 'handler'); - detector.collectChanges(); + detector..watch(list, null, null)..collectChanges(); list[1] = 'b' + 'oo'; expect(detector.collectChanges()).toEqual(null); }); + it('should ignore [NaN] != [NaN]', () { + var list = [double.NAN]; + var record = detector..watch(list, null, null)..collectChanges(); + + expect(detector.collectChanges()).toEqual(null); + }); + it('should remove and add same item', () { var list = ['a', 'b', 'c']; var record = detector.watch(list, null, 'handler'); From 29040973cad613c75d4471bd2f6ca707f54303cb Mon Sep 17 00:00:00 2001 From: Victor Date: Tue, 18 Feb 2014 12:10:10 +0100 Subject: [PATCH 2/2] fix(change detector): fix the handling of NaN & string values for maps --- .../dirty_checking_change_detector.dart | 8 ++++-- .../dirty_checking_change_detector_spec.dart | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/change_detection/dirty_checking_change_detector.dart b/lib/change_detection/dirty_checking_change_detector.dart index d04f2bf11..0bada105e 100644 --- a/lib/change_detection/dirty_checking_change_detector.dart +++ b/lib/change_detection/dirty_checking_change_detector.dart @@ -451,9 +451,13 @@ class _MapChangeRecord implements MapChangeRecord { if (oldSeqRecord != null && key == oldSeqRecord.key) { newSeqRecord = oldSeqRecord; if (!identical(value, oldSeqRecord._currentValue)) { - oldSeqRecord._previousValue = oldSeqRecord._currentValue; + var prev = oldSeqRecord._previousValue = oldSeqRecord._currentValue; oldSeqRecord._currentValue = value; - _addToChanges(oldSeqRecord); + 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); + } } } else { seqChanged = true; diff --git a/test/change_detection/dirty_checking_change_detector_spec.dart b/test/change_detection/dirty_checking_change_detector_spec.dart index 7a6c2b0fc..bd244276b 100644 --- a/test/change_detection/dirty_checking_change_detector_spec.dart +++ b/test/change_detection/dirty_checking_change_detector_spec.dart @@ -395,6 +395,31 @@ main() => describe('DirtyCheckingChangeDetector', () { changes: [], removals: ['a[A -> null]', 'd[D -> null]'])); }); + + it('should test string keys by value rather than by reference', () { + var map = {'foo': 0}; + detector..watch(map, null, null)..collectChanges(); + + map['f' + 'oo'] = 0; + + expect(detector.collectChanges()).toEqual(null); + }); + + it('should test string values by value rather than by reference', () { + var map = {'foo': 'bar'}; + detector..watch(map, null, null)..collectChanges(); + + map['foo'] = 'b' + 'ar'; + + expect(detector.collectChanges()).toEqual(null); + }); + + it('should not see a NaN value as a change', () { + var map = {'foo': double.NAN}; + var record = detector..watch(map, null, null)..collectChanges(); + + expect(detector.collectChanges()).toEqual(null); + }); }); describe('DuplicateMap', () {