From 5e6317fecc16513546f7e4e39f66ab22dfd7d29b Mon Sep 17 00:00:00 2001 From: vsavkin Date: Sun, 16 Aug 2015 19:05:53 -0700 Subject: [PATCH] feat(change_detection): request a change detection check when an event happens Closes #3679 --- .../abstract_change_detector.ts | 4 +- .../src/change_detection/binding_record.ts | 5 ++- .../change_detection_jit_generator.ts | 14 ++++++- .../dynamic_change_detector.ts | 8 ++++ .../src/core/compiler/proto_view_factory.ts | 3 +- .../change_detector_codegen.dart | 12 +++++- .../change_detector_config.ts | 37 ++++++++++++++----- .../change_detection/change_detector_spec.ts | 20 +++++++++- 8 files changed, 82 insertions(+), 21 deletions(-) diff --git a/modules/angular2/src/change_detection/abstract_change_detector.ts b/modules/angular2/src/change_detection/abstract_change_detector.ts index 7fab994116..be535ca395 100644 --- a/modules/angular2/src/change_detection/abstract_change_detector.ts +++ b/modules/angular2/src/change_detection/abstract_change_detector.ts @@ -72,9 +72,7 @@ export class AbstractChangeDetector implements ChangeDetector { return res; } - handleEventInternal(eventName: string, elIndex: number, locals: Locals): boolean { - return false; - } + handleEventInternal(eventName: string, elIndex: number, locals: Locals): boolean { return false; } detectChanges(): void { this.runDetectChanges(false); } diff --git a/modules/angular2/src/change_detection/binding_record.ts b/modules/angular2/src/change_detection/binding_record.ts index 6b38039342..44f2d8275c 100644 --- a/modules/angular2/src/change_detection/binding_record.ts +++ b/modules/angular2/src/change_detection/binding_record.ts @@ -118,8 +118,9 @@ export class BindingRecord { } static createForHostEvent(ast: AST, eventName: string, - directiveIndex: DirectiveIndex): BindingRecord { + directiveRecord: DirectiveRecord): BindingRecord { + var directiveIndex = directiveRecord.directiveIndex; return new BindingRecord(EVENT, directiveIndex, ast, directiveIndex.elementIndex, null, null, - eventName, null, null, null); + eventName, null, null, directiveRecord); } } diff --git a/modules/angular2/src/change_detection/change_detection_jit_generator.ts b/modules/angular2/src/change_detection/change_detection_jit_generator.ts index d4b4946016..4935098f4c 100644 --- a/modules/angular2/src/change_detection/change_detection_jit_generator.ts +++ b/modules/angular2/src/change_detection/change_detection_jit_generator.ts @@ -83,7 +83,7 @@ export class ChangeDetectorJITGenerator { if (this.eventBindings.length > 0) { var handlers = this.eventBindings.map(eb => this._genEventBinding(eb)).join("\n"); return ` - ${this._typeName}.prototype.handleEvent = function(eventName, elIndex, locals) { + ${this._typeName}.prototype.handleEventInternal = function(eventName, elIndex, locals) { var ${this._names.getPreventDefaultAccesor()} = false; ${this._names.genInitEventLocals()} ${handlers} @@ -106,13 +106,23 @@ export class ChangeDetectorJITGenerator { _genEventBindingEval(eb: EventBinding, r: ProtoRecord): string { if (r.lastInBinding) { var evalRecord = this._logic.genEventBindingEvalValue(eb, r); + var markPath = this._genMarkPathToRootAsCheckOnce(r); var prevDefault = this._genUpdatePreventDefault(eb, r); - return `${evalRecord}\n${prevDefault}`; + return `${evalRecord}\n${markPath}\n${prevDefault}`; } else { return this._logic.genEventBindingEvalValue(eb, r); } } + _genMarkPathToRootAsCheckOnce(r: ProtoRecord): string { + var br = r.bindingRecord; + if (br.isOnPushChangeDetection()) { + return `${this._names.getDetectorName(br.directiveRecord.directiveIndex)}.markPathToRootAsCheckOnce();`; + } else { + return ""; + } + } + _genUpdatePreventDefault(eb: EventBinding, r: ProtoRecord): string { var local = this._names.getEventLocalName(eb, r.selfIndex); return `if (${local} === false) { ${this._names.getPreventDefaultAccesor()} = true};`; diff --git a/modules/angular2/src/change_detection/dynamic_change_detector.ts b/modules/angular2/src/change_detection/dynamic_change_detector.ts index 53e14f1de7..4240e26e47 100644 --- a/modules/angular2/src/change_detection/dynamic_change_detector.ts +++ b/modules/angular2/src/change_detection/dynamic_change_detector.ts @@ -53,6 +53,7 @@ export class DynamicChangeDetector extends AbstractChangeDetector { var proto = eb.records[i]; var res = this._calculateCurrValue(proto, values, locals); if (proto.lastInBinding) { + this._markPathAsCheckOnce(proto); return res; } else { this._writeSelf(proto, res, values); @@ -62,6 +63,13 @@ export class DynamicChangeDetector extends AbstractChangeDetector { throw new BaseException("Cannot be reached"); } + _markPathAsCheckOnce(proto: ProtoRecord): void { + if (proto.bindingRecord.isOnPushChangeDetection()) { + var dir = proto.bindingRecord.directiveRecord; + this._getDetectorFor(dir.directiveIndex).markPathToRootAsCheckOnce(); + } + } + _matchingEventBindings(eventName: string, elIndex: number): EventBinding[] { return ListWrapper.filter(this.eventBindings, eb => eb.eventName == eventName && eb.elIndex === elIndex); diff --git a/modules/angular2/src/core/compiler/proto_view_factory.ts b/modules/angular2/src/core/compiler/proto_view_factory.ts index 0ae1a71be1..d31c076cbf 100644 --- a/modules/angular2/src/core/compiler/proto_view_factory.ts +++ b/modules/angular2/src/core/compiler/proto_view_factory.ts @@ -57,8 +57,7 @@ export class BindingRecordsCreator { var directiveMetadata = allDirectiveMetadatas[dir.directiveIndex]; var dirRecord = this._getDirectiveRecord(boundElementIndex, i, directiveMetadata); dir.eventBindings.forEach(heb => { - res.push( - BindingRecord.createForHostEvent(heb.source, heb.fullName, dirRecord.directiveIndex)); + res.push(BindingRecord.createForHostEvent(heb.source, heb.fullName, dirRecord)); }); } } diff --git a/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart b/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart index f075b03363..fdb3a5b485 100644 --- a/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart +++ b/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart @@ -183,13 +183,23 @@ class _CodegenState { String _genEventBindingEval(EventBinding eb, ProtoRecord r){ if (r.lastInBinding) { var evalRecord = _logic.genEventBindingEvalValue(eb, r); + var markPath = _genMarkPathToRootAsCheckOnce(r); var prevDefault = _genUpdatePreventDefault(eb, r); - return "${evalRecord}\n${prevDefault}"; + return "${evalRecord}\n${markPath}\n${prevDefault}"; } else { return _logic.genEventBindingEvalValue(eb, r); } } + String _genMarkPathToRootAsCheckOnce(ProtoRecord r) { + var br = r.bindingRecord; + if (br.isOnPushChangeDetection()) { + return "${_names.getDetectorName(br.directiveRecord.directiveIndex)}.markPathToRootAsCheckOnce();"; + } else { + return ""; + } + } + String _genUpdatePreventDefault(EventBinding eb, ProtoRecord r) { var local = this._names.getEventLocalName(eb, r.selfIndex); return """if (${local} == false) { ${_names.getPreventDefaultAccesor()} = true; }"""; diff --git a/modules/angular2/test/change_detection/change_detector_config.ts b/modules/angular2/test/change_detection/change_detector_config.ts index dc08b94e02..c15149fcd7 100644 --- a/modules/angular2/test/change_detection/change_detector_config.ts +++ b/modules/angular2/test/change_detection/change_detector_config.ts @@ -39,13 +39,14 @@ function _createEventRecords(expression: string): List { return [BindingRecord.createForEvent(ast, eventName, 0)]; } -function _createHostEventRecords(expression: string): List { +function _createHostEventRecords(expression: string, directiveRecord: DirectiveRecord): + List { var parts = expression.split("="); var eventName = parts[0].substring(1, parts[0].length - 1); var exp = parts[1].substring(1, parts[1].length - 1); var ast = _getParser().parseAction(exp, 'location'); - return [BindingRecord.createForHostEvent(ast, eventName, new DirectiveIndex(0, 0))]; + return [BindingRecord.createForHostEvent(ast, eventName, directiveRecord)]; } function _convertLocalsToVariableBindings(locals: Locals): List { @@ -98,7 +99,7 @@ export function getDefinition(id: string): TestDefinition { testDef = new TestDefinition(id, cdDef, null); } else if (ListWrapper.indexOf(_availableHostEventDefinitions, id) >= 0) { - var eventRecords = _createHostEventRecords(id); + var eventRecords = _createHostEventRecords(id, _DirectiveUpdating.basicRecords[0]); let cdDef = new ChangeDetectorDefinition(id, null, [], [], eventRecords, [_DirectiveUpdating.basicRecords[0]], true); testDef = new TestDefinition(id, cdDef, null); @@ -165,12 +166,15 @@ class _ExpressionWithLocals { } class _ExpressionWithMode { - constructor(private _strategy: string, private _withRecords: boolean) {} + constructor(private _strategy: string, private _withRecords: boolean, + private _withEvents: boolean) {} createChangeDetectorDefinition(): ChangeDetectorDefinition { var variableBindings = []; - var bindingRecords = null; - var directiveRecords = null; + var bindingRecords = []; + var directiveRecords = []; + var eventRecords = []; + if (this._withRecords) { var dirRecordWithOnPush = new DirectiveRecord({directiveIndex: new DirectiveIndex(0, 0), changeDetection: ON_PUSH}); @@ -183,8 +187,19 @@ class _ExpressionWithMode { bindingRecords = []; directiveRecords = []; } + + if (this._withEvents) { + var dirRecordWithOnPush = + new DirectiveRecord({directiveIndex: new DirectiveIndex(0, 0), changeDetection: ON_PUSH}); + directiveRecords = [dirRecordWithOnPush]; + + eventRecords = + ListWrapper.concat(_createEventRecords("(event)='false'"), + _createHostEventRecords("(host-event)='false'", dirRecordWithOnPush)) + } + return new ChangeDetectorDefinition('(empty id)', this._strategy, variableBindings, - bindingRecords, [], directiveRecords, true); + bindingRecords, eventRecords, directiveRecords, true); } /** @@ -192,9 +207,11 @@ class _ExpressionWithMode { * Definitions in this map define conditions which allow testing various change detector modes. */ static availableDefinitions: StringMap = { - 'emptyUsingDefaultStrategy': new _ExpressionWithMode(DEFAULT, false), - 'emptyUsingOnPushStrategy': new _ExpressionWithMode(ON_PUSH, false), - 'onPushRecordsUsingDefaultStrategy': new _ExpressionWithMode(DEFAULT, true) + 'emptyUsingDefaultStrategy': new _ExpressionWithMode(DEFAULT, false, false), + 'emptyUsingOnPushStrategy': new _ExpressionWithMode(ON_PUSH, false, false), + 'onPushRecordsUsingDefaultStrategy': new _ExpressionWithMode(DEFAULT, true, false), + 'onPushWithEvent': new _ExpressionWithMode(ON_PUSH, false, true), + 'onPushWithHostEvent': new _ExpressionWithMode(ON_PUSH, false, true) }; } diff --git a/modules/angular2/test/change_detection/change_detector_spec.ts b/modules/angular2/test/change_detection/change_detector_spec.ts index 1c032e0b87..288319aebf 100644 --- a/modules/angular2/test/change_detection/change_detector_spec.ts +++ b/modules/angular2/test/change_detection/change_detector_spec.ts @@ -716,6 +716,25 @@ export function main() { expect(checkedDetector.mode).toEqual(CHECK_ONCE); }); + + it('should mark ON_PUSH detectors as CHECK_ONCE after an event', () => { + var cd = _createWithoutHydrate('onPushWithEvent').changeDetector; + cd.hydrate(_DEFAULT_CONTEXT, null, directives, null); + cd.mode = CHECKED; + + cd.handleEvent("event", 0, null); + + expect(cd.mode).toEqual(CHECK_ONCE); + }); + + it('should mark ON_PUSH detectors as CHECK_ONCE after a host event', () => { + var cd = _createWithoutHydrate('onPushWithHostEvent').changeDetector; + cd.hydrate(_DEFAULT_CONTEXT, null, directives, null); + + cd.handleEvent("host-event", 0, null); + + expect(checkedDetector.mode).toEqual(CHECK_ONCE); + }); }); }); @@ -899,7 +918,6 @@ export function main() { res = val.changeDetector.handleEvent("event", 0, locals); expect(res).toBe(false); }); - }); }); });