From 277681096d78eba35783b31f871771b33d999e64 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Wed, 18 Dec 2019 14:35:22 +0100 Subject: [PATCH] fix(ivy): properly bootstrap components with attribute selectors (#34450) Fixes #34349 PR Close #34450 --- integration/_payload-limits.json | 4 +- packages/core/src/render3/component_ref.ts | 12 ++- .../core/src/render3/node_selector_matcher.ts | 73 ++++++++++++++++ .../core/test/acceptance/bootstrap_spec.ts | 47 +++++++++-- .../core/test/acceptance/component_spec.ts | 38 +++++++++ .../core/test/render3/component_ref_spec.ts | 8 +- .../render3/node_selector_matcher_spec.ts | 83 ++++++++++++++++++- 7 files changed, 246 insertions(+), 19 deletions(-) diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index a54df902bd..f7ecc299a3 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -30,7 +30,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 136594, + "main-es2015": 137141, "polyfills-es2015": 37494 } } @@ -49,7 +49,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 225787, + "main-es2015": 226288, "polyfills-es2015": 36808, "5-es2015": 779 } diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index ecb71c3c9f..6bd001f66c 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -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'; @@ -113,9 +114,7 @@ export class ComponentFactory extends viewEngine_ComponentFactory { private componentDef: ComponentDef, private ngModule?: viewEngine_NgModuleRef) { 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; @@ -135,7 +134,12 @@ export class ComponentFactory extends viewEngine_ComponentFactory { 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; diff --git a/packages/core/src/render3/node_selector_matcher.ts b/packages/core/src/render3/node_selector_matcher.ts index a943f12ce8..f4c9b66a52 100644 --- a/packages/core/src/render3/node_selector_matcher.ts +++ b/packages/core/src/render3/node_selector_matcher.ts @@ -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(','); +} \ No newline at end of file diff --git a/packages/core/test/acceptance/bootstrap_spec.ts b/packages/core/test/acceptance/bootstrap_spec.ts index b11889bfb9..bcd355159f 100644 --- a/packages/core/test/acceptance/bootstrap_spec.ts +++ b/packages/core/test/acceptance/bootstrap_spec.ts @@ -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('
', async() => { + it('should bootstrap using #id selector', + withBody('
before|
', 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('
before|
', async() => { + try { + const ngModuleRef = + await platformBrowserDynamic().bootstrapModule(MultipleSelectorsAppModule); + expect(document.body.textContent).toEqual('before|works!'); ngModuleRef.destroy(); } catch (err) { console.error(err); @@ -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 { +} \ No newline at end of file diff --git a/packages/core/test/acceptance/component_spec.ts b/packages/core/test/acceptance/component_spec.ts index f80ac4c3a0..352cb57ce8 100644 --- a/packages/core/test/acceptance/component_spec.ts +++ b/packages/core/test/acceptance/component_spec.ts @@ -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({ diff --git a/packages/core/test/render3/component_ref_spec.ts b/packages/core/test/render3/component_ref_spec.ts index 9380dbe432..4ed8043b72 100644 --- a/packages/core/test/render3/component_ref_spec.ts +++ b/packages/core/test/render3/component_ref_spec.ts @@ -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, @@ -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([]); @@ -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, @@ -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'}, diff --git a/packages/core/test/render3/node_selector_matcher_spec.ts b/packages/core/test/render3/node_selector_matcher_spec.ts index b6d5c3dc5d..58d8a5e129 100644 --- a/packages/core/test/render3/node_selector_matcher_spec.ts +++ b/packages/core/test/render3/node_selector_matcher_spec.ts @@ -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); @@ -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)'); + }); +});