Skip to content

Commit

Permalink
fix(compiler-cli): add useInlining option to type check config
Browse files Browse the repository at this point in the history
Use `any` type for type constructors that would require inlining.
  • Loading branch information
zarend committed Mar 4, 2021
1 parent 0e08d9a commit a0fba5a
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 22 deletions.
10 changes: 10 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/api/api.ts
Expand Up @@ -260,6 +260,16 @@ export interface TypeCheckingConfig {
* literals are cast to `any` when declared.
*/
strictLiteralTypes: boolean;

/**
* Determines if inline type contructors are available.
*
* If true, uses inline type constuctors when required.
*
* If false, never use inline type constructors. If a type constructor would normally require
* inlining, use `any` type instead to avoid inlining.
*/
useInlineTypeConstructors: boolean;
}


Expand Down
11 changes: 1 addition & 10 deletions packages/compiler-cli/src/ngtsc/typecheck/src/context.ts
Expand Up @@ -219,10 +219,6 @@ export class TypeCheckContextImpl implements TypeCheckContext {
...this.getTemplateDiagnostics(parseErrors, templateId, sourceMapping));
}

// Accumulate a list of any directives which could not have type constructors generated due to
// unsupported inlining operations.
let missingInlines: ClassDeclaration[] = [];

const boundTarget = binder.bind({template});

// Get all of the directives used in the template and record type constructors for all of them.
Expand All @@ -232,7 +228,6 @@ export class TypeCheckContextImpl implements TypeCheckContext {

if (dir.isGeneric && requiresInlineTypeCtor(dirNode, this.reflector)) {
if (this.inlining === InliningMode.Error) {
missingInlines.push(dirNode);
continue;
}
// Add a type constructor operation for the directive.
Expand Down Expand Up @@ -262,7 +257,7 @@ export class TypeCheckContextImpl implements TypeCheckContext {

// If inlining is not supported, but is required for either the TCB or one of its directive
// dependencies, then exit here with an error.
if (this.inlining === InliningMode.Error && (tcbRequiresInline || missingInlines.length > 0)) {
if (this.inlining === InliningMode.Error && tcbRequiresInline) {
// This template cannot be supported because the underlying strategy does not support inlining
// and inlining would be required.

Expand All @@ -271,10 +266,6 @@ export class TypeCheckContextImpl implements TypeCheckContext {
shimData.oobRecorder.requiresInlineTcb(templateId, ref.node);
}

if (missingInlines.length > 0) {
shimData.oobRecorder.requiresInlineTypeConstructors(templateId, ref.node, missingInlines);
}

// Checking this template would be unsupported, so don't try.
return;
}
Expand Down
Expand Up @@ -43,7 +43,7 @@ export class Environment {

constructor(
readonly config: TypeCheckingConfig, protected importManager: ImportManager,
private refEmitter: ReferenceEmitter, private reflector: ReflectionHost,
private refEmitter: ReferenceEmitter, readonly reflector: ReflectionHost,
protected contextFile: ts.SourceFile) {}

/**
Expand Down
67 changes: 63 additions & 4 deletions packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
Expand Up @@ -11,7 +11,7 @@ import * as ts from 'typescript';

import {Reference} from '../../imports';
import {ClassPropertyName} from '../../metadata';
import {ClassDeclaration} from '../../reflection';
import {ClassDeclaration, ReflectionHost} from '../../reflection';
import {TemplateId, TypeCheckableDirectiveMeta, TypeCheckBlockMetadata} from '../api';

import {addExpressionIdentifier, ExpressionIdentifier, markIgnoreDiagnostics} from './comments';
Expand All @@ -21,7 +21,8 @@ import {Environment} from './environment';
import {astToTypescript, NULL_AS_ANY} from './expression';
import {OutOfBandDiagnosticRecorder} from './oob';
import {ExpressionSemanticVisitor} from './template_semantics';
import {tsCallMethod, tsCastToAny, tsCreateElement, tsCreateTypeQueryForCoercedInput, tsCreateVariable, tsDeclareVariable} from './ts_util';
import {checkIfGenericTypesAreUnbound, tsCallMethod, tsCastToAny, tsCreateElement, tsCreateTypeQueryForCoercedInput, tsCreateVariable, tsDeclareVariable} from './ts_util';
import {requiresInlineTypeCtor} from './type_constructor';

/**
* Given a `ts.ClassDeclaration` for a component, and metadata regarding that component, compose a
Expand Down Expand Up @@ -390,6 +391,51 @@ class TcbDirectiveTypeOp extends TcbOp {
}
}

/**
* A `TcbOp` which constructs an instance of a directive _without_ setting any of its inputs. Inputs
* are later set in the `TcbDirectiveInputsOp`. Type checking was found to be faster when done in
* this way as opposed to `TcbDirectiveCtorOp` which is only necessary when the directive is
* generic.
*
* Executing this operation returns a reference to the directive instance variable with its generic
* type parameters set to `any`.
*/
class TcbDirectiveTypeOpWithGenericParams extends TcbOp {
constructor(
private tcb: Context, private scope: Scope, private node: TmplAstTemplate|TmplAstElement,
private dir: TypeCheckableDirectiveMeta) {
super();
}

get optional() {
// 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 {
const dirRef = this.dir.ref as Reference<ClassDeclaration<ts.ClassDeclaration>>;
const id = this.tcb.allocateId();

const rawType = this.tcb.env.referenceType(this.dir.ref);
if (!ts.isTypeReferenceNode(rawType)) {
throw new Error(`Expected TypeReferenceNode when referencing the ctx param for ${
this.dir.ref.debugName}`);
}

const typeArguments =
dirRef.node.typeParameters!.map(() => ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword));

const type = ts.createTypeReferenceNode(rawType.typeName, typeArguments);

addExpressionIdentifier(type, ExpressionIdentifier.DIRECTIVE);
addParseSpanInfo(type, this.node.startSourceSpan || this.node.sourceSpan);
this.scope.addStatement(tsDeclareVariable(id, type));
return id;
}
}

/**
* A `TcbOp` which creates a variable for a local ref in a template.
* The initializer for the variable is the variable expression for the directive, template, or
Expand Down Expand Up @@ -1383,8 +1429,21 @@ class Scope {

const dirMap = new Map<TypeCheckableDirectiveMeta, number>();
for (const dir of directives) {
const directiveOp = dir.isGeneric ? new TcbDirectiveCtorOp(this.tcb, this, node, dir) :
new TcbDirectiveTypeOp(this.tcb, this, node, dir);
let directiveOp: TcbOp;
const host = this.tcb.env.reflector;
const dirRef = dir.ref as Reference<ClassDeclaration<ts.ClassDeclaration>>;

if (dir.isGeneric) {
if (requiresInlineTypeCtor(dirRef.node, host) &&
!this.tcb.env.config.useInlineTypeConstructors) {
directiveOp = new TcbDirectiveTypeOpWithGenericParams(this.tcb, this, node, dir);
} else {
directiveOp = new TcbDirectiveCtorOp(this.tcb, this, node, dir);
}
} else {
directiveOp = new TcbDirectiveTypeOp(this.tcb, this, node, dir);
}

const dirIndex = this.opQueue.push(directiveOp) - 1;
dirMap.set(dir, dirIndex);

Expand Down
27 changes: 20 additions & 7 deletions packages/compiler-cli/src/ngtsc/typecheck/test/test_utils.ts
Expand Up @@ -188,6 +188,7 @@ export const ALL_ENABLED_CONFIG: TypeCheckingConfig = {
useContextGenericType: true,
strictLiteralTypes: true,
enableTemplateTypeChecker: false,
useInlineTypeConstructors: true
};

// Remove 'ref' from TypeCheckableDirectiveMeta and add a 'selector' instead.
Expand All @@ -208,17 +209,27 @@ export type TestPipe = {
file?: AbsoluteFsPath, pipeName: string, type: 'pipe',
};

export type TestDeclaration = TestDirective|TestPipe;
export type TestBase = {
code?: string;
}

export type TestDeclaration = (TestDirective|TestPipe)&TestBase;

export function tcb(
template: string, declarations: TestDeclaration[] = [], config?: TypeCheckingConfig,
template: string, declarations: TestDeclaration[] = [], config?: Partial<TypeCheckingConfig>,
options?: {emitSpans?: boolean}): string {
const classes = ['Test', ...declarations.map(decl => decl.name)];
const code = classes.map(name => `export class ${name}<T extends string> {}`).join('\n');
const codeLines = [`export class Test<T extends string> {}`];

for (const decl of declarations) {
if (decl.code !== undefined) {
codeLines.push(decl.code);
} else {
codeLines.push(`export class ${decl.name}<T extends string> {}`);
}
}
const rootFilePath = absoluteFrom('/synthetic.ts');
const {program, host} = makeProgram([
{name: rootFilePath, contents: code, isRoot: true},
{name: rootFilePath, contents: codeLines.join('\n'), isRoot: true},
]);

const sf = getSourceFileOrError(program, rootFilePath);
Expand All @@ -233,7 +244,7 @@ export function tcb(
const id = 'tcb' as TemplateId;
const meta: TypeCheckBlockMetadata = {id, boundTarget, pipes, schemas: []};

config = config || {
const fullConfig: TypeCheckingConfig = {
applyTemplateContextGuards: true,
checkQueries: false,
checkTypeOfInputBindings: true,
Expand All @@ -253,6 +264,8 @@ export function tcb(
useContextGenericType: true,
strictLiteralTypes: true,
enableTemplateTypeChecker: false,
useInlineTypeConstructors: true,
...config
};
options = options || {
emitSpans: false,
Expand All @@ -265,7 +278,7 @@ export function tcb(
const refEmmiter: ReferenceEmitter = new ReferenceEmitter(
[new LocalIdentifierStrategy(), new RelativePathStrategy(reflectionHost)]);

const env = new TypeCheckFile(fileName, config, refEmmiter, reflectionHost, host);
const env = new TypeCheckFile(fileName, fullConfig, refEmmiter, reflectionHost, host);

const ref = new Reference(clazz)

Expand Down
Expand Up @@ -745,6 +745,7 @@ describe('type check blocks', () => {
useContextGenericType: true,
strictLiteralTypes: true,
enableTemplateTypeChecker: false,
useInlineTypeConstructors: true
};

describe('config.applyTemplateContextGuards', () => {
Expand Down Expand Up @@ -1077,4 +1078,36 @@ describe('type check blocks', () => {
});
});
});

it('should use `any` type for type constructors with bound generic params ' +
'when `useInlineTypeConstructors` is `false`',
() => {
const template = `
<div dir
[inputA]='foo'
[inputB]='bar'
></div>
`;
const declarations: TestDeclaration[] = [{
code: `
interface PrivateInterface{}; export class Dir<T extends PrivateInterface, U extends string> {};
`,
type: 'directive',
name: 'Dir',
selector: '[dir]',
inputs: {
inputA: 'inputA',
inputB: 'inputB',
},
isGeneric: true
}];

const config: Partial<TypeCheckingConfig> = {useInlineTypeConstructors: false};

const renderedTcb = tcb(template, declarations, config);

expect(renderedTcb).toContain(`var _t1: i0.Dir<any, any> = (null!);`);
expect(renderedTcb).toContain(`_t1.inputA = (((ctx).foo));`);
expect(renderedTcb).toContain(`_t1.inputB = (((ctx).bar));`);
});
});

0 comments on commit a0fba5a

Please sign in to comment.