honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ivy): ensure component/directive `class` selectors are properly understood (#27849) 

Angular allows for `<ng-content>` elements to include a selector which
filters which content-projected entries are inserted into the container
depending on whether or not the selector is matched.

With Ivy this feature has not fully worked due to the massive changes
that took place inside of Ivy's styling algorithm code (which is
responsible for assigning classes and styles to an element). This
fix ensures that content-projection can correctly identify which slot
an element should be placed into when class-based selectors are used.

PR Close #27849
This commit is contained in:
Matias Niemelä 2019-01-11 08:37:51 -08:00 committed by Andrew Kushnir
parent 06e5bf1661
commit e62eeed7d4
4 changed files with 179 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
packages/core/src/render3/node_selector_matcher.ts
View File

@ -11,6 +11,7 @@ import '../util/ng_dev_mode';
import {assertDefined, assertNotEqual} from '../util/assert';
import {AttributeMarker, TAttributes, TNode, TNodeType, unusedValueExportToPlacateAjd as unused1} from './interfaces/node';
import {CssSelector, CssSelectorList, NG_PROJECT_AS_ATTR_NAME, SelectorFlags, unusedValueExportToPlacateAjd as unused2} from './interfaces/projection';
import {getInitialClassNameValue} from './styling/class_and_style_bindings';
const unusedValueToPlacateAjd = unused1 + unused2;
@ -58,7 +59,6 @@ function hasTagAndTypeMatch(
export function isNodeMatchingSelector(
tNode: TNode, selector: CssSelector, isProjectionMode: boolean): boolean {
ngDevMode && assertDefined(selector[0], 'Selector should have a tag name');
let mode: SelectorFlags = SelectorFlags.ELEMENT;
const nodeAttrs = tNode.attrs !;
const selectOnlyMarkerIdx = nodeAttrs ? nodeAttrs.indexOf(AttributeMarker.SelectOnly) : -1;
@ -92,7 +92,19 @@ export function isNodeMatchingSelector(
skipToNextSelector = true;
}
} else {
const attrName = mode & SelectorFlags.CLASS ? 'class' : current;
const selectorAttrValue = mode & SelectorFlags.CLASS ? current : selector[++i];
// special case for matching against classes when a tNode has been instantiated with
// class and style values as separate attribute values (e.g. ['title', CLASS, 'foo'])
if ((mode & SelectorFlags.CLASS) && tNode.stylingTemplate) {
if (!isCssClassMatching(readClassValueFromTNode(tNode), selectorAttrValue as string)) {
if (isPositive(mode)) return false;
skipToNextSelector = true;
}
continue;
}
const attrName = (mode & SelectorFlags.CLASS) ? 'class' : current;
const attrIndexInNode = findAttrIndexInNode(attrName, nodeAttrs);
if (attrIndexInNode === -1) {
@ -101,7 +113,6 @@ export function isNodeMatchingSelector(
continue;
}
const selectorAttrValue = mode & SelectorFlags.CLASS ? current : selector[++i];
if (selectorAttrValue !== '') {
let nodeAttrValue: string;
const maybeAttrName = nodeAttrs[attrIndexInNode];
@ -113,8 +124,10 @@ export function isNodeMatchingSelector(
'We do not match directives on namespaced attributes');
nodeAttrValue = nodeAttrs[attrIndexInNode + 1] as string;
}
if (mode & SelectorFlags.CLASS &&
!isCssClassMatching(nodeAttrValue as string, selectorAttrValue as string) ||
const compareAgainstClassName = mode & SelectorFlags.CLASS ? nodeAttrValue : null;
if (compareAgainstClassName &&
!isCssClassMatching(compareAgainstClassName, selectorAttrValue as string) ||
mode & SelectorFlags.ATTRIBUTE && selectorAttrValue !== nodeAttrValue) {
if (isPositive(mode)) return false;
skipToNextSelector = true;
@ -130,6 +143,16 @@ function isPositive(mode: SelectorFlags): boolean {
return (mode & SelectorFlags.NOT) === 0;
}
function readClassValueFromTNode(tNode: TNode): string {
// comparing against CSS class values is complex because the compiler doesn't place them as
// regular attributes when an element is created. Instead, the classes (and styles for
// that matter) are placed in a special styling context that is used for resolving all
// class/style values across static attributes, [style]/[class] and [style.prop]/[class.name]
// bindings. Therefore if and when the styling context exists then the class values are to be
// extracted by the context helper code below...
return tNode.stylingTemplate ? getInitialClassNameValue(tNode.stylingTemplate) : '';
}
/**
* Examines an attributes definition array from a node to find the index of the
* attribute with the specified name.

3
packages/core/test/bundling/todo/bundle.golden_symbols.json
View File

@ -1022,6 +1022,9 @@
{
"name": "queueComponentIndexForCheck"
},
{
"name": "readClassValueFromTNode"
},
{
"name": "readElementValue"
},

226
packages/core/test/linker/projection_integration_spec.ts
View File

@ -81,22 +81,35 @@ describe('projection', () => {
expect(main.nativeElement).toHaveText('');
});
fixmeIvy('FW-833: Directive / projected node matching against class name')
.it('should support multiple content tags', () => {
TestBed.configureTestingModule({declarations: [MultipleContentTagsComponent]});
TestBed.overrideComponent(MainComp, {
set: {
template: '<multiple-content-tags>' +
'<div>B</div>' +
'<div>C</div>' +
'<div class="left">A</div>' +
'</multiple-content-tags>'
}
});
const main = TestBed.createComponent(MainComp);
it('should project a single class-based tag', () => {
TestBed.configureTestingModule({declarations: [SingleContentTagComponent]});
TestBed.overrideComponent(MainComp, {
set: {
template: '<single-content-tag>' +
'<div class="target">I AM PROJECTED</div>' +
'</single-content-tag>'
}
});
const main = TestBed.createComponent(MainComp);
expect(main.nativeElement).toHaveText('(A, BC)');
});
expect(main.nativeElement).toHaveText('I AM PROJECTED');
});
fixmeIvy('unknown').it('should support multiple content tags', () => {
TestBed.configureTestingModule({declarations: [MultipleContentTagsComponent]});
TestBed.overrideComponent(MainComp, {
set: {
template: '<multiple-content-tags>' +
'<div>B</div>' +
'<div>C</div>' +
'<div class="left">A</div>' +
'</multiple-content-tags>'
}
});
const main = TestBed.createComponent(MainComp);
expect(main.nativeElement).toHaveText('(A, BC)');
});
it('should redistribute only direct children', () => {
TestBed.configureTestingModule({declarations: [MultipleContentTagsComponent]});
@ -182,35 +195,34 @@ describe('projection', () => {
expect(main.nativeElement).toHaveText('OUTER(INNER(INNERINNER(A,BC)))');
});
fixmeIvy('FW-833: Directive / projected node matching against class name')
.it('should redistribute when the shadow dom changes', () => {
TestBed.configureTestingModule(
{declarations: [ConditionalContentComponent, ManualViewportDirective]});
TestBed.overrideComponent(MainComp, {
set: {
template: '<conditional-content>' +
'<div class="left">A</div>' +
'<div>B</div>' +
'<div>C</div>' +
'</conditional-content>'
}
});
const main = TestBed.createComponent(MainComp);
fixmeIvy('unknown').it('should redistribute when the shadow dom changes', () => {
TestBed.configureTestingModule(
{declarations: [ConditionalContentComponent, ManualViewportDirective]});
TestBed.overrideComponent(MainComp, {
set: {
template: '<conditional-content>' +
'<div class="left">A</div>' +
'<div>B</div>' +
'<div>C</div>' +
'</conditional-content>'
}
});
const main = TestBed.createComponent(MainComp);
const viewportDirective =
main.debugElement.queryAllNodes(By.directive(ManualViewportDirective))[0].injector.get(
ManualViewportDirective);
const viewportDirective =
main.debugElement.queryAllNodes(By.directive(ManualViewportDirective))[0].injector.get(
ManualViewportDirective);
expect(main.nativeElement).toHaveText('(, BC)');
expect(main.nativeElement).toHaveText('(, BC)');
viewportDirective.show();
main.detectChanges();
expect(main.nativeElement).toHaveText('(A, BC)');
viewportDirective.show();
main.detectChanges();
expect(main.nativeElement).toHaveText('(A, BC)');
viewportDirective.hide();
main.detectChanges();
expect(main.nativeElement).toHaveText('(, BC)');
});
viewportDirective.hide();
main.detectChanges();
expect(main.nativeElement).toHaveText('(, BC)');
});
// GH-2095 - https://github.com/angular/angular/issues/2095
// important as we are removing the ng-content element during compilation,
@ -290,38 +302,36 @@ describe('projection', () => {
expect(main.nativeElement).toHaveText('SIMPLE()START(A)END');
});
fixmeIvy('FW-833: Directive / projected node matching against class name')
.it('should support moving ng-content around', () => {
TestBed.configureTestingModule({
declarations: [ConditionalContentComponent, ProjectDirective, ManualViewportDirective]
});
TestBed.overrideComponent(MainComp, {
set: {
template: '<conditional-content>' +
'<div class="left">A</div>' +
'<div>B</div>' +
'</conditional-content>' +
'START(<div project></div>)END'
}
});
const main = TestBed.createComponent(MainComp);
fixmeIvy('unknown').it('should support moving ng-content around', () => {
TestBed.configureTestingModule(
{declarations: [ConditionalContentComponent, ProjectDirective, ManualViewportDirective]});
TestBed.overrideComponent(MainComp, {
set: {
template: '<conditional-content>' +
'<div class="left">A</div>' +
'<div>B</div>' +
'</conditional-content>' +
'START(<div project></div>)END'
}
});
const main = TestBed.createComponent(MainComp);
const sourceDirective: ManualViewportDirective =
main.debugElement.queryAllNodes(By.directive(ManualViewportDirective))[0].injector.get(
ManualViewportDirective);
const projectDirective: ProjectDirective =
main.debugElement.queryAllNodes(By.directive(ProjectDirective))[0].injector.get(
ProjectDirective);
expect(main.nativeElement).toHaveText('(, B)START()END');
const sourceDirective: ManualViewportDirective =
main.debugElement.queryAllNodes(By.directive(ManualViewportDirective))[0].injector.get(
ManualViewportDirective);
const projectDirective: ProjectDirective =
main.debugElement.queryAllNodes(By.directive(ProjectDirective))[0].injector.get(
ProjectDirective);
expect(main.nativeElement).toHaveText('(, B)START()END');
projectDirective.show(sourceDirective.templateRef);
expect(main.nativeElement).toHaveText('(, B)START(A)END');
projectDirective.show(sourceDirective.templateRef);
expect(main.nativeElement).toHaveText('(, B)START(A)END');
// Stamping ng-content multiple times should not produce the content multiple
// times...
projectDirective.show(sourceDirective.templateRef);
expect(main.nativeElement).toHaveText('(, B)START(A)END');
});
// Stamping ng-content multiple times should not produce the content multiple
// times...
projectDirective.show(sourceDirective.templateRef);
expect(main.nativeElement).toHaveText('(, B)START(A)END');
});
// Note: This does not use a ng-content element, but
// is still important as we are merging proto views independent of
@ -533,49 +543,48 @@ describe('projection', () => {
expect(main.nativeElement).toHaveText('B(A)');
});
fixmeIvy('FW-833: Directive / projected node matching against class name')
.it('should project filled view containers into a view container', () => {
TestBed.configureTestingModule(
{declarations: [ConditionalContentComponent, ManualViewportDirective]});
TestBed.overrideComponent(MainComp, {
set: {
template: '<conditional-content>' +
'<div class="left">A</div>' +
'<ng-template manual class="left">B</ng-template>' +
'<div class="left">C</div>' +
'<div>D</div>' +
'</conditional-content>'
}
});
const main = TestBed.createComponent(MainComp);
fixmeIvy('unknown').it('should project filled view containers into a view container', () => {
TestBed.configureTestingModule(
{declarations: [ConditionalContentComponent, ManualViewportDirective]});
TestBed.overrideComponent(MainComp, {
set: {
template: '<conditional-content>' +
'<div class="left">A</div>' +
'<ng-template manual class="left">B</ng-template>' +
'<div class="left">C</div>' +
'<div>D</div>' +
'</conditional-content>'
}
});
const main = TestBed.createComponent(MainComp);
const conditionalComp = main.debugElement.query(By.directive(ConditionalContentComponent));
const conditionalComp = main.debugElement.query(By.directive(ConditionalContentComponent));
const viewViewportDir =
conditionalComp.queryAllNodes(By.directive(ManualViewportDirective))[0].injector.get(
ManualViewportDirective);
const viewViewportDir =
conditionalComp.queryAllNodes(By.directive(ManualViewportDirective))[0].injector.get(
ManualViewportDirective);
expect(main.nativeElement).toHaveText('(, D)');
expect(main.nativeElement).toHaveText('(, D)');
expect(main.nativeElement).toHaveText('(, D)');
expect(main.nativeElement).toHaveText('(, D)');
viewViewportDir.show();
main.detectChanges();
expect(main.nativeElement).toHaveText('(AC, D)');
viewViewportDir.show();
main.detectChanges();
expect(main.nativeElement).toHaveText('(AC, D)');
const contentViewportDir =
conditionalComp.queryAllNodes(By.directive(ManualViewportDirective))[1].injector.get(
ManualViewportDirective);
const contentViewportDir =
conditionalComp.queryAllNodes(By.directive(ManualViewportDirective))[1].injector.get(
ManualViewportDirective);
contentViewportDir.show();
main.detectChanges();
expect(main.nativeElement).toHaveText('(ABC, D)');
contentViewportDir.show();
main.detectChanges();
expect(main.nativeElement).toHaveText('(ABC, D)');
// hide view viewport, and test that it also hides
// the content viewport's views
viewViewportDir.hide();
main.detectChanges();
expect(main.nativeElement).toHaveText('(, D)');
});
// hide view viewport, and test that it also hides
// the content viewport's views
viewViewportDir.hide();
main.detectChanges();
expect(main.nativeElement).toHaveText('(, D)');
});
});
@Component({selector: 'main', template: ''})
@ -626,6 +635,13 @@ class Empty {
class MultipleContentTagsComponent {
}
@Component({
selector: 'single-content-tag',
template: '<ng-content SELECT=".target"></ng-content>',
})
class SingleContentTagComponent {
}
@Directive({selector: '[manual]'})
class ManualViewportDirective {
constructor(public vc: ViewContainerRef, public templateRef: TemplateRef<Object>) {}

30
packages/core/test/render3/node_selector_matcher_spec.ts
View File

@ -10,6 +10,7 @@ import {AttributeMarker, TAttributes, TNode, TNodeType} from '../../src/render3/
import {CssSelector, CssSelectorList, NG_PROJECT_AS_ATTR_NAME, SelectorFlags,} from '../../src/render3/interfaces/projection';
import {getProjectAsAttrValue, isNodeMatchingSelectorList, isNodeMatchingSelector} from '../../src/render3/node_selector_matcher';
import {initializeStaticContext} from '../../src/render3/styling/class_and_style_bindings';
import {createTNode} from '@angular/core/src/render3/instructions';
import {getLView} from '@angular/core/src/render3/state';
@ -18,9 +19,12 @@ function testLStaticData(tagName: string, attrs: TAttributes | null): TNode {
}
describe('css selector matching', () => {
function isMatching(tagName: string, attrs: TAttributes | null, selector: CssSelector): boolean {
return isNodeMatchingSelector(
createTNode(getLView(), TNodeType.Element, 0, tagName, attrs, null), selector, false);
function isMatching(
tagName: string, attrsOrTNode: TAttributes | TNode | null, selector: CssSelector): boolean {
const tNode = (!attrsOrTNode || Array.isArray(attrsOrTNode)) ?
createTNode(getLView(), TNodeType.Element, 0, tagName, attrsOrTNode as TAttributes, null) :
(attrsOrTNode as TNode);
return isNodeMatchingSelector(tNode, selector, false);
}
describe('isNodeMatchingSimpleSelector', () => {
@ -298,6 +302,26 @@ describe('css selector matching', () => {
// <div class="foo">
expect(isMatching('div', ['class', 'foo'], selector)).toBeFalsy();
});
it('should match against a class value before and after the styling context is created',
() => {
// selector: 'div.abc'
const selector = ['div', SelectorFlags.CLASS, 'abc'];
const tNode = createTNode(getLView(), TNodeType.Element, 0, 'div', [], null);
// <div> (without attrs or styling context)
expect(isMatching('div', tNode, selector)).toBeFalsy();
// <div class="abc"> (with attrs but without styling context)
tNode.attrs = ['class', 'abc'];
tNode.stylingTemplate = null;
expect(isMatching('div', tNode, selector)).toBeTruthy();
// <div class="abc"> (with styling context but without attrs)
tNode.stylingTemplate = initializeStaticContext([AttributeMarker.Classes, 'abc']);
tNode.attrs = null;
expect(isMatching('div', tNode, selector)).toBeTruthy();
});
});
});