From d84993faf18a277b7db2bcf4d312b8e47ff432c3 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 Call new `(de)hydrateDirectives` methods from `(de)hydrate`. Add a null implementation in `AbstractChangeDetector` and only override if necessary for the specific change detector. Update to #3248 --- .../abstract_change_detector.ts | 6 +- .../change_detection_jit_generator.ts | 62 +++++++++++++------ .../change_detector_codegen.dart | 46 +++++++++++--- .../expected/bar.ng_deps.dart | 19 +++--- 4 files changed, 98 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 8f584b9d08..f4efc51b5e 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 { @@ -95,4 +99,4 @@ export class AbstractChangeDetector implements ChangeDetector { null; 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 fb7ed21abd..0410acd181 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()} - ${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); @@ -345,9 +371,9 @@ export class ChangeDetectorJITGenerator { } } - _genCheckNoChanges(typeName: string): string { + _genCheckNoChanges(): string { if (this.generateCheckNoChanges) { - return `${typeName}.prototype.checkNoChanges = function() { this.runDetectChanges(true); }`; + return `${this._typeName}.prototype.checkNoChanges = function() { this.runDetectChanges(true); }`; } else { return ''; } 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 0a43349748..fc8b81e1fd 100644 --- a/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart +++ b/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart @@ -115,7 +115,7 @@ class _CodegenState { this.$_PROTOS_ACCESSOR, this.$_DIRECTIVES_ACCESSOR) : super(${_encodeValue(_changeDetectorDefId)}, $_DISPATCHER_ACCESSOR) { - ${_names.genDehydrateFields()} + dehydrateDirectives(false); } void detectChangesInRecords(throwOnChange) { @@ -123,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); } } @@ -151,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 @@ -184,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(); @@ -412,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 f04e144086..7433464203 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); } } @@ -92,7 +91,7 @@ class _MyComponent_ChangeDetector0 extends _gen.AbstractChangeDetector { } void checkNoChanges() { - this.runDetectChanges(true); + runDetectChanges(true); } void callOnAllChangesDone() { @@ -103,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(