Skip to content

Commit

Permalink
fix(ivy): properly bootstrap components with attribute selectors (#34450
Browse files Browse the repository at this point in the history
)

Fixes #34349

PR Close #34450
  • Loading branch information
pkozlowski-opensource authored and atscott committed Jan 14, 2020
1 parent b3af220 commit 2c0b9ea
Show file tree
Hide file tree
Showing 7 changed files with 246 additions and 19 deletions.
4 changes: 2 additions & 2 deletions integration/_payload-limits.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 1485,
"main-es2015": 136594,
"main-es2015": 137141,

This comment has been minimized.

Copy link
@IgorMinar

IgorMinar Jan 14, 2020

Contributor

😢

"polyfills-es2015": 37494
}
}
Expand All @@ -49,7 +49,7 @@
"master": {
"uncompressed": {
"runtime-es2015": 2289,
"main-es2015": 225787,
"main-es2015": 226288,
"polyfills-es2015": 36808,
"5-es2015": 779
}
Expand Down
12 changes: 8 additions & 4 deletions packages/core/src/render3/component_ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {ComponentDef} from './interfaces/definition';
import {TContainerNode, TElementContainerNode, TElementNode} from './interfaces/node';
import {RNode, RendererFactory3, domRendererFactory3, isProceduralRenderer} from './interfaces/renderer';
import {LView, LViewFlags, TVIEW, TViewType} from './interfaces/view';
import {stringifyCSSSelectorList} from './node_selector_matcher';
import {enterView, leaveView} from './state';
import {defaultScheduler} from './util/misc_utils';
import {getTNode} from './util/view_utils';
Expand Down Expand Up @@ -113,9 +114,7 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
private componentDef: ComponentDef<any>, private ngModule?: viewEngine_NgModuleRef<any>) {
super();
this.componentType = componentDef.type;

// default to 'div' in case this component has an attribute selector
this.selector = componentDef.selectors[0][0] as string || 'div';
this.selector = stringifyCSSSelectorList(componentDef.selectors);
this.ngContentSelectors =
componentDef.ngContentSelectors ? componentDef.ngContentSelectors : [];
this.isBoundToModule = !!ngModule;
Expand All @@ -135,7 +134,12 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {

const hostRNode = rootSelectorOrNode ?
locateHostElement(rendererFactory, rootSelectorOrNode, this.componentDef.encapsulation) :
elementCreate(this.selector, rendererFactory.createRenderer(null, this.componentDef), null);
// Determine a tag name used for creating host elements when this component is created
// dynamically. Default to 'div' if this component did not specify any tag name in its
// selector.
elementCreate(
this.componentDef.selectors[0][0] as string || 'div',
rendererFactory.createRenderer(null, this.componentDef), null);

const rootFlags = this.componentDef.onPush ? LViewFlags.Dirty | LViewFlags.IsRoot :
LViewFlags.CheckAlways | LViewFlags.IsRoot;
Expand Down
73 changes: 73 additions & 0 deletions packages/core/src/render3/node_selector_matcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,3 +297,76 @@ export function isSelectorInSelectorList(selector: CssSelector, list: CssSelecto
}
return false;
}

function maybeWrapInNotSelector(isNegativeMode: boolean, chunk: string): string {
return isNegativeMode ? ':not(' + chunk.trim() + ')' : chunk;
}

function stringifyCSSSelector(selector: CssSelector): string {
let result = selector[0] as string;
let i = 1;
let mode = SelectorFlags.ATTRIBUTE;
let currentChunk = '';
let isNegativeMode = false;
while (i < selector.length) {
let valueOrMarker = selector[i];
if (typeof valueOrMarker === 'string') {
if (mode & SelectorFlags.ATTRIBUTE) {
const attrValue = selector[++i] as string;
currentChunk +=
'[' + valueOrMarker + (attrValue.length > 0 ? '="' + attrValue + '"' : '') + ']';
} else if (mode & SelectorFlags.CLASS) {
currentChunk += '.' + valueOrMarker;
} else if (mode & SelectorFlags.ELEMENT) {
currentChunk += ' ' + valueOrMarker;
}
} else {
//
// Append current chunk to the final result in case we come across SelectorFlag, which
// indicates that the previous section of a selector is over. We need to accumulate content
// between flags to make sure we wrap the chunk later in :not() selector if needed, e.g.
// ```
// ['', Flags.CLASS, '.classA', Flags.CLASS | Flags.NOT, '.classB', '.classC']
// ```
// should be transformed to `.classA :not(.classB .classC)`.
//
// Note: for negative selector part, we accumulate content between flags until we find the
// next negative flag. This is needed to support a case where `:not()` rule contains more than
// one chunk, e.g. the following selector:
// ```
// ['', Flags.ELEMENT | Flags.NOT, 'p', Flags.CLASS, 'foo', Flags.CLASS | Flags.NOT, 'bar']
// ```
// should be stringified to `:not(p.foo) :not(.bar)`
//
if (currentChunk !== '' && !isPositive(valueOrMarker)) {
result += maybeWrapInNotSelector(isNegativeMode, currentChunk);
currentChunk = '';
}
mode = valueOrMarker;
// According to CssSelector spec, once we come across `SelectorFlags.NOT` flag, the negative
// mode is maintained for remaining chunks of a selector.
isNegativeMode = isNegativeMode || !isPositive(mode);
}
i++;
}
if (currentChunk !== '') {
result += maybeWrapInNotSelector(isNegativeMode, currentChunk);
}
return result;
}

/**
* Generates string representation of CSS selector in parsed form.
*
* ComponentDef and DirectiveDef are generated with the selector in parsed form to avoid doing
* additional parsing at runtime (for example, for directive matching). However in some cases (for
* example, while bootstrapping a component), a string version of the selector is required to query
* for the host element on the page. This function takes the parsed form of a selector and returns
* its string representation.
*
* @param selectorList selector in parsed form
* @returns string representation of a given selector
*/
export function stringifyCSSSelectorList(selectorList: CssSelectorList): string {
return selectorList.map(stringifyCSSSelector).join(',');
}
47 changes: 39 additions & 8 deletions packages/core/test/acceptance/bootstrap_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,28 @@
*/

import {Component, NgModule} from '@angular/core';
import {getComponentDef} from '@angular/core/src/render3/definition';
import {BrowserModule} from '@angular/platform-browser';
import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
import {onlyInIvy, withBody} from '@angular/private/testing';
import {withBody} from '@angular/private/testing';

describe('bootstrap', () => {
it('should bootstrap using #id selector', withBody('<div #my-app>', async() => {
it('should bootstrap using #id selector',
withBody('<div>before|</div><button id="my-app"></button>', async() => {
try {
const ngModuleRef = await platformBrowserDynamic().bootstrapModule(MyAppModule);
expect(document.body.textContent).toEqual('works!');
const ngModuleRef = await platformBrowserDynamic().bootstrapModule(IdSelectorAppModule);
expect(document.body.textContent).toEqual('before|works!');
ngModuleRef.destroy();
} catch (err) {
console.error(err);
}
}));

it('should bootstrap using one of selectors from the list',
withBody('<div>before|</div><div class="bar"></div>', async() => {
try {
const ngModuleRef =
await platformBrowserDynamic().bootstrapModule(MultipleSelectorsAppModule);
expect(document.body.textContent).toEqual('before|works!');
ngModuleRef.destroy();
} catch (err) {
console.error(err);
Expand All @@ -28,9 +40,28 @@ describe('bootstrap', () => {
selector: '#my-app',
template: 'works!',
})
export class MyAppComponent {
export class IdSelectorAppComponent {
}

@NgModule({imports: [BrowserModule], declarations: [MyAppComponent], bootstrap: [MyAppComponent]})
export class MyAppModule {
@NgModule({
imports: [BrowserModule],
declarations: [IdSelectorAppComponent],
bootstrap: [IdSelectorAppComponent],
})
export class IdSelectorAppModule {
}

@Component({
selector: '[foo],span,.bar',
template: 'works!',
})
export class MultipleSelectorsAppComponent {
}

@NgModule({
imports: [BrowserModule],
declarations: [MultipleSelectorsAppComponent],
bootstrap: [MultipleSelectorsAppComponent],
})
export class MultipleSelectorsAppModule {
}
38 changes: 38 additions & 0 deletions packages/core/test/acceptance/component_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,44 @@ describe('component', () => {
expect(log).toEqual(['CompB:ngDoCheck']);
});

it('should preserve simple component selector in a component factory', () => {
@Component({selector: '[foo]', template: ''})
class AttSelectorCmp {
}

@NgModule({
declarations: [AttSelectorCmp],
entryComponents: [AttSelectorCmp],
})
class AppModule {
}

TestBed.configureTestingModule({imports: [AppModule]});
const cmpFactoryResolver = TestBed.inject(ComponentFactoryResolver);
const cmpFactory = cmpFactoryResolver.resolveComponentFactory(AttSelectorCmp);

expect(cmpFactory.selector).toBe('[foo]');
});

it('should preserve complex component selector in a component factory', () => {
@Component({selector: '[foo],div:not(.bar)', template: ''})
class ComplexSelectorCmp {
}

@NgModule({
declarations: [ComplexSelectorCmp],
entryComponents: [ComplexSelectorCmp],
})
class AppModule {
}

TestBed.configureTestingModule({imports: [AppModule]});
const cmpFactoryResolver = TestBed.inject(ComponentFactoryResolver);
const cmpFactory = cmpFactoryResolver.resolveComponentFactory(ComplexSelectorCmp);

expect(cmpFactory.selector).toBe('[foo],div:not(.bar)');
});

describe('should clear host element if provided in ComponentFactory.create', () => {
function runTestWithRenderer(rendererProviders: any[]) {
@Component({
Expand Down
8 changes: 4 additions & 4 deletions packages/core/test/render3/component_ref_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('ComponentFactory', () => {
static ɵfac = () => new TestComponent();
static ɵcmp = ɵɵdefineComponent({
type: TestComponent,
selectors: [['test', 'foo'], ['bar']],
selectors: [['test', 'foo', ''], ['bar']],
decls: 0,
vars: 0,
template: () => undefined,
Expand All @@ -32,7 +32,7 @@ describe('ComponentFactory', () => {

const cf = cfr.resolveComponentFactory(TestComponent);

expect(cf.selector).toBe('test');
expect(cf.selector).toBe('test[foo],bar');
expect(cf.componentType).toBe(TestComponent);
expect(cf.ngContentSelectors).toEqual([]);
expect(cf.inputs).toEqual([]);
Expand All @@ -45,7 +45,7 @@ describe('ComponentFactory', () => {
static ɵcmp = ɵɵdefineComponent({
type: TestComponent,
encapsulation: ViewEncapsulation.None,
selectors: [['test', 'foo'], ['bar']],
selectors: [['test', 'foo', ''], ['bar']],
decls: 0,
vars: 0,
template: () => undefined,
Expand All @@ -65,7 +65,7 @@ describe('ComponentFactory', () => {

expect(cf.componentType).toBe(TestComponent);
expect(cf.ngContentSelectors).toEqual(['*', 'a', 'b']);
expect(cf.selector).toBe('test');
expect(cf.selector).toBe('test[foo],bar');

expect(cf.inputs).toEqual([
{propName: 'in1', templateName: 'in1'},
Expand Down
83 changes: 82 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} from '../../src/render3/node_selector_matcher';
import {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 @@ -508,3 +508,84 @@ describe('css selector matching', () => {
});

});

describe('stringifyCSSSelectorList', () => {

it('should stringify selector with a tag name only',
() => { expect(stringifyCSSSelectorList([['button']])).toBe('button'); });

it('should stringify selector with attributes', () => {
expect(stringifyCSSSelectorList([['', 'id', '']])).toBe('[id]');
expect(stringifyCSSSelectorList([['button', 'id', '']])).toBe('button[id]');
expect(stringifyCSSSelectorList([['button', 'id', 'value']])).toBe('button[id="value"]');
expect(stringifyCSSSelectorList([['button', 'id', 'value', 'title', 'other']]))
.toBe('button[id="value"][title="other"]');
});

it('should stringify selector with class names', () => {
expect(stringifyCSSSelectorList([['', SelectorFlags.CLASS, 'foo']])).toBe('.foo');
expect(stringifyCSSSelectorList([['button', SelectorFlags.CLASS, 'foo']])).toBe('button.foo');

expect(stringifyCSSSelectorList([['button', SelectorFlags.CLASS, 'foo', 'bar']]))
.toBe('button.foo.bar');

expect(stringifyCSSSelectorList([
['button', 'id', 'value', 'title', 'other', SelectorFlags.CLASS, 'foo', 'bar']
])).toBe('button[id="value"][title="other"].foo.bar');
});

it('should stringify selector with `:not()` rules', () => {
expect(stringifyCSSSelectorList([['', SelectorFlags.CLASS | SelectorFlags.NOT, 'foo', 'bar']]))
.toBe(':not(.foo.bar)');

expect(stringifyCSSSelectorList([
['button', SelectorFlags.ATTRIBUTE | SelectorFlags.NOT, 'foo', 'bar']
])).toBe('button:not([foo="bar"])');

expect(stringifyCSSSelectorList([['', SelectorFlags.ELEMENT | SelectorFlags.NOT, 'foo']]))
.toBe(':not(foo)');

expect(stringifyCSSSelectorList([
['span', SelectorFlags.CLASS, 'foo', SelectorFlags.CLASS | SelectorFlags.NOT, 'bar', 'baz']
])).toBe('span.foo:not(.bar.baz)');

expect(stringifyCSSSelectorList([
['span', 'id', 'value', SelectorFlags.ATTRIBUTE | SelectorFlags.NOT, 'title', 'other']
])).toBe('span[id="value"]:not([title="other"])');

expect(stringifyCSSSelectorList([[
'', SelectorFlags.CLASS, 'bar', SelectorFlags.ATTRIBUTE | SelectorFlags.NOT, 'foo', '',
SelectorFlags.ELEMENT | SelectorFlags.NOT, 'div'
]])).toBe('.bar:not([foo]):not(div)');

expect(stringifyCSSSelectorList([[
'div', SelectorFlags.ATTRIBUTE | SelectorFlags.NOT, 'foo', '', SelectorFlags.CLASS, 'bar',
SelectorFlags.CLASS | SelectorFlags.NOT, 'baz'
]])).toBe('div:not([foo].bar):not(.baz)');

expect(stringifyCSSSelectorList([[
'div', SelectorFlags.ELEMENT | SelectorFlags.NOT, 'p', SelectorFlags.CLASS, 'bar',
SelectorFlags.CLASS | SelectorFlags.NOT, 'baz'
]])).toBe('div:not(p.bar):not(.baz)');
});

it('should stringify multiple comma-separated selectors', () => {
expect(stringifyCSSSelectorList([
['', 'id', ''], ['button', 'id', 'value']
])).toBe('[id],button[id="value"]');

expect(stringifyCSSSelectorList([
['', 'id', ''], ['button', 'id', 'value'],
['div', SelectorFlags.ATTRIBUTE | SelectorFlags.NOT, 'foo', '']
])).toBe('[id],button[id="value"],div:not([foo])');

expect(stringifyCSSSelectorList([
['', 'id', ''], ['button', 'id', 'value'],
['div', SelectorFlags.ATTRIBUTE | SelectorFlags.NOT, 'foo', ''],
[
'div', SelectorFlags.ELEMENT | SelectorFlags.NOT, 'p', SelectorFlags.CLASS, 'bar',
SelectorFlags.CLASS | SelectorFlags.NOT, 'baz'
]
])).toBe('[id],button[id="value"],div:not([foo]),div:not(p.bar):not(.baz)');
});
});

0 comments on commit 2c0b9ea

Please sign in to comment.