Skip to content

Commit ad9cf02

Browse files
mvuksanojbdeboer
authored andcommitted
feat(ChangeDetector): When watching a methods colorization should not show up as changes
Closes dart-archive#934 fix(DCCD): when watching variables, always fire on first digest
1 parent b97df74 commit ad9cf02

File tree

4 files changed

+132
-10
lines changed

4 files changed

+132
-10
lines changed

lib/change_detection/dirty_checking_change_detector.dart

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -460,8 +460,14 @@ class DirtyCheckingRecord<H> implements Record<H>, WatchRecord<H> {
460460
_mode = _MODE_MAP_FIELD_;
461461
_getter = null;
462462
} else {
463-
_mode = _MODE_GETTER_;
464-
_getter = _fieldGetterFactory.getter(obj, field);
463+
if (_fieldGetterFactory.isMethod(obj, field)) {
464+
_mode = _MODE_IDENTITY_;
465+
previousValue = currentValue = _fieldGetterFactory.method(obj, field)(obj);
466+
assert(previousValue is Function);
467+
} else {
468+
_mode = _MODE_GETTER_;
469+
_getter = _fieldGetterFactory.getter(obj, field);
470+
}
465471
}
466472
}
467473

lib/change_detection/dirty_checking_change_detector_dynamic.dart

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,11 @@ import 'dart:mirrors';
1313
class DynamicFieldGetterFactory implements FieldGetterFactory {
1414
final isMethodInvoke = true;
1515

16-
bool isMethod(Object object, String name) {
17-
try {
18-
return method(object, name) != null;
19-
} catch (e, s) {
20-
return false;
21-
}
16+
isMethod(Object object, String name) {
17+
var declaration = reflectClass(object.runtimeType).declarations[new Symbol(name)];
18+
return declaration != null &&
19+
declaration is MethodMirror &&
20+
(declaration as MethodMirror).isRegularMethod;
2221
}
2322

2423
Function method(Object object, String name) {

test/change_detection/dirty_checking_change_detector_spec.dart

Lines changed: 109 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import '../_specs.dart';
44
import 'package:angular/change_detection/change_detection.dart';
55
import 'package:angular/change_detection/dirty_checking_change_detector.dart';
66
import 'package:angular/change_detection/dirty_checking_change_detector_static.dart';
7+
import 'package:angular/change_detection/dirty_checking_change_detector_dynamic.dart';
78
import 'dart:collection';
89
import 'dart:math';
910

@@ -14,19 +15,80 @@ void main() {
1415
"first": (o) => o.first,
1516
"age": (o) => o.age,
1617
"last": (o) => o.last,
17-
"toString": (o) => o.toString
18+
"toString": (o) => o.toString,
19+
"isUnderAge": (o) => o.isUnderAge,
20+
"isUnderAgeAsVariable": (o) => o.isUnderAgeAsVariable
1821
});
1922

2023
beforeEach(() {
2124
detector = new DirtyCheckingChangeDetector<String>(getterFactory);
2225
});
2326

2427
describe('StaticFieldGetterFactory', () {
28+
DirtyCheckingChangeDetector<String> detector;
29+
var user = new _User('Marko', 'Vuksanovic', 30);
30+
FieldGetterFactory getterFactory = new StaticFieldGetterFactory({
31+
"first": (o) => o.first,
32+
"age": (o) => o.age,
33+
"last": (o) => o.last,
34+
"toString": (o) => o.toString,
35+
"isUnderAge": (o) => o.isUnderAge,
36+
"isUnderAgeAsVariable": (o) => o.isUnderAgeAsVariable,
37+
"list": (o) => o.list,
38+
"map": (o) => o.map
39+
});
40+
41+
beforeEach(() {
42+
detector = new DirtyCheckingChangeDetector<String>(getterFactory);
43+
});
44+
2545
it('should detect methods', () {
2646
var obj = new _User();
2747
expect(getterFactory.isMethod(obj, 'toString')).toEqual(true);
2848
expect(getterFactory.isMethod(obj, 'age')).toEqual(false);
2949
});
50+
51+
it('should return true is method is real method', () {
52+
expect(getterFactory.isMethod(user, 'isUnderAge')).toEqual(true);
53+
});
54+
55+
it('should return false is field is a function', () {
56+
expect(getterFactory.isMethod(user, 'isUnderAgeAsVariable')).toEqual(false);
57+
});
58+
59+
it('should return false is field is a list', () {
60+
expect(getterFactory.isMethod(user, 'list')).toEqual(false);
61+
});
62+
63+
it('should return false is field is a map', () {
64+
expect(getterFactory.isMethod(user, 'map')).toEqual(false);
65+
});
66+
});
67+
68+
describe('Dynamic GetterFactory', () {
69+
DirtyCheckingChangeDetector<String> detector;
70+
var user = new _User('Marko', 'Vuksanovic', 30);
71+
FieldGetterFactory getterFactory = new DynamicFieldGetterFactory();
72+
73+
beforeEach(() {
74+
detector = new DirtyCheckingChangeDetector<String>(getterFactory);
75+
});
76+
77+
it('should return true is method is real method', () {
78+
expect(getterFactory.isMethod(user, 'isUnderAge')).toEqual(true);
79+
});
80+
81+
it('should return false is field is a function', () {
82+
expect(getterFactory.isMethod(user, 'isUnderAgeAsVariable')).toEqual(false);
83+
});
84+
85+
it('should return false is field is a list', () {
86+
expect(getterFactory.isMethod(user, 'list')).toEqual(false);
87+
});
88+
89+
it('should return false is field is a map', () {
90+
expect(getterFactory.isMethod(user, 'map')).toEqual(false);
91+
});
3092
});
3193

3294
describe('object field', () {
@@ -657,6 +719,42 @@ void main() {
657719
});
658720
});
659721

722+
describe('function watching', () {
723+
it('should detect no changes when watching a function', () {
724+
var user = new _User('marko', 'vuksanovic', 15);
725+
Iterator changeIterator;
726+
727+
detector..watch(user, 'isUnderAge', null);
728+
changeIterator = detector.collectChanges();
729+
expect(changeIterator.moveNext()).toEqual(true);
730+
expect(changeIterator.moveNext()).toEqual(false);
731+
732+
user.age = 17;
733+
changeIterator = detector.collectChanges();
734+
expect(changeIterator.moveNext()).toEqual(false);
735+
736+
user.age = 30;
737+
changeIterator = detector.collectChanges();
738+
expect(changeIterator.moveNext()).toEqual(false);
739+
});
740+
741+
it('should detect change when watching a property function', () {
742+
var user = new _User('marko', 'vuksanovic', 30);
743+
Iterator changeIterator;
744+
745+
detector..watch(user, 'isUnderAgeAsVariable', null);
746+
changeIterator = detector.collectChanges();
747+
expect(changeIterator.moveNext()).toEqual(true);
748+
749+
changeIterator = detector.collectChanges();
750+
expect(changeIterator.moveNext()).toEqual(false);
751+
752+
user.isUnderAgeAsVariable = () => false;
753+
changeIterator = detector.collectChanges();
754+
expect(changeIterator.moveNext()).toEqual(true);
755+
});
756+
});
757+
660758
describe('DuplicateMap', () {
661759
DuplicateMap map;
662760
beforeEach(() => map = new DuplicateMap());
@@ -693,8 +791,17 @@ class _User {
693791
String first;
694792
String last;
695793
num age;
794+
var isUnderAgeAsVariable;
795+
List<String> list = ['foo', 'bar', 'baz'];
796+
Map map = {'foo': 'bar', 'baz': 'cux'};
696797

697-
_User([this.first, this.last, this.age]);
798+
_User([this.first, this.last, this.age]) {
799+
isUnderAgeAsVariable = isUnderAge;
800+
}
801+
802+
bool isUnderAge() {
803+
return age != null ? age < 18 : false;
804+
}
698805
}
699806

700807
Matcher toEqualCollectionRecord({collection, previous, additions, moves, removals}) =>

test/directive/ng_repeat_spec.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,16 @@ main() {
116116
expect(element.querySelectorAll('span').length).toEqual(2);
117117
});
118118

119+
it('should support function as a formatter', () {
120+
scope.context['isEven'] = (num) => num % 2 == 0;
121+
var element = compile(
122+
'<div ng-show="true">'
123+
'<span ng-repeat="r in [1, 2] | filter:isEven">{{r}}</span>'
124+
'</div>');
125+
scope.apply();
126+
expect(element.text).toEqual('2');
127+
});
128+
119129

120130
describe('track by', () {
121131
it(r'should track using expression function', () {

0 commit comments

Comments
 (0)