Skip to content

Commit

Permalink
fix(change_detect): Sort DirectiveMetadata properties during proces…
Browse files Browse the repository at this point in the history
…sing

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).
  • Loading branch information
Tim Blasi committed Jul 21, 2015
1 parent 4c8ea12 commit b2a0be8
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 9 deletions.
1 change: 1 addition & 0 deletions modules/angular2/src/facade/collection.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class MapWrapper {
static forEach(Map m, fn(v, k)) {
m.forEach((k, v) => fn(v, k));
}
static get(Map map, key) => map[key];
static int size(Map m) => m.length;
static void delete(Map m, k) {
m.remove(k);
Expand Down
1 change: 1 addition & 0 deletions modules/angular2/src/facade/collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export class MapWrapper {
}
static createFromPairs(pairs: List<any>): Map<any, any> { return createMapFromPairs(pairs); }
static forEach<K, V>(m: Map<K, V>, fn: /*(V, K) => void*/ Function) { m.forEach(<any>fn); }
static get<K, V>(map: Map<K, V>, key: K): V { return map.get(key); }
static size(m: Map<any, any>): number { return m.size; }
static delete<K>(m: Map<K, any>, k: K) { m.delete(k); }
static clearValues(m: Map<any, any>) { _clearValues(m); }
Expand Down
2 changes: 2 additions & 0 deletions modules/angular2/src/facade/lang.dart
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ class StringWrapper {
static bool contains(String s, String substr) {
return s.contains(substr);
}

static int compare(String a, String b) => a.compareTo(b);
}

class StringJoiner {
Expand Down
10 changes: 10 additions & 0 deletions modules/angular2/src/facade/lang.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,16 @@ export class StringWrapper {
}

static contains(s: string, substr: string): boolean { return s.indexOf(substr) != -1; }

static compare(a: string, b: string): int {
if (a < b) {
return -1;
} else if (a > b) {
return 1;
} else {
return 0;
}
}
}

export class StringJoiner {
Expand Down
16 changes: 13 additions & 3 deletions modules/angular2/src/render/dom/compiler/directive_parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,17 @@ export class DirectiveParser implements CompileStep {
});
}
if (isPresent(dirMetadata.hostListeners)) {
MapWrapper.forEach(dirMetadata.hostListeners, (action, eventName) => {
this._sortedKeysForEach(dirMetadata.hostListeners, (action, eventName) => {
this._bindDirectiveEvent(eventName, action, current, directiveBinderBuilder);
});
}
if (isPresent(dirMetadata.hostProperties)) {
MapWrapper.forEach(dirMetadata.hostProperties, (expression, hostPropertyName) => {
this._sortedKeysForEach(dirMetadata.hostProperties, (expression, hostPropertyName) => {
this._bindHostProperty(hostPropertyName, expression, current, directiveBinderBuilder);
});
}
if (isPresent(dirMetadata.hostAttributes)) {
MapWrapper.forEach(dirMetadata.hostAttributes, (hostAttrValue, hostAttrName) => {
this._sortedKeysForEach(dirMetadata.hostAttributes, (hostAttrValue, hostAttrName) => {
this._addHostAttribute(hostAttrName, hostAttrValue, current);
});
}
Expand All @@ -97,6 +97,16 @@ export class DirectiveParser implements CompileStep {
});
}

_sortedKeysForEach(map: Map<string, string>, fn: (value: string, key: string) => void): void {
var keys = MapWrapper.keys(map);
ListWrapper.sort(keys, (a, b) => {
// Ensure a stable sort.
var compareVal = StringWrapper.compare(a, b);
return compareVal == 0 ? -1 : compareVal;
});
ListWrapper.forEach(keys, (key) => { fn(MapWrapper.get(map, key), key); });
}

_ensureHasOnlyOneComponent(elementBinder: ElementBinderBuilder, elDescription: string): void {
if (isPresent(elementBinder.componentId)) {
throw new BaseException(
Expand Down
12 changes: 6 additions & 6 deletions modules/angular2/test/forms/integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -645,13 +645,13 @@ export function main() {

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

dispatchEvent(input, "blur");
rootTC.detectChanges();

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

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

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

dispatchEvent(input, "blur");
rootTC.detectChanges();

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

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

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

dispatchEvent(input, "blur");
rootTC.detectChanges();

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

input.value = "updatedValue";
dispatchEvent(input, "change");
Expand Down

0 comments on commit b2a0be8

Please sign in to comment.