From 28c7a4efbc975295ac1cad9bb18cbbd792e7434e Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Thu, 26 Jul 2018 17:22:41 +0200 Subject: [PATCH] feat(ivy): add basic support for ng-container (#25227) This commit adds basic support for - most of the functionality should work as long as is a child of a regular element. PR Close #25227 --- packages/core/src/render3/di.ts | 11 +- packages/core/src/render3/instructions.ts | 51 ++++++++- .../core/src/render3/interfaces/injector.ts | 4 +- packages/core/src/render3/interfaces/node.ts | 18 ++- packages/core/src/render3/ng_dev_mode.ts | 2 + .../core/src/render3/node_manipulation.ts | 29 +++-- .../core/test/render3/integration_spec.ts | 105 +++++++++++++++++- packages/core/test/render3/query_spec.ts | 39 ++++++- packages/core/test/render3/render_util.ts | 3 +- 9 files changed, 236 insertions(+), 26 deletions(-) diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index 123b2f38f1..0a2b24ceda 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -21,7 +21,7 @@ import {addToViewTree, assertPreviousIsParent, createEmbeddedViewNode, createLCo import {VIEWS} from './interfaces/container'; import {DirectiveDefInternal, RenderFlags} from './interfaces/definition'; import {LInjector} from './interfaces/injector'; -import {AttributeMarker, LContainerNode, LElementNode, LNode, LViewNode, TContainerNode, TElementNode, TNodeFlags, TNodeType} from './interfaces/node'; +import {AttributeMarker, LContainerNode, LElementContainerNode, LElementNode, LNode, LViewNode, TContainerNode, TElementNode, TNodeFlags, TNodeType} from './interfaces/node'; import {LQueries, QueryReadType} from './interfaces/query'; import {Renderer3} from './interfaces/renderer'; import {DECLARATION_VIEW, DIRECTIVES, HOST_NODE, INJECTOR, LViewData, QUERIES, RENDERER, TVIEW, TView} from './interfaces/view'; @@ -91,7 +91,8 @@ export function bloomAdd(injector: LInjector, type: Type): void { export function getOrCreateNodeInjector(): LInjector { ngDevMode && assertPreviousIsParent(); - return getOrCreateNodeInjectorForNode(getPreviousOrParentNode() as LElementNode | LContainerNode); + return getOrCreateNodeInjectorForNode( + getPreviousOrParentNode() as LElementNode | LElementContainerNode | LContainerNode); } /** @@ -100,7 +101,8 @@ export function getOrCreateNodeInjector(): LInjector { * @param node for which an injector should be retrieved / created. * @returns Node injector */ -export function getOrCreateNodeInjectorForNode(node: LElementNode | LContainerNode): LInjector { +export function getOrCreateNodeInjectorForNode( + node: LElementNode | LElementContainerNode | LContainerNode): LInjector { const nodeInjector = node.nodeInjector; const parent = getParentLNode(node); const parentInjector = parent && parent.nodeInjector; @@ -637,7 +639,8 @@ class ViewContainerRef implements viewEngine.ViewContainerRef { private _viewRefs: viewEngine.ViewRef[] = []; constructor( - private _lContainerNode: LContainerNode, private _hostNode: LElementNode|LContainerNode) {} + private _lContainerNode: LContainerNode, + private _hostNode: LElementNode|LElementContainerNode|LContainerNode) {} get element(): ElementRef { const injector = getOrCreateNodeInjectorForNode(this._hostNode); diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index ccce7b5537..3a9d315d0e 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -18,7 +18,7 @@ import {executeHooks, executeInitHooks, queueInitHooks, queueLifecycleHooks} fro import {ACTIVE_INDEX, LContainer, RENDER_PARENT, VIEWS} from './interfaces/container'; import {ComponentDefInternal, ComponentQuery, ComponentTemplate, DirectiveDefInternal, DirectiveDefListOrFactory, InitialStylingFlags, PipeDefListOrFactory, RenderFlags} from './interfaces/definition'; import {LInjector} from './interfaces/injector'; -import {AttributeMarker, InitialInputData, InitialInputs, LContainerNode, LElementNode, LNode, LProjectionNode, LTextNode, LViewNode, PropertyAliasValue, PropertyAliases, TAttributes, TContainerNode, TElementNode, TNode, TNodeFlags, TNodeType} from './interfaces/node'; +import {AttributeMarker, InitialInputData, InitialInputs, LContainerNode, LElementContainerNode, LElementNode, LNode, LProjectionNode, LTextNode, LViewNode, PropertyAliasValue, PropertyAliases, TAttributes, TContainerNode, TElementNode, TNode, TNodeFlags, TNodeType} from './interfaces/node'; import {CssSelectorList, NG_PROJECT_AS_ATTR_NAME} from './interfaces/projection'; import {LQueries} from './interfaces/query'; import {ProceduralRenderer3, RComment, RElement, RText, Renderer3, RendererFactory3, RendererStyleFlags3, isProceduralRenderer} from './interfaces/renderer'; @@ -419,6 +419,9 @@ export function createLNode( export function createLNode( index: number, type: TNodeType.Projection, native: null, name: null, attrs: TAttributes | null, lProjection: null): LProjectionNode; +export function createLNode( + index: number, type: TNodeType.ElementContainer, native: RComment, name: null, + attrs: TAttributes | null, data: null): LElementContainerNode; export function createLNode( index: number, type: TNodeType, native: RText | RElement | RComment | null, name: string | null, attrs: TAttributes | null, state?: null | LViewData | LContainer): LElementNode<extNode& @@ -695,6 +698,50 @@ export function element( elementEnd(); } +/** + * Creates a logical container for other nodes () backed by a comment node in the DOM. + * The instruction must later be followed by `elementContainerEnd()` call. + * + * @param index Index of the element in the LViewData array + * @param attrs Set of attributes to be used when matching directives. + * @param localRefs A set of local reference bindings on the element. + * + * Even if this instruction accepts a set of attributes no actual attribute values are propoagted to + * the DOM (as a comment node can't have attributes). Attributes are here only for directive + * matching purposes and setting initial inputs of directives. + */ +export function elementContainerStart( + index: number, attrs?: TAttributes | null, localRefs?: string[] | null): void { + ngDevMode && + assertEqual(viewData[BINDING_INDEX], -1, 'elements should be created before any bindings'); + + ngDevMode && ngDevMode.rendererCreateComment++; + const native = renderer.createComment(ngDevMode ? 'ng-container' : ''); + + ngDevMode && assertDataInRange(index - 1); + + const node: LElementContainerNode = + createLNode(index, TNodeType.ElementContainer, native, null, attrs || null, null); + + appendChild(getParentLNode(node), native, viewData); + createDirectivesAndLocals(localRefs); +} + +/** Mark the end of the . */ +export function elementContainerEnd(): void { + if (isParent) { + isParent = false; + } else { + ngDevMode && assertHasParent(); + previousOrParentNode = getParentLNode(previousOrParentNode) as LElementContainerNode; + } + + ngDevMode && assertNodeType(previousOrParentNode, TNodeType.ElementContainer); + const queries = previousOrParentNode.queries; + queries && queries.addNode(previousOrParentNode); + queueLifecycleHooks(previousOrParentNode.tNode.flags, tView); +} + /** * Create DOM element. The instruction must later be followed by `elementEnd()` call. * @@ -1086,6 +1133,7 @@ export function hostElement( export function listener( eventName: string, listenerFn: (e?: any) => any, useCapture = false): void { ngDevMode && assertPreviousIsParent(); + ngDevMode && assertNodeOfPossibleTypes(previousOrParentNode, TNodeType.Element); const node = previousOrParentNode; const native = node.native as RElement; ngDevMode && ngDevMode.rendererAddEventListener++; @@ -1779,6 +1827,7 @@ export function container( const currentParent = isParent ? previousOrParentNode : getParentLNode(previousOrParentNode) !; const lContainer = createLContainer(currentParent, viewData); + ngDevMode && ngDevMode.rendererCreateComment++; const comment = renderer.createComment(ngDevMode ? 'container' : ''); const node = createLNode(index, TNodeType.Container, comment, tagName || null, attrs || null, lContainer); diff --git a/packages/core/src/render3/interfaces/injector.ts b/packages/core/src/render3/interfaces/injector.ts index 400e38eaf4..7d76cac0c1 100644 --- a/packages/core/src/render3/interfaces/injector.ts +++ b/packages/core/src/render3/interfaces/injector.ts @@ -11,7 +11,7 @@ import {ElementRef} from '../../linker/element_ref'; import {TemplateRef} from '../../linker/template_ref'; import {ViewContainerRef} from '../../linker/view_container_ref'; -import {LContainerNode, LElementNode} from './node'; +import {LContainerNode, LElementContainerNode, LElementNode} from './node'; export interface LInjector { /** @@ -26,7 +26,7 @@ export interface LInjector { * for DI to retrieve a directive from the data array if injector indicates * it is there. */ - readonly node: LElementNode|LContainerNode; + readonly node: LElementNode|LElementContainerNode|LContainerNode; /** * The following bloom filter determines whether a directive is available diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index 2a67e44baa..a76b30efb8 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -21,11 +21,12 @@ import {LViewData, TView} from './view'; * on how to map a particular set of bits in LNode.flags to the node type. */ export const enum TNodeType { - Container = 0b00, - Projection = 0b01, - View = 0b10, - Element = 0b11, - ViewOrElement = 0b10, + Container = 0b000, + Projection = 0b001, + View = 0b010, + Element = 0b011, + ViewOrElement = 0b010, + ElementContainer = 0b100, } /** @@ -118,6 +119,13 @@ export interface LElementNode extends LNode { readonly data: LViewData|null; } +/** LNode representing . */ +export interface LElementContainerNode extends LNode { + /** The DOM comment associated with this node. */ + readonly native: RComment; + readonly data: null; +} + /** LNode representing a #text node. */ export interface LTextNode extends LNode { /** The text node associated with this node. */ diff --git a/packages/core/src/render3/ng_dev_mode.ts b/packages/core/src/render3/ng_dev_mode.ts index a63a971c46..512e61c426 100644 --- a/packages/core/src/render3/ng_dev_mode.ts +++ b/packages/core/src/render3/ng_dev_mode.ts @@ -29,6 +29,7 @@ declare global { rendererDestroyNode: number; rendererMoveNode: number; rendererRemoveNode: number; + rendererCreateComment: number; } } @@ -61,6 +62,7 @@ export function ngDevModeResetPerfCounters() { rendererDestroyNode: 0, rendererMoveNode: 0, rendererRemoveNode: 0, + rendererCreateComment: 0, }; } diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index 56f99d8e78..24234f1425 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -9,7 +9,7 @@ import {assertDefined} from './assert'; import {callHooks} from './hooks'; import {LContainer, RENDER_PARENT, VIEWS, unusedValueExportToPlacateAjd as unused1} from './interfaces/container'; -import {LContainerNode, LElementNode, LNode, LProjectionNode, LTextNode, LViewNode, TNode, TNodeFlags, TNodeType, unusedValueExportToPlacateAjd as unused2} from './interfaces/node'; +import {LContainerNode, LElementContainerNode, LElementNode, LNode, LProjectionNode, LTextNode, LViewNode, TNode, TNodeFlags, TNodeType, unusedValueExportToPlacateAjd as unused2} from './interfaces/node'; import {unusedValueExportToPlacateAjd as unused3} from './interfaces/projection'; import {ProceduralRenderer3, RComment, RElement, RNode, RText, Renderer3, isProceduralRenderer, unusedValueExportToPlacateAjd as unused4} from './interfaces/renderer'; import {CLEANUP, CONTAINER_INDEX, DIRECTIVES, FLAGS, HEADER_OFFSET, HOST_NODE, HookData, LViewData, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, TVIEW, unusedValueExportToPlacateAjd as unused5} from './interfaces/view'; @@ -38,11 +38,14 @@ export function getChildLNode(node: LNode): LNode|null { } /** Retrieves the parent LNode of a given node. */ -export function getParentLNode(node: LContainerNode | LElementNode | LTextNode | LProjectionNode): - LElementNode|LViewNode; +export function getParentLNode( + node: LContainerNode | LElementNode | LElementContainerNode | LTextNode | + LProjectionNode): LElementNode|LElementContainerNode|LViewNode; export function getParentLNode(node: LViewNode): LContainerNode|null; -export function getParentLNode(node: LNode): LElementNode|LContainerNode|LViewNode|null; -export function getParentLNode(node: LNode): LElementNode|LContainerNode|LViewNode|null { +export function getParentLNode(node: LNode): LElementNode|LElementContainerNode|LContainerNode| + LViewNode|null; +export function getParentLNode(node: LNode): LElementNode|LElementContainerNode|LContainerNode| + LViewNode|null { if (node.tNode.index === -1 && node.tNode.type === TNodeType.View) { // This is a dynamically created view inside a dynamic container. // If the host index is -1, the view has not yet been inserted, so it has no parent. @@ -518,9 +521,10 @@ function executePipeOnDestroys(viewData: LViewData): void { */ export function canInsertNativeNode(parent: LNode, currentView: LViewData): boolean { // We can only insert into a Component or View. Any other type should be an Error. - ngDevMode && assertNodeOfPossibleTypes(parent, TNodeType.Element, TNodeType.View); + ngDevMode && assertNodeOfPossibleTypes( + parent, TNodeType.Element, TNodeType.ElementContainer, TNodeType.View); - if (parent.tNode.type === TNodeType.Element) { + if (parent.tNode.type === TNodeType.Element || parent.tNode.type === TNodeType.ElementContainer) { // Parent is an element. if (parent.view !== currentView) { // If the Parent view is not the same as current view than we are inserting across @@ -584,6 +588,12 @@ export function appendChild(parent: LNode, child: RNode | null, currentView: LVi isProceduralRenderer(renderer) ? renderer.insertBefore(renderParent !.native, child, beforeNode) : renderParent !.native.insertBefore(child, beforeNode, true); + } else if (parent.tNode.type === TNodeType.ElementContainer) { + const beforeNode = parent.native; + const renderParent = getParentLNode(parent) as LElementNode; + isProceduralRenderer(renderer) ? + renderer.insertBefore(renderParent !.native, child, beforeNode) : + renderParent !.native.insertBefore(child, beforeNode, true); } else { isProceduralRenderer(renderer) ? renderer.appendChild(parent.native !as RElement, child) : parent.native !.appendChild(child); @@ -621,8 +631,9 @@ export function removeChild(parent: LNode, child: RNode | null, currentView: LVi * @param currentView Current LView */ export function appendProjectedNode( - node: LElementNode | LTextNode | LContainerNode, currentParent: LElementNode | LViewNode, - currentView: LViewData, renderParent: LElementNode): void { + node: LElementNode | LElementContainerNode | LTextNode | LContainerNode, + currentParent: LElementNode | LElementContainerNode | LViewNode, currentView: LViewData, + renderParent: LElementNode): void { appendChild(currentParent, node.native, currentView); if (node.tNode.type === TNodeType.Container) { // The node we are adding is a container and we are adding it to an element which diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index c49b1ae56a..7d2669b323 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -6,16 +6,17 @@ * found in the LICENSE file at https://angular.io/license */ +import {ElementRef} from '@angular/core'; import {RenderFlags} from '@angular/core/src/render3'; -import {defineComponent, defineDirective} from '../../src/render3/index'; -import {NO_CHANGE, bind, container, containerRefreshEnd, containerRefreshStart, element, elementAttribute, elementClassProp, elementEnd, elementProperty, elementStart, elementStyleProp, elementStyling, elementStylingApply, embeddedViewEnd, embeddedViewStart, interpolation1, interpolation2, interpolation3, interpolation4, interpolation5, interpolation6, interpolation7, interpolation8, interpolationV, load, loadDirective, projection, projectionDef, text, textBinding} from '../../src/render3/instructions'; +import {AttributeMarker, defineComponent, defineDirective, injectElementRef} 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} from '../../src/render3/instructions'; import {InitialStylingFlags} from '../../src/render3/interfaces/definition'; import {HEADER_OFFSET} from '../../src/render3/interfaces/view'; import {sanitizeUrl} from '../../src/sanitization/sanitization'; import {Sanitizer, SecurityContext} from '../../src/sanitization/security'; -import {ComponentFixture, containerEl, renderToHtml} from './render_util'; +import {ComponentFixture, TemplateFixture, containerEl, renderToHtml} from './render_util'; describe('render3 integration test', () => { @@ -418,6 +419,104 @@ describe('render3 integration test', () => { }); + describe('ng-container', () => { + + it('should insert as a child of a regular element', () => { + + /** + *
before|Greetings|after
+ */ + function Template() { + elementStart(0, 'div'); + { + text(1, 'before|'); + elementContainerStart(2); + { + text(3, 'Greetings'); + element(4, 'span'); + } + elementContainerEnd(); + text(5, '|after'); + } + elementEnd(); + } + + const fixture = new TemplateFixture(Template); + expect(fixture.html).toEqual('
before|Greetings|after
'); + }); + + it('should support directives and inject ElementRef', () => { + + class Directive { + constructor(public elRef: ElementRef) {} + + static ngDirectiveDef = defineDirective({ + type: Directive, + selectors: [['', 'dir', '']], + factory: () => new Directive(injectElementRef()), + }); + } + + let directive: Directive; + + /** + *
+ */ + function Template() { + elementStart(0, 'div'); + { + elementContainerStart(1, [AttributeMarker.SelectOnly, 'dir']); + elementContainerEnd(); + directive = loadDirective(0); + } + elementEnd(); + } + + const fixture = new TemplateFixture(Template, () => {}, [Directive]); + expect(fixture.html).toEqual('
'); + expect(directive !.elRef.nativeElement.nodeType).toBe(Node.COMMENT_NODE); + }); + + it('should not set any attributes', () => { + + /** + *
+ */ + function Template() { + elementStart(0, 'div'); + { + elementContainerStart(1, ['id', 'foo']); + elementContainerEnd(); + } + elementEnd(); + } + + const fixture = new TemplateFixture(Template); + expect(fixture.html).toEqual('
'); + }); + + it('should throw when trying to add event listener', () => { + + /** + *
+ */ + function Template() { + elementStart(0, 'div'); + { + elementContainerStart(1); + { + listener('click', function() {}); + } + elementContainerEnd(); + } + elementEnd(); + } + + expect(() => { new TemplateFixture(Template); }).toThrow(); + }); + + }); + describe('tree', () => { interface Tree { beforeLabel?: string; diff --git a/packages/core/test/render3/query_spec.ts b/packages/core/test/render3/query_spec.ts index 6458a64208..6b5f870d9e 100644 --- a/packages/core/test/render3/query_spec.ts +++ b/packages/core/test/render3/query_spec.ts @@ -12,7 +12,7 @@ import {ElementRef, TemplateRef, ViewContainerRef} from '@angular/core'; import {EventEmitter} from '../..'; import {QUERY_READ_CONTAINER_REF, QUERY_READ_ELEMENT_REF, QUERY_READ_FROM_NODE, QUERY_READ_TEMPLATE_REF, getOrCreateNodeInjectorForNode, getOrCreateTemplateRef} from '../../src/render3/di'; import {AttributeMarker, QueryList, defineComponent, defineDirective, detectChanges, injectViewContainerRef} from '../../src/render3/index'; -import {bind, container, containerRefreshEnd, containerRefreshStart, element, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, load, loadDirective, loadElement, loadQueryList, registerContentQuery} from '../../src/render3/instructions'; +import {bind, container, containerRefreshEnd, containerRefreshStart, element, elementContainerEnd, elementContainerStart, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, load, loadDirective, loadElement, loadQueryList, registerContentQuery} from '../../src/render3/instructions'; import {RenderFlags} from '../../src/render3/interfaces/definition'; import {query, queryRefresh} from '../../src/render3/query'; @@ -364,6 +364,43 @@ describe('query', () => { expect(qList.first.nativeElement).toEqual(elToQuery); }); + it('should query for and read ElementRef with a native element pointing to comment node', + () => { + let elToQuery; + /** + * + * class Cmpt { + * @ViewChildren('foo') query; + * } + */ + const Cmpt = createComponent( + 'cmpt', + function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementContainerStart(1, null, ['foo', '']); + elToQuery = loadElement(1).native; + elementContainerEnd(); + } + }, + [], [], + function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + query(0, ['foo'], false, QUERY_READ_ELEMENT_REF); + } + if (rf & RenderFlags.Update) { + let tmp: any; + queryRefresh(tmp = load>(0)) && + (ctx.query = tmp as QueryList); + } + }); + + const cmptInstance = renderComponent(Cmpt); + const qList = (cmptInstance.query as QueryList); + expect(qList.length).toBe(1); + expect(isElementRef(qList.first)).toBeTruthy(); + expect(qList.first.nativeElement).toEqual(elToQuery); + }); + it('should read ViewContainerRef from element nodes when explicitly asked for', () => { /** *
diff --git a/packages/core/test/render3/render_util.ts b/packages/core/test/render3/render_util.ts index dedd1ae988..35888184b3 100644 --- a/packages/core/test/render3/render_util.ts +++ b/packages/core/test/render3/render_util.ts @@ -222,7 +222,8 @@ export function toHtml(componentOrElement: T | RElement): string { .replace(/^
/, '') .replace(/<\/div>$/, '') .replace(' style=""', '') - .replace(//g, ''); + .replace(//g, '') + .replace(//g, ''); } }