From 18c4a252ababace28d1b2ee05c5b0ebbc83d8af9 Mon Sep 17 00:00:00 2001 From: Tim Blasi Date: Mon, 27 Jul 2015 18:31:22 -0700 Subject: [PATCH] refactor(change_detect): Move (de)hydrate logic into dedicated methods Add additional details here! Update to #3248 --- .../abstract_change_detector.ts | 6 +- .../change_detection_jit_generator.ts | 58 ++++++++++++++----- .../change_detector_codegen.dart | 52 +++++++++++++---- .../expected/bar.ng_deps.dart | 21 ++++--- 4 files changed, 102 insertions(+), 35 deletions(-) diff --git a/modules/angular2/src/change_detection/abstract_change_detector.ts b/modules/angular2/src/change_detection/abstract_change_detector.ts index 7271ae897833c3..ac164df4afde3b 100644 --- a/modules/angular2/src/change_detection/abstract_change_detector.ts +++ b/modules/angular2/src/change_detection/abstract_change_detector.ts @@ -60,8 +60,12 @@ export class AbstractChangeDetector implements ChangeDetector { hydrate(context: any, locals: Locals, directives: any, pipes: any): void {} + hydrateDirectives(directives: any): void {} + dehydrate(): void {} + dehydrateDirectives(destroyPipes: boolean): void {} + callOnAllChangesDone(): void {} _detectChangesInLightDomChildren(throwOnChange: boolean): void { @@ -94,4 +98,4 @@ export class AbstractChangeDetector implements ChangeDetector { c["locals"], c["injector"], proto.expressionAsString); throw new ChangeDetectionError(proto, exception, stack, context); } -} \ No newline at end of file +} 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 fb7ed21abdce66..ef0061bd6b7606 100644 --- a/modules/angular2/src/change_detection/change_detection_jit_generator.ts +++ b/modules/angular2/src/change_detection/change_detection_jit_generator.ts @@ -33,17 +33,18 @@ var ALREADY_CHECKED_ACCESSOR = "this.alreadyChecked"; export class ChangeDetectorJITGenerator { _names: CodegenNameUtil; + _typeName: string; constructor(public id: string, public changeDetectionStrategy: string, public records: List, public directiveRecords: List, private generateCheckNoChanges: boolean) { this._names = new CodegenNameUtil(this.records, this.directiveRecords, 'this._', UTIL); + this._typeName = sanitizeName(`ChangeDetector_${this.id}`); } generate(): Function { - var typeName = sanitizeName(`ChangeDetector_${this.id}`); var classDefinition = ` - var ${typeName} = function ${typeName}(dispatcher, protos, directiveRecords) { + var ${this._typeName} = function ${this._typeName}(dispatcher, protos, directiveRecords) { ${ABSTRACT_CHANGE_DETECTOR}.call(this, ${JSON.stringify(this.id)}, dispatcher); ${PROTOS_ACCESSOR} = protos; ${DIRECTIVES_ACCESSOR} = directiveRecords; @@ -51,12 +52,12 @@ export class ChangeDetectorJITGenerator { ${CURRENT_PROTO} = null; ${PIPES_ACCESSOR} = null; ${ALREADY_CHECKED_ACCESSOR} = false; - ${this._names.genDehydrateFields()} + this.dehydrateDirectives(false); } - ${typeName}.prototype = Object.create(${ABSTRACT_CHANGE_DETECTOR}.prototype); + ${this._typeName}.prototype = Object.create(${ABSTRACT_CHANGE_DETECTOR}.prototype); - ${typeName}.prototype.detectChangesInRecords = function(throwOnChange) { + ${this._typeName}.prototype.detectChangesInRecords = function(throwOnChange) { if (!this.hydrated()) { ${UTIL}.throwDehydrated(); } @@ -67,7 +68,7 @@ export class ChangeDetectorJITGenerator { } } - ${typeName}.prototype.__detectChangesInRecords = function(throwOnChange) { + ${this._typeName}.prototype.__detectChangesInRecords = function(throwOnChange) { ${CURRENT_PROTO} = null; ${this._names.genInitLocals()} @@ -81,35 +82,37 @@ export class ChangeDetectorJITGenerator { ${ALREADY_CHECKED_ACCESSOR} = true; } - ${this._genCheckNoChanges(typeName)} + ${this._genCheckNoChanges(this._typeName)} - ${typeName}.prototype.callOnAllChangesDone = function() { + ${this._typeName}.prototype.callOnAllChangesDone = function() { ${this._genCallOnAllChangesDoneBody()} } - ${typeName}.prototype.hydrate = function(context, locals, directives, pipes) { + ${this._typeName}.prototype.hydrate = function(context, locals, directives, pipes) { ${MODE_ACCESSOR} = "${ChangeDetectionUtil.changeDetectionMode(this.changeDetectionStrategy)}"; ${this._names.getContextName()} = context; ${LOCALS_ACCESSOR} = locals; - ${this._genHydrateDirectives()} - ${this._genHydrateDetectors()} + this.hydrateDirectives(directives); ${PIPES_ACCESSOR} = pipes; ${ALREADY_CHECKED_ACCESSOR} = false; } - ${typeName}.prototype.dehydrate = function() { - ${this._names.genPipeOnDestroy()} - ${this._names.genDehydrateFields()} + ${this._maybeGenHydrateDirectives()} + + ${this._typeName}.prototype.dehydrate = function() { + this.dehydrateDirectives(true); ${LOCALS_ACCESSOR} = null; ${PIPES_ACCESSOR} = null; } - ${typeName}.prototype.hydrated = function() { + ${this._maybeGenDehydrateDirectives()} + + ${this._typeName}.prototype.hydrated = function() { return ${this._names.getContextName()} !== null; } return function(dispatcher) { - return new ${typeName}(dispatcher, protos, directiveRecords); + return new ${this._typeName}(dispatcher, protos, directiveRecords); } `; @@ -118,6 +121,29 @@ export class ChangeDetectorJITGenerator { AbstractChangeDetector, ChangeDetectionUtil, this.records, this.directiveRecords); } + _maybeGenDehydrateDirectives(): string { + var destroyPipesCode = this._names.genPipeOnDestroy(); + if (destroyPipesCode) { + destroyPipesCode = `if (destroyPipes) { ${destroyPipesCode} }`; + } + var dehydrateFieldsCode = this._names.genDehydrateFields(); + if (!destroyPipesCode && !dehydrateFieldsCode) return ''; + return `${this._typeName}.prototype.dehydrateDirectives = function(destroyPipes) { + ${destroyPipesCode} + ${dehydrateFieldsCode} + }`; + } + + _maybeGenHydrateDirectives(): string { + var hydrateDirectivesCode = this._genHydrateDirectives(); + var hydrateDetectorsCode = this._genHydrateDetectors(); + if (!hydrateDirectivesCode && !hydrateDetectorsCode) return ''; + return `${this._typeName}.prototype.hydrateDirectives = function(directives) { + ${hydrateDirectivesCode} + ${hydrateDetectorsCode} + }`; + } + _genHydrateDirectives(): string { var directiveFieldNames = this._names.getAllDirectiveNames(); var lines = ListWrapper.createFixedSize(directiveFieldNames.length); 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 c1177579636d8d..8ce31cd20816b9 100644 --- a/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart +++ b/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart @@ -78,7 +78,8 @@ class _CodegenState { _CodegenState._(this._changeDetectorDefId, this._contextTypeName, this._changeDetectorTypeName, String changeDetectionStrategy, - List records, List directiveRecords, this._generateCheckNoChanges) + List records, List directiveRecords, + this._generateCheckNoChanges) : _records = records, _directiveRecords = directiveRecords, _names = new CodegenNameUtil(records, directiveRecords, '_', _UTIL), @@ -92,7 +93,8 @@ class _CodegenState { .forEach((rec) => protoRecords.add(rec, def.variableNames)); var records = coalesce(protoRecords.records); return new _CodegenState._(def.id, typeName, changeDetectorTypeName, - def.strategy, records, def.directiveRecords, def.generateCheckNoChanges); + def.strategy, records, def.directiveRecords, + def.generateCheckNoChanges); } void _writeToBuf(StringBuffer buf) { @@ -113,7 +115,7 @@ class _CodegenState { this.$_PROTOS_ACCESSOR, this.$_DIRECTIVES_ACCESSOR) : super(${_encodeValue(_changeDetectorDefId)}, $_DISPATCHER_ACCESSOR) { - ${_names.genDehydrateFields()} + dehydrateDirectives(false); } void detectChangesInRecords(throwOnChange) { @@ -121,9 +123,9 @@ class _CodegenState { $_UTIL.throwDehydrated(); } try { - this.__detectChangesInRecords(throwOnChange); + __detectChangesInRecords(throwOnChange); } catch (e, s) { - this.throwError($_CURRENT_PROTO, e, s); + throwError($_CURRENT_PROTO, e, s); } } @@ -149,19 +151,21 @@ class _CodegenState { $_MODE_ACCESSOR = '$_changeDetectionMode'; ${_names.getContextName()} = context; $_LOCALS_ACCESSOR = locals; - ${_genHydrateDirectives()} - ${_genHydrateDetectors()} + hydrateDirectives(directives); $_ALREADY_CHECKED_ACCESSOR = false; $_PIPES_ACCESSOR = pipes; } + ${_maybeGenHydrateDirectives()} + void dehydrate() { - ${_names.genPipeOnDestroy()} - ${_names.genDehydrateFields()} + dehydrateDirectives(true); $_LOCALS_ACCESSOR = null; $_PIPES_ACCESSOR = null; } + ${_maybeGenDehydrateDirectives()} + hydrated() => ${_names.getContextName()} != null; static $_GEN_PREFIX.ProtoChangeDetector @@ -182,6 +186,34 @@ class _CodegenState { '''); } + String _maybeGenDehydrateDirectives() { + var destroyPipesParamName = 'destroyPipes'; + var destroyPipesCode = _names.genPipeOnDestroy(); + if (destroyPipesCode.isNotEmpty) { + destroyPipesCode = 'if (${destroyPipesParamName}) { ' + '${destroyPipesCode}' + '}'; + } + var dehydrateFieldsCode = _names.genDehydrateFields(); + if (destroyPipesCode.isEmpty && dehydrateFieldsCode.isEmpty) return ''; + return 'void dehydrateDirectives(${destroyPipesParamName}) {' + '${destroyPipesCode}' + '${dehydrateFieldsCode}' + '}'; + } + + String _maybeGenHydrateDirectives() { + var hydrateDirectivesCode = _genHydrateDirectives(); + var hydrateDetectorsCode = _genHydrateDetectors(); + if (hydrateDirectivesCode.isEmpty && hydrateDetectorsCode.isEmpty) { + return ''; + } + return 'void hydrateDirectives(directives) { ' + '$hydrateDirectivesCode' + '$hydrateDetectorsCode' + '}'; + } + String _genHydrateDirectives() { var buf = new StringBuffer(); var directiveFieldNames = _names.getAllDirectiveNames(); @@ -410,7 +442,7 @@ class _CodegenState { String _genCheckNoChanges() { if (this._generateCheckNoChanges) { - return 'void checkNoChanges() { this.runDetectChanges(true); }'; + return 'void checkNoChanges() { runDetectChanges(true); }'; } else { return ''; } diff --git a/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart b/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart index 7163a35883265d..809288ec934044 100644 --- a/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart +++ b/modules/angular2/test/transform/integration/two_annotations_files/expected/bar.ng_deps.dart @@ -36,8 +36,7 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector { _MyComponent_ChangeDetector0( dynamic dispatcher, this._protos, this._directiveRecords) : super("MyComponent_comp_0", dispatcher) { - _context = null; - _myNum0 = _interpolate1 = _gen.ChangeDetectionUtil.uninitialized; + dehydrateDirectives(false); } void detectChangesInRecords(throwOnChange) { @@ -45,9 +44,9 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector { _gen.ChangeDetectionUtil.throwDehydrated(); } try { - this.__detectChangesInRecords(throwOnChange); + __detectChangesInRecords(throwOnChange); } catch (e, s) { - this.throwError(currentProto, e, s); + throwError(currentProto, e, s); } } @@ -91,7 +90,9 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector { _alreadyChecked = true; } - void checkNoChanges() {this.runDetectChanges(true);} + void checkNoChanges() { + runDetectChanges(true); + } void callOnAllChangesDone() { dispatcher.notifyOnAllChangesDone(); @@ -101,18 +102,22 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector { mode = 'ALWAYS_CHECK'; _context = context; _locals = locals; - + hydrateDirectives(directives); _alreadyChecked = false; _pipes = pipes; } void dehydrate() { - _context = null; - _myNum0 = _interpolate1 = _gen.ChangeDetectionUtil.uninitialized; + dehydrateDirectives(true); _locals = null; _pipes = null; } + void dehydrateDirectives(destroyPipes) { + _context = null; + _myNum0 = _interpolate1 = _gen.ChangeDetectionUtil.uninitialized; + } + hydrated() => _context != null; static _gen.ProtoChangeDetector newProtoChangeDetector(