From 1367cd9569e5aafa1261d711ded6438e9db58d8f Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Mon, 16 Jan 2017 23:52:42 +0200 Subject: [PATCH] fix(upgrade): respect hierarchical injectors for downgraded components (#14037) Correctly wire up hierarchical injectors for downgraded components in `upgrade/static`: Downgraded components inherit the injector of the first downgraded component up the DOM tree. This is similar to (part of) d91a86a, but for `upgrade/static`. POSSIBLE BREAKING CHANGE: In order to enable more control over the wiring of downgraded components and their content (which eventually allows better control over features like injector setup and content projection), it was necessary to change the implementation of the directives generated for downgraed components. The directives are now terminal and manually take care of projecting and compiling their contents in the post-linking function. This is similar to how the dynamic version of `upgrade` does it. This is not expected to affect apps, since the relative order of individual operations is preserved. Still, it is difficult to predict how every possible usecase may be affected. --- .../upgrade/src/common/downgrade_component.ts | 101 +++++++++++++----- .../src/common/downgrade_component_adapter.ts | 58 +++++----- .../upgrade/src/dynamic/upgrade_adapter.ts | 2 +- .../integration/content_projection_spec.ts | 45 ++++---- .../integration/downgrade_component_spec.ts | 31 ++++++ .../integration/upgrade_component_spec.ts | 2 +- 6 files changed, 165 insertions(+), 74 deletions(-) diff --git a/modules/@angular/upgrade/src/common/downgrade_component.ts b/modules/@angular/upgrade/src/common/downgrade_component.ts index 7db8c71343..4815ce1b5e 100644 --- a/modules/@angular/upgrade/src/common/downgrade_component.ts +++ b/modules/@angular/upgrade/src/common/downgrade_component.ts @@ -9,8 +9,9 @@ import {ComponentFactory, ComponentFactoryResolver, Injector, Type} from '@angular/core'; import * as angular from './angular1'; -import {$INJECTOR, $PARSE, INJECTOR_KEY, REQUIRE_NG_MODEL} from './constants'; +import {$COMPILE, $INJECTOR, $PARSE, INJECTOR_KEY, REQUIRE_INJECTOR, REQUIRE_NG_MODEL} from './constants'; import {DowngradeComponentAdapter} from './downgrade_component_adapter'; +import {controllerKey} from './util'; let downgradeCount = 0; @@ -71,43 +72,93 @@ export function downgradeComponent(info: /* ComponentInfo */ { const directiveFactory: angular.IAnnotatedFunction = function( + $compile: angular.ICompileService, $injector: angular.IInjectorService, $parse: angular.IParseService): angular.IDirective { return { restrict: 'E', - require: ['?^' + INJECTOR_KEY, REQUIRE_NG_MODEL], + terminal: true, + require: [REQUIRE_INJECTOR, REQUIRE_NG_MODEL], link: (scope: angular.IScope, element: angular.IAugmentedJQuery, attrs: angular.IAttributes, - required: any[], transclude: angular.ITranscludeFunction) => { - - let parentInjector: Injector = required[0]; - if (parentInjector === null) { - parentInjector = $injector.get(INJECTOR_KEY); - } + required: any[]) => { + // We might have to compile the contents asynchronously, because this might have been + // triggered by `UpgradeNg1ComponentAdapterBuilder`, before the Angular templates have + // been compiled. + const parentInjector: Injector | ParentInjectorPromise = required[0] || $injector.get(INJECTOR_KEY); const ngModel: angular.INgModelController = required[1]; - - const componentFactoryResolver: ComponentFactoryResolver = - parentInjector.get(ComponentFactoryResolver); - const componentFactory: ComponentFactory = - componentFactoryResolver.resolveComponentFactory(info.component); - if (!componentFactory) { - throw new Error('Expecting ComponentFactory for: ' + info.component); + const downgradeFn = (injector: Injector) => { + const componentFactoryResolver: ComponentFactoryResolver = + injector.get(ComponentFactoryResolver); + const componentFactory: ComponentFactory = + componentFactoryResolver.resolveComponentFactory(info.component); + + if (!componentFactory) { + throw new Error('Expecting ComponentFactory for: ' + info.component); + } + + const id = idPrefix + (idCount++); + const injectorPromise = new ParentInjectorPromise(element); + const facade = new DowngradeComponentAdapter( + id, info, element, attrs, scope, ngModel, injector, $compile, $parse, componentFactory); + + const projectableNodes = facade.compileContents(); + facade.createComponent(projectableNodes); + facade.setupInputs(); + facade.setupOutputs(); + facade.registerCleanup(); + + injectorPromise.resolve(facade.getInjector()); + }; + + if (parentInjector instanceof ParentInjectorPromise) { + parentInjector.then(downgradeFn); + } else { + downgradeFn(parentInjector); } - - const facade = new DowngradeComponentAdapter( - idPrefix + (idCount++), info, element, attrs, scope, ngModel, parentInjector, $parse, - componentFactory); - facade.setupInputs(); - facade.createComponent(); - facade.projectContent(); - facade.setupOutputs(); - facade.registerCleanup(); } }; }; - directiveFactory.$inject = [$INJECTOR, $PARSE]; + directiveFactory.$inject = [$COMPILE, $INJECTOR, $PARSE]; return directiveFactory; } + +/** + * Synchronous promise-like object to wrap parent injectors, + * to preserve the synchronous nature of Angular 1's $compile. + */ +class ParentInjectorPromise { + private injector: Injector; + private injectorKey: string = controllerKey(INJECTOR_KEY); + private callbacks: ((injector: Injector) => any)[] = []; + + constructor(private element: angular.IAugmentedJQuery) { + // Store the promise on the element. + element.data(this.injectorKey, this); + } + + then(callback: (injector: Injector) => any) { + if (this.injector) { + callback(this.injector); + } else { + this.callbacks.push(callback); + } + } + + resolve(injector: Injector) { + this.injector = injector; + + // Store the real injector on the element. + this.element.data(this.injectorKey, injector); + + // Release the element to prevent memory leaks. + this.element = null; + + // Run the queued callbacks. + this.callbacks.forEach(callback => callback(injector)); + this.callbacks.length = 0; + } +} diff --git a/modules/@angular/upgrade/src/common/downgrade_component_adapter.ts b/modules/@angular/upgrade/src/common/downgrade_component_adapter.ts index 59987739fa..ceaeb9b189 100644 --- a/modules/@angular/upgrade/src/common/downgrade_component_adapter.ts +++ b/modules/@angular/upgrade/src/common/downgrade_component_adapter.ts @@ -9,42 +9,52 @@ import {ChangeDetectorRef, ComponentFactory, ComponentRef, EventEmitter, Injector, OnChanges, ReflectiveInjector, SimpleChange, SimpleChanges, Type} from '@angular/core'; import * as angular from './angular1'; -import {hookupNgModel} from '../common/util'; import {ComponentInfo, PropertyBinding} from './component_info'; import {$SCOPE} from './constants'; +import {hookupNgModel} from './util'; const INITIAL_VALUE = { __UNINITIALIZED__: true }; export class DowngradeComponentAdapter { - component: any = null; - inputs: Attr; - inputChangeCount: number = 0; - inputChanges: SimpleChanges = null; - componentRef: ComponentRef = null; - changeDetector: ChangeDetectorRef = null; - componentScope: angular.IScope; - childNodes: Node[]; - contentInsertionPoint: Node = null; + private inputChangeCount: number = 0; + private inputChanges: SimpleChanges = null; + private componentScope: angular.IScope; + private componentRef: ComponentRef = null; + private component: any = null; + private changeDetector: ChangeDetectorRef = null; constructor( private id: string, private info: ComponentInfo, private element: angular.IAugmentedJQuery, private attrs: angular.IAttributes, private scope: angular.IScope, private ngModel: angular.INgModelController, private parentInjector: Injector, - private parse: angular.IParseService, private componentFactory: ComponentFactory) { - (this.element[0]).id = id; + private $compile: angular.ICompileService, private $parse: angular.IParseService, + private componentFactory: ComponentFactory) { + (this.element[0] as any).id = id; this.componentScope = scope.$new(); - this.childNodes = element.contents(); } - createComponent() { + compileContents(): Node[][] { + const projectableNodes: Node[][] = []; + const linkFn = this.$compile(this.element.contents()); + + this.element.empty(); + + linkFn(this.scope, (clone: Node[]) => { + projectableNodes.push(clone); + this.element.append(clone); + }); + + return projectableNodes; + } + + createComponent(projectableNodes: Node[][]) { const childInjector = ReflectiveInjector.resolveAndCreate( [{provide: $SCOPE, useValue: this.componentScope}], this.parentInjector); - this.contentInsertionPoint = document.createComment('ng1 insertion point'); - this.componentRef = this.componentFactory.create( - childInjector, [[this.contentInsertionPoint]], this.element[0]); + this.componentRef = + this.componentFactory.create(childInjector, projectableNodes, this.element[0]); this.changeDetector = this.componentRef.changeDetectorRef; this.component = this.componentRef.instance; @@ -110,16 +120,6 @@ export class DowngradeComponentAdapter { this.componentScope.$watch(() => this.changeDetector && this.changeDetector.detectChanges()); } - projectContent() { - const childNodes = this.childNodes; - const parent = this.contentInsertionPoint.parentNode; - if (parent) { - for (let i = 0, ii = childNodes.length; i < ii; i++) { - parent.insertBefore(childNodes[i], this.contentInsertionPoint); - } - } - } - setupOutputs() { const attrs = this.attrs; const outputs = this.info.outputs || []; @@ -147,7 +147,7 @@ export class DowngradeComponentAdapter { } if (expr != null && assignExpr != null) { - const getter = this.parse(expr); + const getter = this.$parse(expr); const setter = getter.assign; if (assignExpr && !setter) { throw new Error(`Expression '${expr}' is not assignable!`); @@ -174,4 +174,6 @@ export class DowngradeComponentAdapter { this.componentRef.destroy(); }); } + + getInjector(): Injector { return this.componentRef && this.componentRef.injector; } } diff --git a/modules/@angular/upgrade/src/dynamic/upgrade_adapter.ts b/modules/@angular/upgrade/src/dynamic/upgrade_adapter.ts index 5353641327..ac32f5b53c 100644 --- a/modules/@angular/upgrade/src/dynamic/upgrade_adapter.ts +++ b/modules/@angular/upgrade/src/dynamic/upgrade_adapter.ts @@ -701,8 +701,8 @@ function ng1ComponentDirective(info: ComponentInfo, idPrefix: string): Function function downgrade(injector: Injector) { const facade = new DowngradeNg2ComponentAdapter( info, element, attrs, scope, ngModel, injector, parse, componentFactory); - facade.setupInputs(); facade.bootstrapNg2(projectableNodes); + facade.setupInputs(); facade.setupOutputs(); facade.registerCleanup(); injectorPromise.resolve(facade.componentRef.injector); diff --git a/modules/@angular/upgrade/test/static/integration/content_projection_spec.ts b/modules/@angular/upgrade/test/static/integration/content_projection_spec.ts index d994aae135..441a4d4a86 100644 --- a/modules/@angular/upgrade/test/static/integration/content_projection_spec.ts +++ b/modules/@angular/upgrade/test/static/integration/content_projection_spec.ts @@ -24,8 +24,10 @@ export function main() { it('should instantiate ng2 in ng1 template and project content', async(() => { // the ng2 component that will be used in ng1 (downgraded) - @Component({selector: 'ng2', template: `{{ 'NG2' }}()`}) + @Component({selector: 'ng2', template: `{{ prop }}()`}) class Ng2Component { + prop = 'NG2'; + ngContent = 'ng2-content'; } // our upgrade module to host the component to downgrade @@ -42,13 +44,16 @@ export function main() { const ng1Module = angular .module('ng1', []) // create an ng1 facade of the ng2 component - .directive('ng2', downgradeComponent({component: Ng2Component})); + .directive('ng2', downgradeComponent({component: Ng2Component})) + .run(($rootScope: angular.IRootScopeService) => { + $rootScope['prop'] = 'NG1'; + $rootScope['ngContent'] = 'ng1-content'; + }); - const element = - html('
{{ \'ng1[\' }}~{{ \'ng-content\' }}~{{ \']\' }}
'); + const element = html('
{{ \'ng1[\' }}~{{ ngContent }}~{{ \']\' }}
'); bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then((upgrade) => { - expect(document.body.textContent).toEqual('ng1[NG2(~ng-content~)]'); + expect(document.body.textContent).toEqual('ng1[NG2(~ng1-content~)]'); }); })); @@ -56,12 +61,13 @@ export function main() { @Component({ selector: 'ng2', - template: `{{ 'ng2(' }}{{'transclude'}}{{ ')' }}`, + template: `{{ 'ng2(' }}{{ transclude }}{{ ')' }}`, }) class Ng2Component { + prop = 'ng2'; + transclude = 'ng2-transclude'; } - @Directive({selector: 'ng1'}) class Ng1WrapperComponent extends UpgradeComponent { constructor(elementRef: ElementRef, injector: Injector) { @@ -78,21 +84,22 @@ export function main() { ngDoBootstrap() {} } - const ng1Module = angular.module('ng1', []) - .directive( - 'ng1', - () => { - return { - transclude: true, - template: '{{ "ng1" }}()' - }; - }) - .directive('ng2', downgradeComponent({component: Ng2Component})); + const ng1Module = + angular.module('ng1', []) + .directive('ng1', () => ({ + transclude: true, + template: '{{ prop }}()' + })) + .directive('ng2', downgradeComponent({component: Ng2Component})) + .run(($rootScope: angular.IRootScopeService) => { + $rootScope['prop'] = 'ng1'; + $rootScope['transclude'] = 'ng1-transclude'; + }); - const element = html('
{{\'ng1(\'}}{{\')\'}}
'); + const element = html('
{{ \'ng1(\' }}{{ \')\' }}
'); bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then((upgrade) => { - expect(document.body.textContent).toEqual('ng1(ng2(ng1(transclude)))'); + expect(document.body.textContent).toEqual('ng1(ng2(ng1(ng2-transclude)))'); }); })); }); diff --git a/modules/@angular/upgrade/test/static/integration/downgrade_component_spec.ts b/modules/@angular/upgrade/test/static/integration/downgrade_component_spec.ts index 3bc42020e3..25783719e2 100644 --- a/modules/@angular/upgrade/test/static/integration/downgrade_component_spec.ts +++ b/modules/@angular/upgrade/test/static/integration/downgrade_component_spec.ts @@ -324,5 +324,36 @@ export function main() { expect(multiTrim(document.body.textContent)).toBe('It works!'); }); })); + + it('should respect hierarchical dependency injection for ng2', async(() => { + @Component({selector: 'parent', template: 'parent()'}) + class ParentComponent { + } + + @Component({selector: 'child', template: 'child'}) + class ChildComponent { + constructor(parent: ParentComponent) {} + } + + @NgModule({ + declarations: [ParentComponent, ChildComponent], + entryComponents: [ParentComponent, ChildComponent], + imports: [BrowserModule, UpgradeModule] + }) + class Ng2Module { + ngDoBootstrap() {} + } + + const ng1Module = + angular.module('ng1', []) + .directive('parent', downgradeComponent({component: ParentComponent})) + .directive('child', downgradeComponent({component: ChildComponent})); + + const element = html(''); + + bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then(upgrade => { + expect(multiTrim(document.body.textContent)).toBe('parent(child)'); + }); + })); }); } diff --git a/modules/@angular/upgrade/test/static/integration/upgrade_component_spec.ts b/modules/@angular/upgrade/test/static/integration/upgrade_component_spec.ts index 33948812cf..812f55f8a8 100644 --- a/modules/@angular/upgrade/test/static/integration/upgrade_component_spec.ts +++ b/modules/@angular/upgrade/test/static/integration/upgrade_component_spec.ts @@ -3028,7 +3028,7 @@ export function main() { }); })); - it('should ng1 > ng1 > ng2 > ng1 (with `require`)', async(() => { + it('should support ng2 > ng1 > ng2 > ng1 (with `require`)', async(() => { // Define `ng1Component` const ng1ComponentA: angular.IComponent = { template: 'ng1A()',