fix(change_detection): update the right change detector when using ON_PUSH mode

Previously, in a case where you have a mix of ON_PUSH and DEFAULT detectors, Angular would update the status of a wrong detector.

Author: vsavkin

modules/angular2/src/change_detection/abstract_change_detector.ts

@@ -210,6 +210,10 @@ export class AbstractChangeDetector<T> implements ChangeDetector {
  return value;
  }
 
+ protected getDetectorFor(directives: any, index: number): ChangeDetector {
+ return directives.getDetectorFor(this.directiveRecords[index].directiveIndex);
+ }
+
  protected notifyDispatcher(value: any): void {
  this.dispatcher.notifyOnBinding(this._currentBinding(), value);
  }

modules/angular2/src/change_detection/change_detection_jit_generator.ts

@@ -143,7 +143,7 @@ export class ChangeDetectorJITGenerator {
 
  _maybeGenHydrateDirectives(): string {
  var hydrateDirectivesCode = this._genHydrateDirectives();
- var hydrateDetectorsCode = this._genHydrateDetectors();
+ var hydrateDetectorsCode = this._logic.genHydrateDetectors(this.directiveRecords);
  if (!hydrateDirectivesCode && !hydrateDetectorsCode) return '';
  return `${this._typeName}.prototype.hydrateDirectives = function(directives) {
  ${hydrateDirectivesCode}
@@ -161,16 +161,6 @@ export class ChangeDetectorJITGenerator {
  return lines.join('\n');
  }
 
- _genHydrateDetectors(): string {
- var detectorFieldNames = this._names.getAllDetectorNames();
- var lines = ListWrapper.createFixedSize(detectorFieldNames.length);
- for (var i = 0, iLen = detectorFieldNames.length; i < iLen; ++i) {
- lines[i] = `${detectorFieldNames[i]} = directives.getDetectorFor(
- ${this._names.getDirectivesAccessorName()}[${i}].directiveIndex);`;
- }
- return lines.join('\n');
- }
-
  _maybeGenCallOnAllChangesDone(): string {
  var notifications = [];
  var dirs = this.directiveRecords;

modules/angular2/src/change_detection/codegen_logic_util.ts

@@ -3,6 +3,7 @@ import {BaseException, Json, StringWrapper} from 'angular2/src/facade/lang';
 import {CodegenNameUtil} from './codegen_name_util';
 import {codify, combineGeneratedStrings, rawString} from './codegen_facade';
 import {ProtoRecord, RecordType} from './proto_record';
+import {DirectiveRecord} from './directive_record';
 
 /**
  * This is an experimental feature. Works only in Dart.
@@ -131,4 +132,16 @@ export class CodegenLogicUtil {
  iVals.push(codify(protoRec.fixedArgs[protoRec.args.length]));
  return combineGeneratedStrings(iVals);
  }
+
+ genHydrateDetectors(directiveRecords: DirectiveRecord[]): string {
+ var res = [];
+ for (var i = 0; i < directiveRecords.length; ++i) {
+ var r = directiveRecords[i];
+ if (!r.isDefaultChangeDetection()) {
+ res.push(
+ `${this._names.getDetectorName(r.directiveIndex)} = this.getDetectorFor(directives, ${i});`);
+ }
+ }
+ return res.join("\n");
+ }
 }

modules/angular2/src/change_detection/codegen_name_util.ts

@@ -200,11 +200,5 @@ export class CodegenNameUtil {
  return this._addFieldPrefix(`directive_${d.name}`);
  }
 
- getAllDetectorNames(): List<string> {
- return ListWrapper.map(
- ListWrapper.filter(this.directiveRecords, r => !r.isDefaultChangeDetection()),
- (d) => this.getDetectorName(d.directiveIndex));
- }
-
  getDetectorName(d: DirectiveIndex): string { return this._addFieldPrefix(`detector_${d.name}`); }
 }

modules/angular2/src/transform/template_compiler/change_detector_codegen.dart

@@ -226,7 +226,7 @@ class _CodegenState {
 
  String _maybeGenHydrateDirectives() {
  var hydrateDirectivesCode = _genHydrateDirectives();
- var hydrateDetectorsCode = _genHydrateDetectors();
+ var hydrateDetectorsCode = _logic.genHydrateDetectors(_directiveRecords);
  if (hydrateDirectivesCode.isEmpty && hydrateDetectorsCode.isEmpty) {
  return '';
  }
@@ -244,16 +244,6 @@ class _CodegenState {
  return '$buf';
  }
 
- String _genHydrateDetectors() {
- var buf = new StringBuffer();
- var detectorFieldNames = _names.getAllDetectorNames();
- for (var i = 0; i < detectorFieldNames.length; ++i) {
- buf.writeln('${detectorFieldNames[i]} = directives.getDetectorFor('
- '${_names.getDirectivesAccessorName()}[$i].directiveIndex);');
- }
- return '$buf';
- }
-
  /// Generates calls to `onAllChangesDone` for all `Directive`s that request
  /// them.
  String _maybeGenCallOnAllChangesDone() {

modules/angular2/test/change_detection/change_detector_config.ts

@@ -183,24 +183,25 @@ class _ExpressionWithMode {
  var directiveRecords = [];
  var eventRecords = [];
 
+ var dirRecordWithDefault =
+ new DirectiveRecord({directiveIndex: new DirectiveIndex(0, 0), changeDetection: DEFAULT});
+ var dirRecordWithOnPush =
+ new DirectiveRecord({directiveIndex: new DirectiveIndex(0, 1), changeDetection: ON_PUSH});
+
  if (this._withRecords) {
- var dirRecordWithOnPush =
- new DirectiveRecord({directiveIndex: new DirectiveIndex(0, 0), changeDetection: ON_PUSH});
+ var updateDirWithOnDefaultRecord =
+ BindingRecord.createForDirective(_getParser().parseBinding('42', 'location'), 'a',
+ (o, v) => (<any>o).a = v, dirRecordWithDefault);
  var updateDirWithOnPushRecord =
  BindingRecord.createForDirective(_getParser().parseBinding('42', 'location'), 'a',
  (o, v) => (<any>o).a = v, dirRecordWithOnPush);
- bindingRecords = [updateDirWithOnPushRecord];
- directiveRecords = [dirRecordWithOnPush];
- } else {
- bindingRecords = [];
- directiveRecords = [];
+
+ directiveRecords = [dirRecordWithDefault, dirRecordWithOnPush];
+ bindingRecords = [updateDirWithOnDefaultRecord, updateDirWithOnPushRecord];
  }
 
  if (this._withEvents) {
- var dirRecordWithOnPush =
- new DirectiveRecord({directiveIndex: new DirectiveIndex(0, 0), changeDetection: ON_PUSH});
- directiveRecords = [dirRecordWithOnPush];
-
+ directiveRecords = [dirRecordWithDefault, dirRecordWithOnPush];
  eventRecords =
  ListWrapper.concat(_createEventRecords("(event)='false'"),
  _createHostEventRecords("(host-event)='false'", dirRecordWithOnPush))

modules/angular2/test/change_detection/change_detector_spec.ts

@@ -704,29 +704,36 @@ export function main() {
  });
 
  describe('marking ON_PUSH detectors as CHECK_ONCE after an update', () => {
- var checkedDetector;
+ var childDirectiveDetectorRegular;
+ var childDirectiveDetectorOnPush;
  var directives;
 
  beforeEach(() => {
- checkedDetector = _createWithoutHydrate('emptyUsingOnPushStrategy').changeDetector;
- checkedDetector.hydrate(_DEFAULT_CONTEXT, null, null, null);
- checkedDetector.mode = CHECKED;
-
- var targetDirective = new TestData(null);
- directives = new FakeDirectives([targetDirective], [checkedDetector]);
+ childDirectiveDetectorRegular = _createWithoutHydrate('10').changeDetector;
+ childDirectiveDetectorRegular.hydrate(_DEFAULT_CONTEXT, null, null, null);
+ childDirectiveDetectorRegular.mode = CHECK_ALWAYS;
+
+ childDirectiveDetectorOnPush =
+ _createWithoutHydrate('emptyUsingOnPushStrategy').changeDetector;
+ childDirectiveDetectorOnPush.hydrate(_DEFAULT_CONTEXT, null, null, null);
+ childDirectiveDetectorOnPush.mode = CHECKED;
+
+ directives =
+ new FakeDirectives([new TestData(null), new TestData(null)],
+ [childDirectiveDetectorRegular, childDirectiveDetectorOnPush]);
  });
 
  it('should set the mode to CHECK_ONCE when a binding is updated', () => {
- var cd = _createWithoutHydrate('onPushRecordsUsingDefaultStrategy').changeDetector;
- cd.hydrate(_DEFAULT_CONTEXT, null, directives, null);
+ var parentDetector =
+ _createWithoutHydrate('onPushRecordsUsingDefaultStrategy').changeDetector;
+ parentDetector.hydrate(_DEFAULT_CONTEXT, null, directives, null);
 
- expect(checkedDetector.mode).toEqual(CHECKED);
+ parentDetector.detectChanges();
 
- // evaluate the record, update the targetDirective, and mark its detector as
- // CHECK_ONCE
- cd.detectChanges();
+ // making sure that we only change the status of ON_PUSH components
+ expect(childDirectiveDetectorRegular.mode).toEqual(CHECK_ALWAYS);
 
- expect(checkedDetector.mode).toEqual(CHECK_ONCE);
+ expect(childDirectiveDetectorOnPush.mode).toEqual(CHECK_ONCE);
  });
 
  it('should mark ON_PUSH detectors as CHECK_ONCE after an event', () => {
@@ -745,7 +752,7 @@ export function main() {
 
  cd.handleEvent("host-event", 0, null);
 
- expect(checkedDetector.mode).toEqual(CHECK_ONCE);
+ expect(childDirectiveDetectorOnPush.mode).toEqual(CHECK_ONCE);
  });
 
  if (IS_DART) {