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

perf(ivy): avoid generating selectors array for directives without a selector (#33431) 

Now that we've replaced `ngBaseDef` with an abstract directive definition, there are a lot more cases where we generate a directive definition without a selector. These changes make it so that we don't generate the `selectors` array if it's going to be empty.

PR Close #33431
This commit is contained in:
crisbeto 2019-10-27 10:59:23 +01:00 committed by Andrew Kushnir
parent d1246a1d10
commit c3e93564d0
7 changed files with 45 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts
View File

@ -310,7 +310,7 @@ A.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: A, selectors: [["", "a", ""]] });
expect(addDefinitionsSpy.calls.first().args[2]) expect(addDefinitionsSpy.calls.first().args[2])
.toEqual( .toEqual(
`UndecoratedBase.ɵfac = function UndecoratedBase_Factory(t) { return new (t || UndecoratedBase)(); }; `UndecoratedBase.ɵfac = function UndecoratedBase_Factory(t) { return new (t || UndecoratedBase)(); };
UndecoratedBase.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: UndecoratedBase, selectors: [], viewQuery: function UndecoratedBase_Query(rf, ctx) { if (rf & 1) { UndecoratedBase.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: UndecoratedBase, viewQuery: function UndecoratedBase_Query(rf, ctx) { if (rf & 1) {
ɵngcc0.ɵɵstaticViewQuery(_c0, true); ɵngcc0.ɵɵstaticViewQuery(_c0, true);
} if (rf & 2) { } if (rf & 2) {
var _t; var _t;

57
packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts
View File

@ -2958,6 +2958,28 @@ describe('compiler compliance', () => {
expect(() => compile(files, angularFiles)).not.toThrow(); expect(() => compile(files, angularFiles)).not.toThrow();
}); });
it('should not generate a selectors array if the directive does not have a selector', () => {
const files = {
app: {
'spec.ts': `
import {Directive} from '@angular/core';
@Directive()
export class AbstractDirective {
}
`
}
};
const expectedOutput = `
// ...
AbstractDirective.ɵdir = $r3$.ɵɵdefineDirective({
type: AbstractDirective
});
// ...
`;
const result = compile(files, angularFiles);
expectEmit(result.source, expectedOutput, 'Invalid directive definition');
});
}); });
@ -3004,7 +3026,6 @@ describe('compiler compliance', () => {
// ... // ...
BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ BaseClass.ɵdir = $r3$.ɵɵdefineDirective({
type: BaseClass, type: BaseClass,
selectors: [],
inputs: { inputs: {
input1: "input1", input1: "input1",
input2: ["alias2", "input2"] input2: ["alias2", "input2"]
@ -3013,7 +3034,7 @@ describe('compiler compliance', () => {
// ... // ...
`; `;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
expectEmit(result.source, expectedOutput, 'Invalid base definition'); expectEmit(result.source, expectedOutput, 'Invalid directive definition');
}); });
it('should add an abstract directive if one or more @Output is present', () => { it('should add an abstract directive if one or more @Output is present', () => {
@ -3052,7 +3073,6 @@ describe('compiler compliance', () => {
// ... // ...
BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ BaseClass.ɵdir = $r3$.ɵɵdefineDirective({
type: BaseClass, type: BaseClass,
selectors: [],
outputs: { outputs: {
output1: "output1", output1: "output1",
output2: "output2" output2: "output2"
@ -3061,7 +3081,7 @@ describe('compiler compliance', () => {
// ... // ...
`; `;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
expectEmit(result.source, expectedOutput, 'Invalid base definition'); expectEmit(result.source, expectedOutput, 'Invalid directive definition');
}); });
it('should add an abstract directive if a mixture of @Input and @Output props are present', it('should add an abstract directive if a mixture of @Input and @Output props are present',
@ -3107,7 +3127,6 @@ describe('compiler compliance', () => {
// ... // ...
BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ BaseClass.ɵdir = $r3$.ɵɵdefineDirective({
type: BaseClass, type: BaseClass,
selectors: [],
inputs: { inputs: {
input1: "input1", input1: "input1",
input2: ["whatever", "input2"] input2: ["whatever", "input2"]
@ -3120,7 +3139,7 @@ describe('compiler compliance', () => {
// ... // ...
`; `;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
expectEmit(result.source, expectedOutput, 'Invalid base definition'); expectEmit(result.source, expectedOutput, 'Invalid directive definition');
}); });
it('should add an abstract directive if a ViewChild query is present', () => { it('should add an abstract directive if a ViewChild query is present', () => {
@ -3151,7 +3170,6 @@ describe('compiler compliance', () => {
// ... // ...
BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ BaseClass.ɵdir = $r3$.ɵɵdefineDirective({
type: BaseClass, type: BaseClass,
selectors: [],
viewQuery: function BaseClass_Query(rf, ctx) { viewQuery: function BaseClass_Query(rf, ctx) {
if (rf & 1) { if (rf & 1) {
$r3$.ɵɵviewQuery($e0_attrs$, true); $r3$.ɵɵviewQuery($e0_attrs$, true);
@ -3165,7 +3183,7 @@ describe('compiler compliance', () => {
// ... // ...
`; `;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
expectEmit(result.source, expectedOutput, 'Invalid base definition'); expectEmit(result.source, expectedOutput, 'Invalid directive definition');
}); });
it('should add an abstract directive if a ViewChildren query is present', () => { it('should add an abstract directive if a ViewChildren query is present', () => {
@ -3198,7 +3216,6 @@ describe('compiler compliance', () => {
// ... // ...
BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ BaseClass.ɵdir = $r3$.ɵɵdefineDirective({
type: BaseClass, type: BaseClass,
selectors: [],
viewQuery: function BaseClass_Query(rf, ctx) { viewQuery: function BaseClass_Query(rf, ctx) {
if (rf & 1) { if (rf & 1) {
$r3$.ɵɵviewQuery(SomeDirective, true); $r3$.ɵɵviewQuery(SomeDirective, true);
@ -3212,7 +3229,7 @@ describe('compiler compliance', () => {
// ... // ...
`; `;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
expectEmit(result.source, expectedOutput, 'Invalid base definition'); expectEmit(result.source, expectedOutput, 'Invalid directive definition');
}); });
it('should add an abstract directive if a ContentChild query is present', () => { it('should add an abstract directive if a ContentChild query is present', () => {
@ -3243,7 +3260,6 @@ describe('compiler compliance', () => {
// ... // ...
BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ BaseClass.ɵdir = $r3$.ɵɵdefineDirective({
type: BaseClass, type: BaseClass,
selectors: [],
contentQueries: function BaseClass_ContentQueries(rf, ctx, dirIndex) { contentQueries: function BaseClass_ContentQueries(rf, ctx, dirIndex) {
if (rf & 1) { if (rf & 1) {
$r3$.ɵɵcontentQuery(dirIndex, $e0_attrs$, true); $r3$.ɵɵcontentQuery(dirIndex, $e0_attrs$, true);
@ -3257,7 +3273,7 @@ describe('compiler compliance', () => {
// ... // ...
`; `;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
expectEmit(result.source, expectedOutput, 'Invalid base definition'); expectEmit(result.source, expectedOutput, 'Invalid directive definition');
}); });
it('should add an abstract directive if a ContentChildren query is present', () => { it('should add an abstract directive if a ContentChildren query is present', () => {
@ -3290,7 +3306,6 @@ describe('compiler compliance', () => {
// ... // ...
BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ BaseClass.ɵdir = $r3$.ɵɵdefineDirective({
type: BaseClass, type: BaseClass,
selectors: [],
contentQueries: function BaseClass_ContentQueries(rf, ctx, dirIndex) { contentQueries: function BaseClass_ContentQueries(rf, ctx, dirIndex) {
if (rf & 1) { if (rf & 1) {
$r3$.ɵɵcontentQuery(dirIndex, SomeDirective, false); $r3$.ɵɵcontentQuery(dirIndex, SomeDirective, false);
@ -3304,7 +3319,7 @@ describe('compiler compliance', () => {
// ... // ...
`; `;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
expectEmit(result.source, expectedOutput, 'Invalid base definition'); expectEmit(result.source, expectedOutput, 'Invalid directive definition');
}); });
it('should add an abstract directive if a host binding is present', () => { it('should add an abstract directive if a host binding is present', () => {
@ -3335,7 +3350,6 @@ describe('compiler compliance', () => {
// ... // ...
BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ BaseClass.ɵdir = $r3$.ɵɵdefineDirective({
type: BaseClass, type: BaseClass,
selectors: [],
hostBindings: function BaseClass_HostBindings(rf, ctx, elIndex) { hostBindings: function BaseClass_HostBindings(rf, ctx, elIndex) {
if (rf & 1) { if (rf & 1) {
$r3$.ɵɵallocHostVars(1); $r3$.ɵɵallocHostVars(1);
@ -3348,7 +3362,7 @@ describe('compiler compliance', () => {
// ... // ...
`; `;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
expectEmit(result.source, expectedOutput, 'Invalid base definition'); expectEmit(result.source, expectedOutput, 'Invalid directive definition');
}); });
it('should add an abstract directive if a host listener is present', () => { it('should add an abstract directive if a host listener is present', () => {
@ -3379,7 +3393,6 @@ describe('compiler compliance', () => {
// ... // ...
BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ BaseClass.ɵdir = $r3$.ɵɵdefineDirective({
type: BaseClass, type: BaseClass,
selectors: [],
hostBindings: function BaseClass_HostBindings(rf, ctx, elIndex) { hostBindings: function BaseClass_HostBindings(rf, ctx, elIndex) {
if (rf & 1) { if (rf & 1) {
$r3$.ɵɵlistener("mousedown", function BaseClass_mousedown_HostBindingHandler($event) { $r3$.ɵɵlistener("mousedown", function BaseClass_mousedown_HostBindingHandler($event) {
@ -3391,7 +3404,7 @@ describe('compiler compliance', () => {
// ... // ...
`; `;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
expectEmit(result.source, expectedOutput, 'Invalid base definition'); expectEmit(result.source, expectedOutput, 'Invalid directive definition');
}); });
it('should add an abstract directive when using any lifecycle hook', () => { it('should add an abstract directive when using any lifecycle hook', () => {
@ -3420,13 +3433,12 @@ describe('compiler compliance', () => {
const expectedOutput = ` const expectedOutput = `
// ... // ...
BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ BaseClass.ɵdir = $r3$.ɵɵdefineDirective({
type: BaseClass, type: BaseClass
selectors: []
}); });
// ... // ...
`; `;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
expectEmit(result.source, expectedOutput, 'Invalid base definition'); expectEmit(result.source, expectedOutput, 'Invalid directive definition');
}); });
@ -3457,13 +3469,12 @@ describe('compiler compliance', () => {
// ... // ...
BaseClass.ɵdir = $r3$.ɵɵdefineDirective({ BaseClass.ɵdir = $r3$.ɵɵdefineDirective({
type: BaseClass, type: BaseClass,
selectors: [],
features: [$r3$.ɵɵNgOnChangesFeature()] features: [$r3$.ɵɵNgOnChangesFeature()]
}); });
// ... // ...
`; `;
const result = compile(files, angularFiles); const result = compile(files, angularFiles);
expectEmit(result.source, expectedOutput, 'Invalid base definition'); expectEmit(result.source, expectedOutput, 'Invalid directive definition');
}); });
it('should NOT add an abstract directive if @Component is present', () => { it('should NOT add an abstract directive if @Component is present', () => {

3
packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
View File

@ -516,8 +516,7 @@ runInEachFileSystem(os => {
const jsContents = env.getContents('test.js'); const jsContents = env.getContents('test.js');
expect(jsContents) expect(jsContents)
.toContain( .toContain('i0.ɵɵdefineDirective({ type: TestBase, inputs: { input: "input" } });');
'i0.ɵɵdefineDirective({ type: TestBase, selectors: [], inputs: { input: "input" } });');
const dtsContents = env.getContents('test.d.ts'); const dtsContents = env.getContents('test.d.ts');
expect(dtsContents) expect(dtsContents)

14
packages/compiler/src/render3/view/compiler.ts
View File

@ -38,20 +38,19 @@ const EMPTY_ARRAY: any[] = [];
// If there is a match, the first matching group will contain the attribute name to bind. // If there is a match, the first matching group will contain the attribute name to bind.
const ATTR_REGEX = /attr\.([^\]]+)/; const ATTR_REGEX = /attr\.([^\]]+)/;
function getStylingPrefix(name: string): string {
return name.substring(0, 5); // style or class
}
function baseDirectiveFields( function baseDirectiveFields(
meta: R3DirectiveMetadata, constantPool: ConstantPool, meta: R3DirectiveMetadata, constantPool: ConstantPool,
bindingParser: BindingParser): DefinitionMap { bindingParser: BindingParser): DefinitionMap {
const definitionMap = new DefinitionMap(); const definitionMap = new DefinitionMap();
const selectors = core.parseSelectorToR3Selector(meta.selector);
// e.g. `type: MyDirective` // e.g. `type: MyDirective`
definitionMap.set('type', meta.type); definitionMap.set('type', meta.type);
// e.g. `selectors: [['', 'someDir', '']]` // e.g. `selectors: [['', 'someDir', '']]`
definitionMap.set('selectors', createDirectiveSelector(meta.selector)); if (selectors.length > 0) {
definitionMap.set('selectors', asLiteral(selectors));
}
if (meta.queries.length > 0) { if (meta.queries.length > 0) {
// e.g. `contentQueries: (rf, ctx, dirIndex) => { ... } // e.g. `contentQueries: (rf, ctx, dirIndex) => { ... }
@ -406,11 +405,6 @@ function prepareQueryParams(query: R3QueryMetadata, constantPool: ConstantPool):
return parameters; return parameters;
} }
// Turn a directive selector into an R3-compatible selector for directive def
function createDirectiveSelector(selector: string | null): o.Expression {
return asLiteral(core.parseSelectorToR3Selector(selector));
}
function convertAttributesToExpressions(attributes: {[name: string]: o.Expression}): function convertAttributesToExpressions(attributes: {[name: string]: o.Expression}):
o.Expression[] { o.Expression[] {
const values: o.Expression[] = []; const values: o.Expression[] = [];

6
packages/core/src/render3/definition.ts
View File

@ -48,7 +48,7 @@ export function ɵɵdefineComponent<T>(componentDefinition: {
type: Type<T>; type: Type<T>;
/** The selectors that will be used to match nodes to this component. */ /** The selectors that will be used to match nodes to this component. */
selectors: CssSelectorList; selectors?: CssSelectorList;
/** /**
* The number of nodes, local refs, and pipes in this component template. * The number of nodes, local refs, and pipes in this component template.
@ -275,7 +275,7 @@ export function ɵɵdefineComponent<T>(componentDefinition: {
onPush: componentDefinition.changeDetection === ChangeDetectionStrategy.OnPush, onPush: componentDefinition.changeDetection === ChangeDetectionStrategy.OnPush,
directiveDefs: null !, // assigned in noSideEffects directiveDefs: null !, // assigned in noSideEffects
pipeDefs: null !, // assigned in noSideEffects pipeDefs: null !, // assigned in noSideEffects
selectors: componentDefinition.selectors, selectors: componentDefinition.selectors || EMPTY_ARRAY,
viewQuery: componentDefinition.viewQuery || null, viewQuery: componentDefinition.viewQuery || null,
features: componentDefinition.features as DirectiveDefFeature[] || null, features: componentDefinition.features as DirectiveDefFeature[] || null,
data: componentDefinition.data || {}, data: componentDefinition.data || {},
@ -507,7 +507,7 @@ export const ɵɵdefineDirective = ɵɵdefineComponent as any as<T>(directiveDef
type: Type<T>; type: Type<T>;
/** The selectors that will be used to match nodes to this directive. */ /** The selectors that will be used to match nodes to this directive. */
selectors: CssSelectorList; selectors?: CssSelectorList;
/** /**
* A map of input names. * A map of input names.

1
packages/core/test/render3/render_util.ts
View File

@ -261,7 +261,6 @@ export function renderTemplate<T>(
enterView(hostLView, null); enterView(hostLView, null);
const def: ComponentDef<any> = ɵɵdefineComponent({ const def: ComponentDef<any> = ɵɵdefineComponent({
selectors: [],
type: Object, type: Object,
template: templateFn, template: templateFn,
decls: decls, decls: decls,

4
tools/public_api_guard/core/core.d.ts vendored
View File

@ -747,7 +747,7 @@ export declare const ɵɵdefaultStyleSanitizer: StyleSanitizeFn;
export declare function ɵɵdefineComponent<T>(componentDefinition: { export declare function ɵɵdefineComponent<T>(componentDefinition: {
type: Type<T>; type: Type<T>;
selectors: CssSelectorList; selectors?: CssSelectorList;
decls: number; decls: number;
vars: number; vars: number;
inputs?: { inputs?: {
@ -777,7 +777,7 @@ export declare function ɵɵdefineComponent<T>(componentDefinition: {
export declare const ɵɵdefineDirective: <T>(directiveDefinition: { export declare const ɵɵdefineDirective: <T>(directiveDefinition: {
type: Type<T>; type: Type<T>;
selectors: (string | SelectorFlags)[][]; selectors?: (string | SelectorFlags)[][] | undefined;
inputs?: { [P in keyof T]?: string | [string, string] | undefined; } | undefined; inputs?: { [P in keyof T]?: string | [string, string] | undefined; } | undefined;
outputs?: { [P_1 in keyof T]?: string | undefined; } | undefined; outputs?: { [P_1 in keyof T]?: string | undefined; } | undefined;
features?: DirectiveDefFeature[] | undefined; features?: DirectiveDefFeature[] | undefined;