From 67d05eb65fa5e8436625d6a0baacd774470fe6ed Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Mon, 25 Apr 2016 08:39:53 -0700 Subject: [PATCH] fix(compiler): use DI order for change detection order. Closes #8198 --- .../angular2/src/compiler/provider_parser.ts | 2 +- modules/angular2/src/compiler/template_ast.ts | 10 +++- .../test/compiler/template_parser_spec.ts | 16 +++++- .../change_detection_integration_spec.ts | 56 ++++++++++++++++++- 4 files changed, 78 insertions(+), 6 deletions(-) diff --git a/modules/angular2/src/compiler/provider_parser.ts b/modules/angular2/src/compiler/provider_parser.ts index 5a9788a211..ff618cc07d 100644 --- a/modules/angular2/src/compiler/provider_parser.ts +++ b/modules/angular2/src/compiler/provider_parser.ts @@ -109,7 +109,7 @@ export class ProviderElementContext { var sortedProviderTypes = this._transformedProviders.values().map(provider => provider.token.identifier); var sortedDirectives = ListWrapper.clone(this._directiveAsts); - ListWrapper.sort(this._directiveAsts, + ListWrapper.sort(sortedDirectives, (dir1, dir2) => sortedProviderTypes.indexOf(dir1.directive.type) - sortedProviderTypes.indexOf(dir2.directive.type)); return sortedDirectives; diff --git a/modules/angular2/src/compiler/template_ast.ts b/modules/angular2/src/compiler/template_ast.ts index 558956010a..f328131649 100644 --- a/modules/angular2/src/compiler/template_ast.ts +++ b/modules/angular2/src/compiler/template_ast.ts @@ -118,9 +118,13 @@ export class ElementAst implements TemplateAst { * Get the component associated with this element, if any. */ getComponent(): CompileDirectiveMetadata { - return this.directives.length > 0 && this.directives[0].directive.isComponent ? - this.directives[0].directive : - null; + for (var i = 0; i < this.directives.length; i++) { + var dirAst = this.directives[i]; + if (dirAst.directive.isComponent) { + return dirAst.directive; + } + } + return null; } } diff --git a/modules/angular2/test/compiler/template_parser_spec.ts b/modules/angular2/test/compiler/template_parser_spec.ts index df266448bb..9e4ecd80dc 100644 --- a/modules/angular2/test/compiler/template_parser_spec.ts +++ b/modules/angular2/test/compiler/template_parser_spec.ts @@ -633,7 +633,7 @@ export function main() { `Mixing multi and non multi provider is not possible for token service0 ("[ERROR ->]
"): TestComp@0:0`); }); - it('should sort providers and directives by their DI order', () => { + it('should sort providers by their DI order', () => { var provider0 = createProvider('service0', {deps: ['type:[dir2]']}); var provider1 = createProvider('service1'); var dir2 = createDir('[dir2]', {deps: ['service1']}); @@ -646,6 +646,20 @@ export function main() { expect(elAst.providers[3].providers).toEqual([provider0]); }); + it('should sort directives by their DI order', () => { + var dir0 = createDir('[dir0]', {deps: ['type:my-comp']}); + var dir1 = createDir('[dir1]', {deps: ['type:[dir0]']}); + var dir2 = createDir('[dir2]', {deps: ['type:[dir1]']}); + var comp = createDir('my-comp'); + var elAst: ElementAst = + parse('', [comp, dir2, dir0, dir1])[0]; + expect(elAst.providers.length).toBe(4); + expect(elAst.directives[0].directive).toBe(comp); + expect(elAst.directives[1].directive).toBe(dir0); + expect(elAst.directives[2].directive).toBe(dir1); + expect(elAst.directives[3].directive).toBe(dir2); + }); + it('should mark directives and dependencies of directives as eager', () => { var provider0 = createProvider('service0'); var provider1 = createProvider('service1'); diff --git a/modules/angular2/test/core/linker/change_detection_integration_spec.ts b/modules/angular2/test/core/linker/change_detection_integration_spec.ts index 4275c44422..20386313e9 100644 --- a/modules/angular2/test/core/linker/change_detection_integration_spec.ts +++ b/modules/angular2/test/core/linker/change_detection_integration_spec.ts @@ -1056,6 +1056,18 @@ export function main() { expect(renderLog.log).toEqual([]); })); }); + + describe('multi directive order', () => { + it('should follow the DI order for the same element', fakeAsync(() => { + var ctx = + createCompFixture('
'); + + ctx.detectChanges(false); + ctx.destroy(); + + expect(directiveLog.filter(['set'])).toEqual(['0.set', '1.set', '2.set']); + })); + }); }); } @@ -1066,7 +1078,10 @@ const ALL_DIRECTIVES = CONST_EXPR([ forwardRef(() => TestLocals), forwardRef(() => CompWithRef), forwardRef(() => EmitterDirective), - forwardRef(() => PushComp) + forwardRef(() => PushComp), + forwardRef(() => OrderCheckDirective2), + forwardRef(() => OrderCheckDirective0), + forwardRef(() => OrderCheckDirective1), ]); const ALL_PIPES = CONST_EXPR([ @@ -1296,6 +1311,45 @@ class TestDirective implements OnInit, DoCheck, OnChanges, AfterContentInit, Aft } } +@Directive({selector: '[orderCheck0]'}) +class OrderCheckDirective0 { + private _name: string; + + @Input('orderCheck0') + set name(value: string) { + this._name = value; + this.log.add(this._name, 'set'); + } + + constructor(public log: DirectiveLog) {} +} + +@Directive({selector: '[orderCheck1]'}) +class OrderCheckDirective1 { + private _name: string; + + @Input('orderCheck1') + set name(value: string) { + this._name = value; + this.log.add(this._name, 'set'); + } + + constructor(public log: DirectiveLog, _check0: OrderCheckDirective0) {} +} + +@Directive({selector: '[orderCheck2]'}) +class OrderCheckDirective2 { + private _name: string; + + @Input('orderCheck2') + set name(value: string) { + this._name = value; + this.log.add(this._name, 'set'); + } + + constructor(public log: DirectiveLog, _check1: OrderCheckDirective1) {} +} + @Directive({selector: '[testLocals]'}) class TestLocals { constructor(templateRef: TemplateRef, vcRef: ViewContainerRef) {