From 9240b09011fd792829b509cfddb702374855816c Mon Sep 17 00:00:00 2001 From: vsavkin Date: Fri, 6 Feb 2015 13:19:47 -0800 Subject: [PATCH] refactor(directives): directives use declare that they listen to onChange in the annotations --- .../src/core/annotations/annotations.js | 11 +++++-- .../src/core/compiler/element_injector.js | 33 ++++++++++++++----- modules/angular2/src/core/compiler/view.js | 14 +++++--- modules/angular2/src/directives/foreach.js | 3 +- .../test/core/annotations/annotations_spec.js | 23 +++++++++++++ .../core/compiler/element_injector_spec.js | 19 ++++++++--- .../angular2/test/core/compiler/view_spec.js | 15 +++++---- 7 files changed, 91 insertions(+), 27 deletions(-) create mode 100644 modules/angular2/test/core/annotations/annotations_spec.js diff --git a/modules/angular2/src/core/annotations/annotations.js b/modules/angular2/src/core/annotations/annotations.js index 285b74ce4e..9a40aa376d 100644 --- a/modules/angular2/src/core/annotations/annotations.js +++ b/modules/angular2/src/core/annotations/annotations.js @@ -1,5 +1,5 @@ -import {ABSTRACT, CONST, normalizeBlank} from 'angular2/src/facade/lang'; -import {List} from 'angular2/src/facade/collection'; +import {ABSTRACT, CONST, normalizeBlank, isPresent} from 'angular2/src/facade/lang'; +import {ListWrapper, List} from 'angular2/src/facade/collection'; import {TemplateConfig} from './template_config'; @ABSTRACT() @@ -30,6 +30,10 @@ export class Directive { this.bind = bind; this.lifecycle = lifecycle; } + + hasLifecycleHook(hook:string):boolean { + return isPresent(this.lifecycle) ? ListWrapper.contains(this.lifecycle, hook) : false; + } } export class Component extends Directive { @@ -133,4 +137,5 @@ export class Template extends Directive { } } -export var onDestroy = "onDestroy"; +export const onDestroy = "onDestroy"; +export const onChange = "onChange"; diff --git a/modules/angular2/src/core/compiler/element_injector.js b/modules/angular2/src/core/compiler/element_injector.js index 46cfbd2da4..b154e5148e 100644 --- a/modules/angular2/src/core/compiler/element_injector.js +++ b/modules/angular2/src/core/compiler/element_injector.js @@ -4,12 +4,11 @@ import {List, ListWrapper, MapWrapper} from 'angular2/src/facade/collection'; import {Injector, Key, Dependency, bind, Binding, NoProviderError, ProviderError, CyclicDependencyError} from 'angular2/di'; import {Parent, Ancestor} from 'angular2/src/core/annotations/visibility'; import {EventEmitter} from 'angular2/src/core/annotations/events'; -import {onDestroy} from 'angular2/src/core/annotations/annotations'; import {View, ProtoView} from 'angular2/src/core/compiler/view'; import {LightDom, SourceLightDom, DestinationLightDom} from 'angular2/src/core/compiler/shadow_dom_emulation/light_dom'; import {ViewPort} from 'angular2/src/core/compiler/viewport'; import {NgElement} from 'angular2/src/core/dom/element'; -import {Directive} from 'angular2/src/core/annotations/annotations' +import {Directive, onChange, onDestroy} from 'angular2/src/core/annotations/annotations' import {BindingPropagationConfig} from 'angular2/src/core/compiler/binding_propagation_config' var _MAX_DIRECTIVE_CONSTRUCTION_COUNTER = 10; @@ -123,18 +122,19 @@ export class DirectiveDependency extends Dependency { export class DirectiveBinding extends Binding { callOnDestroy:boolean; + callOnChange:boolean; + onCheck:boolean; - constructor(key:Key, factory:Function, dependencies:List, providedAsPromise:boolean, callOnDestroy:boolean) { + constructor(key:Key, factory:Function, dependencies:List, providedAsPromise:boolean, annotation:Directive) { super(key, factory, dependencies, providedAsPromise); - this.callOnDestroy = callOnDestroy; + this.callOnDestroy = isPresent(annotation) && annotation.hasLifecycleHook(onDestroy); + this.callOnChange = isPresent(annotation) && annotation.hasLifecycleHook(onChange); + //this.onCheck = isPresent(annotation) && annotation.hasLifecycleHook(onCheck); } static createFromBinding(b:Binding, annotation:Directive):Binding { var deps = ListWrapper.map(b.dependencies, DirectiveDependency.createFrom); - var callOnDestroy = isPresent(annotation) && isPresent(annotation.lifecycle) ? - ListWrapper.contains(annotation.lifecycle, onDestroy) : - false; - return new DirectiveBinding(b.key, b.factory, deps, b.providedAsPromise, callOnDestroy); + return new DirectiveBinding(b.key, b.factory, deps, b.providedAsPromise, annotation); } static createFromType(type:Type, annotation:Directive):Binding { @@ -567,7 +567,7 @@ export class ElementInjector extends TreeNode { return _undefined; } - getAtIndex(index:int) { + getDirectiveAtIndex(index:int) { if (index == 0) return this._obj0; if (index == 1) return this._obj1; if (index == 2) return this._obj2; @@ -581,6 +581,21 @@ export class ElementInjector extends TreeNode { throw new OutOfBoundsAccess(index); } + getDirectiveBindingAtIndex(index:int) { + var p = this._proto; + if (index == 0) return p._binding0; + if (index == 1) return p._binding1; + if (index == 2) return p._binding2; + if (index == 3) return p._binding3; + if (index == 4) return p._binding4; + if (index == 5) return p._binding5; + if (index == 6) return p._binding6; + if (index == 7) return p._binding7; + if (index == 8) return p._binding8; + if (index == 9) return p._binding9; + throw new OutOfBoundsAccess(index); + } + hasInstances() { return this._constructionCounter > 0; } diff --git a/modules/angular2/src/core/compiler/view.js b/modules/angular2/src/core/compiler/view.js index 27f59d90fa..923818b7a4 100644 --- a/modules/angular2/src/core/compiler/view.js +++ b/modules/angular2/src/core/compiler/view.js @@ -12,7 +12,6 @@ import {FIELD, IMPLEMENTS, int, isPresent, isBlank, BaseException} from 'angular import {Injector} from 'angular2/di'; import {NgElement} from 'angular2/src/core/dom/element'; import {ViewPort} from './viewport'; -import {OnChange} from './interfaces'; import {Content} from './shadow_dom_emulation/content_tag'; import {LightDom, DestinationLightDom} from './shadow_dom_emulation/light_dom'; import {ShadowDomStrategy} from './shadow_dom_strategy'; @@ -211,7 +210,9 @@ export class View { _notifyDirectiveAboutChanges(groupMemento, records:List) { var dir = groupMemento.directive(this.elementInjectors); - if (dir instanceof OnChange) { + var binding = groupMemento.directiveBinding(this.elementInjectors); + + if (binding.callOnChange) { dir.onChange(this._collectChanges(records)); } } @@ -552,7 +553,7 @@ export class DirectivePropertyMemento { invoke(record:ChangeRecord, elementInjectors:List) { var elementInjector:ElementInjector = elementInjectors[this._elementInjectorIndex]; - var directive = elementInjector.getAtIndex(this._directiveIndex); + var directive = elementInjector.getDirectiveAtIndex(this._directiveIndex); this._setter(directive, record.currentValue); } } @@ -581,7 +582,12 @@ class DirectivePropertyGroupMemento { directive(elementInjectors:List) { var elementInjector:ElementInjector = elementInjectors[this._elementInjectorIndex]; - return elementInjector.getAtIndex(this._directiveIndex); + return elementInjector.getDirectiveAtIndex(this._directiveIndex); + } + + directiveBinding(elementInjectors:List) { + var elementInjector:ElementInjector = elementInjectors[this._elementInjectorIndex]; + return elementInjector.getDirectiveBindingAtIndex(this._directiveIndex); } } diff --git a/modules/angular2/src/directives/foreach.js b/modules/angular2/src/directives/foreach.js index 16de07fe31..8d13f15081 100644 --- a/modules/angular2/src/directives/foreach.js +++ b/modules/angular2/src/directives/foreach.js @@ -1,4 +1,4 @@ -import {Template} from 'angular2/src/core/annotations/annotations'; + import {Template, onChange} from 'angular2/src/core/annotations/annotations'; import {OnChange} from 'angular2/src/core/compiler/interfaces'; import {ViewPort} from 'angular2/src/core/compiler/viewport'; import {View} from 'angular2/src/core/compiler/view'; @@ -7,6 +7,7 @@ import {ListWrapper} from 'angular2/src/facade/collection'; @Template({ selector: '[foreach][in]', + lifecycle: [onChange], bind: { 'in': 'iterable[]' } diff --git a/modules/angular2/test/core/annotations/annotations_spec.js b/modules/angular2/test/core/annotations/annotations_spec.js new file mode 100644 index 0000000000..67f97f080f --- /dev/null +++ b/modules/angular2/test/core/annotations/annotations_spec.js @@ -0,0 +1,23 @@ +import {ddescribe, describe, it, iit, expect, beforeEach} from 'angular2/test_lib'; +import {Directive, onChange} from 'angular2/src/core/annotations/annotations'; + +export function main() { + describe("Directive", () => { + describe("lifecycle", () => { + it("should be false when no lifecycle specified", () => { + var d = new Directive(); + expect(d.hasLifecycleHook(onChange)).toBe(false); + }); + + it("should be false when the lifecycle does not contain the hook", () => { + var d = new Directive({lifecycle:[]}); + expect(d.hasLifecycleHook(onChange)).toBe(false); + }); + + it("should be true otherwise", () => { + var d = new Directive({lifecycle:[onChange]}); + expect(d.hasLifecycleHook(onChange)).toBe(true); + }); + }); + }); +} diff --git a/modules/angular2/test/core/compiler/element_injector_spec.js b/modules/angular2/test/core/compiler/element_injector_spec.js index 52c6e37fae..a0ed9f6fd5 100644 --- a/modules/angular2/test/core/compiler/element_injector_spec.js +++ b/modules/angular2/test/core/compiler/element_injector_spec.js @@ -330,14 +330,25 @@ export function main() { expect(inj.get(SimpleDirective)).toBeAnInstanceOf(SimpleDirective); }); - it("should allow for direct access using getAtIndex", function () { + it("should allow for direct access using getDirectiveAtIndex", function () { var inj = injector([ DirectiveBinding.createFromBinding(bind(SimpleDirective).toClass(SimpleDirective), null) ]); - expect(inj.getAtIndex(0)).toBeAnInstanceOf(SimpleDirective); - expect(() => inj.getAtIndex(-1)).toThrowError( + expect(inj.getDirectiveAtIndex(0)).toBeAnInstanceOf(SimpleDirective); + expect(() => inj.getDirectiveAtIndex(-1)).toThrowError( 'Index -1 is out-of-bounds.'); - expect(() => inj.getAtIndex(10)).toThrowError( + expect(() => inj.getDirectiveAtIndex(10)).toThrowError( + 'Index 10 is out-of-bounds.'); + }); + + it("should allow for direct access using getBindingAtIndex", function () { + var inj = injector([ + DirectiveBinding.createFromBinding(bind(SimpleDirective).toClass(SimpleDirective), null) + ]); + expect(inj.getDirectiveBindingAtIndex(0)).toBeAnInstanceOf(DirectiveBinding); + expect(() => inj.getDirectiveBindingAtIndex(-1)).toThrowError( + 'Index -1 is out-of-bounds.'); + expect(() => inj.getDirectiveBindingAtIndex(10)).toThrowError( 'Index 10 is out-of-bounds.'); }); diff --git a/modules/angular2/test/core/compiler/view_spec.js b/modules/angular2/test/core/compiler/view_spec.js index 2a783285ba..970f9ee3d7 100644 --- a/modules/angular2/test/core/compiler/view_spec.js +++ b/modules/angular2/test/core/compiler/view_spec.js @@ -1,10 +1,9 @@ import {describe, xit, it, expect, beforeEach, ddescribe, iit, el} from 'angular2/test_lib'; import {ProtoView, ElementPropertyMemento, DirectivePropertyMemento} from 'angular2/src/core/compiler/view'; -import {ProtoElementInjector, ElementInjector} from 'angular2/src/core/compiler/element_injector'; +import {ProtoElementInjector, ElementInjector, DirectiveBinding} from 'angular2/src/core/compiler/element_injector'; import {EmulatedShadowDomStrategy, NativeShadowDomStrategy} from 'angular2/src/core/compiler/shadow_dom_strategy'; import {DirectiveMetadataReader} from 'angular2/src/core/compiler/directive_metadata_reader'; -import {Component, Decorator, Template} from 'angular2/src/core/annotations/annotations'; -import {OnChange} from 'angular2/core'; +import {Component, Decorator, Template, Directive, onChange} from 'angular2/src/core/annotations/annotations'; import {Lexer, Parser, DynamicProtoChangeDetector, ChangeDetector} from 'angular2/change_detection'; import {TemplateConfig} from 'angular2/src/core/annotations/template_config'; @@ -546,7 +545,9 @@ export function main() { var pv = new ProtoView(el('
'), new DynamicProtoChangeDetector(), null); - pv.bindElement(new ProtoElementInjector(null, 0, [DirectiveImplementingOnChange])); + pv.bindElement(new ProtoElementInjector(null, 0, [ + DirectiveBinding.createFromType(DirectiveImplementingOnChange, new Directive({lifecycle: [onChange]})) + ])); pv.bindDirectiveProperty( 0, parser.parseBinding('a', null), 'a', reflector.setter('a'), false); pv.bindDirectiveProperty( 0, parser.parseBinding('b', null), 'b', reflector.setter('b'), false); createViewAndChangeDetector(pv); @@ -563,7 +564,9 @@ export function main() { var pv = new ProtoView(el('
'), new DynamicProtoChangeDetector(), null); - pv.bindElement(new ProtoElementInjector(null, 0, [DirectiveImplementingOnChange])); + pv.bindElement(new ProtoElementInjector(null, 0, [ + DirectiveBinding.createFromType(DirectiveImplementingOnChange, new Directive({lifecycle: [onChange]})) + ])); pv.bindDirectiveProperty( 0, parser.parseBinding('a', null), 'a', reflector.setter('a'), false); pv.bindDirectiveProperty( 0, parser.parseBinding('b', null), 'b', reflector.setter('b'), false); createViewAndChangeDetector(pv); @@ -616,7 +619,7 @@ class SomeDirective { } } -class DirectiveImplementingOnChange extends OnChange { +class DirectiveImplementingOnChange { a; b; c;