Skip to content

Commit b2a0be8

Browse files
author
Tim Blasi
committed
fix(change_detect): Sort DirectiveMetadata properties during processing
The Angular 2 render compiler can get out of sync between its transformer execution and its runtime execution, leading to incorrect change detectors with out-of-order property values. Stable sorting solves this problem (temporarily).
1 parent 4c8ea12 commit b2a0be8

File tree

6 files changed

+33
-9
lines changed

6 files changed

+33
-9
lines changed

modules/angular2/src/facade/collection.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ class MapWrapper {
4242
static forEach(Map m, fn(v, k)) {
4343
m.forEach((k, v) => fn(v, k));
4444
}
45+
static get(Map map, key) => map[key];
4546
static int size(Map m) => m.length;
4647
static void delete(Map m, k) {
4748
m.remove(k);

modules/angular2/src/facade/collection.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ export class MapWrapper {
6464
}
6565
static createFromPairs(pairs: List<any>): Map<any, any> { return createMapFromPairs(pairs); }
6666
static forEach<K, V>(m: Map<K, V>, fn: /*(V, K) => void*/ Function) { m.forEach(<any>fn); }
67+
static get<K, V>(map: Map<K, V>, key: K): V { return map.get(key); }
6768
static size(m: Map<any, any>): number { return m.size; }
6869
static delete<K>(m: Map<K, any>, k: K) { m.delete(k); }
6970
static clearValues(m: Map<any, any>) { _clearValues(m); }

modules/angular2/src/facade/lang.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ class StringWrapper {
9797
static bool contains(String s, String substr) {
9898
return s.contains(substr);
9999
}
100+
101+
static int compare(String a, String b) => a.compareTo(b);
100102
}
101103

102104
class StringJoiner {

modules/angular2/src/facade/lang.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,16 @@ export class StringWrapper {
158158
}
159159

160160
static contains(s: string, substr: string): boolean { return s.indexOf(substr) != -1; }
161+
162+
static compare(a: string, b: string): int {
163+
if (a < b) {
164+
return -1;
165+
} else if (a > b) {
166+
return 1;
167+
} else {
168+
return 0;
169+
}
170+
}
161171
}
162172

163173
export class StringJoiner {

modules/angular2/src/render/dom/compiler/directive_parser.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,17 +76,17 @@ export class DirectiveParser implements CompileStep {
7676
});
7777
}
7878
if (isPresent(dirMetadata.hostListeners)) {
79-
MapWrapper.forEach(dirMetadata.hostListeners, (action, eventName) => {
79+
this._sortedKeysForEach(dirMetadata.hostListeners, (action, eventName) => {
8080
this._bindDirectiveEvent(eventName, action, current, directiveBinderBuilder);
8181
});
8282
}
8383
if (isPresent(dirMetadata.hostProperties)) {
84-
MapWrapper.forEach(dirMetadata.hostProperties, (expression, hostPropertyName) => {
84+
this._sortedKeysForEach(dirMetadata.hostProperties, (expression, hostPropertyName) => {
8585
this._bindHostProperty(hostPropertyName, expression, current, directiveBinderBuilder);
8686
});
8787
}
8888
if (isPresent(dirMetadata.hostAttributes)) {
89-
MapWrapper.forEach(dirMetadata.hostAttributes, (hostAttrValue, hostAttrName) => {
89+
this._sortedKeysForEach(dirMetadata.hostAttributes, (hostAttrValue, hostAttrName) => {
9090
this._addHostAttribute(hostAttrName, hostAttrValue, current);
9191
});
9292
}
@@ -97,6 +97,16 @@ export class DirectiveParser implements CompileStep {
9797
});
9898
}
9999

100+
_sortedKeysForEach(map: Map<string, string>, fn: (value: string, key: string) => void): void {
101+
var keys = MapWrapper.keys(map);
102+
ListWrapper.sort(keys, (a, b) => {
103+
// Ensure a stable sort.
104+
var compareVal = StringWrapper.compare(a, b);
105+
return compareVal == 0 ? -1 : compareVal;
106+
});
107+
ListWrapper.forEach(keys, (key) => { fn(MapWrapper.get(map, key), key); });
108+
}
109+
100110
_ensureHasOnlyOneComponent(elementBinder: ElementBinderBuilder, elDescription: string): void {
101111
if (isPresent(elementBinder.componentId)) {
102112
throw new BaseException(

modules/angular2/test/forms/integration_spec.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -645,13 +645,13 @@ export function main() {
645645

646646
var input = rootTC.query(By.css("input")).nativeElement;
647647
expect(DOM.classList(input))
648-
.toEqual(['ng-binding', 'ng-untouched', 'ng-pristine', 'ng-invalid']);
648+
.toEqual(['ng-binding', 'ng-invalid', 'ng-pristine', 'ng-untouched']);
649649

650650
dispatchEvent(input, "blur");
651651
rootTC.detectChanges();
652652

653653
expect(DOM.classList(input))
654-
.toEqual(["ng-binding", "ng-pristine", "ng-invalid", "ng-touched"]);
654+
.toEqual(["ng-binding", "ng-invalid", "ng-pristine", "ng-touched"]);
655655

656656
input.value = "updatedValue";
657657
dispatchEvent(input, "change");
@@ -675,13 +675,13 @@ export function main() {
675675

676676
var input = rootTC.query(By.css("input")).nativeElement;
677677
expect(DOM.classList(input))
678-
.toEqual(["ng-binding", "ng-untouched", "ng-pristine", "ng-invalid"]);
678+
.toEqual(["ng-binding", "ng-invalid", "ng-pristine", "ng-untouched"]);
679679

680680
dispatchEvent(input, "blur");
681681
rootTC.detectChanges();
682682

683683
expect(DOM.classList(input))
684-
.toEqual(["ng-binding", "ng-pristine", "ng-invalid", "ng-touched"]);
684+
.toEqual(["ng-binding", "ng-invalid", "ng-pristine", "ng-touched"]);
685685

686686
input.value = "updatedValue";
687687
dispatchEvent(input, "change");
@@ -703,13 +703,13 @@ export function main() {
703703

704704
var input = rootTC.query(By.css("input")).nativeElement;
705705
expect(DOM.classList(input))
706-
.toEqual(["ng-binding", "ng-untouched", "ng-pristine", "ng-invalid"]);
706+
.toEqual(["ng-binding", "ng-invalid", "ng-pristine", "ng-untouched"]);
707707

708708
dispatchEvent(input, "blur");
709709
rootTC.detectChanges();
710710

711711
expect(DOM.classList(input))
712-
.toEqual(["ng-binding", "ng-pristine", "ng-invalid", "ng-touched"]);
712+
.toEqual(["ng-binding", "ng-invalid", "ng-pristine", "ng-touched"]);
713713

714714
input.value = "updatedValue";
715715
dispatchEvent(input, "change");

0 commit comments

Comments
 (0)