From 32c61f434c7600e0492e21178a252d55b816bd44 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Wed, 23 Jan 2019 13:26:48 +0100 Subject: [PATCH] fix(ivy): ngUpgrade should distinguish element and module injectors (#28313) There are cases where we should check an element injector but don't go into the associated module injector if a token is not found. In both the view engine and ngIvy this is acheived by passing the `NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR` as the `notFoundValue`. Before this fix the view engine and ngIvy were using different objects to represent `NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR`. This was causing problems as ngUpgrade is using `NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR` const in its `NgAdapterInjector` to prevent searching of module injectors. This commit makes sure that ngIvy is using the same object to represent `NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR` as the view engine. PR Close #28313 --- packages/core/src/render3/component_ref.ts | 3 +- .../integration/downgrade_component_spec.ts | 80 ++++--- .../integration/downgrade_module_spec.ts | 202 +++++++++--------- 3 files changed, 140 insertions(+), 145 deletions(-) diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index 18318a9a64..9945c57f40 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -19,6 +19,7 @@ import {RendererFactory2} from '../render/api'; import {Sanitizer} from '../sanitization/security'; import {assertDefined} from '../util/assert'; import {VERSION} from '../version'; +import {NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR} from '../view/provider'; import {assertComponentType} from './assert'; import {LifecycleHooksFeature, createRootComponent, createRootComponentView, createRootContext} from './component'; @@ -74,8 +75,6 @@ export const SCHEDULER = new InjectionToken<((fn: () => void) => void)>('SCHEDUL factory: () => defaultScheduler, }); -const NOT_FOUND_CHECK_ONLY_ELEMENT_INJECTOR = {}; - function createChainedInjector(rootViewInjector: Injector, moduleInjector: Injector): Injector { return { get: (token: Type| InjectionToken, notFoundValue?: T): T => { diff --git a/packages/upgrade/test/static/integration/downgrade_component_spec.ts b/packages/upgrade/test/static/integration/downgrade_component_spec.ts index fe78f26f03..286d9db4ac 100644 --- a/packages/upgrade/test/static/integration/downgrade_component_spec.ts +++ b/packages/upgrade/test/static/integration/downgrade_component_spec.ts @@ -736,56 +736,54 @@ withEachNg1Version(() => { }); })); - fixmeIvy( - 'FW-717: Injector on lazy loaded components are not the same as their NgModule\'s injector') - .it('should work with ng2 lazy loaded components', async(() => { - let componentInjector: Injector; + it('should work with ng2 lazy loaded components', async(() => { + let componentInjector: Injector; - @Component({selector: 'ng2', template: ''}) - class Ng2Component { - constructor(injector: Injector) { componentInjector = injector; } - } + @Component({selector: 'ng2', template: ''}) + class Ng2Component { + constructor(injector: Injector) { componentInjector = injector; } + } - @NgModule({ - declarations: [Ng2Component], - entryComponents: [Ng2Component], - imports: [BrowserModule, UpgradeModule], - }) - class Ng2Module { - ngDoBootstrap() {} - } + @NgModule({ + declarations: [Ng2Component], + entryComponents: [Ng2Component], + imports: [BrowserModule, UpgradeModule], + }) + class Ng2Module { + ngDoBootstrap() {} + } - @Component({template: ''}) - class LazyLoadedComponent { - constructor(public module: NgModuleRef) {} - } + @Component({template: ''}) + class LazyLoadedComponent { + constructor(public module: NgModuleRef) {} + } - @NgModule({ - declarations: [LazyLoadedComponent], - entryComponents: [LazyLoadedComponent], - }) - class LazyLoadedModule { - } + @NgModule({ + declarations: [LazyLoadedComponent], + entryComponents: [LazyLoadedComponent], + }) + class LazyLoadedModule { + } - const ng1Module = angular.module('ng1', []).directive( - 'ng2', downgradeComponent({component: Ng2Component})); + const ng1Module = angular.module('ng1', []).directive( + 'ng2', downgradeComponent({component: Ng2Component})); - const element = html(''); + const element = html(''); - bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then(upgrade => { - const modInjector = upgrade.injector; - // Emulate the router lazy loading a module and creating a component - const compiler = modInjector.get(Compiler); - const modFactory = compiler.compileModuleSync(LazyLoadedModule); - const childMod = modFactory.create(modInjector); - const cmpFactory = childMod.componentFactoryResolver.resolveComponentFactory( - LazyLoadedComponent) !; - const lazyCmp = cmpFactory.create(componentInjector); + bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then(upgrade => { + const modInjector = upgrade.injector; + // Emulate the router lazy loading a module and creating a component + const compiler = modInjector.get(Compiler); + const modFactory = compiler.compileModuleSync(LazyLoadedModule); + const childMod = modFactory.create(modInjector); + const cmpFactory = + childMod.componentFactoryResolver.resolveComponentFactory(LazyLoadedComponent) !; + const lazyCmp = cmpFactory.create(componentInjector); - expect(lazyCmp.instance.module.injector === childMod.injector).toBe(true); - }); + expect(lazyCmp.instance.module.injector === childMod.injector).toBe(true); + }); - })); + })); it('should throw if `downgradedModule` is specified', async(() => { @Component({selector: 'ng2', template: ''}) diff --git a/packages/upgrade/test/static/integration/downgrade_module_spec.ts b/packages/upgrade/test/static/integration/downgrade_module_spec.ts index 663158ae11..051a87a88d 100644 --- a/packages/upgrade/test/static/integration/downgrade_module_spec.ts +++ b/packages/upgrade/test/static/integration/downgrade_module_spec.ts @@ -416,127 +416,125 @@ withEachNg1Version(() => { }); })); - fixmeIvy('FW-873: projected component injector hierarchy not wired up correctly') - .it('should correctly traverse the injector tree of downgraded components (from different modules)', - async(() => { - @Component({ - selector: 'ng2A', - template: 'ng2A()', - providers: [ - {provide: 'FOO', useValue: 'CompA-foo'}, - {provide: 'BAR', useValue: 'CompA-bar'}, - ], - }) - class Ng2ComponentA { - } + it('should correctly traverse the injector tree of downgraded components (from different modules)', + async(() => { + @Component({ + selector: 'ng2A', + template: 'ng2A()', + providers: [ + {provide: 'FOO', useValue: 'CompA-foo'}, + {provide: 'BAR', useValue: 'CompA-bar'}, + ], + }) + class Ng2ComponentA { + } - @Component({ - selector: 'ng2B', - template: ` + @Component({ + selector: 'ng2B', + template: ` FOO:{{ foo }} BAR:{{ bar }} BAZ:{{ baz }} QUX:{{ qux }} QUUX:{{ quux }} `, - providers: [ - {provide: 'FOO', useValue: 'CompB-foo'}, - ], - }) - class Ng2ComponentB { - constructor( - @Inject('FOO') public foo: string, @Inject('BAR') public bar: string, - @Inject('BAZ') public baz: string, @Inject('QUX') public qux: string, - @Inject('QUUX') public quux: string) {} - } + providers: [ + {provide: 'FOO', useValue: 'CompB-foo'}, + ], + }) + class Ng2ComponentB { + constructor( + @Inject('FOO') public foo: string, @Inject('BAR') public bar: string, + @Inject('BAZ') public baz: string, @Inject('QUX') public qux: string, + @Inject('QUUX') public quux: string) {} + } - @NgModule({ - declarations: [Ng2ComponentA], - entryComponents: [Ng2ComponentA], - imports: [BrowserModule], - providers: [ - {provide: 'FOO', useValue: 'ModA-foo'}, - {provide: 'BAR', useValue: 'ModA-bar'}, - {provide: 'BAZ', useValue: 'ModA-baz'}, - {provide: 'QUX', useValue: 'ModA-qux'}, - ], - }) - class Ng2ModuleA { - ngDoBootstrap() {} - } + @NgModule({ + declarations: [Ng2ComponentA], + entryComponents: [Ng2ComponentA], + imports: [BrowserModule], + providers: [ + {provide: 'FOO', useValue: 'ModA-foo'}, + {provide: 'BAR', useValue: 'ModA-bar'}, + {provide: 'BAZ', useValue: 'ModA-baz'}, + {provide: 'QUX', useValue: 'ModA-qux'}, + ], + }) + class Ng2ModuleA { + ngDoBootstrap() {} + } - @NgModule({ - declarations: [Ng2ComponentB], - entryComponents: [Ng2ComponentB], - imports: [BrowserModule], - providers: [ - {provide: 'FOO', useValue: 'ModB-foo'}, - {provide: 'BAR', useValue: 'ModB-bar'}, - {provide: 'BAZ', useValue: 'ModB-baz'}, - ], - }) - class Ng2ModuleB { - ngDoBootstrap() {} - } + @NgModule({ + declarations: [Ng2ComponentB], + entryComponents: [Ng2ComponentB], + imports: [BrowserModule], + providers: [ + {provide: 'FOO', useValue: 'ModB-foo'}, + {provide: 'BAR', useValue: 'ModB-bar'}, + {provide: 'BAZ', useValue: 'ModB-baz'}, + ], + }) + class Ng2ModuleB { + ngDoBootstrap() {} + } - const doDowngradeModule = (module: Type) => { - const bootstrapFn = (extraProviders: StaticProvider[]) => { - const platformRef = getPlatform() || platformBrowserDynamic([ - ...extraProviders, - {provide: 'FOO', useValue: 'Plat-foo'}, - {provide: 'BAR', useValue: 'Plat-bar'}, - {provide: 'BAZ', useValue: 'Plat-baz'}, - {provide: 'QUX', useValue: 'Plat-qux'}, - {provide: 'QUUX', useValue: 'Plat-quux'}, - ]); - return platformRef.bootstrapModule(module); - }; - return downgradeModule(bootstrapFn); - }; + const doDowngradeModule = (module: Type) => { + const bootstrapFn = (extraProviders: StaticProvider[]) => { + const platformRef = getPlatform() || platformBrowserDynamic([ + ...extraProviders, + {provide: 'FOO', useValue: 'Plat-foo'}, + {provide: 'BAR', useValue: 'Plat-bar'}, + {provide: 'BAZ', useValue: 'Plat-baz'}, + {provide: 'QUX', useValue: 'Plat-qux'}, + {provide: 'QUUX', useValue: 'Plat-quux'}, + ]); + return platformRef.bootstrapModule(module); + }; + return downgradeModule(bootstrapFn); + }; - const downModA = doDowngradeModule(Ng2ModuleA); - const downModB = doDowngradeModule(Ng2ModuleB); - const ng1Module = angular.module('ng1', [downModA, downModB]) - .directive('ng2A', downgradeComponent({ - component: Ng2ComponentA, - downgradedModule: downModA, propagateDigest, - })) - .directive('ng2B', downgradeComponent({ - component: Ng2ComponentB, - downgradedModule: downModB, propagateDigest, - })); + const downModA = doDowngradeModule(Ng2ModuleA); + const downModB = doDowngradeModule(Ng2ModuleB); + const ng1Module = angular.module('ng1', [downModA, downModB]) + .directive('ng2A', downgradeComponent({ + component: Ng2ComponentA, + downgradedModule: downModA, propagateDigest, + })) + .directive('ng2B', downgradeComponent({ + component: Ng2ComponentB, + downgradedModule: downModB, propagateDigest, + })); - const element = html(` + const element = html(` `); - const $injector = angular.bootstrap(element, [ng1Module.name]); - const $rootScope = $injector.get($ROOT_SCOPE) as angular.IRootScopeService; + const $injector = angular.bootstrap(element, [ng1Module.name]); + const $rootScope = $injector.get($ROOT_SCOPE) as angular.IRootScopeService; - // Wait for module A to be bootstrapped. - setTimeout(() => { - expect(multiTrim(element.textContent)).toBe('ng2A()'); + // Wait for module A to be bootstrapped. + setTimeout(() => { + expect(multiTrim(element.textContent)).toBe('ng2A()'); - // Nested component B. - $rootScope.$apply('showB1 = true'); + // Nested component B. + $rootScope.$apply('showB1 = true'); - // Wait for module B to be bootstrapped. - setTimeout(() => { - // It is debatable, whether the order of traversal should be: - // CompB > CompA > ModB > ModA > Plat (similar to how lazy-loaded components - // work) - expect(multiTrim(element.children[0].textContent)) - .toBe( - 'ng2A( FOO:CompB-foo BAR:CompA-bar BAZ:ModB-baz QUX:Plat-qux QUUX:Plat-quux )'); + // Wait for module B to be bootstrapped. + setTimeout(() => { + // It is debatable, whether the order of traversal should be: + // CompB > CompA > ModB > ModA > Plat (similar to how lazy-loaded components + // work) + expect(multiTrim(element.children[0].textContent)) + .toBe( + 'ng2A( FOO:CompB-foo BAR:CompA-bar BAZ:ModB-baz QUX:Plat-qux QUUX:Plat-quux )'); - // Standalone component B. - $rootScope.$apply('showB2 = true'); - expect(multiTrim(element.children[1].textContent)) - .toBe( - 'FOO:CompB-foo BAR:ModB-bar BAZ:ModB-baz QUX:Plat-qux QUUX:Plat-quux'); - }); - }); - })); + // Standalone component B. + $rootScope.$apply('showB2 = true'); + expect(multiTrim(element.children[1].textContent)) + .toBe('FOO:CompB-foo BAR:ModB-bar BAZ:ModB-baz QUX:Plat-qux QUUX:Plat-quux'); + }); + }); + })); it('should support downgrading a component and propagate inputs', async(() => { @Component(