From 90bf5d8961979469e39ad27a9cdce4ec132ef11c Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Fri, 4 May 2018 15:58:42 +0200 Subject: [PATCH] feat(ivy): separate attributes for directive matching purposes (#23991) In ngIvy directives matching (determining which directives are active based on a CSS seletor) happens at runtime. This means that runtime needs to have enough context to match directives. This PR takes care of cases where a directive's selector should match bindings (ex. [foo]="exp") and event handlers (ex. (out)="do()"). In the mentioned cases we need to have binding / output "attributes" for directive's CSS selector matching purposes. At the same time those are not regular attributes and as such should not be reflected in the DOM. Closes #23706 PR Close #23991 --- packages/core/src/render3/di.ts | 10 +- packages/core/src/render3/index.ts | 4 + packages/core/src/render3/instructions.ts | 46 ++--- packages/core/src/render3/interfaces/node.ts | 41 ++++- .../core/src/render3/node_selector_matcher.ts | 22 ++- packages/core/test/render3/content_spec.ts | 44 ++++- packages/core/test/render3/di_spec.ts | 68 ++++++-- packages/core/test/render3/directive_spec.ts | 160 ++++++++++++++++-- .../render3/node_selector_matcher_spec.ts | 28 ++- 9 files changed, 349 insertions(+), 74 deletions(-) diff --git a/packages/core/src/render3/di.ts b/packages/core/src/render3/di.ts index 7d66fca500..b57926f254 100644 --- a/packages/core/src/render3/di.ts +++ b/packages/core/src/render3/di.ts @@ -22,7 +22,7 @@ import {assertGreaterThan, assertLessThan, assertNotNull} from './assert'; import {addToViewTree, assertPreviousIsParent, createLContainer, createLNodeObject, createTNode, createTView, getDirectiveInstance, getPreviousOrParentNode, getRenderer, isComponent, renderEmbeddedTemplate, resolveDirective} from './instructions'; import {ComponentTemplate, DirectiveDef, DirectiveDefList, PipeDefList} from './interfaces/definition'; import {LInjector} from './interfaces/injector'; -import {LContainerNode, LElementNode, LNode, LViewNode, TNodeFlags, TNodeType} from './interfaces/node'; +import {AttributeMarker, LContainerNode, LElementNode, LNode, LViewNode, TNodeFlags, TNodeType} from './interfaces/node'; import {QueryReadType} from './interfaces/query'; import {Renderer3} from './interfaces/renderer'; import {LView, TView} from './interfaces/view'; @@ -251,7 +251,7 @@ export function injectChangeDetectorRef(): viewEngine_ChangeDetectorRef { * * @experimental */ -export function injectAttribute(attrName: string): string|undefined { +export function injectAttribute(attrNameToInject: string): string|undefined { ngDevMode && assertPreviousIsParent(); const lElement = getPreviousOrParentNode() as LElementNode; ngDevMode && assertNodeType(lElement, TNodeType.Element); @@ -260,8 +260,10 @@ export function injectAttribute(attrName: string): string|undefined { const attrs = tElement.attrs; if (attrs) { for (let i = 0; i < attrs.length; i = i + 2) { - if (attrs[i] == attrName) { - return attrs[i + 1]; + const attrName = attrs[i]; + if (attrName === AttributeMarker.SELECT_ONLY) break; + if (attrName == attrNameToInject) { + return attrs[i + 1] as string; } } } diff --git a/packages/core/src/render3/index.ts b/packages/core/src/render3/index.ts index 421b421364..0e8e091bc3 100644 --- a/packages/core/src/render3/index.ts +++ b/packages/core/src/render3/index.ts @@ -73,6 +73,10 @@ export { tick, } from './instructions'; +export { + AttributeMarker +} from './interfaces/node'; + export { pipe as Pp, pipeBind1 as pb1, diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index cc90d6c24d..20752d436a 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -15,7 +15,7 @@ import {CssSelectorList, LProjection, NG_PROJECT_AS_ATTR_NAME} from './interface import {LQueries} from './interfaces/query'; import {CurrentMatchesList, LView, LViewFlags, LifecycleStage, RootContext, TData, TView} from './interfaces/view'; -import {LContainerNode, LElementNode, LNode, TNodeType, TNodeFlags, LProjectionNode, LTextNode, LViewNode, TNode, TContainerNode, InitialInputData, InitialInputs, PropertyAliases, PropertyAliasValue, TElementNode,} from './interfaces/node'; +import {AttributeMarker, TAttributes, LContainerNode, LElementNode, LNode, TNodeType, TNodeFlags, LProjectionNode, LTextNode, LViewNode, TNode, TContainerNode, InitialInputData, InitialInputs, PropertyAliases, PropertyAliasValue, TElementNode,} from './interfaces/node'; import {assertNodeType} from './node_assert'; import {appendChild, insertView, appendProjectedNode, removeView, canInsertNativeNode, createTextNode, getNextLNode, getChildLNode, getParentLNode} from './node_manipulation'; import {isNodeMatchingSelectorList, matchingSelectorIndex} from './node_selector_matcher'; @@ -366,19 +366,19 @@ export function createLNodeObject( */ export function createLNode( index: number | null, type: TNodeType.Element, native: RElement | RText | null, - name: string | null, attrs: string[] | null, lView?: LView | null): LElementNode; + name: string | null, attrs: TAttributes | null, lView?: LView | null): LElementNode; export function createLNode( index: number | null, type: TNodeType.View, native: null, name: null, attrs: null, lView: LView): LViewNode; export function createLNode( index: number, type: TNodeType.Container, native: undefined, name: string | null, - attrs: string[] | null, lContainer: LContainer): LContainerNode; + attrs: TAttributes | null, lContainer: LContainer): LContainerNode; export function createLNode( - index: number, type: TNodeType.Projection, native: null, name: null, attrs: string[] | null, + index: number, type: TNodeType.Projection, native: null, name: null, attrs: TAttributes | null, lProjection: LProjection): LProjectionNode; export function createLNode( index: number | null, type: TNodeType, native: RText | RElement | null | undefined, - name: string | null, attrs: string[] | null, state?: null | LView | LContainer | + name: string | null, attrs: TAttributes | null, state?: null | LView | LContainer | LProjection): LElementNode<extNode&LViewNode&LContainerNode&LProjectionNode { const parent = isParent ? previousOrParentNode : previousOrParentNode && getParentLNode(previousOrParentNode) !as LNode; @@ -586,7 +586,8 @@ function getRenderFlags(view: LView): RenderFlags { * ['id', 'warning5', 'class', 'alert'] */ export function elementStart( - index: number, name: string, attrs?: string[] | null, localRefs?: string[] | null): RElement { + index: number, name: string, attrs?: TAttributes | null, + localRefs?: string[] | null): RElement { ngDevMode && assertEqual( currentView.bindingStartIndex, -1, 'elements should be created before any bindings'); @@ -600,23 +601,18 @@ export function elementStart( if (attrs) setUpAttributes(native, attrs); appendChild(getParentLNode(node), native, currentView); - createDirectivesAndLocals(index, name, attrs, localRefs, false); + createDirectivesAndLocals(localRefs); return native; } /** * Creates directive instances and populates local refs. * - * @param index Index of the current node (to create TNode) - * @param name Tag name of the current node - * @param attrs Attrs of the current node * @param localRefs Local refs of the current node - * @param inlineViews Whether or not this node will create inline views */ -function createDirectivesAndLocals( - index: number, name: string | null, attrs: string[] | null | undefined, - localRefs: string[] | null | undefined, inlineViews: boolean) { +function createDirectivesAndLocals(localRefs?: string[] | null) { const node = previousOrParentNode; + if (firstTemplatePass) { ngDevMode && ngDevMode.firstTemplatePass++; cacheMatchingDirectivesForNode(node.tNode, currentView.tView, localRefs || null); @@ -822,17 +818,18 @@ export function createTView( }; } -function setUpAttributes(native: RElement, attrs: string[]): void { - ngDevMode && assertEqual(attrs.length % 2, 0, 'each attribute should have a key and a value'); - +function setUpAttributes(native: RElement, attrs: TAttributes): void { const isProc = isProceduralRenderer(renderer); for (let i = 0; i < attrs.length; i += 2) { const attrName = attrs[i]; + if (attrName === AttributeMarker.SELECT_ONLY) break; if (attrName !== NG_PROJECT_AS_ATTR_NAME) { const attrVal = attrs[i + 1]; ngDevMode && ngDevMode.rendererSetAttribute++; - isProc ? (renderer as ProceduralRenderer3).setAttribute(native, attrName, attrVal) : - native.setAttribute(attrName, attrVal); + isProc ? + (renderer as ProceduralRenderer3) + .setAttribute(native, attrName as string, attrVal as string) : + native.setAttribute(attrName as string, attrVal as string); } } } @@ -1046,7 +1043,7 @@ export function elementProperty( * @returns the TNode object */ export function createTNode( - type: TNodeType, index: number | null, tagName: string | null, attrs: string[] | null, + type: TNodeType, index: number | null, tagName: string | null, attrs: TAttributes | null, parent: TElementNode | TContainerNode | null, tViews: TView[] | null): TNode { ngDevMode && ngDevMode.tNode++; return { @@ -1442,10 +1439,13 @@ function generateInitialInputs( for (let i = 0; i < attrs.length; i += 2) { const attrName = attrs[i]; const minifiedInputName = inputs[attrName]; + const attrValue = attrs[i + 1]; + + if (attrName === AttributeMarker.SELECT_ONLY) break; if (minifiedInputName !== undefined) { const inputsToStore: InitialInputs = initialInputData[directiveIndex] || (initialInputData[directiveIndex] = []); - inputsToStore.push(minifiedInputName, attrs[i + 1]); + inputsToStore.push(minifiedInputName, attrValue as string); } } return initialInputData; @@ -1484,7 +1484,7 @@ export function createLContainer( * @param localRefs A set of local reference bindings on the element. */ export function container( - index: number, template?: ComponentTemplate, tagName?: string | null, attrs?: string[], + index: number, template?: ComponentTemplate, tagName?: string | null, attrs?: TAttributes, localRefs?: string[] | null): void { ngDevMode && assertEqual( currentView.bindingStartIndex, -1, @@ -1501,7 +1501,7 @@ export function container( // Containers are added to the current view tree instead of their embedded views // because views can be removed and re-inserted. addToViewTree(currentView, node.data); - createDirectivesAndLocals(index, tagName || null, attrs, localRefs, template == null); + createDirectivesAndLocals(localRefs); isParent = false; ngDevMode && assertNodeType(previousOrParentNode, TNodeType.Container); diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index c7d316ab92..80b04fecbe 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -158,6 +158,29 @@ export interface LProjectionNode extends LNode { dynamicLContainerNode: null; } +/** + * A set of marker values to be used in the attributes arrays. Those markers indicate that some + * items are not regular attributes and the processing should be adapted accordingly. + */ +export const enum AttributeMarker { + NS = 0, // namespace. Has to be repeated. + + /** + * This marker indicates that the following attribute names were extracted from bindings (ex.: + * [foo]="exp") and / or event handlers (ex. (bar)="doSth()"). + * Taking the above bindings and outputs as an example an attributes array could look as follows: + * ['class', 'fade in', AttributeMarker.SELECT_ONLY, 'foo', 'bar'] + */ + SELECT_ONLY = 1 +} + +/** + * A combination of: + * - attribute names and values + * - special markers acting as flags to alter attributes processing. + */ +export type TAttributes = (string | AttributeMarker)[]; + /** * LNode binding data (flyweight) for a particular node that is shared between all templates * of a specific type. @@ -198,18 +221,20 @@ export interface TNode { tagName: string|null; /** - * Static attributes associated with an element. We need to store - * static attributes to support content projection with selectors. - * Attributes are stored statically because reading them from the DOM - * would be way too slow for content projection and queries. + * Attributes associated with an element. We need to store attributes to support various use-cases + * (attribute injection, content projection with selectors, directives matching). + * Attributes are stored statically because reading them from the DOM would be way too slow for + * content projection and queries. * - * Since attrs will always be calculated first, they will never need - * to be marked undefined by other instructions. + * Since attrs will always be calculated first, they will never need to be marked undefined by + * other instructions. * - * The name of the attribute and its value alternate in the array. + * For regular attributes a name of an attribute and its value alternate in the array. * e.g. ['role', 'checkbox'] + * This array can contain flags that will indicate "special attributes" (attributes with + * namespaces, attributes extracted from bindings and outputs). */ - attrs: string[]|null; + attrs: TAttributes|null; /** * A set of local names under which a given element is exported in a template and diff --git a/packages/core/src/render3/node_selector_matcher.ts b/packages/core/src/render3/node_selector_matcher.ts index e92e3f287d..73f2ab55ec 100644 --- a/packages/core/src/render3/node_selector_matcher.ts +++ b/packages/core/src/render3/node_selector_matcher.ts @@ -9,7 +9,7 @@ import './ng_dev_mode'; import {assertNotNull} from './assert'; -import {TNode, unusedValueExportToPlacateAjd as unused1} from './interfaces/node'; +import {AttributeMarker, TAttributes, TNode, unusedValueExportToPlacateAjd as unused1} from './interfaces/node'; import {CssSelector, CssSelectorList, NG_PROJECT_AS_ATTR_NAME, SelectorFlags, unusedValueExportToPlacateAjd as unused2} from './interfaces/projection'; const unusedValueToPlacateAjd = unused1 + unused2; @@ -40,6 +40,7 @@ export function isNodeMatchingSelector(tNode: TNode, selector: CssSelector): boo let mode: SelectorFlags = SelectorFlags.ELEMENT; const nodeAttrs = tNode.attrs !; + const selectOnlyMarkerIdx = nodeAttrs ? nodeAttrs.indexOf(AttributeMarker.SELECT_ONLY) : -1; // When processing ":not" selectors, we skip to the next ":not" if the // current one doesn't match @@ -80,9 +81,11 @@ export function isNodeMatchingSelector(tNode: TNode, selector: CssSelector): boo const selectorAttrValue = mode & SelectorFlags.CLASS ? current : selector[++i]; if (selectorAttrValue !== '') { - const nodeAttrValue = nodeAttrs[attrIndexInNode + 1]; + const nodeAttrValue = selectOnlyMarkerIdx > -1 && attrIndexInNode > selectOnlyMarkerIdx ? + '' : + nodeAttrs[attrIndexInNode + 1]; if (mode & SelectorFlags.CLASS && - !isCssClassMatching(nodeAttrValue, selectorAttrValue as string) || + !isCssClassMatching(nodeAttrValue as string, selectorAttrValue as string) || mode & SelectorFlags.ATTRIBUTE && selectorAttrValue !== nodeAttrValue) { if (isPositive(mode)) return false; skipToNextSelector = true; @@ -98,10 +101,15 @@ function isPositive(mode: SelectorFlags): boolean { return (mode & SelectorFlags.NOT) === 0; } -function findAttrIndexInNode(name: string, attrs: string[] | null): number { +function findAttrIndexInNode(name: string, attrs: TAttributes | null): number { + let step = 2; if (attrs === null) return -1; - for (let i = 0; i < attrs.length; i += 2) { - if (attrs[i] === name) return i; + for (let i = 0; i < attrs.length; i += step) { + const attrName = attrs[i]; + if (attrName === name) return i; + if (attrName === AttributeMarker.SELECT_ONLY) { + step = 1; + } } return -1; } @@ -123,7 +131,7 @@ export function getProjectAsAttrValue(tNode: TNode): string|null { // only check for ngProjectAs in attribute names, don't accidentally match attribute's value // (attribute names are stored at even indexes) if ((ngProjectAsAttrIdx & 1) === 0) { - return nodeAttrs[ngProjectAsAttrIdx + 1]; + return nodeAttrs[ngProjectAsAttrIdx + 1] as string; } } return null; diff --git a/packages/core/test/render3/content_spec.ts b/packages/core/test/render3/content_spec.ts index 45469996af..71d7dda90b 100644 --- a/packages/core/test/render3/content_spec.ts +++ b/packages/core/test/render3/content_spec.ts @@ -8,10 +8,11 @@ import {SelectorFlags} from '@angular/core/src/render3/interfaces/projection'; -import {detectChanges} from '../../src/render3/index'; -import {container, containerRefreshEnd, containerRefreshStart, elementEnd, elementStart, embeddedViewEnd, embeddedViewStart, load, loadDirective, projection, projectionDef, text} from '../../src/render3/instructions'; +import {AttributeMarker, detectChanges} from '../../src/render3/index'; +import {bind, container, containerRefreshEnd, containerRefreshStart, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, loadDirective, projection, projectionDef, text} from '../../src/render3/instructions'; import {RenderFlags} from '../../src/render3/interfaces/definition'; -import {createComponent, renderComponent, toHtml} from './render_util'; + +import {ComponentFixture, createComponent, renderComponent, toHtml} from './render_util'; describe('content projection', () => { it('should project content', () => { @@ -583,6 +584,43 @@ describe('content projection', () => { '
1
2
'); }); + // https://stackblitz.com/edit/angular-psokum?file=src%2Fapp%2Fapp.module.ts + it('should project nodes where attribute selector matches a binding', () => { + /** + * + */ + const Child = createComponent('child', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + projectionDef(0, [[['', 'title', '']]], ['[title]']); + { projection(1, 0, 1); } + } + }); + + /** + * + * Has title + * + */ + const Parent = createComponent('parent', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'child'); + { + elementStart(1, 'span', [AttributeMarker.SELECT_ONLY, 'title']); + { text(2, 'Has title'); } + elementEnd(); + } + elementEnd(); + } + if (rf & RenderFlags.Update) { + elementProperty(1, 'title', bind('Some title')); + } + }, [Child]); + + const fixture = new ComponentFixture(Parent); + expect(fixture.html).toEqual('Has title'); + + }); + it('should project nodes using class selectors', () => { /** *
diff --git a/packages/core/test/render3/di_spec.ts b/packages/core/test/render3/di_spec.ts index 73dc163330..c5bceace20 100644 --- a/packages/core/test/render3/di_spec.ts +++ b/packages/core/test/render3/di_spec.ts @@ -12,9 +12,9 @@ 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, injectTemplateRef, injectViewContainerRef} from '../../src/render3/index'; -import {bind, container, containerRefreshEnd, containerRefreshStart, createLNode, createLView, createTView, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, enterView, interpolation2, leaveView, load, projection, projectionDef, text, textBinding} from '../../src/render3/instructions'; +import {bind, container, containerRefreshEnd, containerRefreshStart, createLNode, createLView, createTView, elementEnd, elementStart, embeddedViewEnd, embeddedViewStart, enterView, interpolation2, leaveView, load, projection, projectionDef, text, textBinding} from '../../src/render3/instructions'; import {LInjector} from '../../src/render3/interfaces/injector'; -import {TNodeType} from '../../src/render3/interfaces/node'; +import {AttributeMarker, TNodeType} from '../../src/render3/interfaces/node'; import {LViewFlags} from '../../src/render3/interfaces/view'; import {ViewRef} from '../../src/render3/view_ref'; @@ -1199,21 +1199,61 @@ describe('di', () => { }); }); - it('should injectAttribute', () => { - let exist: string|undefined = 'wrong'; - let nonExist: string|undefined = 'wrong'; + describe('@Attribute', () => { - const MyApp = createComponent('my-app', function(rf: RenderFlags, ctx: any) { - if (rf & RenderFlags.Create) { - elementStart(0, 'div', ['exist', 'existValue', 'other', 'ignore']); - exist = injectAttribute('exist'); - nonExist = injectAttribute('nonExist'); - } + it('should inject attribute', () => { + let exist: string|undefined = 'wrong'; + let nonExist: string|undefined = 'wrong'; + + const MyApp = createComponent('my-app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'div', ['exist', 'existValue', 'other', 'ignore']); + exist = injectAttribute('exist'); + nonExist = injectAttribute('nonExist'); + } + }); + + const fixture = new ComponentFixture(MyApp); + expect(exist).toEqual('existValue'); + expect(nonExist).toEqual(undefined); }); - const app = renderComponent(MyApp); - expect(exist).toEqual('existValue'); - expect(nonExist).toEqual(undefined); + // https://stackblitz.com/edit/angular-8ytqkp?file=src%2Fapp%2Fapp.component.ts + it('should not inject attributes representing bindings and outputs', () => { + let exist: string|undefined = 'wrong'; + let nonExist: string|undefined = 'wrong'; + + const MyApp = createComponent('my-app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'div', ['exist', 'existValue', AttributeMarker.SELECT_ONLY, 'nonExist']); + exist = injectAttribute('exist'); + nonExist = injectAttribute('nonExist'); + } + }); + + const fixture = new ComponentFixture(MyApp); + expect(exist).toEqual('existValue'); + expect(nonExist).toEqual(undefined); + }); + + it('should not accidentally inject attributes representing bindings and outputs', () => { + let exist: string|undefined = 'wrong'; + let nonExist: string|undefined = 'wrong'; + + const MyApp = createComponent('my-app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + elementStart(0, 'div', [ + 'exist', 'existValue', AttributeMarker.SELECT_ONLY, 'binding1', 'nonExist', 'binding2' + ]); + exist = injectAttribute('exist'); + nonExist = injectAttribute('nonExist'); + } + }); + + const fixture = new ComponentFixture(MyApp); + expect(exist).toEqual('existValue'); + expect(nonExist).toEqual(undefined); + }); }); describe('inject', () => { diff --git a/packages/core/test/render3/directive_spec.ts b/packages/core/test/render3/directive_spec.ts index d088989ea1..58f0c22349 100644 --- a/packages/core/test/render3/directive_spec.ts +++ b/packages/core/test/render3/directive_spec.ts @@ -6,10 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ -import {defineDirective} from '../../src/render3/index'; -import {bind, elementEnd, elementProperty, elementStart, loadDirective} from '../../src/render3/instructions'; -import {RenderFlags} from '../../src/render3/interfaces/definition'; -import {renderToHtml} from './render_util'; +import {EventEmitter} from '@angular/core'; + +import {AttributeMarker, defineDirective} from '../../src/render3/index'; +import {bind, elementEnd, elementProperty, elementStart, listener, loadDirective} from '../../src/render3/instructions'; + +import {TemplateFixture} from './render_util'; describe('directive', () => { @@ -31,17 +33,151 @@ describe('directive', () => { }); } - function Template(rf: RenderFlags, ctx: any) { - if (rf & RenderFlags.Create) { - elementStart(0, 'span', ['dir', '']); - elementEnd(); + function Template() { + elementStart(0, 'span', [AttributeMarker.SELECT_ONLY, 'dir']); + elementEnd(); + } + + const fixture = new TemplateFixture(Template, () => {}, [Directive]); + expect(fixture.html).toEqual(''); + + directiveInstance !.klass = 'bar'; + fixture.update(); + expect(fixture.html).toEqual(''); + }); + + }); + + describe('selectors', () => { + + it('should match directives with attribute selectors on bindings', () => { + let directiveInstance: Directive; + + class Directive { + static ngDirectiveDef = defineDirective({ + type: Directive, + selectors: [['', 'test', '']], + factory: () => directiveInstance = new Directive, + inputs: {test: 'test', other: 'other'} + }); + + testValue: boolean; + other: boolean; + + /** + * A setter to assert that a binding is not invoked with stringified attribute value + */ + set test(value: any) { + // if a binding is processed correctly we should only be invoked with a false Boolean + // and never with the "false" string literal + this.testValue = value; + if (value !== false) { + fail('Should only be called with a false Boolean value, got a non-falsy value'); + } } } - const defs = [Directive]; - expect(renderToHtml(Template, {}, defs)).toEqual(''); - directiveInstance !.klass = 'bar'; - expect(renderToHtml(Template, {}, defs)).toEqual(''); + /** + * + */ + function createTemplate() { + // using 2 bindings to show example shape of attributes array + elementStart(0, 'span', ['class', 'fade', AttributeMarker.SELECT_ONLY, 'test', 'other']); + elementEnd(); + } + + function updateTemplate() { elementProperty(0, 'test', bind(false)); } + + const fixture = new TemplateFixture(createTemplate, updateTemplate, [Directive]); + + // the "test" attribute should not be reflected in the DOM as it is here only for directive + // matching purposes + expect(fixture.html).toEqual(''); + expect(directiveInstance !.testValue).toBe(false); + }); + + it('should not accidentally set inputs from attributes extracted from bindings / outputs', + () => { + let directiveInstance: Directive; + + class Directive { + static ngDirectiveDef = defineDirective({ + type: Directive, + selectors: [['', 'test', '']], + factory: () => directiveInstance = new Directive, + inputs: {test: 'test', prop1: 'prop1', prop2: 'prop2'} + }); + + prop1: boolean; + prop2: boolean; + testValue: boolean; + + + /** + * A setter to assert that a binding is not invoked with stringified attribute value + */ + set test(value: any) { + // if a binding is processed correctly we should only be invoked with a false Boolean + // and never with the "false" string literal + this.testValue = value; + if (value !== false) { + fail('Should only be called with a false Boolean value, got a non-falsy value'); + } + } + } + + /** + * + */ + function createTemplate() { + // putting name (test) in the "usual" value position + elementStart( + 0, 'span', ['class', 'fade', AttributeMarker.SELECT_ONLY, 'prop1', 'test', 'prop2']); + elementEnd(); + } + + function updateTemplate() { + elementProperty(0, 'prop1', bind(true)); + elementProperty(0, 'test', bind(false)); + elementProperty(0, 'prop2', bind(true)); + } + + const fixture = new TemplateFixture(createTemplate, updateTemplate, [Directive]); + + // the "test" attribute should not be reflected in the DOM as it is here only for directive + // matching purposes + expect(fixture.html).toEqual(''); + expect(directiveInstance !.testValue).toBe(false); + }); + + it('should match directives with attribute selectors on outputs', () => { + let directiveInstance: Directive; + + class Directive { + static ngDirectiveDef = defineDirective({ + type: Directive, + selectors: [['', 'out', '']], + factory: () => directiveInstance = new Directive, + outputs: {out: 'out'} + }); + + out = new EventEmitter(); + } + + /** + * + */ + function createTemplate() { + elementStart(0, 'span', [AttributeMarker.SELECT_ONLY, 'out']); + { listener('out', () => {}); } + elementEnd(); + } + + const fixture = new TemplateFixture(createTemplate, () => {}, [Directive]); + + // "out" should not be part of reflected attributes + expect(fixture.html).toEqual(''); + expect(directiveInstance !).not.toBeUndefined(); }); }); diff --git a/packages/core/test/render3/node_selector_matcher_spec.ts b/packages/core/test/render3/node_selector_matcher_spec.ts index 53b432ea56..3a143bcadf 100644 --- a/packages/core/test/render3/node_selector_matcher_spec.ts +++ b/packages/core/test/render3/node_selector_matcher_spec.ts @@ -6,12 +6,12 @@ * found in the LICENSE file at https://angular.io/license */ -import {TNode, TNodeType} from '../../src/render3/interfaces/node'; +import {AttributeMarker, TAttributes, TNode, TNodeType} from '../../src/render3/interfaces/node'; import {CssSelector, CssSelectorList, NG_PROJECT_AS_ATTR_NAME, SelectorFlags,} from '../../src/render3/interfaces/projection'; import {getProjectAsAttrValue, isNodeMatchingSelectorList, isNodeMatchingSelector} from '../../src/render3/node_selector_matcher'; -function testLStaticData(tagName: string, attrs: string[] | null): TNode { +function testLStaticData(tagName: string, attrs: TAttributes | null): TNode { return { type: TNodeType.Element, index: 0, @@ -29,7 +29,7 @@ function testLStaticData(tagName: string, attrs: string[] | null): TNode { } describe('css selector matching', () => { - function isMatching(tagName: string, attrs: string[] | null, selector: CssSelector): boolean { + function isMatching(tagName: string, attrs: TAttributes | null, selector: CssSelector): boolean { return isNodeMatchingSelector(testLStaticData(tagName, attrs), selector); } @@ -177,6 +177,28 @@ describe('css selector matching', () => { '', 'class', 'foo' ])).toBeTruthy(`Selector '[class="foo"]' should match `); }); + + it('should take optional binding attribute names into account', () => { + expect(isMatching('span', [AttributeMarker.SELECT_ONLY, 'directive'], [ + '', 'directive', '' + ])).toBeTruthy(`Selector '[directive]' should match `); + }); + + it('should not match optional binding attribute names if attribute selector has value', + () => { + expect(isMatching('span', [AttributeMarker.SELECT_ONLY, 'directive'], [ + '', 'directive', 'value' + ])).toBeFalsy(`Selector '[directive=value]' should not match `); + }); + + it('should not match optional binding attribute names if attribute selector has value and next name equals to value', + () => { + expect(isMatching( + 'span', [AttributeMarker.SELECT_ONLY, 'directive', 'value'], + ['', 'directive', 'value'])) + .toBeFalsy( + `Selector '[directive=value]' should not match `); + }); }); describe('class matching', () => {