From d5f47d6b710c0d6fd6b8a14e2319880127ad838b Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Thu, 20 Sep 2018 11:48:06 -0700 Subject: [PATCH] refactor(ivy): special injection tokens should not be cached (#26048) PR Close #26048 --- packages/core/src/render3/component.ts | 3 +- packages/core/src/render3/di.ts | 160 +++++------ packages/core/src/render3/instructions.ts | 41 +-- .../core/src/render3/interfaces/injector.ts | 16 -- packages/core/src/render3/query.ts | 17 +- packages/core/src/render3/view_ref.ts | 18 +- .../hello_world/bundle.golden_symbols.json | 3 - .../bundling/todo/bundle.golden_symbols.json | 30 +-- .../todo_r2/bundle.golden_symbols.json | 30 +-- packages/core/test/render3/di_spec.ts | 254 ++++++++++-------- .../core/test/render3/integration_spec.ts | 2 +- 11 files changed, 268 insertions(+), 306 deletions(-) diff --git a/packages/core/src/render3/component.ts b/packages/core/src/render3/component.ts index bdf468c8ee..0250a22a86 100644 --- a/packages/core/src/render3/component.ts +++ b/packages/core/src/render3/component.ts @@ -14,7 +14,7 @@ import {Sanitizer} from '../sanitization/security'; import {assertComponentType, assertDefined} from './assert'; import {queueInitHooks, queueLifecycleHooks} from './hooks'; -import {CLEAN_PROMISE, baseDirectiveCreate, createLViewData, createTView, detectChangesInternal, enterView, executeInitAndContentHooks, getRootView, hostElement, initChangeDetectorIfExisting, leaveView, locateHostElement, setHostBindings, queueHostBindingForCheck,} from './instructions'; +import {CLEAN_PROMISE, baseDirectiveCreate, createLViewData, createTView, detectChangesInternal, enterView, executeInitAndContentHooks, getRootView, hostElement, leaveView, locateHostElement, setHostBindings, queueHostBindingForCheck,} from './instructions'; import {ComponentDef, ComponentDefInternal, ComponentType} from './interfaces/definition'; import {LElementNode} from './interfaces/node'; import {RElement, RendererFactory3, domRendererFactory3} from './interfaces/renderer'; @@ -150,7 +150,6 @@ export function createRootComponent( if (componentDef.hostBindings) queueHostBindingForCheck(0, componentDef.hostVars); rootContext.components.push(component); (elementNode.data as LViewData)[CONTEXT] = component; - initChangeDetectorIfExisting(elementNode.nodeInjector, component, elementNode.data as LViewData); hostFeatures && hostFeatures.forEach((feature) => feature(component, componentDef)); setHostBindings(rootView[TVIEW].hostBindings); diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index 84d4161b70..1299ed2558 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -145,10 +145,6 @@ export function getOrCreateNodeInjectorForNode( cbf5: parentInjector == null ? 0 : parentInjector.cbf5 | parentInjector.bf5, cbf6: parentInjector == null ? 0 : parentInjector.cbf6 | parentInjector.bf6, cbf7: parentInjector == null ? 0 : parentInjector.cbf7 | parentInjector.bf7, - templateRef: null, - viewContainerRef: null, - elementRef: null, - changeDetectorRef: null, }; } @@ -207,7 +203,7 @@ export function directiveInject( * @returns The ElementRef instance to use */ export function injectElementRef(): viewEngine_ElementRef { - return getOrCreateElementRef(getOrCreateNodeInjector()); + return createElementRef(getPreviousOrParentTNode(), _getViewData()); } /** @@ -217,7 +213,7 @@ export function injectElementRef(): viewEngine_ElementRef { * @returns The TemplateRef instance to use */ export function injectTemplateRef(): viewEngine_TemplateRef { - return getOrCreateTemplateRef(getOrCreateNodeInjector()); + return createTemplateRef(getPreviousOrParentTNode(), _getViewData()); } /** @@ -227,12 +223,14 @@ export function injectTemplateRef(): viewEngine_TemplateRef { * @returns The ViewContainerRef instance to use */ export function injectViewContainerRef(): viewEngine_ViewContainerRef { - return getOrCreateContainerRef(getOrCreateNodeInjector()); + const previousTNode = + getPreviousOrParentTNode() as TElementNode | TElementContainerNode | TContainerNode; + return createContainerRef(previousTNode, _getViewData()); } /** Returns a ChangeDetectorRef (a.k.a. a ViewRef) */ export function injectChangeDetectorRef(): viewEngine_ChangeDetectorRef { - return getOrCreateChangeDetectorRef(getOrCreateNodeInjector(), null); + return createViewRef(getPreviousOrParentTNode(), _getViewData(), null); } /** @@ -302,34 +300,25 @@ export function injectAttribute(attrNameToInject: string): string|undefined { /** * Creates a ViewRef and stores it on the injector as ChangeDetectorRef (public alias). - * Or, if it already exists, retrieves the existing instance. * + * @param hostTNode The node that is requesting a ChangeDetectorRef + * @param hostView The view to which the node belongs + * @param context The context for this change detector ref * @returns The ChangeDetectorRef to use */ -export function getOrCreateChangeDetectorRef( - di: LInjector, context: any): viewEngine_ChangeDetectorRef { - if (di.changeDetectorRef) return di.changeDetectorRef; - - const currentTNode = di.tNode; - if (isComponent(currentTNode)) { - return di.changeDetectorRef = - new ViewRef(getLNode(currentTNode, di.view).data as LViewData, context); - } else if (currentTNode.type === TNodeType.Element) { - return di.changeDetectorRef = getOrCreateHostChangeDetector(di.view); +export function createViewRef( + hostTNode: TNode, hostView: LViewData, context: any): viewEngine_ChangeDetectorRef { + if (isComponent(hostTNode)) { + const componentIndex = hostTNode.flags >> TNodeFlags.DirectiveStartingIndexShift; + const componentView = getLNode(hostTNode, hostView).data as LViewData; + return new ViewRef(componentView, context, componentIndex); + } else if (hostTNode.type === TNodeType.Element) { + const hostComponentView = findComponentView(hostView); + return new ViewRef(hostComponentView, hostComponentView[CONTEXT], -1); } return null !; } -/** Gets or creates ChangeDetectorRef for the closest host component */ -function getOrCreateHostChangeDetector(currentView: LViewData): viewEngine_ChangeDetectorRef { - const hostComponentView = findComponentView(currentView); - const hostNode = getHostElementNode(hostComponentView) !; - const hostInjector = hostNode.nodeInjector; - const existingRef = hostInjector && hostInjector.changeDetectorRef; - - return existingRef ? existingRef : new ViewRef(hostComponentView, hostComponentView[CONTEXT]); -} - function getOrCreateRenderer2(di: LInjector): Renderer2 { const renderer = di.view[RENDERER]; if (isProceduralRenderer(renderer)) { @@ -537,44 +526,44 @@ function sameHostView(injector: LInjector): boolean { } export class ReadFromInjectorFn { - constructor(readonly read: (injector: LInjector, tNode: TNode, directiveIndex?: number) => T) {} + constructor(readonly read: (tNode: TNode, view: LViewData, directiveIndex?: number) => T) {} } /** * Creates an ElementRef for a given node injector and stores it on the injector. - * Or, if the ElementRef already exists, retrieves the existing ElementRef. * * @param di The node injector where we should store a created ElementRef * @returns The ElementRef instance to use */ -export function getOrCreateElementRef(di: LInjector): viewEngine_ElementRef { - return di.elementRef || (di.elementRef = new ElementRef(getLNode(di.tNode, di.view).native)); +export function createElementRef(tNode: TNode, view: LViewData): viewEngine_ElementRef { + return new ElementRef(getLNode(tNode, view).native); } export const QUERY_READ_TEMPLATE_REF = >>( new ReadFromInjectorFn>( - (injector: LInjector) => getOrCreateTemplateRef(injector)) as any); + (tNode: TNode, view: LViewData) => { return createTemplateRef(tNode, view);}) as any); export const QUERY_READ_CONTAINER_REF = >( new ReadFromInjectorFn( - (injector: LInjector) => getOrCreateContainerRef(injector)) as any); + (tNode: TNode, view: LViewData) => createContainerRef( + tNode as TElementNode | TContainerNode | TElementContainerNode, view)) as any); export const QUERY_READ_ELEMENT_REF = >(new ReadFromInjectorFn( - (injector: LInjector) => getOrCreateElementRef(injector)) as any); + (tNode: TNode, view: LViewData) => createElementRef(tNode, view)) as any); export const QUERY_READ_FROM_NODE = - (new ReadFromInjectorFn((injector: LInjector, tNode: TNode, directiveIdx: number) => { + (new ReadFromInjectorFn((tNode: TNode, view: LViewData, directiveIdx: number) => { ngDevMode && assertNodeOfPossibleTypes( tNode, TNodeType.Container, TNodeType.Element, TNodeType.ElementContainer); if (directiveIdx > -1) { - return injector.view[DIRECTIVES] ![directiveIdx]; + return view[DIRECTIVES] ![directiveIdx]; } if (tNode.type === TNodeType.Element || tNode.type === TNodeType.ElementContainer) { - return getOrCreateElementRef(injector); + return createElementRef(tNode, view); } if (tNode.type === TNodeType.Container) { - return getOrCreateTemplateRef(injector); + return createTemplateRef(tNode, view); } if (ngDevMode) { // should never happen @@ -586,42 +575,38 @@ export const QUERY_READ_FROM_NODE = class ElementRef extends viewEngine_ElementRef {} /** - * Creates a ViewContainerRef and stores it on the injector. Or, if the ViewContainerRef - * already exists, retrieves the existing ViewContainerRef. + * Creates a ViewContainerRef and stores it on the injector. * + * @param hostTNode The node that is requesting a ViewContainerRef + * @param hostView The view to which the node belongs * @returns The ViewContainerRef instance to use */ -export function getOrCreateContainerRef(di: LInjector): viewEngine_ViewContainerRef { - if (!di.viewContainerRef) { - const hostLNode = - getPreviousOrParentNode() as LElementNode | LContainerNode | LElementContainerNode; - const hostTNode = getPreviousOrParentTNode() as TElementNode | TContainerNode; - ngDevMode && assertNodeOfPossibleTypes( - hostTNode, TNodeType.Container, TNodeType.Element, TNodeType.ElementContainer); +export function createContainerRef( + hostTNode: TElementNode | TContainerNode | TElementContainerNode, + hostView: LViewData): viewEngine_ViewContainerRef { + const hostLNode = getLNode(hostTNode, hostView); + ngDevMode && assertNodeOfPossibleTypes( + hostTNode, TNodeType.Container, TNodeType.Element, TNodeType.ElementContainer); - const hostView = di.view; - const lContainer = createLContainer(hostView, true); - const comment = hostView[RENDERER].createComment(ngDevMode ? 'container' : ''); - const lContainerNode: LContainerNode = - createLNodeObject(TNodeType.Container, hostLNode.nodeInjector, comment, lContainer); + const lContainer = createLContainer(hostView, true); + const comment = hostView[RENDERER].createComment(ngDevMode ? 'container' : ''); + const lContainerNode: LContainerNode = + createLNodeObject(TNodeType.Container, hostLNode.nodeInjector, comment, lContainer); - lContainer[RENDER_PARENT] = getRenderParent(hostTNode, hostView); + lContainer[RENDER_PARENT] = getRenderParent(hostTNode, hostView); - appendChild(comment, hostTNode, hostView); + appendChild(comment, hostTNode, hostView); - if (!hostTNode.dynamicContainerNode) { - hostTNode.dynamicContainerNode = - createTNode(TNodeType.Container, -1, null, null, hostTNode, null); - } - - hostLNode.dynamicLContainerNode = lContainerNode; - addToViewTree(hostView, hostTNode.index as number, lContainer); - - di.viewContainerRef = new ViewContainerRef( - lContainer, hostTNode.dynamicContainerNode as TContainerNode, hostTNode, hostView); + if (!hostTNode.dynamicContainerNode) { + hostTNode.dynamicContainerNode = + createTNode(TNodeType.Container, -1, null, null, hostTNode, null); } - return di.viewContainerRef; + hostLNode.dynamicLContainerNode = lContainerNode; + addToViewTree(hostView, hostTNode.index as number, lContainer); + + return new ViewContainerRef( + lContainer, hostTNode.dynamicContainerNode as TContainerNode, hostTNode, hostView); } export class NodeInjector implements Injector { @@ -629,16 +614,16 @@ export class NodeInjector implements Injector { get(token: any): any { if (token === viewEngine_TemplateRef) { - return getOrCreateTemplateRef(this._lInjector); + return createTemplateRef(this._lInjector.tNode, this._lInjector.view); } if (token === viewEngine_ViewContainerRef) { - return getOrCreateContainerRef(this._lInjector); + return createContainerRef(this._lInjector.tNode, this._lInjector.view); } if (token === viewEngine_ElementRef) { - return getOrCreateElementRef(this._lInjector); + return createElementRef(this._lInjector.tNode, this._lInjector.view); } if (token === viewEngine_ChangeDetectorRef) { - return getOrCreateChangeDetectorRef(this._lInjector, null); + return createViewRef(this._lInjector.tNode, this._lInjector.view, null); } if (token === Renderer2) { return getOrCreateRenderer2(this._lInjector); @@ -666,7 +651,7 @@ class ViewContainerRef extends viewEngine_ViewContainerRef { // TODO: Remove LNode lookup when removing LNode.nodeInjector const injector = getOrCreateNodeInjectorForNode(this._getHostNode(), this._hostTNode, this._hostView); - return getOrCreateElementRef(injector); + return createElementRef(injector.tNode, injector.view); } get injector(): Injector { @@ -776,23 +761,20 @@ class ViewContainerRef extends viewEngine_ViewContainerRef { } /** - * Creates a TemplateRef and stores it on the injector. Or, if the TemplateRef already - * exists, retrieves the existing TemplateRef. + * Creates a TemplateRef and stores it on the injector. * - * @param di The node injector where we should store a created TemplateRef + * @param hostTNode The node that is requesting a TemplateRef + * @param hostView The view to which the node belongs * @returns The TemplateRef instance to use */ -export function getOrCreateTemplateRef(di: LInjector): viewEngine_TemplateRef { - if (!di.templateRef) { - const hostNode = getPreviousOrParentNode() as LContainerNode; - const hostTNode = getPreviousOrParentTNode() as TContainerNode; - ngDevMode && assertNodeType(hostTNode, TNodeType.Container); - ngDevMode && assertDefined(hostTNode.tViews, 'TView must be allocated'); - di.templateRef = new TemplateRef( - di.view, getOrCreateElementRef(di), hostTNode.tViews as TView, getRenderer(), - hostNode.data ![QUERIES]); - } - return di.templateRef; +export function createTemplateRef( + hostTNode: TNode, hostView: LViewData): viewEngine_TemplateRef { + const hostNode = getLNode(hostTNode, hostView); + ngDevMode && assertNodeType(hostTNode, TNodeType.Container); + ngDevMode && assertDefined(hostTNode.tViews, 'TView must be allocated'); + return new TemplateRef( + hostView, createElementRef(hostTNode, hostView), hostTNode.tViews as TView, getRenderer(), + hostNode.data ![QUERIES]); } export function getFactoryOf(type: Type): ((type?: Type) => T)|null { @@ -835,7 +817,7 @@ class TemplateRef extends viewEngine_TemplateRef { insertView(lView, container, hostView !, index !, tContainerNode !.parent !.index); } renderEmbeddedTemplate(lView, this._tView, context, RenderFlags.Create); - const viewRef = new ViewRef(lView, context); + const viewRef = new ViewRef(lView, context, -1); viewRef._tViewNode = lView[HOST_NODE] as TViewNode; return viewRef; } @@ -846,7 +828,5 @@ class TemplateRef extends viewEngine_TemplateRef { * `` element. */ export function templateRefExtractor(tNode: TContainerNode, currentView: LViewData) { - // TODO: remove this lookup with removing LNode.nodeInjector - const lNode = getLNode(tNode, currentView) as LContainerNode; - return getOrCreateTemplateRef(getOrCreateNodeInjectorForNode(lNode, tNode, currentView)); + return createTemplateRef(tNode, currentView); } diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 0bad2ce790..cb3397ac7f 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -899,12 +899,18 @@ function findDirectiveMatches(tNode: TNode): CurrentMatchesList|null { for (let i = 0; i < registry.length; i++) { const def = registry[i]; if (isNodeMatchingSelectorList(tNode, def.selectors !)) { + matches || (matches = []); if ((def as ComponentDefInternal).template) { if (tNode.flags & TNodeFlags.isComponent) throwMultipleComponentError(tNode); + addComponentLogic(def as ComponentDefInternal); tNode.flags = TNodeFlags.isComponent; + + // The component is always stored first with directives after. + matches.unshift(def, null); + } else { + matches.push(def, null); } if (def.diPublic) def.diPublic(def); - (matches || (matches = [])).push(def, null); } } } @@ -948,14 +954,6 @@ export function queueHostBindingForCheck(dirIndex: number, hostVars: number): vo ])).push(dirIndex, previousOrParentTNode.index - HEADER_OFFSET); } -/** Sets the context for a ChangeDetectorRef to the given instance. */ -export function initChangeDetectorIfExisting( - injector: LInjector | null, instance: any, view: LViewData): void { - if (injector && injector.changeDetectorRef != null) { - (injector.changeDetectorRef as ViewRef)._setComponentContext(view, instance); - } -} - /** * This function instantiates the given directives. */ @@ -976,6 +974,12 @@ function instantiateDirectivesDirectly() { for (let i = start; i < end; i++) { const def: DirectiveDefInternal = tDirectives[i]; + + // Component view must be set on node before the factory is created so + // ChangeDetectorRefs have a way to store component view on creation. + if ((def as ComponentDefInternal).template) { + addComponentLogic(def as ComponentDefInternal); + } directiveCreate(i, def.factory(), def); } } @@ -1698,13 +1702,11 @@ export function textBinding(index: number, value: T | NO_CHANGE): void { export function directiveCreate( directiveDefIdx: number, directive: T, directiveDef: DirectiveDefInternal| ComponentDefInternal): T { - const hostNode = getPreviousOrParentNode() !; + const hostNode = getLNode(previousOrParentTNode, viewData); const instance = baseDirectiveCreate(directiveDefIdx, directive, directiveDef, hostNode); - const isComponent = (directiveDef as ComponentDefInternal).template; - if (isComponent) { - addComponentLogic( - directiveDefIdx, directive, directiveDef as ComponentDefInternal, hostNode); + if ((directiveDef as ComponentDefInternal).template) { + hostNode.data ![CONTEXT] = directive; } if (firstTemplatePass) { @@ -1727,8 +1729,9 @@ export function directiveCreate( return instance; } -function addComponentLogic( - directiveIndex: number, instance: T, def: ComponentDefInternal, hostNode: LNode): void { +function addComponentLogic(def: ComponentDefInternal): void { + const hostNode = getLNode(previousOrParentTNode, viewData); + const tView = getOrCreateTView( def.template, def.consts, def.vars, def.directiveDefs, def.pipeDefs, def.viewQuery); @@ -1737,15 +1740,13 @@ function addComponentLogic( const componentView = addToViewTree( viewData, previousOrParentTNode.index as number, createLViewData( - rendererFactory.createRenderer(hostNode.native as RElement, def), tView, instance, + rendererFactory.createRenderer(hostNode.native as RElement, def), tView, null, def.onPush ? LViewFlags.Dirty : LViewFlags.CheckAlways, getCurrentSanitizer())); // We need to set the host node/data here because when the component LNode was created, // we didn't yet know it was a component (just an element). (hostNode as{data: LViewData}).data = componentView; - (componentView as LViewData)[HOST_NODE] = getPreviousOrParentTNode() as TElementNode; - - initChangeDetectorIfExisting(hostNode.nodeInjector, instance, componentView); + (componentView as LViewData)[HOST_NODE] = previousOrParentTNode as TElementNode; if (firstTemplatePass) queueComponentIndexForCheck(); } diff --git a/packages/core/src/render3/interfaces/injector.ts b/packages/core/src/render3/interfaces/injector.ts index abaf485e45..a20fb83dad 100644 --- a/packages/core/src/render3/interfaces/injector.ts +++ b/packages/core/src/render3/interfaces/injector.ts @@ -70,22 +70,6 @@ export interface LInjector { cbf5: number; cbf6: number; cbf7: number; - - /** Stores the TemplateRef so subsequent injections of the TemplateRef get the same instance. */ - templateRef: TemplateRef|null; - - /** Stores the ViewContainerRef so subsequent injections of the ViewContainerRef get the same - * instance. */ - viewContainerRef: ViewContainerRef|null; - - /** Stores the ElementRef so subsequent injections of the ElementRef get the same instance. */ - elementRef: ElementRef|null; - - /** - * Stores the ChangeDetectorRef so subsequent injections of the ChangeDetectorRef get the - * same instance. - */ - changeDetectorRef: ChangeDetectorRef|null; } // Note: This hack is necessary so we don't erroneously get a circular dependency diff --git a/packages/core/src/render3/query.ts b/packages/core/src/render3/query.ts index 725c442e96..14b3a9cd32 100644 --- a/packages/core/src/render3/query.ts +++ b/packages/core/src/render3/query.ts @@ -265,10 +265,10 @@ function getIdxOfMatchingDirective(tNode: TNode, currentView: LViewData, type: T } function readFromNodeInjector( - nodeInjector: LInjector, tNode: TNode, currentView: LViewData, - read: QueryReadType| Type, directiveIdx: number): any { + tNode: TNode, currentView: LViewData, read: QueryReadType| Type, + directiveIdx: number): any { if (read instanceof ReadFromInjectorFn) { - return read.read(nodeInjector, tNode, directiveIdx); + return read.read(tNode, currentView, directiveIdx); } else { const matchingIdx = getIdxOfMatchingDirective(tNode, currentView, read as Type); if (matchingIdx !== null) { @@ -282,10 +282,6 @@ function add( query: LQuery| null, tNode: TElementNode | TContainerNode | TElementContainerNode) { const currentView = _getViewData(); - // TODO: remove this lookup when nodeInjector is removed from LNode - const nodeInjector = - getOrCreateNodeInjectorForNode(getLNode(tNode, currentView), tNode, currentView); - while (query) { const predicate = query.predicate; const type = predicate.type; @@ -294,8 +290,8 @@ function add( if (directiveIdx !== null) { // a node is matching a predicate - determine what to read // if read token and / or strategy is not specified, use type as read token - const result = readFromNodeInjector( - nodeInjector, tNode, currentView, predicate.read || type, directiveIdx); + const result = + readFromNodeInjector(tNode, currentView, predicate.read || type, directiveIdx); if (result !== null) { addMatch(query, result); } @@ -308,8 +304,7 @@ function add( // a node is matching a predicate - determine what to read // note that queries using name selector must specify read strategy ngDevMode && assertDefined(predicate.read, 'the node should have a predicate'); - const result = readFromNodeInjector( - nodeInjector, tNode, currentView, predicate.read !, directiveIdx); + const result = readFromNodeInjector(tNode, currentView, predicate.read !, directiveIdx); if (result !== null) { addMatch(query, result); } diff --git a/packages/core/src/render3/view_ref.ts b/packages/core/src/render3/view_ref.ts index f86c462ded..f43247d074 100644 --- a/packages/core/src/render3/view_ref.ts +++ b/packages/core/src/render3/view_ref.ts @@ -13,7 +13,7 @@ import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, InternalViewRef as viewEn import {checkNoChanges, checkNoChangesInRootView, detectChanges, detectChangesInRootView, getRendererFactory, markViewDirty, storeCleanupFn, viewAttached} from './instructions'; import {TViewNode} from './interfaces/node'; -import {FLAGS, LViewData, LViewFlags} from './interfaces/view'; +import {DIRECTIVES, FLAGS, LViewData, LViewFlags, PARENT} from './interfaces/view'; import {destroyLView} from './node_manipulation'; @@ -37,20 +37,14 @@ export class ViewRef implements viewEngine_EmbeddedViewRef, viewEngine_Int */ _tViewNode: TViewNode|null = null; - context: T; // TODO(issue/24571): remove '!'. rootNodes !: any[]; - constructor(_view: LViewData, context: T|null) { - this.context = context !; + constructor(_view: LViewData, private _context: T|null, private _componentIndex: number) { this._view = _view; } - /** @internal */ - _setComponentContext(view: LViewData, context: T) { - this._view = view; - this.context = context; - } + get context(): T { return this._context ? this._context : this._lookUpContext(); } get destroyed(): boolean { return (this._view[FLAGS] & LViewFlags.Destroyed) === LViewFlags.Destroyed; @@ -260,11 +254,15 @@ export class ViewRef implements viewEngine_EmbeddedViewRef, viewEngine_Int detachFromAppRef() { this._appRef = null; } attachToAppRef(appRef: ApplicationRef) { this._appRef = appRef; } + + private _lookUpContext(): T { + return this._context = this._view[PARENT] ![DIRECTIVES] ![this._componentIndex] as T; + } } /** @internal */ export class RootViewRef extends ViewRef { - constructor(public _view: LViewData) { super(_view, null); } + constructor(public _view: LViewData) { super(_view, null, -1); } detectChanges(): void { detectChangesInRootView(this._view); } diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index 3cd87edf5b..9721bfa9f4 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -266,9 +266,6 @@ { "name": "hostElement" }, - { - "name": "initChangeDetectorIfExisting" - }, { "name": "invertObject" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 33db03f04b..418f627aad 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -404,9 +404,15 @@ { "name": "contextViewData" }, + { + "name": "createContainerRef" + }, { "name": "createDirectivesAndLocals" }, + { + "name": "createElementRef" + }, { "name": "createEmbeddedViewAndNode" }, @@ -443,6 +449,9 @@ { "name": "createTView" }, + { + "name": "createTemplateRef" + }, { "name": "createTextNode" }, @@ -452,6 +461,9 @@ { "name": "createViewQuery" }, + { + "name": "createViewRef" + }, { "name": "defineComponent" }, @@ -620,18 +632,6 @@ { "name": "getMultiStartIndex" }, - { - "name": "getOrCreateChangeDetectorRef" - }, - { - "name": "getOrCreateContainerRef" - }, - { - "name": "getOrCreateElementRef" - }, - { - "name": "getOrCreateHostChangeDetector" - }, { "name": "getOrCreateInjectable" }, @@ -647,9 +647,6 @@ { "name": "getOrCreateTView" }, - { - "name": "getOrCreateTemplateRef" - }, { "name": "getParentLNode" }, @@ -719,9 +716,6 @@ { "name": "hostElement" }, - { - "name": "initChangeDetectorIfExisting" - }, { "name": "inject" }, diff --git a/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json b/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json index 04c09413bb..0cad4f25ca 100644 --- a/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo_r2/bundle.golden_symbols.json @@ -1274,9 +1274,15 @@ { "name": "couldBeInjectableType" }, + { + "name": "createContainerRef" + }, { "name": "createDirectivesAndLocals" }, + { + "name": "createElementRef" + }, { "name": "createEmbeddedViewAndNode" }, @@ -1325,6 +1331,9 @@ { "name": "createTView" }, + { + "name": "createTemplateRef" + }, { "name": "createTextNode" }, @@ -1334,6 +1343,9 @@ { "name": "createViewQuery" }, + { + "name": "createViewRef" + }, { "name": "dateGetter" }, @@ -1706,18 +1718,6 @@ { "name": "getNumberOfCurrencyDigits" }, - { - "name": "getOrCreateChangeDetectorRef" - }, - { - "name": "getOrCreateContainerRef" - }, - { - "name": "getOrCreateElementRef" - }, - { - "name": "getOrCreateHostChangeDetector" - }, { "name": "getOrCreateInjectable" }, @@ -1733,9 +1733,6 @@ { "name": "getOrCreateTView" }, - { - "name": "getOrCreateTemplateRef" - }, { "name": "getOriginalError" }, @@ -1844,9 +1841,6 @@ { "name": "identity" }, - { - "name": "initChangeDetectorIfExisting" - }, { "name": "initDomAdapter" }, diff --git a/packages/core/test/render3/di_spec.ts b/packages/core/test/render3/di_spec.ts index 51dc33b2c4..d9cfe95038 100644 --- a/packages/core/test/render3/di_spec.ts +++ b/packages/core/test/render3/di_spec.ts @@ -11,17 +11,18 @@ import {RenderFlags} from '@angular/core/src/render3/interfaces/definition'; import {defineComponent} from '../../src/render3/definition'; import {bloomAdd, bloomFindPossibleInjector, getOrCreateNodeInjector, injectAttribute} from '../../src/render3/di'; -import {NgOnChangesFeature, PublicFeature, defineDirective, directiveInject, injectChangeDetectorRef, injectElementRef, injectRenderer2, injectTemplateRef, injectViewContainerRef} from '../../src/render3/index'; +import {PublicFeature, defineDirective, directiveInject, injectChangeDetectorRef, injectElementRef, injectRenderer2, injectTemplateRef, injectViewContainerRef, load} from '../../src/render3/index'; import {bind, container, containerRefreshEnd, containerRefreshStart, createNodeAtIndex, createLViewData, createTView, element, elementEnd, elementStart, embeddedViewEnd, embeddedViewStart, enterView, interpolation2, leaveView, projection, projectionDef, reference, template, text, textBinding, loadDirective, elementContainerStart, elementContainerEnd} from '../../src/render3/instructions'; import {LInjector} from '../../src/render3/interfaces/injector'; import {isProceduralRenderer} from '../../src/render3/interfaces/renderer'; -import {AttributeMarker, TNodeType} from '../../src/render3/interfaces/node'; +import {AttributeMarker, LContainerNode, LElementNode, TNodeType} from '../../src/render3/interfaces/node'; import {LViewFlags} from '../../src/render3/interfaces/view'; import {ViewRef} from '../../src/render3/view_ref'; import {getRendererFactory2} from './imported_renderer2'; import {ComponentFixture, createComponent, createDirective, renderComponent, toHtml} from './render_util'; +import {NgIf} from './common_with_def'; describe('di', () => { describe('no dependencies', () => { @@ -787,6 +788,10 @@ describe('di', () => { describe('ElementRef', () => { it('should create directive with ElementRef dependencies', () => { + let dir !: Directive; + let dirSameInstance !: DirectiveSameInstance; + let divNode !: LElementNode; + class Directive { value: string; constructor(public elementRef: ElementRef) { @@ -795,47 +800,75 @@ describe('di', () => { static ngDirectiveDef = defineDirective({ type: Directive, selectors: [['', 'dir', '']], - factory: () => new Directive(injectElementRef()), + factory: () => dir = new Directive(injectElementRef()), features: [PublicFeature], exportAs: 'dir' }); } class DirectiveSameInstance { - value: boolean; - constructor(elementRef: ElementRef, directive: Directive) { - this.value = (elementRef === directive.elementRef) && elementRef instanceof ElementRef; + isSameInstance: boolean; + constructor(public elementRef: ElementRef, directive: Directive) { + this.isSameInstance = elementRef === directive.elementRef; } static ngDirectiveDef = defineDirective({ type: DirectiveSameInstance, selectors: [['', 'dirSame', '']], - factory: () => new DirectiveSameInstance(injectElementRef(), directiveInject(Directive)), + factory: () => dirSameInstance = + new DirectiveSameInstance(injectElementRef(), directiveInject(Directive)), exportAs: 'dirSame' }); } - /** - *
- * {{ dir.value }} - {{ dirSame.value }} - *
- */ + /**
*/ const App = createComponent('app', function(rf: RenderFlags, ctx: any) { if (rf & RenderFlags.Create) { - elementStart(0, 'div', ['dir', '', 'dirSame', ''], ['dirSame', 'dirSame', 'dir', 'dir']); - { text(3); } + elementStart(0, 'div', ['dir', '', 'dirSame', '']); elementEnd(); + divNode = load(0); } - - if (rf & RenderFlags.Update) { - const tmp1 = reference(1) as any; - const tmp2 = reference(2) as any; - textBinding(3, interpolation2('', tmp2.value, '-', tmp1.value, '')); - } - }, 4, 2, [Directive, DirectiveSameInstance]); + }, 1, 0, [Directive, DirectiveSameInstance]); const fixture = new ComponentFixture(App); - expect(fixture.html).toEqual('
ElementRef-true
'); + expect(dir.value).toEqual('ElementRef'); + expect(dir.elementRef.nativeElement).toEqual(divNode.native); + expect(dirSameInstance.elementRef.nativeElement).toEqual(divNode.native); + + // Each ElementRef instance should be unique + expect(dirSameInstance.isSameInstance).toBe(false); }); + + it('should create ElementRef with comment if requesting directive is on node', + () => { + let dir !: Directive; + let commentNode !: LContainerNode; + + class Directive { + value: string; + constructor(public elementRef: ElementRef) { + this.value = (elementRef.constructor as any).name; + } + static ngDirectiveDef = defineDirective({ + type: Directive, + selectors: [['', 'dir', '']], + factory: () => dir = new Directive(injectElementRef()), + features: [PublicFeature], + exportAs: 'dir' + }); + } + + /** */ + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + template(0, () => {}, 0, 0, null, ['dir', '']); + commentNode = load(0); + } + }, 1, 0, [Directive]); + + const fixture = new ComponentFixture(App); + expect(dir.value).toEqual('ElementRef'); + expect(dir.elementRef.nativeElement).toEqual(commentNode.native); + }); }); describe('TemplateRef', () => { @@ -855,9 +888,9 @@ describe('di', () => { } class DirectiveSameInstance { - value: boolean; + isSameInstance: boolean; constructor(templateRef: TemplateRef, directive: Directive) { - this.value = templateRef === directive.templateRef; + this.isSameInstance = templateRef === directive.templateRef; } static ngDirectiveDef = defineDirective({ type: DirectiveSameInstance, @@ -881,12 +914,13 @@ describe('di', () => { if (rf & RenderFlags.Update) { const tmp1 = reference(1) as any; const tmp2 = reference(2) as any; - textBinding(3, interpolation2('', tmp1.value, '-', tmp2.value, '')); + textBinding(3, interpolation2('', tmp1.value, '-', tmp2.isSameInstance, '')); } }, 4, 2, [Directive, DirectiveSameInstance]); const fixture = new ComponentFixture(App); - expect(fixture.html).toEqual('TemplateRef-true'); + // Each TemplateRef instance should be unique + expect(fixture.html).toEqual('TemplateRef-false'); }); }); @@ -907,9 +941,9 @@ describe('di', () => { } class DirectiveSameInstance { - value: boolean; + isSameInstance: boolean; constructor(viewContainerRef: ViewContainerRef, directive: Directive) { - this.value = viewContainerRef === directive.viewContainerRef; + this.isSameInstance = viewContainerRef === directive.viewContainerRef; } static ngDirectiveDef = defineDirective({ type: DirectiveSameInstance, @@ -934,12 +968,13 @@ describe('di', () => { if (rf & RenderFlags.Update) { const tmp1 = reference(1) as any; const tmp2 = reference(2) as any; - textBinding(3, interpolation2('', tmp1.value, '-', tmp2.value, '')); + textBinding(3, interpolation2('', tmp1.value, '-', tmp2.isSameInstance, '')); } }, 4, 2, [Directive, DirectiveSameInstance]); const fixture = new ComponentFixture(App); - expect(fixture.html).toEqual('
ViewContainerRef-true
'); + // Each ViewContainerRef instance should be unique + expect(fixture.html).toEqual('
ViewContainerRef-false
'); }); }); @@ -990,53 +1025,33 @@ describe('di', () => { }); } - class IfDirective { - /* @Input */ - myIf = true; + const directives = [MyComp, Directive, DirectiveSameInstance, NgIf]; - constructor(public template: TemplateRef, public vcr: ViewContainerRef) {} + it('should inject current component ChangeDetectorRef into directives on the same node as components', + () => { + /** {{ dir.value }} */ + const MyApp = createComponent('my-app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + element(0, 'my-comp', ['dir', '', 'dirSame', ''], ['dir', 'dir']); + text(2); + } + if (rf & RenderFlags.Update) { + const tmp = reference(1) as any; + textBinding(2, bind(tmp.value)); + } + }, 3, 1, directives); - ngOnChanges() { - if (this.myIf) { - this.vcr.createEmbeddedView(this.template); - } - } + const app = renderComponent(MyApp); + // ChangeDetectorRef is the token, ViewRef has historically been the constructor + expect(toHtml(app)).toEqual('ViewRef'); + expect((comp !.cdr as ViewRef).context).toBe(comp); - static ngDirectiveDef = defineDirective({ - type: IfDirective, - selectors: [['', 'myIf', '']], - factory: () => new IfDirective(injectTemplateRef(), injectViewContainerRef()), - inputs: {myIf: 'myIf'}, - features: [PublicFeature, NgOnChangesFeature] - }); - } + // Each ChangeDetectorRef instance should be unique + expect(dir !.cdr).not.toBe(comp !.cdr); + expect(dir !.cdr).not.toBe(dirSameInstance !.cdr); + }); - - const directives = [MyComp, Directive, DirectiveSameInstance, IfDirective]; - - it('should inject current component ChangeDetectorRef into directives on components', () => { - /** {{ dir.value }} */ - const MyApp = createComponent('my-app', function(rf: RenderFlags, ctx: any) { - if (rf & RenderFlags.Create) { - element(0, 'my-comp', ['dir', '', 'dirSame', ''], ['dir', 'dir']); - text(2); - } - if (rf & RenderFlags.Update) { - const tmp = reference(1) as any; - textBinding(2, bind(tmp.value)); - } - }, 3, 1, directives); - - const app = renderComponent(MyApp); - // ChangeDetectorRef is the token, ViewRef has historically been the constructor - expect(toHtml(app)).toEqual('ViewRef'); - expect((comp !.cdr as ViewRef).context).toBe(comp); - - expect(dir !.cdr).toBe(comp !.cdr); - expect(dir !.cdr).toBe(dirSameInstance !.cdr); - }); - - it('should inject host component ChangeDetectorRef into directives on elements', () => { + it('should inject host component ChangeDetectorRef into directives on normal elements', () => { class MyApp { constructor(public cdr: ChangeDetectorRef) {} @@ -1067,49 +1082,52 @@ describe('di', () => { expect(toHtml(app)).toEqual('
ViewRef
'); expect((app !.cdr as ViewRef).context).toBe(app); - expect(dir !.cdr).toBe(app.cdr); - expect(dir !.cdr).toBe(dirSameInstance !.cdr); + // Each ChangeDetectorRef instance should be unique + expect(dir !.cdr).not.toBe(app.cdr); + expect(dir !.cdr).not.toBe(dirSameInstance !.cdr); }); - it('should inject host component ChangeDetectorRef into directives in ContentChildren', () => { - class MyApp { - constructor(public cdr: ChangeDetectorRef) {} + it('should inject host component ChangeDetectorRef into directives in a component\'s ContentChildren', + () => { + class MyApp { + constructor(public cdr: ChangeDetectorRef) {} - static ngComponentDef = defineComponent({ - type: MyApp, - selectors: [['my-app']], - consts: 4, - vars: 1, - factory: () => new MyApp(injectChangeDetectorRef()), - /** - * - *
- * - * {{ dir.value }} - */ - template: function(rf: RenderFlags, ctx: any) { - if (rf & RenderFlags.Create) { - elementStart(0, 'my-comp'); - { element(1, 'div', ['dir', '', 'dirSame', ''], ['dir', 'dir']); } - elementEnd(); - text(3); - } - if (rf & RenderFlags.Update) { - const tmp = reference(2) as any; - textBinding(3, bind(tmp.value)); - } - }, - directives: directives - }); - } + static ngComponentDef = defineComponent({ + type: MyApp, + selectors: [['my-app']], + consts: 4, + vars: 1, + factory: () => new MyApp(injectChangeDetectorRef()), + /** + * + *
+ * + * {{ dir.value }} + */ + template: function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'my-comp'); + { element(1, 'div', ['dir', '', 'dirSame', ''], ['dir', 'dir']); } + elementEnd(); + text(3); + } + if (rf & RenderFlags.Update) { + const tmp = reference(2) as any; + textBinding(3, bind(tmp.value)); + } + }, + directives: directives + }); + } - const app = renderComponent(MyApp); - expect(toHtml(app)).toEqual('
ViewRef'); - expect((app !.cdr as ViewRef).context).toBe(app); + const app = renderComponent(MyApp); + expect(toHtml(app)).toEqual('
ViewRef'); + expect((app !.cdr as ViewRef).context).toBe(app); - expect(dir !.cdr).toBe(app !.cdr); - expect(dir !.cdr).toBe(dirSameInstance !.cdr); - }); + // Each ChangeDetectorRef instance should be unique + expect(dir !.cdr).not.toBe(app !.cdr); + expect(dir !.cdr).not.toBe(dirSameInstance !.cdr); + }); it('should inject host component ChangeDetectorRef into directives in embedded views', () => { @@ -1161,8 +1179,9 @@ describe('di', () => { expect(toHtml(app)).toEqual('
ViewRef
'); expect((app !.cdr as ViewRef).context).toBe(app); - expect(dir !.cdr).toBe(app.cdr); - expect(dir !.cdr).toBe(dirSameInstance !.cdr); + // Each ChangeDetectorRef instance should be unique + expect(dir !.cdr).not.toBe(app.cdr); + expect(dir !.cdr).not.toBe(dirSameInstance !.cdr); }); it('should inject host component ChangeDetectorRef into directives on containers', () => { @@ -1189,10 +1208,10 @@ describe('di', () => { factory: () => new MyApp(injectChangeDetectorRef()), consts: 1, vars: 0, - /**
{{ dir.value }}
*/ + /**
{{ dir.value }}
*/ template: function(rf: RenderFlags, ctx: MyApp) { if (rf & RenderFlags.Create) { - template(0, C1, 3, 1, null, ['myIf', 'showing']); + template(0, C1, 3, 1, null, ['ngIf', 'showing']); } }, directives: directives @@ -1203,8 +1222,9 @@ describe('di', () => { expect(toHtml(app)).toEqual('
ViewRef
'); expect((app !.cdr as ViewRef).context).toBe(app); - expect(dir !.cdr).toBe(app.cdr); - expect(dir !.cdr).toBe(dirSameInstance !.cdr); + // Each ChangeDetectorRef instance should be unique + expect(dir !.cdr).not.toBe(app.cdr); + expect(dir !.cdr).not.toBe(dirSameInstance !.cdr); }); }); diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index 5cfa83cb68..15ba6895f8 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -10,7 +10,7 @@ import {ElementRef, TemplateRef, ViewContainerRef} from '@angular/core'; import {RenderFlags} from '@angular/core/src/render3'; import {RendererStyleFlags2, RendererType2} from '../../src/render/api'; -import {getOrCreateNodeInjectorForNode, getOrCreateTemplateRef} from '../../src/render3/di'; +import {createTemplateRef, getOrCreateNodeInjectorForNode} from '../../src/render3/di'; import {AttributeMarker, defineComponent, defineDirective, injectElementRef, injectTemplateRef, injectViewContainerRef} from '../../src/render3/index'; import {NO_CHANGE, bind, container, containerRefreshEnd, containerRefreshStart, element, elementAttribute, elementClassProp, elementContainerEnd, elementContainerStart, elementEnd, elementProperty, elementStart, elementStyleProp, elementStyling, elementStylingApply, embeddedViewEnd, embeddedViewStart, interpolation1, interpolation2, interpolation3, interpolation4, interpolation5, interpolation6, interpolation7, interpolation8, interpolationV, listener, load, loadDirective, projection, projectionDef, text, textBinding, template} from '../../src/render3/instructions';