Skip to content

Commit b724642

Browse files
committed
feat: Removing implicit map dereference in expressions
Closes dart-archive#915 1) Now "a.b" return a.b and "a['b']" return a['b'] Before this change a.b would have match either a.b or a['b']. The reason is that the context was a Map (before dart-archive#914) and we don't want to write ctrl['foo'] but ctrl.foo. It was also not possible to access Map properties, ie map.length would have returned map['length']. 2) Accessing probes The probes are no more accessible directly in the rootScope context but in a $probes map on the rootScope context. Before: <p probe="prb"></p> var probe = rootScope.context['prb']; After: <p probe="prb"></p> var probe = rootScope.context.$probes['prb']; Conflicts: lib/change_detection/context_locals.dart
1 parent b6d56a3 commit b724642

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+1687
-1809
lines changed

benchmark/watch_group_perf.dart

-34
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ var _dynamicFieldGetterFactory = new DynamicFieldGetterFactory();
2727
main() {
2828
_fieldRead();
2929
_fieldReadGetter();
30-
_mapRead();
3130
_methodInvoke0();
3231
_methodInvoke1();
3332
_function2();
@@ -107,39 +106,6 @@ _fieldReadGetter() {
107106
time('fieldReadGetter', () => watchGrp.detectChanges());
108107
}
109108

110-
_mapRead() {
111-
var map = {
112-
'a': 0, 'b': 1, 'c': 2, 'd': 3, 'e': 4,
113-
'f': 0, 'g': 1, 'h': 2, 'i': 3, 'j': 4,
114-
'k': 0, 'l': 1, 'm': 2, 'n': 3, 'o': 4,
115-
'p': 0, 'q': 1, 'r': 2, 's': 3, 't': 4};
116-
var watchGrp = new RootWatchGroup(_dynamicFieldGetterFactory,
117-
new DirtyCheckingChangeDetector(_dynamicFieldGetterFactory), map)
118-
..watch(_parse('a'), _reactionFn)
119-
..watch(_parse('b'), _reactionFn)
120-
..watch(_parse('c'), _reactionFn)
121-
..watch(_parse('d'), _reactionFn)
122-
..watch(_parse('e'), _reactionFn)
123-
..watch(_parse('f'), _reactionFn)
124-
..watch(_parse('g'), _reactionFn)
125-
..watch(_parse('h'), _reactionFn)
126-
..watch(_parse('i'), _reactionFn)
127-
..watch(_parse('j'), _reactionFn)
128-
..watch(_parse('k'), _reactionFn)
129-
..watch(_parse('l'), _reactionFn)
130-
..watch(_parse('m'), _reactionFn)
131-
..watch(_parse('n'), _reactionFn)
132-
..watch(_parse('o'), _reactionFn)
133-
..watch(_parse('p'), _reactionFn)
134-
..watch(_parse('q'), _reactionFn)
135-
..watch(_parse('r'), _reactionFn)
136-
..watch(_parse('s'), _reactionFn)
137-
..watch(_parse('t'), _reactionFn);
138-
139-
print('Watch: ${watchGrp.fieldCost}; eval: ${watchGrp.evalCost}');
140-
time('mapRead', () => watchGrp.detectChanges());
141-
}
142-
143109
_methodInvoke0() {
144110
var context = new _Obj();
145111
context.a = new _Obj();

lib/change_detection/dirty_checking_change_detector.dart

+9-14
Original file line numberDiff line numberDiff line change
@@ -467,22 +467,17 @@ class DirtyCheckingRecord<H> implements WatchRecord<H> {
467467
return;
468468
}
469469

470-
if (object is Map) {
471-
_mode = _MODE_MAP_FIELD_;
472-
_getter = null;
473-
} else {
474-
while (object is ContextLocals) {
475-
var ctx = object as ContextLocals;
476-
if (ctx.hasProperty(field)) {
477-
_mode = _MODE_MAP_FIELD_;
478-
_getter = null;
479-
return;
480-
}
481-
object = ctx.parentContext;
470+
while (object is ContextLocals) {
471+
var ctx = object as ContextLocals;
472+
if (ctx.hasProperty(field)) {
473+
_mode = _MODE_MAP_FIELD_;
474+
_getter = null;
475+
return;
482476
}
483-
_mode = _MODE_GETTER_OR_METHOD_CLOSURE_;
484-
_getter = _fieldGetterFactory.getter(object, field);
477+
object = ctx.parentContext;
485478
}
479+
_mode = _MODE_GETTER_OR_METHOD_CLOSURE_;
480+
_getter = _fieldGetterFactory.getter(object, field);
486481
}
487482

488483
bool check() {

lib/core/parser/eval_access.dart

-4
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,13 @@ class AccessKeyed extends syntax.AccessKeyed {
3838
* where we have a pair of pre-compiled getter and setter functions that we
3939
* use to do the access the field.
4040
*/
41-
// todo(vicb) - parser should not depend on ContextLocals
42-
// todo(vicb) - Map should not be a special case so that we can access the props
4341
abstract class AccessFast {
4442
String get name;
4543
Getter get getter;
4644
Setter get setter;
4745

4846
dynamic _eval(holder) {
4947
if (holder == null) return null;
50-
if (holder is Map) return holder[name];
5148
return getter(holder);
5249
}
5350

@@ -56,7 +53,6 @@ abstract class AccessFast {
5653
_assignToNonExisting(scope, value);
5754
return value;
5855
} else {
59-
if (holder is Map) return holder[name] = value;
6056
return setter(holder, value);
6157
}
6258
}

lib/core/parser/parser_dynamic.dart

+7-26
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,14 @@ class DynamicClosureMap implements ClosureMap {
88
final Map<String, Symbol> symbols = {};
99
Getter lookupGetter(String name) {
1010
var symbol = new Symbol(name);
11-
return (o) {
12-
if (o is Map) {
13-
return o[name];
14-
} else {
15-
return reflect(o).getField(symbol).reflectee;
16-
}
17-
};
11+
return (o) => reflect(o).getField(symbol).reflectee;
1812
}
1913

2014
Setter lookupSetter(String name) {
2115
var symbol = new Symbol(name);
2216
return (o, value) {
23-
if (o is Map) {
24-
return o[name] = value;
25-
} else {
26-
reflect(o).setField(symbol, value);
27-
return value;
28-
}
17+
reflect(o).setField(symbol, value);
18+
return value;
2919
};
3020
}
3121

@@ -37,19 +27,10 @@ class DynamicClosureMap implements ClosureMap {
3727
var symbol = symbols.putIfAbsent(name, () => new Symbol(name));
3828
sNamedArgs[symbol] = value;
3929
});
40-
if (o is Map) {
41-
var fn = o[name];
42-
if (fn is Function) {
43-
return Function.apply(fn, posArgs, sNamedArgs);
44-
} else {
45-
throw "Property '$name' is not of type function.";
46-
}
47-
} else {
48-
try {
49-
return reflect(o).invoke(symbol, posArgs, sNamedArgs).reflectee;
50-
} on NoSuchMethodError catch (e) {
51-
throw 'Undefined function $name';
52-
}
30+
try {
31+
return reflect(o).invoke(symbol, posArgs, sNamedArgs).reflectee;
32+
} on NoSuchMethodError catch (e) {
33+
throw 'Undefined function $name';
5334
}
5435
};
5536
}

lib/core/parser/parser_static.dart

+1-10
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,7 @@ class StaticClosureMap extends ClosureMap {
2727
return (o, posArgs, namedArgs) {
2828
var sNamedArgs = {};
2929
namedArgs.forEach((name, value) => sNamedArgs[symbols[name]] = value);
30-
if (o is Map) {
31-
var fn = o[name];
32-
if (fn is Function) {
33-
return Function.apply(fn, posArgs, sNamedArgs);
34-
} else {
35-
throw "Property '$name' is not of type function.";
36-
}
37-
} else {
38-
return Function.apply(fn(o), posArgs, sNamedArgs);
39-
}
30+
return Function.apply(fn(o), posArgs, sNamedArgs);
4031
};
4132
}
4233

lib/core/scope.dart

+8-8
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ class ScopeDigestTTL {
9696
* a `scope` setter:
9797
*
9898
* @Component(...)
99-
* class MyComponent {
99+
* class MyComponent implements ScopeAware {
100100
* Watch watch;
101101
*
102102
* MyComponent(Dependency myDep) {
@@ -111,7 +111,7 @@ class ScopeDigestTTL {
111111
* }
112112
*/
113113
abstract class ScopeAware {
114-
void set context(ctx);
114+
void set scope(Scope scope);
115115
}
116116

117117
/**
@@ -128,7 +128,7 @@ class Scope {
128128
/**
129129
* The default execution context for [watch]es [observe]ers, and [eval]uation.
130130
*/
131-
final Object context;
131+
final context;
132132

133133
/**
134134
* The [RootScope] of the application.
@@ -165,20 +165,20 @@ class Scope {
165165
// TODO(misko): WatchGroup should be private.
166166
// Instead we should expose performance stats about the watches
167167
// such as # of watches, checks/1ms, field checks, function checks, etc
168-
WatchGroup _readWriteGroup;
169-
WatchGroup _readOnlyGroup;
168+
final WatchGroup _readWriteGroup;
169+
final WatchGroup _readOnlyGroup;
170170

171171
Scope _childHead, _childTail, _next, _prev;
172172
_Streams _streams;
173173

174174
/// Do not use. Exposes internal state for testing.
175175
bool get hasOwnStreams => _streams != null && _streams._scope == this;
176176

177-
Scope(this.context, this.rootScope, this._parentScope,
177+
Scope(Object this.context, this.rootScope, this._parentScope,
178178
this._readWriteGroup, this._readOnlyGroup, this.id,
179179
this._stats)
180180
{
181-
if (context is ScopeAware) context.scope = this;
181+
if (context is ScopeAware) (context as ScopeAware).scope = this;
182182
}
183183

184184
/**
@@ -636,7 +636,7 @@ class RootScope extends Scope {
636636
_zone.onTurnDone = apply;
637637
_zone.onError = (e, s, ls) => _exceptionHandler(e, s);
638638
_zone.onScheduleMicrotask = runAsync;
639-
cacheRegister.registerCache("ScopeWatchASTs", astCache);
639+
cacheRegister.registerCache("ScopeWatchASTs", astCache);
640640
}
641641

642642
RootScope get rootScope => this;

lib/directive/ng_class.dart

+7-16
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,7 @@ abstract class _NgClassBase {
171171
nodeAttrs.observe('class', (String cls) {
172172
if (prevCls != cls) {
173173
prevCls = cls;
174-
var index = _hasLocal(_scope, r'$index') ? _getLocal(_scope, r'$index') : null;
175-
_applyChanges(index);
174+
_applyChanges(_getLocal(_scope, r'$index', null));
176175
}
177176
});
178177
}
@@ -181,8 +180,7 @@ abstract class _NgClassBase {
181180
if (_watchExpression != null) _watchExpression.remove();
182181
_watchExpression = _scope.watch(expression, (v, _) {
183182
_computeChanges(v);
184-
var index = _hasLocal(_scope, r'$index') ? _getLocal(_scope, r'$index') : null;
185-
_applyChanges(index);
183+
_applyChanges(_getLocal(_scope, r'$index', null));
186184
},
187185
canChangeModel: false,
188186
collection: true);
@@ -279,20 +277,13 @@ abstract class _NgClassBase {
279277
}
280278
}
281279

282-
bool _hasLocal(context, name) {
283-
var ctx = context;
284-
while (ctx is ContextLocals) {
285-
if (ctx.hasProperty(name)) return true;
286-
ctx = ctx.parentScope;
287-
}
288-
return false;
289-
}
290-
291-
dynamic _getLocal(context, name) {
292-
var ctx = context;
280+
/// Returns a value from the [scope] context.
281+
/// Walks up the [ContextLocals] tree and returns the [defaultValue] when the value is not found.
282+
dynamic _getLocal(Scope scope, String name, defaultValue) {
283+
var ctx = scope.context;
293284
while (ctx is ContextLocals) {
294285
if (ctx.hasProperty(name)) return ctx[name];
295286
ctx = ctx.parentScope;
296287
}
297-
return null;
288+
return defaultValue;
298289
}

lib/directive/ng_control.dart

+3-3
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ part of angular.directive;
33
/**
44
* Contains info and error states used during form and input validation.
55
*
6-
* NgControl is a common superclass for forms and input controls that handles info and error states, as well as
7-
* status flags. NgControl is used with the form and fieldset as well as all other directives that are used for
8-
* user input with NgModel.
6+
* [NgControl] is a common superclass for forms and input controls that handles info and error
7+
* states, as well as status flags. NgControl is used with the form and fieldset as well as all
8+
* other directives that are used for user input with NgModel.
99
*/
1010
abstract class NgControl implements AttachAware, DetachAware {
1111
static const NG_VALID = "ng-valid";

lib/directive/ng_form.dart

+2-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ class NgForm extends NgControl {
5757
set name(String value) {
5858
if (value != null) {
5959
super.name = value;
60-
_scope.context[name] = this;
60+
// todo(vicb) find a way to publish this info in the context (ContextLocals)
61+
//_scope.context[name] = this;
6162
}
6263
}
6364

0 commit comments

Comments
 (0)