From a9ce454b2176514e1b655d7b005c32f0aa14ab98 Mon Sep 17 00:00:00 2001 From: vsavkin Date: Thu, 20 Aug 2015 15:11:12 -0700 Subject: [PATCH] fix(change_detection): fixed reflect properties as attributes Closes #3761 --- modules/angular2/change_detection.ts | 1 + modules/angular2/render.ts | 1 - .../abstract_change_detector.ts | 4 ++ .../src/change_detection/binding_record.ts | 3 +- .../src/change_detection/change_detection.ts | 44 +++++++----- .../change_detection_jit_generator.ts | 4 +- .../dynamic_change_detector.ts | 4 ++ .../src/change_detection/interfaces.ts | 6 +- .../angular2/src/core/application_common.ts | 10 ++- modules/angular2/src/core/compiler/view.ts | 11 +++ .../angular2/src/forms/directives/ng_model.ts | 2 +- .../angular2/src/render/dom/dom_renderer.ts | 13 +--- modules/angular2/src/render/dom/dom_tokens.ts | 4 -- .../angular2/src/test_lib/test_injector.ts | 4 +- .../src/transform/common/options.dart | 6 ++ .../src/transform/common/options_reader.dart | 3 + .../change_detector_codegen.dart | 3 + .../template_compiler/generator.dart | 5 +- .../template_compiler/transformer.dart | 3 +- .../src/web-workers/ui/di_bindings.ts | 13 ++-- .../web-workers/worker/application_common.ts | 8 +-- .../change_detection/change_detection_spec.ts | 4 +- .../change_detector_config.ts | 16 +++-- .../change_detection/change_detector_spec.ts | 31 ++++++++- .../test/core/compiler/integration_spec.ts | 29 +++++++- .../dom/dom_renderer_integration_spec.ts | 68 ------------------- .../expected/bar.ng_deps.dart | 1 + .../change_detection_benchmark.ts | 9 ++- 28 files changed, 168 insertions(+), 142 deletions(-) diff --git a/modules/angular2/change_detection.ts b/modules/angular2/change_detection.ts index bb06ca84a3..17f490204e 100644 --- a/modules/angular2/change_detection.ts +++ b/modules/angular2/change_detection.ts @@ -28,4 +28,5 @@ export { KeyValueDiffers, KeyValueDiffer, KeyValueDifferFactory + } from 'angular2/src/change_detection/change_detection'; diff --git a/modules/angular2/render.ts b/modules/angular2/render.ts index f0b3f69e8e..ceb3a8be4c 100644 --- a/modules/angular2/render.ts +++ b/modules/angular2/render.ts @@ -17,6 +17,5 @@ export { ViewDefinition, DOCUMENT, APP_ID, - DOM_REFLECT_PROPERTIES_AS_ATTRIBUTES, MAX_IN_MEMORY_ELEMENTS_PER_TEMPLATE } from './src/render/render'; diff --git a/modules/angular2/src/change_detection/abstract_change_detector.ts b/modules/angular2/src/change_detection/abstract_change_detector.ts index 98abbbe6e9..7e3b786bdf 100644 --- a/modules/angular2/src/change_detection/abstract_change_detector.ts +++ b/modules/angular2/src/change_detection/abstract_change_detector.ts @@ -211,6 +211,10 @@ export class AbstractChangeDetector implements ChangeDetector { this.dispatcher.notifyOnBinding(this._currentBinding(), value); } + protected logBindingUpdate(value: any): void { + this.dispatcher.logBindingUpdate(this._currentBinding(), value); + } + protected addChange(changes: StringMap, oldValue: any, newValue: any): StringMap { if (isBlank(changes)) { diff --git a/modules/angular2/src/change_detection/binding_record.ts b/modules/angular2/src/change_detection/binding_record.ts index 272ab345b0..7723c224f5 100644 --- a/modules/angular2/src/change_detection/binding_record.ts +++ b/modules/angular2/src/change_detection/binding_record.ts @@ -64,7 +64,8 @@ export class BindingRecord { static createForDirective(ast: AST, propertyName: string, setter: SetterFn, directiveRecord: DirectiveRecord): BindingRecord { - var t = new BindingTarget(DIRECTIVE, null, propertyName, null, ast.toString()); + var elementIndex = directiveRecord.directiveIndex.elementIndex; + var t = new BindingTarget(DIRECTIVE, elementIndex, propertyName, null, ast.toString()); return new BindingRecord(DIRECTIVE, t, 0, ast, setter, null, directiveRecord); } diff --git a/modules/angular2/src/change_detection/change_detection.ts b/modules/angular2/src/change_detection/change_detection.ts index 62f77a58ac..df20cf3d9c 100644 --- a/modules/angular2/src/change_detection/change_detection.ts +++ b/modules/angular2/src/change_detection/change_detection.ts @@ -84,8 +84,6 @@ export const defaultKeyValueDiffers = CONST_EXPR(new KeyValueDiffers(keyValDiff) // dart2js. See https://github.com/dart-lang/sdk/issues/23630 for details. export var preGeneratedProtoDetectors: StringMap = {}; -export const PROTO_CHANGE_DETECTOR = CONST_EXPR(new OpaqueToken('ProtoChangeDetectors')); - /** * Implements change detection using a map of pregenerated proto detectors. */ @@ -93,14 +91,19 @@ export const PROTO_CHANGE_DETECTOR = CONST_EXPR(new OpaqueToken('ProtoChangeDete export class PreGeneratedChangeDetection extends ChangeDetection { _dynamicChangeDetection: ChangeDetection; _protoChangeDetectorFactories: StringMap; + _genConfig: ChangeDetectorGenConfig; - constructor(@Inject(PROTO_CHANGE_DETECTOR) @Optional() protoChangeDetectorsForTest?: - StringMap) { + constructor(config?: ChangeDetectorGenConfig, + protoChangeDetectorsForTest?: StringMap) { super(); this._dynamicChangeDetection = new DynamicChangeDetection(); this._protoChangeDetectorFactories = isPresent(protoChangeDetectorsForTest) ? protoChangeDetectorsForTest : preGeneratedProtoDetectors; + + this._genConfig = + isPresent(config) ? config : new ChangeDetectorGenConfig(assertionsEnabled(), + assertionsEnabled(), false); } static isSupported(): boolean { return PregenProtoChangeDetector.isSupported(); } @@ -112,11 +115,8 @@ export class PreGeneratedChangeDetection extends ChangeDetection { return this._dynamicChangeDetection.getProtoChangeDetector(id, definition); } + get genConfig(): ChangeDetectorGenConfig { return this._genConfig; } get generateDetectors(): boolean { return true; } - - get genConfig(): ChangeDetectorGenConfig { - return new ChangeDetectorGenConfig(assertionsEnabled(), assertionsEnabled()); - } } @@ -127,15 +127,21 @@ export class PreGeneratedChangeDetection extends ChangeDetection { */ @Injectable() export class DynamicChangeDetection extends ChangeDetection { + _genConfig: ChangeDetectorGenConfig; + + constructor(config?: ChangeDetectorGenConfig) { + super(); + this._genConfig = + isPresent(config) ? config : new ChangeDetectorGenConfig(assertionsEnabled(), + assertionsEnabled(), false); + } + getProtoChangeDetector(id: string, definition: ChangeDetectorDefinition): ProtoChangeDetector { return new DynamicProtoChangeDetector(definition); } + get genConfig(): ChangeDetectorGenConfig { return this._genConfig; } get generateDetectors(): boolean { return true; } - - get genConfig(): ChangeDetectorGenConfig { - return new ChangeDetectorGenConfig(assertionsEnabled(), assertionsEnabled()); - } } /** @@ -145,17 +151,21 @@ export class DynamicChangeDetection extends ChangeDetection { * {@link DynamicChangeDetection} and {@link PreGeneratedChangeDetection}. */ @Injectable() -@CONST() export class JitChangeDetection extends ChangeDetection { + _genConfig: ChangeDetectorGenConfig; + constructor(config?: ChangeDetectorGenConfig) { + super(); + this._genConfig = + isPresent(config) ? config : new ChangeDetectorGenConfig(assertionsEnabled(), + assertionsEnabled(), false); + } + static isSupported(): boolean { return JitProtoChangeDetector.isSupported(); } getProtoChangeDetector(id: string, definition: ChangeDetectorDefinition): ProtoChangeDetector { return new JitProtoChangeDetector(definition); } + get genConfig(): ChangeDetectorGenConfig { return this._genConfig; } get generateDetectors(): boolean { return true; } - - get genConfig(): ChangeDetectorGenConfig { - return new ChangeDetectorGenConfig(assertionsEnabled(), assertionsEnabled()); - } } 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 fe32b84ced..02d533e610 100644 --- a/modules/angular2/src/change_detection/change_detection_jit_generator.ts +++ b/modules/angular2/src/change_detection/change_detection_jit_generator.ts @@ -82,7 +82,6 @@ export class ChangeDetectorJITGenerator { return new ${this._typeName}(dispatcher); } `; - return new Function(ABSTRACT_CHANGE_DETECTOR, UTIL, classDefinition)(AbstractChangeDetector, ChangeDetectionUtil); } @@ -301,6 +300,7 @@ export class ChangeDetectorJITGenerator { var newValue = this._names.getLocalName(r.selfIndex); var oldValue = this._names.getFieldName(r.selfIndex); + var notifyDebug = this.genConfig.logBindingUpdate ? `this.logBindingUpdate(${newValue});` : ""; var br = r.bindingRecord; if (br.target.isDirective()) { @@ -309,12 +309,14 @@ export class ChangeDetectorJITGenerator { return ` ${this._genThrowOnChangeCheck(oldValue, newValue)} ${directiveProperty} = ${newValue}; + ${notifyDebug} ${IS_CHANGED_LOCAL} = true; `; } else { return ` ${this._genThrowOnChangeCheck(oldValue, newValue)} this.notifyDispatcher(${newValue}); + ${notifyDebug} `; } } diff --git a/modules/angular2/src/change_detection/dynamic_change_detector.ts b/modules/angular2/src/change_detection/dynamic_change_detector.ts index a8268c79fa..035cc6a874 100644 --- a/modules/angular2/src/change_detection/dynamic_change_detector.ts +++ b/modules/angular2/src/change_detection/dynamic_change_detector.ts @@ -179,6 +179,10 @@ export class DynamicChangeDetector extends AbstractChangeDetector { var directiveIndex = bindingRecord.directiveRecord.directiveIndex; bindingRecord.setter(this._getDirectiveFor(directiveIndex), change.currentValue); } + + if (this.genConfig.logBindingUpdate) { + super.logBindingUpdate(change.currentValue); + } } _addChange(bindingRecord: BindingRecord, change, changes) { diff --git a/modules/angular2/src/change_detection/interfaces.ts b/modules/angular2/src/change_detection/interfaces.ts index 3a3fdae57f..6560c7b0ed 100644 --- a/modules/angular2/src/change_detection/interfaces.ts +++ b/modules/angular2/src/change_detection/interfaces.ts @@ -27,7 +27,7 @@ import {ChangeDetectorRef} from './change_detector_ref'; * * # Example * ```javascript - * bootstrap(MyApp, [bind(ChangeDetection).toClass(DynamicChangeDetection)]); + * bootstrap(MyApp, [bind(ChangeDetection).toValue(new DynamicChangeDetection())]); * ``` */ @CONST() @@ -49,6 +49,7 @@ export class DebugContext { export interface ChangeDispatcher { getDebugContext(elementIndex: number, directiveIndex: DirectiveIndex): DebugContext; notifyOnBinding(bindingTarget: BindingTarget, value: any): void; + logBindingUpdate(bindingTarget: BindingTarget, value: any): void; notifyOnAllChangesDone(): void; } @@ -74,7 +75,8 @@ export interface ChangeDetector { export interface ProtoChangeDetector { instantiate(dispatcher: ChangeDispatcher): ChangeDetector; } export class ChangeDetectorGenConfig { - constructor(public genCheckNoChanges: boolean, public genDebugInfo: boolean) {} + constructor(public genCheckNoChanges: boolean, public genDebugInfo: boolean, + public logBindingUpdate: boolean) {} } export class ChangeDetectorDefinition { diff --git a/modules/angular2/src/core/application_common.ts b/modules/angular2/src/core/application_common.ts index c1a5b57107..324ef742c0 100644 --- a/modules/angular2/src/core/application_common.ts +++ b/modules/angular2/src/core/application_common.ts @@ -60,7 +60,6 @@ import {Renderer, RenderCompiler} from 'angular2/src/render/api'; import { DomRenderer, DOCUMENT, - DOM_REFLECT_PROPERTIES_AS_ATTRIBUTES, DefaultDomCompiler, APP_ID_RANDOM_BINDING, MAX_IN_MEMORY_ELEMENTS_PER_TEMPLATE, @@ -84,16 +83,15 @@ var _rootInjector: Injector; var _rootBindings = [bind(Reflector).toValue(reflector), TestabilityRegistry]; function _injectorBindings(appComponentType): List> { - var bestChangeDetection: Type = DynamicChangeDetection; + var bestChangeDetection = new DynamicChangeDetection(); if (PreGeneratedChangeDetection.isSupported()) { - bestChangeDetection = PreGeneratedChangeDetection; + bestChangeDetection = new PreGeneratedChangeDetection(); } else if (JitChangeDetection.isSupported()) { - bestChangeDetection = JitChangeDetection; + bestChangeDetection = new JitChangeDetection(); } return [ bind(DOCUMENT) .toValue(DOM.defaultDoc()), - bind(DOM_REFLECT_PROPERTIES_AS_ATTRIBUTES).toValue(false), bind(APP_COMPONENT).toValue(appComponentType), bind(APP_COMPONENT_REF_PROMISE) .toFactory( @@ -141,7 +139,7 @@ function _injectorBindings(appComponentType): List> { DEFAULT_PIPES, bind(IterableDiffers).toValue(defaultIterableDiffers), bind(KeyValueDiffers).toValue(defaultKeyValueDiffers), - bind(ChangeDetection).toClass(bestChangeDetection), + bind(ChangeDetection).toValue(bestChangeDetection), ViewLoader, DirectiveResolver, PipeResolver, diff --git a/modules/angular2/src/core/compiler/view.ts b/modules/angular2/src/core/compiler/view.ts index 1efcb35a2a..1a0b3fb478 100644 --- a/modules/angular2/src/core/compiler/view.ts +++ b/modules/angular2/src/core/compiler/view.ts @@ -32,9 +32,12 @@ import {RenderEventDispatcher} from 'angular2/src/render/api'; import {ViewRef, ProtoViewRef, internalView} from './view_ref'; import {ElementRef} from './element_ref'; import {ProtoPipes} from 'angular2/src/core/pipes/pipes'; +import {camelCaseToDashCase} from 'angular2/src/render/dom/util'; export {DebugContext} from 'angular2/src/change_detection/interfaces'; +const REFLECT_PREFIX: string = 'ng-reflect-'; + export class AppProtoViewMergeMapping { renderProtoViewRef: renderApi.RenderProtoViewRef; renderFragmentCount: number; @@ -193,6 +196,14 @@ export class AppView implements ChangeDispatcher, RenderEventDispatcher { } } + logBindingUpdate(b: BindingTarget, value: any): void { + if (b.isDirective() || b.isElementProperty()) { + var elementRef = this.elementRefs[this.elementOffset + b.elementIndex]; + this.renderer.setElementAttribute( + elementRef, `${REFLECT_PREFIX}${camelCaseToDashCase(b.name)}`, `${value}`); + } + } + notifyOnAllChangesDone(): void { var eiCount = this.proto.elementBinders.length; var ei = this.elementInjectors; diff --git a/modules/angular2/src/forms/directives/ng_model.ts b/modules/angular2/src/forms/directives/ng_model.ts index c598619434..4b3fc62418 100644 --- a/modules/angular2/src/forms/directives/ng_model.ts +++ b/modules/angular2/src/forms/directives/ng_model.ts @@ -72,4 +72,4 @@ export class NgModel extends NgControl { this.viewModel = newValue; ObservableWrapper.callNext(this.update, newValue); } -} +} \ No newline at end of file diff --git a/modules/angular2/src/render/dom/dom_renderer.ts b/modules/angular2/src/render/dom/dom_renderer.ts index e91206ecbf..bb3df68f52 100644 --- a/modules/angular2/src/render/dom/dom_renderer.ts +++ b/modules/angular2/src/render/dom/dom_renderer.ts @@ -35,7 +35,7 @@ import { import {TemplateCloner} from './template_cloner'; -import {DOCUMENT, DOM_REFLECT_PROPERTIES_AS_ATTRIBUTES} from './dom_tokens'; +import {DOCUMENT} from './dom_tokens'; const REFLECT_PREFIX: string = 'ng-reflect-'; @@ -43,15 +43,11 @@ const REFLECT_PREFIX: string = 'ng-reflect-'; @Injectable() export class DomRenderer extends Renderer { _document; - _reflectPropertiesAsAttributes: boolean; constructor(private _eventManager: EventManager, private _domSharedStylesHost: DomSharedStylesHost, - private _templateCloner: TemplateCloner, @Inject(DOCUMENT) document, - @Inject(DOM_REFLECT_PROPERTIES_AS_ATTRIBUTES) reflectPropertiesAsAttributes: - boolean) { + private _templateCloner: TemplateCloner, @Inject(DOCUMENT) document) { super(); - this._reflectPropertiesAsAttributes = reflectPropertiesAsAttributes; this._document = document; } @@ -165,11 +161,6 @@ export class DomRenderer extends Renderer { } var view = resolveInternalDomView(location.renderView); view.setElementProperty(location.renderBoundElementIndex, propertyName, propertyValue); - // Reflect the property value as an attribute value with ng-reflect- prefix. - if (this._reflectPropertiesAsAttributes) { - this.setElementAttribute(location, `${REFLECT_PREFIX}${camelCaseToDashCase(propertyName)}`, - `${propertyValue}`); - } } setElementAttribute(location: RenderElementRef, attributeName: string, attributeValue: string): diff --git a/modules/angular2/src/render/dom/dom_tokens.ts b/modules/angular2/src/render/dom/dom_tokens.ts index 8d8d1c0313..0c73ea5970 100644 --- a/modules/angular2/src/render/dom/dom_tokens.ts +++ b/modules/angular2/src/render/dom/dom_tokens.ts @@ -3,10 +3,6 @@ import {CONST_EXPR, StringWrapper, Math} from 'angular2/src/facade/lang'; export const DOCUMENT: OpaqueToken = CONST_EXPR(new OpaqueToken('DocumentToken')); -export const DOM_REFLECT_PROPERTIES_AS_ATTRIBUTES: OpaqueToken = - CONST_EXPR(new OpaqueToken('DomReflectPropertiesAsAttributes')); - - /** * A unique id (string) for an angular application. */ diff --git a/modules/angular2/src/test_lib/test_injector.ts b/modules/angular2/src/test_lib/test_injector.ts index bbd1c6d3dd..53cb95973f 100644 --- a/modules/angular2/src/test_lib/test_injector.ts +++ b/modules/angular2/src/test_lib/test_injector.ts @@ -54,7 +54,6 @@ import {RenderCompiler, Renderer} from 'angular2/src/render/api'; import { DomRenderer, DOCUMENT, - DOM_REFLECT_PROPERTIES_AS_ATTRIBUTES, DefaultDomCompiler, APP_ID, SharedStylesHost, @@ -111,7 +110,6 @@ function _getAppBindings() { bind(ElementSchemaRegistry).toValue(new DomElementSchemaRegistry()), DomSharedStylesHost, bind(SharedStylesHost).toAlias(DomSharedStylesHost), - bind(DOM_REFLECT_PROPERTIES_AS_ATTRIBUTES).toValue(false), ProtoViewFactory, AppViewPool, AppViewManager, @@ -125,7 +123,7 @@ function _getAppBindings() { DEFAULT_PIPES, bind(IterableDiffers).toValue(defaultIterableDiffers), bind(KeyValueDiffers).toValue(defaultKeyValueDiffers), - bind(ChangeDetection).toClass(DynamicChangeDetection), + bind(ChangeDetection).toValue(new DynamicChangeDetection()), Log, ViewLoader, DynamicComponentLoader, diff --git a/modules/angular2/src/transform/common/options.dart b/modules/angular2/src/transform/common/options.dart index 1e8ff41bd4..b681a8b540 100644 --- a/modules/angular2/src/transform/common/options.dart +++ b/modules/angular2/src/transform/common/options.dart @@ -10,6 +10,7 @@ const CUSTOM_ANNOTATIONS_PARAM = 'custom_annotations'; const ENTRY_POINT_PARAM = 'entry_points'; const FORMAT_CODE_PARAM = 'format_code'; const GENERATE_CHANGE_DETECTORS_PARAM = 'generate_change_detectors'; +const REFLECT_PROPERTIES_AS_ATTRIBUTES = 'reflectPropertiesAsAttributes'; const INIT_REFLECTOR_PARAM = 'init_reflector'; const INLINE_VIEWS_PARAM = 'inline_views'; const MIRROR_MODE_PARAM = 'mirror_mode'; @@ -43,6 +44,8 @@ class TransformerOptions { /// Whether to create change detector classes for discovered `@View`s. final bool generateChangeDetectors; + final bool reflectPropertiesAsAttributes; + /// The number of phases to spend optimizing output size. /// Each additional phase adds time to the transformation but may decrease /// final output size. There is a limit beyond which this will no longer @@ -66,6 +69,7 @@ class TransformerOptions { this.annotationMatcher, this.optimizationPhases, {this.generateChangeDetectors, + this.reflectPropertiesAsAttributes, this.inlineViews, this.formatCode}); @@ -78,6 +82,7 @@ class TransformerOptions { int optimizationPhases: DEFAULT_OPTIMIZATION_PHASES, bool inlineViews: true, bool generateChangeDetectors: true, + bool reflectPropertiesAsAttributes: true, bool formatCode: false}) { if (reflectionEntryPoints == null || reflectionEntryPoints.isEmpty) { reflectionEntryPoints = entryPoints; @@ -94,6 +99,7 @@ class TransformerOptions { annotationMatcher, optimizationPhases, generateChangeDetectors: generateChangeDetectors, + reflectPropertiesAsAttributes: reflectPropertiesAsAttributes, inlineViews: inlineViews, formatCode: formatCode); } diff --git a/modules/angular2/src/transform/common/options_reader.dart b/modules/angular2/src/transform/common/options_reader.dart index 565bb2cd1d..3b5edb3430 100644 --- a/modules/angular2/src/transform/common/options_reader.dart +++ b/modules/angular2/src/transform/common/options_reader.dart @@ -15,6 +15,8 @@ TransformerOptions parseBarbackSettings(BarbackSettings settings) { var inlineViews = _readBool(config, INLINE_VIEWS_PARAM, defaultValue: true); var generateChangeDetectors = _readBool(config, GENERATE_CHANGE_DETECTORS_PARAM, defaultValue: true); + var reflectPropertiesAsAttributes = + _readBool(config, REFLECT_PROPERTIES_AS_ATTRIBUTES, defaultValue: false); var formatCode = _readBool(config, FORMAT_CODE_PARAM, defaultValue: false); String mirrorModeVal = config.containsKey(MIRROR_MODE_PARAM) ? config[MIRROR_MODE_PARAM] : ''; @@ -41,6 +43,7 @@ TransformerOptions parseBarbackSettings(BarbackSettings settings) { customAnnotationDescriptors: _readCustomAnnotations(config), optimizationPhases: optimizationPhases, generateChangeDetectors: generateChangeDetectors, + reflectPropertiesAsAttributes: reflectPropertiesAsAttributes, formatCode: formatCode); } 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 c8dad2c33e..ca33c8aa28 100644 --- a/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart +++ b/modules/angular2/src/transform/template_compiler/change_detector_codegen.dart @@ -399,6 +399,7 @@ class _CodegenState { var newValue = _names.getLocalName(r.selfIndex); var oldValue = _names.getFieldName(r.selfIndex); + var notifyDebug = _genConfig.logBindingUpdate ? "this.logBindingUpdate(${newValue});" : ""; var br = r.bindingRecord; if (br.target.isDirective()) { @@ -407,12 +408,14 @@ class _CodegenState { return ''' ${_genThrowOnChangeCheck(oldValue, newValue)} $directiveProperty = $newValue; + ${notifyDebug} $_IS_CHANGED_LOCAL = true; '''; } else { return ''' ${_genThrowOnChangeCheck(oldValue, newValue)} this.notifyDispatcher(${newValue}); + ${notifyDebug} '''; } } diff --git a/modules/angular2/src/transform/template_compiler/generator.dart b/modules/angular2/src/transform/template_compiler/generator.dart index acc782092a..6f4997fe07 100644 --- a/modules/angular2/src/transform/template_compiler/generator.dart +++ b/modules/angular2/src/transform/template_compiler/generator.dart @@ -37,7 +37,7 @@ import 'view_definition_creator.dart'; /// This method assumes a {@link DomAdapter} has been registered. Future processTemplates(AssetReader reader, AssetId entryPoint, {bool generateRegistrations: true, - bool generateChangeDetectors: true}) async { + bool generateChangeDetectors: true, bool reflectPropertiesAsAttributes: false}) async { var viewDefResults = await createViewDefinitions(reader, entryPoint); // Note: TemplateCloner(-1) never serializes Nodes into strings. // we might want to change this to TemplateCloner(0) to force the serialization @@ -58,7 +58,8 @@ Future processTemplates(AssetReader reader, AssetId entryPoint, } if (generateChangeDetectors) { var saved = reflector.reflectionCapabilities; - var genConfig = new ChangeDetectorGenConfig(assertionsEnabled(), assertionsEnabled()); + var genConfig = new ChangeDetectorGenConfig(assertionsEnabled(), assertionsEnabled(), reflectPropertiesAsAttributes); + reflector.reflectionCapabilities = const NullReflectionCapabilities(); var defs = getChangeDetectorDefinitions(viewDefEntry.hostMetadata, protoView, viewDefEntry.viewDef.directives, genConfig); diff --git a/modules/angular2/src/transform/template_compiler/transformer.dart b/modules/angular2/src/transform/template_compiler/transformer.dart index 25b7ba5f4f..6135cf64c7 100644 --- a/modules/angular2/src/transform/template_compiler/transformer.dart +++ b/modules/angular2/src/transform/template_compiler/transformer.dart @@ -33,7 +33,8 @@ class TemplateCompiler extends Transformer { var id = transform.primaryInput.id; var reader = new AssetReader.fromTransform(transform); var transformedCode = formatter.format(await processTemplates(reader, id, - generateChangeDetectors: options.generateChangeDetectors)); + generateChangeDetectors: options.generateChangeDetectors, + reflectPropertiesAsAttributes: options.reflectPropertiesAsAttributes)); transform.addOutput(new Asset.fromString(id, transformedCode)); }); } diff --git a/modules/angular2/src/web-workers/ui/di_bindings.ts b/modules/angular2/src/web-workers/ui/di_bindings.ts index accaeb88a4..c5b9c0a82c 100644 --- a/modules/angular2/src/web-workers/ui/di_bindings.ts +++ b/modules/angular2/src/web-workers/ui/di_bindings.ts @@ -1,7 +1,6 @@ // TODO (jteplitz602): This whole file is nearly identical to core/application.ts. // There should be a way to refactor application so that this file is unnecessary. See #3277 import {Injector, bind, Binding} from "angular2/di"; -import {Type, isBlank, isPresent} from "angular2/src/facade/lang"; import {Reflector, reflector} from 'angular2/src/reflection/reflection'; import {List, ListWrapper} from 'angular2/src/facade/collection'; import { @@ -24,7 +23,6 @@ import {AppRootUrl} from 'angular2/src/services/app_root_url'; import { DomRenderer, DOCUMENT, - DOM_REFLECT_PROPERTIES_AS_ATTRIBUTES, DefaultDomCompiler, APP_ID_RANDOM_BINDING, MAX_IN_MEMORY_ELEMENTS_PER_TEMPLATE, @@ -75,12 +73,12 @@ var _rootBindings = [bind(Reflector).toValue(reflector)]; // TODO: This code is nearly identitcal to core/application. There should be a way to only write it // once -function _injectorBindings(): List> { - var bestChangeDetection: Type = DynamicChangeDetection; +function _injectorBindings(): List { + var bestChangeDetection = new DynamicChangeDetection(); if (PreGeneratedChangeDetection.isSupported()) { - bestChangeDetection = PreGeneratedChangeDetection; + bestChangeDetection = new PreGeneratedChangeDetection(); } else if (JitChangeDetection.isSupported()) { - bestChangeDetection = JitChangeDetection; + bestChangeDetection = new JitChangeDetection(); } return [ @@ -94,7 +92,6 @@ function _injectorBindings(): List> { return new EventManager(plugins, ngZone); }, [NgZone]), - bind(DOM_REFLECT_PROPERTIES_AS_ATTRIBUTES).toValue(false), DomRenderer, bind(Renderer).toAlias(DomRenderer), APP_ID_RANDOM_BINDING, @@ -119,7 +116,7 @@ function _injectorBindings(): List> { CompilerCache, ViewResolver, DEFAULT_PIPES, - bind(ChangeDetection).toClass(bestChangeDetection), + bind(ChangeDetection).toValue(bestChangeDetection), ViewLoader, DirectiveResolver, Parser, diff --git a/modules/angular2/src/web-workers/worker/application_common.ts b/modules/angular2/src/web-workers/worker/application_common.ts index 3e70625d72..f422fa4d73 100644 --- a/modules/angular2/src/web-workers/worker/application_common.ts +++ b/modules/angular2/src/web-workers/worker/application_common.ts @@ -79,11 +79,11 @@ class PrintLogger { function _injectorBindings(appComponentType, bus: MessageBusInterface, initData: StringMap): List> { - var bestChangeDetection: Type = DynamicChangeDetection; + var bestChangeDetection = new DynamicChangeDetection(); if (PreGeneratedChangeDetection.isSupported()) { - bestChangeDetection = PreGeneratedChangeDetection; + bestChangeDetection = new PreGeneratedChangeDetection(); } else if (JitChangeDetection.isSupported()) { - bestChangeDetection = JitChangeDetection; + bestChangeDetection = new JitChangeDetection(); } return [ bind(APP_COMPONENT) @@ -123,7 +123,7 @@ function _injectorBindings(appComponentType, bus: MessageBusInterface, DEFAULT_PIPES, bind(IterableDiffers).toValue(defaultIterableDiffers), bind(KeyValueDiffers).toValue(defaultKeyValueDiffers), - bind(ChangeDetection).toClass(bestChangeDetection), + bind(ChangeDetection).toValue(bestChangeDetection), DirectiveResolver, PipeResolver, Parser, diff --git a/modules/angular2/test/change_detection/change_detection_spec.ts b/modules/angular2/test/change_detection/change_detection_spec.ts index a019b15616..e180097891 100644 --- a/modules/angular2/test/change_detection/change_detection_spec.ts +++ b/modules/angular2/test/change_detection/change_detection_spec.ts @@ -28,13 +28,13 @@ export function main() { it("should return a proto change detector when one is available", () => { var map = {'id': (def) => proto}; - var cd = new PreGeneratedChangeDetection(map); + var cd = new PreGeneratedChangeDetection(null, map); expect(cd.getProtoChangeDetector('id', def)).toBe(proto) }); it("should delegate to dynamic change detection otherwise", () => { - var cd = new PreGeneratedChangeDetection({}); + var cd = new PreGeneratedChangeDetection(null, {}); expect(cd.getProtoChangeDetector('id', def)).toBeAnInstanceOf(DynamicProtoChangeDetector); }); }); diff --git a/modules/angular2/test/change_detection/change_detector_config.ts b/modules/angular2/test/change_detection/change_detector_config.ts index 41c049431b..eab0fa6d3b 100644 --- a/modules/angular2/test/change_detection/change_detector_config.ts +++ b/modules/angular2/test/change_detection/change_detector_config.ts @@ -66,7 +66,7 @@ export var PROP_NAME = 'propName'; * In this case, we expect `id` and `expression` to be the same string. */ export function getDefinition(id: string): TestDefinition { - var genConfig = new ChangeDetectorGenConfig(true, true); + var genConfig = new ChangeDetectorGenConfig(true, true, true); var testDef = null; if (StringMapWrapper.contains(_ExpressionWithLocals.availableDefinitions, id)) { let val = StringMapWrapper.get(_ExpressionWithLocals.availableDefinitions, id); @@ -110,6 +110,12 @@ export function getDefinition(id: string): TestDefinition { var records = _createBindingRecords("a"); let cdDef = new ChangeDetectorDefinition(id, "ON_PUSH_OBSERVE", [], records, [], [], genConfig); testDef = new TestDefinition(id, cdDef, null); + + } else if (id == "updateElementProduction") { + var genConfig = new ChangeDetectorGenConfig(false, false, false); + var records = _createBindingRecords("name"); + let cdDef = new ChangeDetectorDefinition(id, null, [], records, [], [], genConfig); + testDef = new TestDefinition(id, cdDef, null); } @@ -138,7 +144,7 @@ export function getAllDefinitions(): List { ListWrapper.concat(allDefs, StringMapWrapper.keys(_DirectiveUpdating.availableDefinitions)); allDefs = ListWrapper.concat(allDefs, _availableEventDefinitions); allDefs = ListWrapper.concat(allDefs, _availableHostEventDefinitions); - allDefs = ListWrapper.concat(allDefs, ["onPushObserve"]); + allDefs = ListWrapper.concat(allDefs, ["onPushObserve", "updateElementProduction"]); return ListWrapper.map(allDefs, (id) => getDefinition(id)); } @@ -150,7 +156,7 @@ class _ExpressionWithLocals { var variableBindings = _convertLocalsToVariableBindings(this.locals); var bindingRecords = _createBindingRecords(this._expression); var directiveRecords = []; - var genConfig = new ChangeDetectorGenConfig(true, true); + var genConfig = new ChangeDetectorGenConfig(true, true, true); return new ChangeDetectorDefinition('(empty id)', strategy, variableBindings, bindingRecords, [], directiveRecords, genConfig); } @@ -210,7 +216,7 @@ class _ExpressionWithMode { _createHostEventRecords("(host-event)='false'", dirRecordWithOnPush)) } - var genConfig = new ChangeDetectorGenConfig(true, true); + var genConfig = new ChangeDetectorGenConfig(true, true, true); return new ChangeDetectorDefinition('(empty id)', this._strategy, variableBindings, bindingRecords, eventRecords, directiveRecords, genConfig); @@ -236,7 +242,7 @@ class _DirectiveUpdating { createChangeDetectorDefinition(): ChangeDetectorDefinition { var strategy = null; var variableBindings = []; - var genConfig = new ChangeDetectorGenConfig(true, true); + var genConfig = new ChangeDetectorGenConfig(true, true, true); return new ChangeDetectorDefinition('(empty id)', strategy, variableBindings, this._bindingRecords, [], this._directiveRecords, diff --git a/modules/angular2/test/change_detection/change_detector_spec.ts b/modules/angular2/test/change_detection/change_detector_spec.ts index 1c8893f89e..86dea2eece 100644 --- a/modules/angular2/test/change_detection/change_detector_spec.ts +++ b/modules/angular2/test/change_detection/change_detector_spec.ts @@ -535,6 +535,31 @@ export function main() { }); }); + describe("logBindingUpdate", () => { + it('should be called for element updates in the dev mode', () => { + var person = new Person('bob'); + var val = _createChangeDetector('name', person); + val.changeDetector.detectChanges(); + expect(val.dispatcher.debugLog).toEqual(['propName=bob']); + }); + + it('should be called for directive updates in the dev mode', () => { + var val = _createWithoutHydrate('directNoDispatcher'); + val.changeDetector.hydrate(_DEFAULT_CONTEXT, null, + new FakeDirectives([new TestDirective()], []), null); + val.changeDetector.detectChanges(); + expect(val.dispatcher.debugLog).toEqual(["a=42"]); + }); + + it('should not be called in the prod mode', () => { + var person = new Person('bob'); + var val = _createChangeDetector('updateElementProduction', person); + val.changeDetector.detectChanges(); + expect(val.dispatcher.debugLog).toEqual([]); + }); + + }); + describe('reading directives', () => { it('should read directive properties', () => { var directive = new TestDirective(); @@ -1123,7 +1148,8 @@ class FakeDirectives { } class TestDispatcher implements ChangeDispatcher { - log: List; + log: string[]; + debugLog: string[]; loggedValues: List; onAllChangesDoneCalled: boolean = false; @@ -1131,6 +1157,7 @@ class TestDispatcher implements ChangeDispatcher { clear() { this.log = []; + this.debugLog = []; this.loggedValues = []; this.onAllChangesDoneCalled = true; } @@ -1140,6 +1167,8 @@ class TestDispatcher implements ChangeDispatcher { this.loggedValues.push(value); } + logBindingUpdate(target, value) { this.debugLog.push(`${target.name}=${this._asString(value)}`); } + notifyOnAllChangesDone() { this.onAllChangesDoneCalled = true; } getDebugContext(a, b) { return null; } diff --git a/modules/angular2/test/core/compiler/integration_spec.ts b/modules/angular2/test/core/compiler/integration_spec.ts index aba8405e68..693bd90f31 100644 --- a/modules/angular2/test/core/compiler/integration_spec.ts +++ b/modules/angular2/test/core/compiler/integration_spec.ts @@ -58,7 +58,10 @@ import { import { PipeTransform, ChangeDetectorRef, - ON_PUSH + ON_PUSH, + ChangeDetection, + DynamicChangeDetection, + ChangeDetectorGenConfig } from 'angular2/src/change_detection/change_detection'; import {Directive, Component, View, ViewMetadata, Attribute, Query, Pipe} from 'angular2/metadata'; @@ -1518,6 +1521,30 @@ export function main() { })); }); + describe('logging property updates', () => { + beforeEachBindings(() => [ + bind(ChangeDetection) + .toValue(new DynamicChangeDetection(new ChangeDetectorGenConfig(true, true, true))) + ]); + + it('should reflect property values as attributes', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { + var tpl = '
' + + '
' + + '
'; + tcb.overrideView(MyComp, new ViewMetadata({template: tpl, directives: [MyDir]})) + + .createAsync(MyComp) + .then((rootTC) => { + rootTC.componentInstance.ctxProp = 'hello'; + rootTC.detectChanges(); + + expect(DOM.getInnerHTML(rootTC.nativeElement)) + .toContain('ng-reflect-dir-prop="hello"'); + async.done(); + }); + })); + }); describe('different proto view storages', () => { function runWithMode(mode: string) { diff --git a/modules/angular2/test/render/dom/dom_renderer_integration_spec.ts b/modules/angular2/test/render/dom/dom_renderer_integration_spec.ts index b280131eff..40c5342fb7 100644 --- a/modules/angular2/test/render/dom/dom_renderer_integration_spec.ts +++ b/modules/angular2/test/render/dom/dom_renderer_integration_spec.ts @@ -24,8 +24,6 @@ import { RenderViewRef, ViewEncapsulation } from 'angular2/src/render/api'; -import {DOM_REFLECT_PROPERTIES_AS_ATTRIBUTES} from 'angular2/src/render/dom/dom_tokens'; -import {bind} from 'angular2/di'; export function main() { describe('DomRenderer integration', () => { @@ -106,72 +104,6 @@ export function main() { }); })); - - it('should NOT reflect property values as attributes if flag is NOT set', - inject([AsyncTestCompleter, DomTestbed], (async, tb) => { - tb.compileAndMerge(someComponent, - [ - new ViewDefinition({ - componentId: 'someComponent', - template: '', - directives: [] - }) - ]) - .then((protoViewMergeMappings) => { - var rootView = tb.createView(protoViewMergeMappings); - var el = DOM.childNodes(rootView.hostElement)[0]; - tb.renderer.setElementProperty(elRef(rootView.viewRef, 1), 'maxLength', '20'); - expect(DOM.getAttribute(el, 'ng-reflect-max-length')) - .toEqual(null); - - async.done(); - }); - })); - - describe('reflection', () => { - beforeEachBindings(() => [bind(DOM_REFLECT_PROPERTIES_AS_ATTRIBUTES).toValue(true)]); - - it('should reflect property values as attributes if flag is set', - inject([AsyncTestCompleter, DomTestbed], (async, tb) => { - tb.compileAndMerge(someComponent, - [ - new ViewDefinition({ - componentId: 'someComponent', - template: '', - directives: [] - }) - ]) - .then((protoViewMergeMappings) => { - var rootView = tb.createView(protoViewMergeMappings); - var el = DOM.childNodes(rootView.hostElement)[0]; - tb.renderer.setElementProperty(elRef(rootView.viewRef, 1), 'maxLength', '20'); - expect(DOM.getAttribute(el, 'ng-reflect-max-length')) - .toEqual('20'); - async.done(); - }); - })); - - it('should reflect non-string property values as attributes if flag is set', - inject([AsyncTestCompleter, DomTestbed], (async, tb) => { - tb.compileAndMerge(someComponent, - [ - new ViewDefinition({ - componentId: 'someComponent', - template: '', - directives: [] - }) - ]) - .then((protoViewMergeMappings) => { - var rootView = tb.createView(protoViewMergeMappings); - var el = DOM.childNodes(rootView.hostElement)[0]; - tb.renderer.setElementProperty(elRef(rootView.viewRef, 1), 'maxLength', 20); - expect(DOM.getAttribute(el, 'ng-reflect-max-length')) - .toEqual('20'); - async.done(); - }); - })); - }); - if (DOM.supportsDOMEvents()) { it('should call actions on the element independent of the compilation', inject([AsyncTestCompleter, DomTestbed], (async, tb: DomTestbed) => { 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 68a9b41211..2f47337566 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 @@ -58,6 +58,7 @@ class _MyComponent_ChangeDetector0 } this.notifyDispatcher(l_interpolate1); + this.logBindingUpdate(l_interpolate1); this.interpolate1 = l_interpolate1; } diff --git a/modules/benchmarks/src/change_detection/change_detection_benchmark.ts b/modules/benchmarks/src/change_detection/change_detection_benchmark.ts index 2678fdbc7e..f033b5219d 100644 --- a/modules/benchmarks/src/change_detection/change_detection_benchmark.ts +++ b/modules/benchmarks/src/change_detection/change_detection_benchmark.ts @@ -12,6 +12,7 @@ import { DynamicChangeDetection, JitChangeDetection, ChangeDetectorDefinition, + ChangeDetectorGenConfig, BindingRecord, DirectiveRecord, DirectiveIndex, @@ -249,8 +250,9 @@ function setUpChangeDetection(changeDetection: ChangeDetection, iterations, obje var dispatcher = new DummyDispatcher(); var parser = new Parser(new Lexer()); + var genConfig = new ChangeDetectorGenConfig(false, false, false); var parentProto = changeDetection.getProtoChangeDetector( - "id", new ChangeDetectorDefinition('parent', null, [], [], [], [], false)); + "id", new ChangeDetectorDefinition('parent', null, [], [], [], [], genConfig)); var parentCd = parentProto.instantiate(dispatcher); var directiveRecord = new DirectiveRecord({directiveIndex: new DirectiveIndex(0, 0)}); @@ -279,7 +281,7 @@ function setUpChangeDetection(changeDetection: ChangeDetection, iterations, obje var proto = changeDetection.getProtoChangeDetector( "id", - new ChangeDetectorDefinition("proto", null, [], bindings, [], [directiveRecord], false)); + new ChangeDetectorDefinition("proto", null, [], bindings, [], [directiveRecord], genConfig)); var targetObj = new Obj(); parentCd.hydrate(object, null, new FakeDirectives(targetObj), null); @@ -385,6 +387,7 @@ class DummyDispatcher implements ChangeDispatcher { getDebugContext(elementIndex: number, directiveIndex: DirectiveIndex): DebugContext { throw "getDebugContext not implemented."; } - notifyOnBinding(bindingRecord, newValue) { throw "Should not be used"; } + notifyOnBinding(bindingTarget, newValue) { throw "Should not be used"; } + logBindingUpdate(bindingTarget, newValue) { throw "Should not be used"; } notifyOnAllChangesDone() {} }