From e3c11045bf335b141cc986be411ed7d71dd4ed5b Mon Sep 17 00:00:00 2001 From: vsavkin Date: Mon, 27 Apr 2015 15:14:30 -0700 Subject: [PATCH] fix(compiler): changed the compiler to set up event listeners and host properties on host view elements Closes #1584 --- .../angular2/src/core/compiler/compiler.js | 4 +- .../src/render/dom/compiler/compiler.js | 22 ++++-- .../render/dom/compiler/directive_parser.js | 11 ++- .../src/render/dom/compiler/selector.js | 7 ++ .../src/render/dom/direct_dom_renderer.js | 16 +--- .../compiler/dynamic_component_loader_spec.js | 42 ++++++++++- .../test/core/compiler/integration_spec.js | 20 +++-- .../dom/compiler/compiler_browser_spec.js | 2 +- .../dom/compiler/compiler_common_tests.js | 23 +++++- .../dom/compiler/directive_parser_spec.js | 74 +++++++++++-------- .../direct_dom_renderer_integration_spec.js | 16 ++-- .../test/render/dom/integration_testbed.js | 11 +-- .../shadow_dom_emulation_integration_spec.js | 28 ++++--- modules/angular2/test/router/outlet_spec.js | 4 +- 14 files changed, 187 insertions(+), 93 deletions(-) diff --git a/modules/angular2/src/core/compiler/compiler.js b/modules/angular2/src/core/compiler/compiler.js index a27f334730..4573b8a789 100644 --- a/modules/angular2/src/core/compiler/compiler.js +++ b/modules/angular2/src/core/compiler/compiler.js @@ -90,7 +90,9 @@ export class Compiler { compileInHost(componentTypeOrBinding:any):Promise { var componentBinding = this._bindDirective(componentTypeOrBinding); this._assertTypeIsComponent(componentBinding); - return this._renderer.createHostProtoView('host').then( (hostRenderPv) => { + + var directiveMetadata = Compiler.buildRenderDirective(componentBinding); + return this._renderer.createHostProtoView(directiveMetadata).then( (hostRenderPv) => { return this._compileNestedProtoViews(null, hostRenderPv, [componentBinding], true); }); } diff --git a/modules/angular2/src/render/dom/compiler/compiler.js b/modules/angular2/src/render/dom/compiler/compiler.js index 927328f597..dc09b9e048 100644 --- a/modules/angular2/src/render/dom/compiler/compiler.js +++ b/modules/angular2/src/render/dom/compiler/compiler.js @@ -2,8 +2,9 @@ import {Injectable} from 'angular2/di'; import {PromiseWrapper, Promise} from 'angular2/src/facade/async'; import {BaseException} from 'angular2/src/facade/lang'; +import {DOM} from 'angular2/src/dom/dom_adapter'; -import {ViewDefinition, ProtoViewDto} from '../../api'; +import {ViewDefinition, ProtoViewDto, DirectiveMetadata} from '../../api'; import {CompilePipeline} from './compile_pipeline'; import {TemplateLoader} from 'angular2/src/render/dom/compiler/template_loader'; import {CompileStepFactory, DefaultStepFactory} from './compile_step_factory'; @@ -32,12 +33,21 @@ export class Compiler { ); } - _compileTemplate(template: ViewDefinition, tplElement):Promise { - var subTaskPromises = []; - var pipeline = new CompilePipeline(this._stepFactory.createSteps(template, subTaskPromises)); - var compileElements; + compileHost(directiveMetadata: DirectiveMetadata):Promise { + var hostViewDef = new ViewDefinition({ + componentId: directiveMetadata.id, + absUrl: null, + template: null, + directives: [directiveMetadata] + }); + var element = DOM.createElement(directiveMetadata.selector); + return this._compileTemplate(hostViewDef, element); + } - compileElements = pipeline.process(tplElement, template.componentId); + _compileTemplate(viewDef: ViewDefinition, tplElement):Promise { + var subTaskPromises = []; + var pipeline = new CompilePipeline(this._stepFactory.createSteps(viewDef, subTaskPromises)); + var compileElements = pipeline.process(tplElement, viewDef.componentId); var protoView = compileElements[0].inheritedProtoView.build(); diff --git a/modules/angular2/src/render/dom/compiler/directive_parser.js b/modules/angular2/src/render/dom/compiler/directive_parser.js index 9c57fbb606..6268c13e34 100644 --- a/modules/angular2/src/render/dom/compiler/directive_parser.js +++ b/modules/angular2/src/render/dom/compiler/directive_parser.js @@ -27,11 +27,20 @@ export class DirectiveParser extends CompileStep { this._selectorMatcher = new SelectorMatcher(); this._directives = directives; for (var i=0; i { - var rootElement = DOM.createElement('div'); - var hostProtoViewBuilder = new ProtoViewBuilder(rootElement); - var elBinder = hostProtoViewBuilder.bindElement(rootElement, 'root element'); - elBinder.setComponentId(componentId); - elBinder.bindDirective(0); - - this._shadowDomStrategy.processElement(null, componentId, rootElement); - return PromiseWrapper.resolve(hostProtoViewBuilder.build()); + createHostProtoView(directiveMetadata:api.DirectiveMetadata):Promise { + return this._compiler.compileHost(directiveMetadata); } createImperativeComponentProtoView(rendererId):Promise { @@ -97,10 +89,10 @@ export class DirectDomRenderer extends api.Renderer { return PromiseWrapper.resolve(protoViewBuilder.build()); } - compile(template:api.ViewDefinition):Promise { + compile(view:api.ViewDefinition):Promise { // Note: compiler already uses a DirectDomProtoViewRef, so we don't // need to do anything here - return this._compiler.compile(template); + return this._compiler.compile(view); } mergeChildComponentProtoViews(protoViewRef:api.ProtoViewRef, protoViewRefs:List) { diff --git a/modules/angular2/test/core/compiler/dynamic_component_loader_spec.js b/modules/angular2/test/core/compiler/dynamic_component_loader_spec.js index 90dbf7e0da..70620dfab5 100644 --- a/modules/angular2/test/core/compiler/dynamic_component_loader_spec.js +++ b/modules/angular2/test/core/compiler/dynamic_component_loader_spec.js @@ -22,6 +22,8 @@ import {DynamicComponentLoader} from 'angular2/src/core/compiler/dynamic_compone import {ElementRef} from 'angular2/src/core/compiler/element_injector'; import {If} from 'angular2/src/directives/if'; import {DirectDomRenderer} from 'angular2/src/render/dom/direct_dom_renderer'; +import {DOM} from 'angular2/src/dom/dom_adapter'; + export function main() { describe('DynamicComponentLoader', function () { @@ -134,6 +136,29 @@ export function main() { }); }); })); + + it('should update host properties', inject([DynamicComponentLoader, TestBed, AsyncTestCompleter], + (loader, tb, async) => { + tb.overrideView(MyComp, new View({ + template: '
', + directives: [Location] + })); + + tb.createView(MyComp).then((view) => { + var location = view.rawView.locals.get("loc"); + + loader.loadNextToExistingLocation(DynamicallyLoadedWithHostProps, location.elementRef).then(ref => { + ref.instance.id = "new value"; + + view.detectChanges(); + + var newlyInsertedElement = DOM.childNodesAsList(view.rootNodes[0])[1]; + expect(newlyInsertedElement.id).toEqual("new value") + + async.done(); + }); + }); + })); }); describe('loading into a new location', () => { @@ -242,6 +267,19 @@ class DynamicallyLoaded { class DynamicallyLoaded2 { } +@Component({ + selector: 'dummy', + hostProperties: {'id' : 'id'} +}) +@View({template: "DynamicallyLoadedWithHostProps;"}) +class DynamicallyLoadedWithHostProps { + id:string; + + constructor() { + this.id = "default"; + } +} + @Component({ selector: 'location' }) @@ -254,7 +292,9 @@ class Location { } } -@Component() +@Component({ + selector: 'my-comp' +}) @View({ directives: [] }) diff --git a/modules/angular2/test/core/compiler/integration_spec.js b/modules/angular2/test/core/compiler/integration_spec.js index f70f84b0c9..6ba577df23 100644 --- a/modules/angular2/test/core/compiler/integration_spec.js +++ b/modules/angular2/test/core/compiler/integration_spec.js @@ -255,20 +255,19 @@ export function main() { tb.overrideView(MyComp, new View({ template: '

', - directives: [IdComponent] + directives: [IdDir] })); tb.createView(MyComp, {context: ctx}).then((view) => { + var idDir = view.rawView.elementInjectors[0].get(IdDir); ctx.ctxProp = 'some_id'; view.detectChanges(); - expect(view.rootNodes[0].id).toEqual('some_id'); - expect(view.rootNodes).toHaveText('Matched on id with some_id'); + expect(idDir.id).toEqual('some_id'); ctx.ctxProp = 'other_id'; view.detectChanges(); - expect(view.rootNodes[0].id).toEqual('other_id'); - expect(view.rootNodes).toHaveText('Matched on id with other_id'); + expect(idDir.id).toEqual('other_id'); async.done(); }); @@ -932,7 +931,9 @@ class PushCmpWithRef { } } -@Component() +@Component({ + selector: 'my-comp' +}) @View({directives: [ ]}) class MyComp { @@ -1192,14 +1193,11 @@ class DecoratorListeningDomEventNoPrevent { } } -@Component({ +@Decorator({ selector: '[id]', properties: {'id': 'id'} }) -@View({ - template: '
Matched on id with {{id}}
' -}) -class IdComponent { +class IdDir { id: string; } diff --git a/modules/angular2/test/render/dom/compiler/compiler_browser_spec.js b/modules/angular2/test/render/dom/compiler/compiler_browser_spec.js index 963ee1b8d2..aedb546227 100644 --- a/modules/angular2/test/render/dom/compiler/compiler_browser_spec.js +++ b/modules/angular2/test/render/dom/compiler/compiler_browser_spec.js @@ -1,4 +1,4 @@ -/* + /* * Runs compiler tests using in-browser DOM adapter. */ diff --git a/modules/angular2/test/render/dom/compiler/compiler_common_tests.js b/modules/angular2/test/render/dom/compiler/compiler_common_tests.js index 947ffd4bcf..a1966d6d84 100644 --- a/modules/angular2/test/render/dom/compiler/compiler_common_tests.js +++ b/modules/angular2/test/render/dom/compiler/compiler_common_tests.js @@ -17,7 +17,7 @@ import {Type, isBlank, stringify, isPresent} from 'angular2/src/facade/lang'; import {PromiseWrapper, Promise} from 'angular2/src/facade/async'; import {Compiler, CompilerCache} from 'angular2/src/render/dom/compiler/compiler'; -import {ProtoViewDto, ViewDefinition} from 'angular2/src/render/api'; +import {ProtoViewDto, ViewDefinition, DirectiveMetadata} from 'angular2/src/render/api'; import {CompileElement} from 'angular2/src/render/dom/compiler/compile_element'; import {CompileStep} from 'angular2/src/render/dom/compiler/compile_step' import {CompileStepFactory} from 'angular2/src/render/dom/compiler/compile_step_factory'; @@ -54,6 +54,22 @@ export function runCompilerCommonTests() { }); })); + it('should run the steps and build the proto view', inject([AsyncTestCompleter], (async) => { + var compiler = createCompiler((parent, current, control) => { + current.inheritedProtoView.bindVariable('b', 'a'); + }); + + var dirMetadata = new DirectiveMetadata({id: 'id', selector: 'CUSTOM', type: DirectiveMetadata.COMPONENT_TYPE}); + compiler.compileHost(dirMetadata).then( (protoView) => { + expect(DOM.tagName(protoView.render.delegate.element)).toEqual('CUSTOM') + expect(mockStepFactory.viewDef.directives).toEqual([dirMetadata]); + expect(protoView.variableBindings).toEqual(MapWrapper.createFromStringMap({ + 'a': 'b' + })); + async.done(); + }); + })); + it('should use the inline template and compile in sync', inject([AsyncTestCompleter], (async) => { var compiler = createCompiler(EMPTY_STEP); compiler.compile(new ViewDefinition({ @@ -124,11 +140,14 @@ export function runCompilerCommonTests() { class MockStepFactory extends CompileStepFactory { steps:List; subTaskPromises:List; + viewDef:ViewDefinition; + constructor(steps) { super(); this.steps = steps; } - createSteps(template, subTaskPromises) { + createSteps(viewDef, subTaskPromises) { + this.viewDef = viewDef; this.subTaskPromises = subTaskPromises; ListWrapper.forEach(this.subTaskPromises, (p) => ListWrapper.push(subTaskPromises, p) ); return this.steps; diff --git a/modules/angular2/test/render/dom/compiler/directive_parser_spec.js b/modules/angular2/test/render/dom/compiler/directive_parser_spec.js index 05a8da3ce0..4b11389a3c 100644 --- a/modules/angular2/test/render/dom/compiler/directive_parser_spec.js +++ b/modules/angular2/test/render/dom/compiler/directive_parser_spec.js @@ -1,5 +1,5 @@ import {describe, beforeEach, it, xit, expect, iit, ddescribe, el} from 'angular2/test_lib'; -import {isPresent, assertionsEnabled} from 'angular2/src/facade/lang'; +import {isPresent, isBlank, assertionsEnabled} from 'angular2/src/facade/lang'; import {ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection'; import {DirectiveParser} from 'angular2/src/render/dom/compiler/directive_parser'; import {CompilePipeline} from 'angular2/src/render/dom/compiler/compile_pipeline'; @@ -17,11 +17,11 @@ export function main() { annotatedDirectives = [ someComponent, someComponent2, - someComponent3, someViewport, someViewport2, someDecorator, someDecoratorIgnoringChildren, + decoratorWithMultipleAttrs, someDecoratorWithProps, someDecoratorWithHostProperties, someDecoratorWithEvents, @@ -30,7 +30,9 @@ export function main() { parser = new Parser(new Lexer()); }); - function createPipeline(propertyBindings = null) { + function createPipeline(propertyBindings = null, directives = null) { + if (isBlank(directives)) directives = annotatedDirectives; + return new CompilePipeline([ new MockStep( (parent, current, control) => { if (isPresent(propertyBindings)) { @@ -39,12 +41,12 @@ export function main() { }); } }), - new DirectiveParser(parser, annotatedDirectives) + new DirectiveParser(parser, directives) ]); } - function process(el, propertyBindings = null) { - var pipeline = createPipeline(propertyBindings); + function process(el, propertyBindings = null, directives = null) { + var pipeline = createPipeline(propertyBindings, directives); return ListWrapper.map(pipeline.process(el), (ce) => ce.inheritedElementBinder ); } @@ -63,7 +65,7 @@ export function main() { it('should detect directives with multiple attributes', () => { var results = process(el('')); expect(results[0].directives[0].directiveIndex).toBe( - annotatedDirectives.indexOf(someComponent3) + annotatedDirectives.indexOf(decoratorWithMultipleAttrs) ); }); @@ -176,30 +178,27 @@ export function main() { describe('component directives', () => { it('should save the component id', () => { var results = process( - el('
') + el('') ); expect(results[0].componentId).toEqual('someComponent'); }); + it('should throw when the provided selector is not an element selector', () => { + expect( () => { + createPipeline(null, [componentWithNonElementSelector]); + }).toThrowError(`Component 'componentWithNonElementSelector' can only have an element selector, but had '[attr]'`); + }); + it('should not allow multiple component directives on the same element', () => { expect( () => { process( - el('
') + el(''), + null, + [someComponent, someComponentDup] ); - }).toThrowError('Only one component directive is allowed per element - check ' - + (assertionsEnabled() ? '
' : 'null')); + }).toThrowError(new RegExp('Only one component directive is allowed per element' )); }); - - it('should not allow component directives on