Skip to content

Commit

Permalink
fix(ivy): add attributes and classes to host elements based on select…
Browse files Browse the repository at this point in the history
…or (#34481)

In View Engine, host element of dynamically created component received attributes and classes extracted from component's selector. For example, if component selector is `[attr] .class`, the `attr` attribute and `.class` class will be add to host element. This commit adds similar logic to Ivy, to make sure this behavior is aligned with View Engine.

PR Close #34481
  • Loading branch information
AndrewKushnir authored and alxhub committed Feb 19, 2020
1 parent fd4ce84 commit 03a8b16
Show file tree
Hide file tree
Showing 5 changed files with 206 additions and 16 deletions.
16 changes: 5 additions & 11 deletions packages/core/src/render3/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {CLEAN_PROMISE, addHostBindingsToExpandoInstructions, addToViewTree, crea
import {ComponentDef, ComponentType, RenderFlags} from './interfaces/definition';
import {TElementNode, TNode, TNodeType} from './interfaces/node';
import {PlayerHandler} from './interfaces/player';
import {RElement, Renderer3, RendererFactory3, domRendererFactory3, isProceduralRenderer} from './interfaces/renderer';
import {RElement, Renderer3, RendererFactory3, domRendererFactory3} from './interfaces/renderer';
import {CONTEXT, HEADER_OFFSET, LView, LViewFlags, RootContext, RootContextFlags, TVIEW, TViewType} from './interfaces/view';
import {writeDirectClass, writeDirectStyle} from './node_manipulation';
import {enterView, getPreviousOrParentTNode, leaveView, setSelectedIndex} from './state';
Expand Down Expand Up @@ -139,7 +139,7 @@ export function renderComponent<T>(
try {
if (rendererFactory.begin) rendererFactory.begin();
const componentView = createRootComponentView(
hostRNode, componentDef, rootView, rendererFactory, renderer, null, sanitizer);
hostRNode, componentDef, rootView, rendererFactory, renderer, sanitizer);
component = createRootComponent(
componentView, componentDef, rootView, rootContext, opts.hostFeatures || null);

Expand Down Expand Up @@ -169,8 +169,8 @@ export function renderComponent<T>(
*/
export function createRootComponentView(
rNode: RElement | null, def: ComponentDef<any>, rootView: LView,
rendererFactory: RendererFactory3, hostRenderer: Renderer3, addVersion: string | null,
sanitizer: Sanitizer | null): LView {
rendererFactory: RendererFactory3, hostRenderer: Renderer3,
sanitizer?: Sanitizer | null): LView {
const tView = rootView[TVIEW];
ngDevMode && assertDataInRange(rootView, 0 + HEADER_OFFSET);
rootView[0 + HEADER_OFFSET] = rNode;
Expand All @@ -188,14 +188,8 @@ export function createRootComponentView(
}
}
}
const viewRenderer = rendererFactory.createRenderer(rNode, def);
if (rNode !== null && addVersion) {
ngDevMode && ngDevMode.rendererSetAttribute++;
isProceduralRenderer(hostRenderer) ?
hostRenderer.setAttribute(rNode, 'ng-version', addVersion) :
rNode.setAttribute('ng-version', addVersion);
}

const viewRenderer = rendererFactory.createRenderer(rNode, def);
const componentView = createLView(
rootView, getOrCreateTComponentView(def), null,
def.onPush ? LViewFlags.Dirty : LViewFlags.CheckAlways, rootView[HEADER_OFFSET], tNode,
Expand Down
24 changes: 21 additions & 3 deletions packages/core/src/render3/component_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ import {TContainerNode, TElementContainerNode, TElementNode} from './interfaces/
import {RNode, RendererFactory3, domRendererFactory3} from './interfaces/renderer';
import {LView, LViewFlags, TVIEW, TViewType} from './interfaces/view';
import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces';
import {stringifyCSSSelectorList} from './node_selector_matcher';
import {writeDirectClass} from './node_manipulation';
import {extractAttrsAndClassesFromSelector, stringifyCSSSelectorList} from './node_selector_matcher';
import {enterView, leaveView} from './state';
import {setUpAttributes} from './util/attrs_utils';
import {defaultScheduler} from './util/misc_utils';
import {getTNode} from './util/view_utils';
import {createElementRef} from './view_engine_compatibility';
Expand Down Expand Up @@ -165,7 +167,6 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
const rootLView = createLView(
null, rootTView, rootContext, rootFlags, null, null, rendererFactory, hostRenderer,
sanitizer, rootViewInjector);
const addVersion = rootSelectorOrNode && hostRNode ? VERSION.full : null;

// rootView is the parent when bootstrapping
// TODO(misko): it looks like we are entering view here but we don't really need to as
Expand All @@ -179,7 +180,24 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {

try {
const componentView = createRootComponentView(
hostRNode, this.componentDef, rootLView, rendererFactory, hostRenderer, addVersion, null);
hostRNode, this.componentDef, rootLView, rendererFactory, hostRenderer);
if (hostRNode) {
if (rootSelectorOrNode) {
setUpAttributes(hostRenderer, hostRNode, ['ng-version', VERSION.full]);
} else {
// If host element is created as a part of this function call (i.e. `rootSelectorOrNode`
// is not defined), also apply attributes and classes extracted from component selector.
// Extract attributes and classes from the first selector only to match VE behavior.
const {attrs, classes} =
extractAttrsAndClassesFromSelector(this.componentDef.selectors[0]);
if (attrs) {
setUpAttributes(hostRenderer, hostRNode, attrs);
}
if (classes && classes.length > 0) {
writeDirectClass(hostRenderer, hostRNode, classes.join(' '));
}
}
}

tElementNode = getTNode(rootLView[TVIEW], 0) as TElementNode;

Expand Down
38 changes: 38 additions & 0 deletions packages/core/src/render3/node_selector_matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,4 +388,42 @@ function stringifyCSSSelector(selector: CssSelector): string {
*/
export function stringifyCSSSelectorList(selectorList: CssSelectorList): string {
return selectorList.map(stringifyCSSSelector).join(',');
}

/**
* Extracts attributes and classes information from a given CSS selector.
*
* This function is used while creating a component dynamically. In this case, the host element
* (that is created dynamically) should contain attributes and classes specified in component's CSS
* selector.
*
* @param selector CSS selector in parsed form (in a form of array)
* @returns object with `attrs` and `classes` fields that contain extracted information
*/
export function extractAttrsAndClassesFromSelector(selector: CssSelector):
{attrs: string[], classes: string[]} {
const attrs: string[] = [];
const classes: string[] = [];
let i = 1;
let mode = SelectorFlags.ATTRIBUTE;
while (i < selector.length) {
let valueOrMarker = selector[i];
if (typeof valueOrMarker === 'string') {
if (mode === SelectorFlags.ATTRIBUTE) {
if (valueOrMarker !== '') {
attrs.push(valueOrMarker, selector[++i] as string);
}
} else if (mode === SelectorFlags.CLASS) {
classes.push(valueOrMarker);
}
} else {
// According to CssSelector spec, once we come across `SelectorFlags.NOT` flag, the negative
// mode is maintained for remaining chunks of a selector. Since attributes and classes are
// extracted only for "positive" part of the selector, we can stop here.
if (!isPositive(mode)) break;
mode = valueOrMarker;
}
i++;
}
return {attrs, classes};
}
94 changes: 93 additions & 1 deletion packages/core/test/acceptance/view_container_ref_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {CommonModule, DOCUMENT} from '@angular/common';
import {computeMsgId} from '@angular/compiler';
import {Compiler, Component, ComponentFactoryResolver, Directive, DoCheck, ElementRef, EmbeddedViewRef, ErrorHandler, NO_ERRORS_SCHEMA, NgModule, OnInit, Pipe, PipeTransform, QueryList, Renderer2, RendererFactory2, RendererType2, Sanitizer, TemplateRef, ViewChild, ViewChildren, ViewContainerRef, ɵsetDocument} from '@angular/core';
import {Compiler, Component, ComponentFactoryResolver, Directive, DoCheck, ElementRef, EmbeddedViewRef, ErrorHandler, Injector, NO_ERRORS_SCHEMA, NgModule, OnInit, Pipe, PipeTransform, QueryList, RendererFactory2, RendererType2, Sanitizer, TemplateRef, ViewChild, ViewChildren, ViewContainerRef, ɵsetDocument} from '@angular/core';
import {Input} from '@angular/core/src/metadata';
import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode';
import {TestBed, TestComponentRenderer} from '@angular/core/testing';
Expand Down Expand Up @@ -251,6 +251,98 @@ describe('ViewContainerRef', () => {
// Also test with selector that has element name in uppercase
runTestWithSelectors('SVG[some-attr]', 'MATH[some-attr]');
});

it('should apply attributes and classes to host element based on selector', () => {
@Component({
selector: '[attr-a=a].class-a:not(.class-b):not([attr-b=b]).class-c[attr-c]',
template: 'Hello'
})
class HelloComp {
}

@NgModule({entryComponents: [HelloComp], declarations: [HelloComp]})
class HelloCompModule {
}

@Component({
template: `
<div id="factory" attr-a="a-original" class="class-original"></div>
<div id="vcr">
<ng-container #container></ng-container>
</div>
`
})
class TestComp {
@ViewChild('container', {read: ViewContainerRef}) vcRef !: ViewContainerRef;

private helloCompFactory = this.cfr.resolveComponentFactory(HelloComp);

constructor(public cfr: ComponentFactoryResolver, public injector: Injector) {}

createComponentViaVCRef() {
this.vcRef.createComponent(this.helloCompFactory); //
}

createComponentViaFactory() {
this.helloCompFactory.create(this.injector, undefined, '#factory');
}
}

TestBed.configureTestingModule({declarations: [TestComp], imports: [HelloCompModule]});
const fixture = TestBed.createComponent(TestComp);
fixture.detectChanges();
fixture.componentInstance.createComponentViaVCRef();
fixture.componentInstance.createComponentViaFactory();
fixture.detectChanges();

// Verify host element for a component created via `vcRef.createComponent` method
const vcrHostElement = fixture.nativeElement.querySelector('#vcr > div');

expect(vcrHostElement.classList.contains('class-a')).toBe(true);
// `class-b` should not be present, since it's wrapped in `:not()` selector
expect(vcrHostElement.classList.contains('class-b')).toBe(false);
expect(vcrHostElement.classList.contains('class-c')).toBe(true);

expect(vcrHostElement.getAttribute('attr-a')).toBe('a');
// `attr-b` should not be present, since it's wrapped in `:not()` selector
expect(vcrHostElement.getAttribute('attr-b')).toBe(null);
expect(vcrHostElement.getAttribute('attr-c')).toBe('');

// Verify host element for a component created using `factory.createComponent` method when
// also passing element selector as an argument
const factoryHostElement = fixture.nativeElement.querySelector('#factory');

if (ivyEnabled) {
// In Ivy, if selector is passed when component is created, matched host node (found using
// this selector) retains all attrs/classes and selector-based attrs/classes should *not* be
// added

// Verify original attrs and classes are still present
expect(factoryHostElement.classList.contains('class-original')).toBe(true);
expect(factoryHostElement.getAttribute('attr-a')).toBe('a-original');

// Make sure selector-based attrs and classes were not added to the host element
expect(factoryHostElement.classList.contains('class-a')).toBe(false);
expect(factoryHostElement.getAttribute('attr-c')).toBe(null);

} else {
// In View Engine, selector-based attrs/classes are *always* added to the host element

expect(factoryHostElement.classList.contains('class-a')).toBe(true);
// `class-b` should not be present, since it's wrapped in `:not()` selector
expect(factoryHostElement.classList.contains('class-b')).toBe(false);
expect(factoryHostElement.classList.contains('class-c')).toBe(true);
// Make sure classes are overridden with ones used in component selector
expect(factoryHostElement.classList.contains('class-original')).toBe(false);

// Note: `attr-a` attr is also present on host element, but we update the value with the
// value from component selector (i.e. using `[attr-a=a]`)
expect(factoryHostElement.getAttribute('attr-a')).toBe('a');
// `attr-b` should not be present, since it's wrapped in `:not()` selector
expect(factoryHostElement.getAttribute('attr-b')).toBe(null);
expect(factoryHostElement.getAttribute('attr-c')).toBe('');
}
});
});

describe('insert', () => {
Expand Down
50 changes: 49 additions & 1 deletion packages/core/test/render3/node_selector_matcher_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {createTNode} from '@angular/core/src/render3/instructions/shared';

import {AttributeMarker, TAttributes, TNode, TNodeType} from '../../src/render3/interfaces/node';
import {CssSelector, CssSelectorList, SelectorFlags} from '../../src/render3/interfaces/projection';
import {getProjectAsAttrValue, isNodeMatchingSelector, isNodeMatchingSelectorList, stringifyCSSSelectorList} from '../../src/render3/node_selector_matcher';
import {extractAttrsAndClassesFromSelector, getProjectAsAttrValue, isNodeMatchingSelector, isNodeMatchingSelectorList, stringifyCSSSelectorList} from '../../src/render3/node_selector_matcher';

function testLStaticData(tagName: string, attrs: TAttributes | null): TNode {
return createTNode(null !, null, TNodeType.Element, 0, tagName, attrs);
Expand Down Expand Up @@ -569,3 +569,51 @@ describe('stringifyCSSSelectorList', () => {
])).toBe('[id],button[id="value"],div:not([foo]),div:not(p.bar):not(.baz)');
});
});

describe('extractAttrsAndClassesFromSelector', () => {
const cases = [
[
['div', '', ''],
[],
[],
],
[
['div', 'attr-a', 'a', 'attr-b', 'b', 'attr-c', ''],
['attr-a', 'a', 'attr-b', 'b', 'attr-c', ''],
[],
],
[
['div', 'attr-a', 'a', SelectorFlags.CLASS, 'class-a', 'class-b', 'class-c'],
['attr-a', 'a'],
['class-a', 'class-b', 'class-c'],
],
[
['', 'attr-a', 'a', SelectorFlags.CLASS, 'class-a', SelectorFlags.ATTRIBUTE, 'attr-b', 'b'],
['attr-a', 'a', 'attr-b', 'b'],
['class-a'],
],
[
[
'', '', '', SelectorFlags.ATTRIBUTE, 'attr-a', 'a',
(SelectorFlags.CLASS | SelectorFlags.NOT), 'class-b'
],
['attr-a', 'a'],
[],
],
[
[
'', '', '', (SelectorFlags.CLASS | SelectorFlags.NOT), 'class-a',
(SelectorFlags.ATTRIBUTE | SelectorFlags.NOT), 'attr-b', 'b'
],
[],
[],
],
];
cases.forEach(([selector, attrs, classes]) => {
it(`should process ${JSON.stringify(selector)} selector`, () => {
const extracted = extractAttrsAndClassesFromSelector(selector);
expect(extracted.attrs).toEqual(attrs as string[]);
expect(extracted.classes).toEqual(classes as string[]);
});
});
});

0 comments on commit 03a8b16

Please sign in to comment.