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

perf(compiler-cli): only generate directive declarations when used (#38418) 

The template type-checker would always generate a directive declaration
even if its type was never used. For example, directives without any
input nor output bindings nor exportAs references don't need the
directive to be declared, as its type would never be used.

This commit makes the `TcbOp`s that are responsible for declaring a
directive as optional, such that they are only executed when requested
from another operation.

PR Close #38418
This commit is contained in:
JoostK 2020-08-11 23:09:05 +02:00 committed by Andrew Scott
parent f42e6ce917
commit fb8f4b4d72
3 changed files with 105 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
View File

@ -355,7 +355,10 @@ class TcbDirectiveTypeOp extends TcbOp {
}
get optional() {
return false;
// The statement generated by this operation is only used to declare the directive's type and
// won't report diagnostics by itself, so the operation is marked as optional to avoid
// generating declarations for directives that don't have any inputs/outputs.
return true;
}
execute(): ts.Identifier {
@ -387,7 +390,9 @@ class TcbDirectiveCtorOp extends TcbOp {
}
get optional() {
return false;
// The statement generated by this operation is only used to infer the directive's type and
// won't report diagnostics by itself, so the operation is marked as optional.
return true;
}
execute(): ts.Identifier {
@ -452,7 +457,7 @@ class TcbDirectiveInputsOp extends TcbOp {
}
execute(): null {
const dirId = this.scope.resolve(this.node, this.dir);
let dirId: ts.Expression|null = null;
// TODO(joost): report duplicate properties
@ -502,6 +507,10 @@ class TcbDirectiveInputsOp extends TcbOp {
// (i.e. private/protected/readonly), generate an assignment into a temporary variable
// that has the type of the field. This achieves type-checking but circumvents the access
// modifiers.
if (dirId === null) {
dirId = this.scope.resolve(this.node, this.dir);
}
const id = this.tcb.allocateId();
const dirTypeRef = this.tcb.env.referenceType(this.dir.ref);
if (!ts.isTypeReferenceNode(dirTypeRef)) {
@ -515,6 +524,10 @@ class TcbDirectiveInputsOp extends TcbOp {
this.scope.addStatement(temp);
target = id;
} else {
if (dirId === null) {
dirId = this.scope.resolve(this.node, this.dir);
}
// To get errors assign directly to the fields on the instance, using property access
// when possible. String literal fields may not be valid JS identifiers so we use
// literal element access instead for those cases.
@ -718,7 +731,8 @@ class TcbDirectiveOutputsOp extends TcbOp {
}
execute(): null {
const dirId = this.scope.resolve(this.node, this.dir);
let dirId: ts.Expression|null = null;
// `dir.outputs` is an object map of field names on the directive class to event names.
// This is backwards from what's needed to match event handlers - a map of event names to field
@ -748,6 +762,9 @@ class TcbDirectiveOutputsOp extends TcbOp {
// that has a `subscribe` method that properly carries the `T` into the handler function.
const handler = tcbCreateEventHandler(output, this.tcb, this.scope, EventParamType.Infer);
if (dirId === null) {
dirId = this.scope.resolve(this.node, this.dir);
}
const outputField = ts.createElementAccess(dirId, ts.createStringLiteral(field));
const outputHelper =
ts.createCall(this.tcb.env.declareOutputHelper(), undefined, [outputField]);

111
packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts
View File

@ -156,8 +156,7 @@ describe('type check blocks', () => {
isGeneric: true,
}];
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain(
'var _t1 = Dir.ngTypeCtor({ "color": (null as any), "strong": (null as any), "enabled": (null as any) });');
expect(block).not.toContain('Dir.ngTypeCtor');
expect(block).toContain('"blue"; false; true;');
});
@ -204,9 +203,9 @@ describe('type check blocks', () => {
];
expect(tcb(TEMPLATE, DIRECTIVES))
.toContain(
'var _t3 = DirA.ngTypeCtor((null!)); ' +
'var _t2 = DirB.ngTypeCtor({ "inputB": (_t3) }); ' +
'var _t1 = DirA.ngTypeCtor({ "inputA": (_t2) });');
'var _t3 = DirB.ngTypeCtor((null!)); ' +
'var _t2 = DirA.ngTypeCtor({ "inputA": (_t3) }); ' +
'var _t1 = DirB.ngTypeCtor({ "inputB": (_t2) });');
});
it('should handle empty bindings', () => {
@ -247,9 +246,8 @@ describe('type check blocks', () => {
}];
expect(tcb(TEMPLATE, DIRECTIVES))
.toContain(
'var _t1 = Dir.ngTypeCtor({ "fieldA": (((ctx).foo)) }); ' +
'var _t2: typeof Dir.ngAcceptInputType_fieldA = (null!); ' +
'_t2 = (((ctx).foo));');
'var _t1: typeof Dir.ngAcceptInputType_fieldA = (null!); ' +
'_t1 = (((ctx).foo));');
});
});
@ -264,6 +262,58 @@ describe('type check blocks', () => {
expect(block).toContain('(ctx).handle(_t1);');
});
it('should only generate directive declarations that have bindings or are referenced', () => {
const TEMPLATE = `
<div
hasInput [input]="value"
hasOutput (output)="handle()"
hasReference #ref="ref"
noReference
noBindings>{{ref.a}}</div>
`;
const DIRECTIVES: TestDeclaration[] = [
{
type: 'directive',
name: 'HasInput',
selector: '[hasInput]',
inputs: {input: 'input'},
},
{
type: 'directive',
name: 'HasOutput',
selector: '[hasOutput]',
outputs: {output: 'output'},
},
{
type: 'directive',
name: 'HasReference',
selector: '[hasReference]',
exportAs: ['ref'],
},
{
type: 'directive',
name: 'NoReference',
selector: '[noReference]',
exportAs: ['no-ref'],
},
{
type: 'directive',
name: 'NoBindings',
selector: '[noBindings]',
inputs: {unset: 'unset'},
},
];
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('var _t1: HasInput = (null!)');
expect(block).toContain('_t1.input = (((ctx).value));');
expect(block).toContain('var _t2: HasOutput = (null!)');
expect(block).toContain('_t2["output"]');
expect(block).toContain('var _t3: HasReference = (null!)');
expect(block).toContain('(_t3).a');
expect(block).not.toContain('NoBindings');
expect(block).not.toContain('NoReference');
});
it('should generate a forward element reference correctly', () => {
const TEMPLATE = `
{{ i.value }}
@ -310,7 +360,7 @@ describe('type check blocks', () => {
inputs: {'color': 'color', 'strong': 'strong', 'enabled': 'enabled'},
}];
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).toContain('var _t1: Dir = (null!);');
expect(block).not.toContain('var _t1: Dir = (null!);');
expect(block).not.toContain('"color"');
expect(block).not.toContain('"strong"');
expect(block).not.toContain('"enabled"');
@ -357,10 +407,10 @@ describe('type check blocks', () => {
];
expect(tcb(TEMPLATE, DIRECTIVES))
.toContain(
'var _t1: DirA = (null!); ' +
'var _t2: DirB = (null!); ' +
'_t1.inputA = (_t2); ' +
'_t2.inputA = (_t1);');
'var _t1: DirB = (null!); ' +
'var _t2: DirA = (null!); ' +
'_t2.inputA = (_t1); ' +
'_t1.inputA = (_t2);');
});
it('should handle undeclared properties', () => {
@ -374,10 +424,9 @@ describe('type check blocks', () => {
},
undeclaredInputFields: ['fieldA']
}];
expect(tcb(TEMPLATE, DIRECTIVES))
.toContain(
'var _t1: Dir = (null!); ' +
'(((ctx).foo)); ');
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).not.toContain('var _t1: Dir = (null!);');
expect(block).toContain('(((ctx).foo)); ');
});
it('should assign restricted properties to temp variables by default', () => {
@ -448,9 +497,9 @@ describe('type check blocks', () => {
}];
expect(tcb(TEMPLATE, DIRECTIVES))
.toContain(
'var _t1: Dir = (null!); ' +
'var _t2: typeof Dir.ngAcceptInputType_field1 = (null!); ' +
'_t1.field2 = _t2 = (((ctx).foo));');
'var _t1: typeof Dir.ngAcceptInputType_field1 = (null!); ' +
'var _t2: Dir = (null!); ' +
'_t2.field2 = _t1 = (((ctx).foo));');
});
it('should handle a single property bound to multiple fields, where one of them is undeclared',
@ -483,11 +532,11 @@ describe('type check blocks', () => {
},
coercedInputFields: ['fieldA'],
}];
expect(tcb(TEMPLATE, DIRECTIVES))
.toContain(
'var _t1: Dir = (null!); ' +
'var _t2: typeof Dir.ngAcceptInputType_fieldA = (null!); ' +
'_t2 = (((ctx).foo));');
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).not.toContain('var _t1: Dir = (null!);');
expect(block).toContain(
'var _t1: typeof Dir.ngAcceptInputType_fieldA = (null!); ' +
'_t1 = (((ctx).foo));');
});
it('should use coercion types if declared, even when backing field is not declared', () => {
@ -502,11 +551,11 @@ describe('type check blocks', () => {
coercedInputFields: ['fieldA'],
undeclaredInputFields: ['fieldA'],
}];
expect(tcb(TEMPLATE, DIRECTIVES))
.toContain(
'var _t1: Dir = (null!); ' +
'var _t2: typeof Dir.ngAcceptInputType_fieldA = (null!); ' +
'_t2 = (((ctx).foo));');
const block = tcb(TEMPLATE, DIRECTIVES);
expect(block).not.toContain('var _t1: Dir = (null!);');
expect(block).toContain(
'var _t1: typeof Dir.ngAcceptInputType_fieldA = (null!); ' +
'_t1 = (((ctx).foo));');
});
it('should handle $any casts', () => {
@ -721,7 +770,7 @@ describe('type check blocks', () => {
expect(block).toContain('function ($event: any): any { (ctx).foo($event); }');
// Note that DOM events are still checked, that is controlled by `checkTypeOfDomEvents`
expect(block).toContain(
'_t2.addEventListener("nonDirOutput", function ($event): any { (ctx).foo($event); });');
'_t1.addEventListener("nonDirOutput", function ($event): any { (ctx).foo($event); });');
});
});

6
packages/compiler-cli/src/ngtsc/typecheck/test/type_checker_spec.ts
View File

@ -276,11 +276,13 @@ runInEachFileSystem(os => {
selector: '[dir]',
file: dirFile,
type: 'directive',
inputs: {'input': 'input'},
isGeneric: true,
}]
},
{
fileName: dirFile,
source: `export class TestDir {}`,
source: `export class TestDir<T> {}`,
templates: {},
}
],
@ -294,7 +296,7 @@ runInEachFileSystem(os => {
const tcbReal = templateTypeChecker.getTypeCheckBlock(cmp)!;
expect(tcbReal.getSourceFile().text).not.toContain('TestDir');
templateTypeChecker.overrideComponentTemplate(cmp, '<div dir></div>');
templateTypeChecker.overrideComponentTemplate(cmp, '<div dir [input]="value"></div>');
const tcbOverridden = templateTypeChecker.getTypeCheckBlock(cmp);
expect(tcbOverridden).not.toBeNull();