fix(ivy): should support components without selector (#27169)

PR Close #27169
This commit is contained in:
Marc Laval 2018-11-22 15:38:28 +01:00 committed by Jason Aden
parent d767e0b2c0
commit c2f30542e7
10 changed files with 467 additions and 425 deletions

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {ConstantPool, CssSelector, Expression, R3ComponentMetadata, R3DirectiveMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr, compileComponentFromMetadata, makeBindingParser, parseTemplate} from '@angular/compiler'; import {ConstantPool, CssSelector, DomElementSchemaRegistry, ElementSchemaRegistry, Expression, R3ComponentMetadata, R3DirectiveMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr, compileComponentFromMetadata, makeBindingParser, parseTemplate} from '@angular/compiler';
import * as path from 'path'; import * as path from 'path';
import * as ts from 'typescript'; import * as ts from 'typescript';
@ -41,6 +41,7 @@ export class ComponentDecoratorHandler implements
private resourceLoader: ResourceLoader, private rootDirs: string[]) {} private resourceLoader: ResourceLoader, private rootDirs: string[]) {}
private literalCache = new Map<Decorator, ts.ObjectLiteralExpression>(); private literalCache = new Map<Decorator, ts.ObjectLiteralExpression>();
private elementSchemaRegistry = new DomElementSchemaRegistry();
detect(node: ts.Declaration, decorators: Decorator[]|null): Decorator|undefined { detect(node: ts.Declaration, decorators: Decorator[]|null): Decorator|undefined {
@ -74,8 +75,9 @@ export class ComponentDecoratorHandler implements
// @Component inherits @Directive, so begin by extracting the @Directive metadata and building // @Component inherits @Directive, so begin by extracting the @Directive metadata and building
// on it. // on it.
const directiveResult = const directiveResult = extractDirectiveMetadata(
extractDirectiveMetadata(node, decorator, this.checker, this.reflector, this.isCore); node, decorator, this.checker, this.reflector, this.isCore,
this.elementSchemaRegistry.getDefaultComponentElementName());
if (directiveResult === undefined) { if (directiveResult === undefined) {
// `extractDirectiveMetadata` returns undefined when the @Directive has `jit: true`. In this // `extractDirectiveMetadata` returns undefined when the @Directive has `jit: true`. In this
// case, compilation of the decorator is skipped. Returning an empty object signifies // case, compilation of the decorator is skipped. Returning an empty object signifies

View File

@ -93,7 +93,7 @@ export class DirectiveDecoratorHandler implements
*/ */
export function extractDirectiveMetadata( export function extractDirectiveMetadata(
clazz: ts.ClassDeclaration, decorator: Decorator, checker: ts.TypeChecker, clazz: ts.ClassDeclaration, decorator: Decorator, checker: ts.TypeChecker,
reflector: ReflectionHost, isCore: boolean): { reflector: ReflectionHost, isCore: boolean, defaultSelector: string | null = null): {
decorator: Map<string, ts.Expression>, decorator: Map<string, ts.Expression>,
metadata: R3DirectiveMetadata, metadata: R3DirectiveMetadata,
decoratedElements: ClassMember[], decoratedElements: ClassMember[],
@ -154,7 +154,7 @@ export function extractDirectiveMetadata(
} }
// Parse the selector. // Parse the selector.
let selector = ''; let selector = defaultSelector;
if (directive.has('selector')) { if (directive.has('selector')) {
const expr = directive.get('selector') !; const expr = directive.get('selector') !;
const resolved = staticallyResolve(expr, reflector, checker); const resolved = staticallyResolve(expr, reflector, checker);

View File

@ -561,6 +561,62 @@ describe('compiler compliance', () => {
expectEmit(source, OtherDirectiveDefinition, 'Incorrect OtherDirective.ngDirectiveDef'); expectEmit(source, OtherDirectiveDefinition, 'Incorrect OtherDirective.ngDirectiveDef');
}); });
it('should support components without selector', () => {
const files = {
app: {
'spec.ts': `
import {Component, Directive, NgModule} from '@angular/core';
@Directive({})
export class EmptyOutletDirective {}
@Component({template: '<router-outlet></router-outlet>'})
export class EmptyOutletComponent {}
@NgModule({declarations: [EmptyOutletComponent]})
export class MyModule{}
`
}
};
// EmptyOutletDirective definition should be:
const EmptyOutletDirectiveDefinition = `
EmptyOutletDirective.ngDirectiveDef = $r3$.ɵdefineDirective({
type: EmptyOutletDirective,
selectors: [],
factory: function EmptyOutletDirective_Factory(t) { return new (t || EmptyOutletDirective)(); }
});
`;
// EmptyOutletComponent definition should be:
const EmptyOutletComponentDefinition = `
EmptyOutletComponent.ngComponentDef = $r3$.ɵdefineComponent({
type: EmptyOutletComponent,
selectors: [["ng-component"]],
factory: function EmptyOutletComponent_Factory(t) { return new (t || EmptyOutletComponent)(); },
consts: 1,
vars: 0,
template: function EmptyOutletComponent_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵelement(0, "router-outlet");
}
},
encapsulation: 2
});
`;
const result = compile(files, angularFiles);
const source = result.source;
expectEmit(
source, EmptyOutletDirectiveDefinition,
'Incorrect EmptyOutletDirective.ngDirectiveDefDef');
expectEmit(
source, EmptyOutletComponentDefinition, 'Incorrect EmptyOutletComponent.ngComponentDef');
});
it('should not treat ElementRef, ViewContainerRef, or ChangeDetectorRef specially when injecting', it('should not treat ElementRef, ViewContainerRef, or ChangeDetectorRef specially when injecting',
() => { () => {
const files = { const files = {

View File

@ -358,9 +358,8 @@ function parserSelectorToR3Selector(selector: CssSelector): R3CssSelector {
return positive.concat(...negative); return positive.concat(...negative);
} }
export function parseSelectorToR3Selector(selector: string): R3CssSelectorList { export function parseSelectorToR3Selector(selector: string | null): R3CssSelectorList {
const selectors = CssSelector.parse(selector); return selector ? CssSelector.parse(selector).map(parserSelectorToR3Selector) : [];
return selectors.map(parserSelectorToR3Selector);
} }
// Pasted from render3/interfaces/definition since it cannot be referenced directly // Pasted from render3/interfaces/definition since it cannot be referenced directly

View File

@ -20,9 +20,11 @@ import {R3Reference} from './render3/util';
import {R3DirectiveMetadata, R3QueryMetadata} from './render3/view/api'; import {R3DirectiveMetadata, R3QueryMetadata} from './render3/view/api';
import {compileComponentFromMetadata, compileDirectiveFromMetadata, parseHostBindings} from './render3/view/compiler'; import {compileComponentFromMetadata, compileDirectiveFromMetadata, parseHostBindings} from './render3/view/compiler';
import {makeBindingParser, parseTemplate} from './render3/view/template'; import {makeBindingParser, parseTemplate} from './render3/view/template';
import {DomElementSchemaRegistry} from './schema/dom_element_schema_registry';
export class CompilerFacadeImpl implements CompilerFacade { export class CompilerFacadeImpl implements CompilerFacade {
R3ResolvedDependencyType = R3ResolvedDependencyType as any; R3ResolvedDependencyType = R3ResolvedDependencyType as any;
private elementSchemaRegistry = new DomElementSchemaRegistry();
compilePipe(angularCoreEnv: CoreEnvironment, sourceMapUrl: string, facade: R3PipeMetadataFacade): compilePipe(angularCoreEnv: CoreEnvironment, sourceMapUrl: string, facade: R3PipeMetadataFacade):
any { any {
@ -118,6 +120,7 @@ export class CompilerFacadeImpl implements CompilerFacade {
{ {
...facade as R3ComponentMetadataFacadeNoPropAndWhitespace, ...facade as R3ComponentMetadataFacadeNoPropAndWhitespace,
...convertDirectiveFacadeToMetadata(facade), ...convertDirectiveFacadeToMetadata(facade),
selector: facade.selector || this.elementSchemaRegistry.getDefaultComponentElementName(),
template, template,
viewQueries: facade.viewQueries.map(convertToR3QueryMetadata), viewQueries: facade.viewQueries.map(convertToR3QueryMetadata),
wrapDirectivesAndPipesInClosure: false, wrapDirectivesAndPipesInClosure: false,

View File

@ -46,7 +46,7 @@ function baseDirectiveFields(
definitionMap.set('type', meta.type); definitionMap.set('type', meta.type);
// e.g. `selectors: [['', 'someDir', '']]` // e.g. `selectors: [['', 'someDir', '']]`
definitionMap.set('selectors', createDirectiveSelector(meta.selector !)); definitionMap.set('selectors', createDirectiveSelector(meta.selector));
// e.g. `factory: () => new MyApp(directiveInject(ElementRef))` // e.g. `factory: () => new MyApp(directiveInject(ElementRef))`
@ -494,7 +494,7 @@ function createQueryDefinition(
} }
// Turn a directive selector into an R3-compatible selector for directive def // Turn a directive selector into an R3-compatible selector for directive def
function createDirectiveSelector(selector: string): o.Expression { function createDirectiveSelector(selector: string | null): o.Expression {
return asLiteral(core.parseSelectorToR3Selector(selector)); return asLiteral(core.parseSelectorToR3Selector(selector));
} }

View File

@ -65,7 +65,7 @@ export function isNodeMatchingSelector(tNode: TNode, selector: CssSelector): boo
if (mode & SelectorFlags.ELEMENT) { if (mode & SelectorFlags.ELEMENT) {
mode = SelectorFlags.ATTRIBUTE | mode & SelectorFlags.NOT; mode = SelectorFlags.ATTRIBUTE | mode & SelectorFlags.NOT;
if (current !== '' && current !== tNode.tagName) { if (current !== '' && current !== tNode.tagName || current === '' && selector.length === 1) {
if (isPositive(mode)) return false; if (isPositive(mode)) return false;
skipToNextSelector = true; skipToNextSelector = true;
} }

View File

@ -47,6 +47,9 @@ describe('css selector matching', () => {
.toBeFalsy(`Selector 'span' should NOT match <SPAN>'`); .toBeFalsy(`Selector 'span' should NOT match <SPAN>'`);
}); });
it('should never match empty string selector', () => {
expect(isMatching('span', null, [''])).toBeFalsy(`Selector '' should NOT match <span>`);
});
}); });
describe('attributes matching', () => { describe('attributes matching', () => {

View File

@ -477,7 +477,6 @@ describe('Integration', () => {
expect(fixture.nativeElement).toHaveText('[simple]'); expect(fixture.nativeElement).toHaveText('[simple]');
})); }));
fixmeIvy('FW-???: TypeError: Cannot read property \'componentInstance\' of undefined') &&
it('should update location when navigating', fakeAsync(() => { it('should update location when navigating', fakeAsync(() => {
@Component({template: `record`}) @Component({template: `record`})
class RecordLocationCmp { class RecordLocationCmp {
@ -673,7 +672,6 @@ describe('Integration', () => {
]); ]);
}))); })));
fixmeIvy('FW-???: TypeError: Cannot read property \'componentInstance\' of undefined') &&
it('should update the location when the matched route does not change', it('should update the location when the matched route does not change',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => { fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = createRoot(router, RootCmp); const fixture = createRoot(router, RootCmp);
@ -835,7 +833,6 @@ describe('Integration', () => {
expect(fixture.nativeElement).toHaveText('query: 2 fragment: fragment2'); expect(fixture.nativeElement).toHaveText('query: 2 fragment: fragment2');
}))); })));
fixmeIvy('FW-???: TypeError: Cannot read property \'componentInstance\' of undefined') &&
it('should ignore null and undefined query params', it('should ignore null and undefined query params',
fakeAsync(inject([Router], (router: Router) => { fakeAsync(inject([Router], (router: Router) => {
const fixture = createRoot(router, RootCmp); const fixture = createRoot(router, RootCmp);
@ -859,7 +856,7 @@ describe('Integration', () => {
])).toThrowError(`The requested path contains undefined segment at index 0`); ])).toThrowError(`The requested path contains undefined segment at index 0`);
}))); })));
fixmeIvy('FW-???: TypeError: Cannot read property \'componentInstance\' of undefined') && fixmeIvy('FW-???: Error: ExpressionChangedAfterItHasBeenCheckedError') &&
it('should push params only when they change', it('should push params only when they change',
fakeAsync(inject([Router], (router: Router) => { fakeAsync(inject([Router], (router: Router) => {
const fixture = createRoot(router, RootCmp); const fixture = createRoot(router, RootCmp);
@ -909,7 +906,7 @@ describe('Integration', () => {
expect(fixture.nativeElement).toHaveText('simple'); expect(fixture.nativeElement).toHaveText('simple');
}))); })));
fixmeIvy('FW-???: TypeError: Cannot read property \'componentInstance\' of undefined') && fixmeIvy('FW-???: Error: ExpressionChangedAfterItHasBeenCheckedError') &&
it('should cancel in-flight navigations', fakeAsync(inject([Router], (router: Router) => { it('should cancel in-flight navigations', fakeAsync(inject([Router], (router: Router) => {
const fixture = createRoot(router, RootCmp); const fixture = createRoot(router, RootCmp);
@ -1308,7 +1305,6 @@ describe('Integration', () => {
expect(cmp.deactivations[0] instanceof BlankCmp).toBe(true); expect(cmp.deactivations[0] instanceof BlankCmp).toBe(true);
})); }));
fixmeIvy('FW-???: TypeError: Cannot read property \'componentInstance\' of undefined') &&
it('should update url and router state before activating components', it('should update url and router state before activating components',
fakeAsync(inject([Router], (router: Router) => { fakeAsync(inject([Router], (router: Router) => {
@ -1345,7 +1341,6 @@ describe('Integration', () => {
}); });
}); });
fixmeIvy('FW-???: TypeError: Cannot read property \'componentInstance\' of undefined') &&
it('should provide resolved data', fakeAsync(inject([Router], (router: Router) => { it('should provide resolved data', fakeAsync(inject([Router], (router: Router) => {
const fixture = createRoot(router, RootCmpWithTwoOutlets); const fixture = createRoot(router, RootCmpWithTwoOutlets);
@ -1354,8 +1349,7 @@ describe('Integration', () => {
data: {one: 1}, data: {one: 1},
resolve: {two: 'resolveTwo'}, resolve: {two: 'resolveTwo'},
children: [ children: [
{path: '', data: {three: 3}, resolve: {four: 'resolveFour'}, component: RouteCmp}, {path: '', data: {three: 3}, resolve: {four: 'resolveFour'}, component: RouteCmp}, {
{
path: '', path: '',
data: {five: 5}, data: {five: 5},
resolve: {six: 'resolveSix'}, resolve: {six: 'resolveSix'},
@ -1425,7 +1419,6 @@ describe('Integration', () => {
expect(e).toEqual(null); expect(e).toEqual(null);
}))); })));
fixmeIvy('FW-???: TypeError: Cannot read property \'componentInstance\' of undefined') &&
it('should preserve resolved data', fakeAsync(inject([Router], (router: Router) => { it('should preserve resolved data', fakeAsync(inject([Router], (router: Router) => {
const fixture = createRoot(router, RootCmp); const fixture = createRoot(router, RootCmp);
@ -1449,7 +1442,6 @@ describe('Integration', () => {
expect(cmp.route.snapshot.data).toEqual({two: 2}); expect(cmp.route.snapshot.data).toEqual({two: 2});
}))); })));
fixmeIvy('FW-???: TypeError: Cannot read property \'componentInstance\' of undefined') &&
it('should rerun resolvers when the urls segments of a wildcard route change', it('should rerun resolvers when the urls segments of a wildcard route change',
fakeAsync(inject([Router, Location], (router: Router) => { fakeAsync(inject([Router, Location], (router: Router) => {
const fixture = createRoot(router, RootCmp); const fixture = createRoot(router, RootCmp);
@ -1525,7 +1517,7 @@ describe('Integration', () => {
}); });
describe('router links', () => { describe('router links', () => {
fixmeIvy('FW-???: ASSERTION ERROR: The provided value must be an instance of an HTMLElement') && fixmeIvy('FW-???: Error: ExpressionChangedAfterItHasBeenCheckedError') &&
it('should support skipping location update for anchor router links', it('should support skipping location update for anchor router links',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => { fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = TestBed.createComponent(RootCmp); const fixture = TestBed.createComponent(RootCmp);
@ -1775,7 +1767,6 @@ describe('Integration', () => {
expect(fixture.nativeElement).toHaveText('team 22 [ simple, right: ]'); expect(fixture.nativeElement).toHaveText('team 22 [ simple, right: ]');
}))); })));
fixmeIvy('FW-662: Components without selector are not supported') &&
it('should support top-level link', fakeAsync(inject([Router], (router: Router) => { it('should support top-level link', fakeAsync(inject([Router], (router: Router) => {
const fixture = createRoot(router, RelativeLinkInIfCmp); const fixture = createRoot(router, RelativeLinkInIfCmp);
advance(fixture); advance(fixture);
@ -2201,7 +2192,7 @@ describe('Integration', () => {
return fixture; return fixture;
} }
fixmeIvy('FW-???: TypeError: Cannot read property \'data\' of undefined') &&
it('should rerun guards and resolvers when params change', it('should rerun guards and resolvers when params change',
fakeAsync(inject([Router], (router: Router) => { fakeAsync(inject([Router], (router: Router) => {
const fixture = configureRouter(router, 'paramsChange'); const fixture = configureRouter(router, 'paramsChange');
@ -2229,7 +2220,6 @@ describe('Integration', () => {
expect(recordedData).toEqual([{data: 0}, {data: 1}, {data: 2}]); expect(recordedData).toEqual([{data: 0}, {data: 1}, {data: 2}]);
}))); })));
fixmeIvy('FW-???: TypeError: Cannot read property \'data\' of undefined') &&
it('should rerun guards and resolvers when query params change', it('should rerun guards and resolvers when query params change',
fakeAsync(inject([Router], (router: Router) => { fakeAsync(inject([Router], (router: Router) => {
const fixture = configureRouter(router, 'paramsOrQueryParamsChange'); const fixture = configureRouter(router, 'paramsOrQueryParamsChange');
@ -2262,7 +2252,6 @@ describe('Integration', () => {
expect(recordedData).toEqual([{data: 0}, {data: 1}, {data: 2}, {data: 3}]); expect(recordedData).toEqual([{data: 0}, {data: 1}, {data: 2}, {data: 3}]);
}))); })));
fixmeIvy('FW-???: TypeError: Cannot read property \'data\' of undefined') &&
it('should always rerun guards and resolvers', it('should always rerun guards and resolvers',
fakeAsync(inject([Router], (router: Router) => { fakeAsync(inject([Router], (router: Router) => {
const fixture = configureRouter(router, 'always'); const fixture = configureRouter(router, 'always');
@ -2292,14 +2281,10 @@ describe('Integration', () => {
router.navigateByUrl('/a;p=2(right:b)?q=1'); router.navigateByUrl('/a;p=2(right:b)?q=1');
advance(fixture); advance(fixture);
expect(guardRunCount).toEqual(5); expect(guardRunCount).toEqual(5);
expect(recordedData).toEqual([ expect(recordedData).toEqual([{data: 0}, {data: 1}, {data: 2}, {data: 3}, {data: 4}]);
{data: 0}, {data: 1}, {data: 2}, {data: 3}, {data: 4}
]);
}))); })));
fixmeIvy('FW-???: TypeError: Cannot read property \'data\' of undefined') && it('should not rerun guards and resolvers', fakeAsync(inject([Router], (router: Router) => {
it('should not rerun guards and resolvers',
fakeAsync(inject([Router], (router: Router) => {
const fixture = configureRouter(router, 'pathParamsChange'); const fixture = configureRouter(router, 'pathParamsChange');
const cmp: RouteCmp = fixture.debugElement.children[1].componentInstance; const cmp: RouteCmp = fixture.debugElement.children[1].componentInstance;
@ -2497,7 +2482,6 @@ describe('Integration', () => {
expect(canceledStatus).toEqual(false); expect(canceledStatus).toEqual(false);
}))); })));
fixmeIvy('FW-???: TypeError: Cannot read property \'componentInstance\' of undefined') &&
it('works with componentless routes', it('works with componentless routes',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => { fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = createRoot(router, RootCmp); const fixture = createRoot(router, RootCmp);
@ -2601,7 +2585,6 @@ describe('Integration', () => {
}))); })));
}); });
fixmeIvy('FW-???: TypeError: Cannot read property \'componentInstance\' of undefined') &&
it('should not create a route state if navigation is canceled', it('should not create a route state if navigation is canceled',
fakeAsync(inject([Router, Location], (router: Router, location: Location) => { fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
const fixture = createRoot(router, RootCmp); const fixture = createRoot(router, RootCmp);
@ -3365,8 +3348,6 @@ describe('Integration', () => {
expect(native.className).toEqual('active'); expect(native.className).toEqual('active');
}))); })));
fixmeIvy('FW-662: Components without selector are not supported') &&
it('should expose an isActive property', fakeAsync(() => { it('should expose an isActive property', fakeAsync(() => {
@Component({ @Component({
template: `<a routerLink="/team" routerLinkActive #rla="routerLinkActive"></a> template: `<a routerLink="/team" routerLinkActive #rla="routerLinkActive"></a>

View File

@ -16,7 +16,6 @@ import {RouterTestingModule} from '@angular/router/testing';
describe('Integration', () => { describe('Integration', () => {
describe('routerLinkActive', () => { describe('routerLinkActive', () => {
fixmeIvy('FW-662: Components without selector are not supported') &&
it('should not cause infinite loops in the change detection - #15825', fakeAsync(() => { it('should not cause infinite loops in the change detection - #15825', fakeAsync(() => {
@Component({selector: 'simple', template: 'simple'}) @Component({selector: 'simple', template: 'simple'})
class SimpleCmp { class SimpleCmp {
@ -59,7 +58,6 @@ describe('Integration', () => {
expect(() => advance(fixture)).not.toThrow(); expect(() => advance(fixture)).not.toThrow();
})); }));
fixmeIvy('FW-662: Components without selector are not supported') &&
it('should set isActive right after looking at its children -- #18983', fakeAsync(() => { it('should set isActive right after looking at its children -- #18983', fakeAsync(() => {
@Component({ @Component({
template: ` template: `