From 74cb57521926f17fdca876de2b57fed11542e559 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Thu, 9 Feb 2017 20:07:11 +0200 Subject: [PATCH] fix(upgrade): correctly project content on downgraded components with structural directives (#14274) Previously, downgraded component adapters were compiling and projecting the contents of the template element instead of the link-element. This didn't make any difference is most cases (as the elements are the same), but broke with structural directives (e.g. `ngRepeat`), which compile the orginal (template) element once and then create and link clones of it. This commit fixes it by always compiling and projecting the contents of the correct (link) element. Fixes #14260 --- .../@angular/upgrade/src/upgrade_adapter.ts | 82 +++++++++---------- .../integration/content_projection_spec.ts | 42 +++++++++- modules/@angular/upgrade/test/upgrade_spec.ts | 44 +++++++++- 3 files changed, 117 insertions(+), 51 deletions(-) diff --git a/modules/@angular/upgrade/src/upgrade_adapter.ts b/modules/@angular/upgrade/src/upgrade_adapter.ts index d4f4b2de176..ac305581289 100644 --- a/modules/@angular/upgrade/src/upgrade_adapter.ts +++ b/modules/@angular/upgrade/src/upgrade_adapter.ts @@ -656,67 +656,59 @@ function ng1ComponentDirective(info: ComponentInfo, idPrefix: string): Function restrict: 'E', terminal: true, require: REQUIRE_INJECTOR, - compile: (templateElement: angular.IAugmentedJQuery, templateAttributes: angular.IAttributes, - transclude: angular.ITranscludeFunction) => { + link: (scope: angular.IScope, element: angular.IAugmentedJQuery, attrs: angular.IAttributes, + parentInjector: Injector | ParentInjectorPromise): void => { // We might have compile the contents lazily, because this might have been triggered by the // UpgradeNg1ComponentAdapterBuilder, when the ng2 templates have not been compiled yet - return { - post: (scope: angular.IScope, element: angular.IAugmentedJQuery, - attrs: angular.IAttributes, parentInjector: Injector | ParentInjectorPromise, - transclude: angular.ITranscludeFunction): void => { - let id = idPrefix + (idCount++); - (element[0]).id = id; - let injectorPromise = new ParentInjectorPromise(element); + let id = idPrefix + (idCount++); + (element[0]).id = id; - const ng2Compiler = ng1Injector.get(NG2_COMPILER) as Compiler; - const ngContentSelectors = ng2Compiler.getNgContentSelectors(info.type); - const linkFns = compileProjectedNodes(templateElement, ngContentSelectors); + let injectorPromise = new ParentInjectorPromise(element); - const componentFactory: ComponentFactory = componentFactoryRefMap[info.selector]; - if (!componentFactory) - throw new Error('Expecting ComponentFactory for: ' + info.selector); + const ng2Compiler = ng1Injector.get(NG2_COMPILER) as Compiler; + const ngContentSelectors = ng2Compiler.getNgContentSelectors(info.type); + const linkFns = compileProjectedNodes(element, ngContentSelectors); - element.empty(); - let projectableNodes = linkFns.map(link => { - let projectedClone: Node[]; - link(scope, (clone: Node[]) => { - projectedClone = clone; - element.append(clone); - }); - return projectedClone; - }); + const componentFactory: ComponentFactory = componentFactoryRefMap[info.selector]; + if (!componentFactory) throw new Error('Expecting ComponentFactory for: ' + info.selector); - parentInjector = parentInjector || ng1Injector.get(NG2_INJECTOR); + element.empty(); + let projectableNodes = linkFns.map(link => { + let projectedClone: Node[]; + link(scope, (clone: Node[]) => { + projectedClone = clone; + element.append(clone); + }); + return projectedClone; + }); - if (parentInjector instanceof ParentInjectorPromise) { - parentInjector.then((resolvedInjector: Injector) => downgrade(resolvedInjector)); - } else { - downgrade(parentInjector); - } + parentInjector = parentInjector || ng1Injector.get(NG2_INJECTOR); - function downgrade(injector: Injector) { - const facade = new DowngradeNg2ComponentAdapter( - info, element, attrs, scope, injector, parse, componentFactory); - facade.setupInputs(); - facade.bootstrapNg2(projectableNodes); - facade.setupOutputs(); - facade.registerCleanup(); - injectorPromise.resolve(facade.componentRef.injector); - } - } - }; + if (parentInjector instanceof ParentInjectorPromise) { + parentInjector.then((resolvedInjector: Injector) => downgrade(resolvedInjector)); + } else { + downgrade(parentInjector); + } + + function downgrade(injector: Injector) { + const facade = new DowngradeNg2ComponentAdapter( + info, element, attrs, scope, injector, parse, componentFactory); + facade.setupInputs(); + facade.bootstrapNg2(projectableNodes); + facade.setupOutputs(); + facade.registerCleanup(); + injectorPromise.resolve(facade.componentRef.injector); + } } }; function compileProjectedNodes( - templateElement: angular.IAugmentedJQuery, - ngContentSelectors: string[]): angular.ILinkFn[] { + element: angular.IAugmentedJQuery, ngContentSelectors: string[]): angular.ILinkFn[] { if (!ngContentSelectors) throw new Error('Expecting ngContentSelectors for: ' + info.selector); // We have to sort the projected content before we compile it, hence the terminal: true - let projectableTemplateNodes = - sortProjectableNodes(ngContentSelectors, templateElement.contents()); + let projectableTemplateNodes = sortProjectableNodes(ngContentSelectors, element.contents()); return projectableTemplateNodes.map(nodes => ng1Compile(nodes)); } } diff --git a/modules/@angular/upgrade/test/aot/integration/content_projection_spec.ts b/modules/@angular/upgrade/test/aot/integration/content_projection_spec.ts index 7c48ac8b46f..e9b5adbf01a 100644 --- a/modules/@angular/upgrade/test/aot/integration/content_projection_spec.ts +++ b/modules/@angular/upgrade/test/aot/integration/content_projection_spec.ts @@ -6,14 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ -import {Component, Directive, ElementRef, Injector, NgModule, destroyPlatform} from '@angular/core'; +import {Component, Directive, ElementRef, Injector, Input, NgModule, destroyPlatform} from '@angular/core'; import {async} from '@angular/core/testing'; import {BrowserModule} from '@angular/platform-browser'; import {platformBrowserDynamic} from '@angular/platform-browser-dynamic'; import * as angular from '@angular/upgrade/src/angular_js'; import {UpgradeComponent, UpgradeModule, downgradeComponent} from '@angular/upgrade/static'; -import {bootstrap, html} from '../test_helpers'; +import {bootstrap, html, multiTrim} from '../test_helpers'; export function main() { describe('content projection', () => { @@ -52,6 +52,44 @@ export function main() { }); })); + it('should correctly project structural directives', async(() => { + @Component({selector: 'ng2', template: 'ng2-{{ itemId }}()'}) + class Ng2Component { + @Input() itemId: string; + } + + @NgModule({ + imports: [BrowserModule, UpgradeModule], + declarations: [Ng2Component], + entryComponents: [Ng2Component] + }) + class Ng2Module { + ngDoBootstrap() {} + } + + const ng1Module = + angular.module('ng1', []) + .directive( + 'ng2', downgradeComponent({component: Ng2Component, inputs: ['itemId']})) + .run(($rootScope: angular.IRootScopeService) => { + $rootScope['items'] = [ + {id: 'a', subitems: [1, 2, 3]}, {id: 'b', subitems: [4, 5, 6]}, + {id: 'c', subitems: [7, 8, 9]} + ]; + }); + + const element = html(` + +
{{ subitem }}
+ + `); + + bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then(upgrade => { + expect(multiTrim(document.body.textContent)) + .toBe('ng2-a( 123 )ng2-b( 456 )ng2-c( 789 )'); + }); + })); + it('should instantiate ng1 in ng2 template and project content', async(() => { @Component({ diff --git a/modules/@angular/upgrade/test/upgrade_spec.ts b/modules/@angular/upgrade/test/upgrade_spec.ts index 9fe7e66b8d7..174c593d29f 100644 --- a/modules/@angular/upgrade/test/upgrade_spec.ts +++ b/modules/@angular/upgrade/test/upgrade_spec.ts @@ -491,6 +491,39 @@ export function main() { }); })); + it('should correctly project structural directives', async(() => { + @Component({selector: 'ng2', template: 'ng2-{{ itemId }}()'}) + class Ng2Component { + @Input() itemId: string; + } + + @NgModule({imports: [BrowserModule], declarations: [Ng2Component]}) + class Ng2Module { + } + + const adapter: UpgradeAdapter = new UpgradeAdapter(Ng2Module); + const ng1Module = angular.module('ng1', []) + .directive('ng2', adapter.downgradeNg2Component(Ng2Component)) + .run(($rootScope: angular.IRootScopeService) => { + $rootScope['items'] = [ + {id: 'a', subitems: [1, 2, 3]}, {id: 'b', subitems: [4, 5, 6]}, + {id: 'c', subitems: [7, 8, 9]} + ]; + }); + + const element = html(` + +
{{ subitem }}
+ + `); + + adapter.bootstrap(element, [ng1Module.name]).ready(ref => { + expect(multiTrim(document.body.textContent)) + .toBe('ng2-a( 123 )ng2-b( 456 )ng2-c( 789 )'); + ref.dispose(); + }); + })); + it('should allow attribute selectors for components in ng2', async(() => { const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => MyNg2Module)); const ng1Module = angular.module('myExample', []); @@ -1901,14 +1934,17 @@ function multiTrim(text: string): string { } function html(html: string): Element { + // Don't return `body` itself, because using it as a `$rootElement` for ng1 + // will attach `$injector` to it and that will affect subsequent tests. const body = document.body; - body.innerHTML = html; + body.innerHTML = `
${html.trim()}
`; + const div = document.body.firstChild as Element; - if (body.childNodes.length == 1 && body.firstChild instanceof HTMLElement) { - return body.firstChild; + if (div.childNodes.length === 1 && div.firstChild instanceof HTMLElement) { + return div.firstChild; } - return body; + return div; } function nodes(html: string) {