From 9eefe25e2f324eec024e421fa5dc1703683d7840 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Fri, 19 Jul 2019 18:45:21 +0200 Subject: [PATCH] fix(ivy): components created with wrong context when passing root node to component factory (#31661) The way the `ComponentFactory.create` is set up at the moment is that if a `rootSelectorOrNode` is passed in, the root context will be injected instead of creating dedicated one for the component. As far as I can tell, there doesn't seem to be a reason to do this and nothing seems to break because of it. These changes switch to always create the root context. PR Close #31661 --- packages/core/src/render3/component_ref.ts | 24 ++---- .../core/test/acceptance/component_spec.ts | 84 ++++++++++++++++++- 2 files changed, 89 insertions(+), 19 deletions(-) diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index 7f3a321288..a2f787db70 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -9,7 +9,6 @@ import {ChangeDetectorRef as ViewEngine_ChangeDetectorRef} from '../change_detection/change_detector_ref'; import {InjectionToken} from '../di/injection_token'; import {Injector} from '../di/injector'; -import {ɵɵinject} from '../di/injector_compatibility'; import {InjectFlags} from '../di/interface/injector'; import {Type} from '../interface/type'; import {ComponentFactory as viewEngine_ComponentFactory, ComponentRef as viewEngine_ComponentRef} from '../linker/component_factory'; @@ -29,7 +28,7 @@ import {addToViewTree, assignTViewNodeToLView, createLView, createTView, element import {ComponentDef} from './interfaces/definition'; import {TContainerNode, TElementContainerNode, TElementNode} from './interfaces/node'; import {RNode, RendererFactory3, domRendererFactory3, isProceduralRenderer} from './interfaces/renderer'; -import {LView, LViewFlags, RootContext, TVIEW} from './interfaces/view'; +import {LView, LViewFlags, TVIEW} from './interfaces/view'; import {enterView, leaveView, namespaceHTMLInternal} from './state'; import {defaultScheduler} from './util/misc_utils'; import {getTNode} from './util/view_utils'; @@ -60,13 +59,6 @@ function toRefArray(map: {[key: string]: string}): {propName: string; templateNa return array; } -/** - * Default {@link RootContext} for all components rendered with {@link renderComponent}. - */ -export const ROOT_CONTEXT = new InjectionToken( - 'ROOT_CONTEXT_TOKEN', - {providedIn: 'root', factory: () => createRootContext(ɵɵinject(SCHEDULER))}); - /** * A change detection scheduler token for {@link RootContext}. This token is the default value used * for the default `RootContext` found in the {@link ROOT_CONTEXT} token. @@ -130,7 +122,6 @@ export class ComponentFactory extends viewEngine_ComponentFactory { create( injector: Injector, projectableNodes?: any[][]|undefined, rootSelectorOrNode?: any, ngModule?: viewEngine_NgModuleRef|undefined): viewEngine_ComponentRef { - const isInternalRootView = rootSelectorOrNode === undefined; ngModule = ngModule || this.ngModule; const rootViewInjector = @@ -143,9 +134,9 @@ export class ComponentFactory extends viewEngine_ComponentFactory { // Ensure that the namespace for the root node is correct, // otherwise the browser might not render out the element properly. namespaceHTMLInternal(); - const hostRNode = isInternalRootView ? - elementCreate(this.selector, rendererFactory.createRenderer(null, this.componentDef)) : - locateHostElement(rendererFactory, rootSelectorOrNode); + const hostRNode = rootSelectorOrNode ? + locateHostElement(rendererFactory, rootSelectorOrNode) : + elementCreate(this.selector, rendererFactory.createRenderer(null, this.componentDef)); const rootFlags = this.componentDef.onPush ? LViewFlags.Dirty | LViewFlags.IsRoot : LViewFlags.CheckAlways | LViewFlags.IsRoot; @@ -157,10 +148,7 @@ export class ComponentFactory extends viewEngine_ComponentFactory { const isIsolated = typeof rootSelectorOrNode === 'string' && /^#root-ng-internal-isolated-\d+/.test(rootSelectorOrNode); - const rootContext: RootContext = (isInternalRootView || isIsolated) ? - createRootContext() : - rootViewInjector.get(ROOT_CONTEXT); - + const rootContext = createRootContext(); const renderer = rendererFactory.createRenderer(hostRNode, this.componentDef); if (rootSelectorOrNode && hostRNode) { @@ -214,7 +202,7 @@ export class ComponentFactory extends viewEngine_ComponentFactory { this.componentType, component, createElementRef(viewEngine_ElementRef, tElementNode, rootLView), rootLView, tElementNode); - if (isInternalRootView || isIsolated) { + if (!rootSelectorOrNode || isIsolated) { // The host element of the internal or isolated root view is attached to the component's host // view node. componentRef.hostView._tViewNode !.child = tElementNode; diff --git a/packages/core/test/acceptance/component_spec.ts b/packages/core/test/acceptance/component_spec.ts index be2040bdb6..0997e46423 100644 --- a/packages/core/test/acceptance/component_spec.ts +++ b/packages/core/test/acceptance/component_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Component, ComponentFactoryResolver, ComponentRef, ElementRef, InjectionToken, NgModule, OnDestroy, Renderer2, Type, ViewChild, ViewContainerRef, ViewEncapsulation} from '@angular/core'; +import {Component, ComponentFactoryResolver, ComponentRef, ElementRef, InjectionToken, Injector, Input, NgModule, OnDestroy, Renderer2, Type, ViewChild, ViewContainerRef, ViewEncapsulation} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; @@ -253,4 +253,86 @@ describe('component', () => { .toBe(true, 'Expected renderers to be different.'); }); + it('components should not share the same context when creating with a root element', () => { + const log: string[] = []; + @Component({ + selector: 'comp-a', + template: '
{{ a }}
', + }) + class CompA { + @Input() a: string = ''; + ngDoCheck() { log.push('CompA:ngDoCheck'); } + } + + @Component({ + selector: 'comp-b', + template: '
{{ b }}
', + }) + class CompB { + @Input() b: string = ''; + ngDoCheck() { log.push('CompB:ngDoCheck'); } + } + + @Component({template: ``}) + class MyCompA { + constructor( + private _componentFactoryResolver: ComponentFactoryResolver, + private _injector: Injector) {} + + createComponent() { + const componentFactoryA = this._componentFactoryResolver.resolveComponentFactory(CompA); + const compRefA = + componentFactoryA.create(this._injector, [], document.createElement('div')); + return compRefA; + } + } + + @Component({template: ``}) + class MyCompB { + constructor(private cfr: ComponentFactoryResolver, private injector: Injector) {} + + createComponent() { + const componentFactoryB = this.cfr.resolveComponentFactory(CompB); + const compRefB = componentFactoryB.create(this.injector, [], document.createElement('div')); + return compRefB; + } + } + + @NgModule({ + declarations: [CompA], + entryComponents: [CompA], + }) + class MyModuleA { + } + + @NgModule({ + declarations: [CompB], + entryComponents: [CompB], + }) + class MyModuleB { + } + + TestBed.configureTestingModule({ + declarations: [MyCompA, MyCompB], + imports: [MyModuleA, MyModuleB], + }); + const fixtureA = TestBed.createComponent(MyCompA); + fixtureA.detectChanges(); + const compA = fixtureA.componentInstance.createComponent(); + compA.instance.a = 'a'; + compA.changeDetectorRef.detectChanges(); + + expect(log).toEqual(['CompA:ngDoCheck']); + + log.length = 0; // reset the log + + const fixtureB = TestBed.createComponent(MyCompB); + fixtureB.detectChanges(); + const compB = fixtureB.componentInstance.createComponent(); + compB.instance.b = 'b'; + compB.changeDetectorRef.detectChanges(); + + expect(log).toEqual(['CompB:ngDoCheck']); + }); + });