fix(ivy): don't include query fields in type constructors (#30094)

Previously, ngtsc included query fields in the list of fields which can
affect the type of a directive via its type constructor. This feature
however has yet to be built, and View Engine in default mode does not
do this inference.

This caused an unexpected bug where private query fields (which should be
an error but are allowed by View Engine) cause the type constructor
signature to be invalid. This commit fixes that issue by disabling the
logic to include query fields.

PR Close #30094
This commit is contained in:
Alex Rickabaugh 2019-04-24 11:53:12 -07:00 committed by Andrew Kushnir
parent 79141f4424
commit d316a18dc6
7 changed files with 66 additions and 12 deletions

View File

@ -393,6 +393,7 @@ export class NgtscProgram implements api.Program {
if (this.options.fullTemplateTypeCheck) {
typeCheckingConfig = {
applyTemplateContextGuards: true,
checkQueries: false,
checkTemplateBodies: true,
checkTypeOfBindings: true,
checkTypeOfPipes: true,
@ -401,6 +402,7 @@ export class NgtscProgram implements api.Program {
} else {
typeCheckingConfig = {
applyTemplateContextGuards: false,
checkQueries: false,
checkTemplateBodies: false,
checkTypeOfBindings: false,
checkTypeOfPipes: false,

View File

@ -95,4 +95,11 @@ export interface TypeCheckingConfig {
* Whether to descend into template bodies and check any bindings there.
*/
checkTemplateBodies: boolean;
/**
* Whether to check resolvable queries.
*
* This is currently an unsupported feature.
*/
checkQueries: false;
}

View File

@ -112,7 +112,7 @@ export class TypeCheckContext {
const ops = this.opMap.get(sf) !;
// Push a `TypeCtorOp` into the operation queue for the source file.
ops.push(new TypeCtorOp(ref, ctorMeta));
ops.push(new TypeCtorOp(ref, ctorMeta, this.config));
}
/**
@ -274,7 +274,7 @@ class TcbOp implements Op {
class TypeCtorOp implements Op {
constructor(
readonly ref: Reference<ClassDeclaration<ts.ClassDeclaration>>,
readonly meta: TypeCtorMetadata) {}
readonly meta: TypeCtorMetadata, private config: TypeCheckingConfig) {}
/**
* Type constructor operations are inserted immediately before the end of the directive class.
@ -283,7 +283,7 @@ class TypeCtorOp implements Op {
execute(im: ImportManager, sf: ts.SourceFile, refEmitter: ReferenceEmitter, printer: ts.Printer):
string {
const tcb = generateInlineTypeCtor(this.ref.node, this.meta);
const tcb = generateInlineTypeCtor(this.ref.node, this.meta, this.config);
return printer.printNode(ts.EmitHint.Unspecified, tcb, sf);
}
}

View File

@ -80,7 +80,7 @@ export class Environment {
queries: dir.queries,
}
};
const typeCtor = generateTypeCtorDeclarationFn(node, meta, nodeTypeRef.typeName);
const typeCtor = generateTypeCtorDeclarationFn(node, meta, nodeTypeRef.typeName, this.config);
this.typeCtorStatements.push(typeCtor);
const fnId = ts.createIdentifier(fnName);
this.typeCtors.set(node, fnId);

View File

@ -9,12 +9,13 @@
import * as ts from 'typescript';
import {ClassDeclaration} from '../../reflection';
import {TypeCtorMetadata} from './api';
import {TypeCheckingConfig, TypeCtorMetadata} from './api';
import {checkIfGenericTypesAreUnbound} from './ts_util';
export function generateTypeCtorDeclarationFn(
node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCtorMetadata,
nodeTypeRef: ts.Identifier | ts.QualifiedName): ts.Statement {
nodeTypeRef: ts.Identifier | ts.QualifiedName, config: TypeCheckingConfig): ts.Statement {
if (requiresInlineTypeCtor(node)) {
throw new Error(`${node.name.text} requires an inline type constructor`);
}
@ -23,7 +24,7 @@ export function generateTypeCtorDeclarationFn(
node.typeParameters !== undefined ? generateGenericArgs(node.typeParameters) : undefined;
const rawType: ts.TypeNode = ts.createTypeReferenceNode(nodeTypeRef, rawTypeArgs);
const initParam = constructTypeCtorParameter(node, meta, rawType);
const initParam = constructTypeCtorParameter(node, meta, rawType, config.checkQueries);
const typeParameters = typeParametersWithDefaultTypes(node.typeParameters);
@ -83,7 +84,8 @@ export function generateTypeCtorDeclarationFn(
* @returns a `ts.MethodDeclaration` for the type constructor.
*/
export function generateInlineTypeCtor(
node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCtorMetadata): ts.MethodDeclaration {
node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCtorMetadata,
config: TypeCheckingConfig): ts.MethodDeclaration {
// Build rawType, a `ts.TypeNode` of the class with its generic parameters passed through from
// the definition without any type bounds. For example, if the class is
// `FooDirective<T extends Bar>`, its rawType would be `FooDirective<T>`.
@ -91,7 +93,7 @@ export function generateInlineTypeCtor(
node.typeParameters !== undefined ? generateGenericArgs(node.typeParameters) : undefined;
const rawType: ts.TypeNode = ts.createTypeReferenceNode(node.name, rawTypeArgs);
const initParam = constructTypeCtorParameter(node, meta, rawType);
const initParam = constructTypeCtorParameter(node, meta, rawType, config.checkQueries);
// If this constructor is being generated into a .ts file, then it needs a fake body. The body
// is set to a return of `null!`. If the type constructor is being generated into a .d.ts file,
@ -117,8 +119,8 @@ export function generateInlineTypeCtor(
}
function constructTypeCtorParameter(
node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCtorMetadata,
rawType: ts.TypeNode): ts.ParameterDeclaration {
node: ClassDeclaration<ts.ClassDeclaration>, meta: TypeCtorMetadata, rawType: ts.TypeNode,
includeQueries: boolean): ts.ParameterDeclaration {
// initType is the type of 'init', the single argument to the type constructor method.
// If the Directive has any inputs, outputs, or queries, its initType will be:
//
@ -134,8 +136,10 @@ function constructTypeCtorParameter(
const keys: string[] = [
...meta.fields.inputs,
...meta.fields.outputs,
...meta.fields.queries,
];
if (includeQueries) {
keys.push(...meta.fields.queries);
}
if (keys.length === 0) {
// Special case - no inputs, outputs, or other fields which could influence the result type.
initType = ts.createTypeLiteralNode([]);

View File

@ -93,6 +93,7 @@ describe('type check blocks', () => {
}];
const BASE_CONFIG: TypeCheckingConfig = {
applyTemplateContextGuards: true,
checkQueries: false,
checkTemplateBodies: true,
checkTypeOfBindings: true,
checkTypeOfPipes: true,
@ -287,6 +288,7 @@ function tcb(
config = config || {
applyTemplateContextGuards: true,
checkQueries: false,
checkTypeOfBindings: true,
checkTypeOfPipes: true,
checkTemplateBodies: true,

View File

@ -27,6 +27,7 @@ const LIB_D_TS = {
const ALL_ENABLED_CONFIG: TypeCheckingConfig = {
applyTemplateContextGuards: true,
checkQueries: false,
checkTemplateBodies: true,
checkTypeOfBindings: true,
checkTypeOfPipes: true,
@ -70,5 +71,43 @@ TestClass.ngTypeCtor({value: 'test'});
});
ctx.calculateTemplateDiagnostics(program, host, options);
});
it('should not consider query fields', () => {
const files = [
LIB_D_TS, {
name: 'main.ts',
contents: `class TestClass { value: any; }`,
}
];
const {program, host, options} = makeProgram(files, undefined, undefined, false);
const checker = program.getTypeChecker();
const logicalFs = new LogicalFileSystem(getRootDirs(host, options));
const emitter = new ReferenceEmitter([
new LocalIdentifierStrategy(),
new AbsoluteModuleStrategy(program, checker, options, host),
new LogicalProjectStrategy(checker, logicalFs),
]);
const ctx = new TypeCheckContext(
ALL_ENABLED_CONFIG, emitter, AbsoluteFsPath.fromUnchecked('/_typecheck_.ts'));
const TestClass = getDeclaration(program, 'main.ts', 'TestClass', isNamedClassDeclaration);
ctx.addInlineTypeCtor(program.getSourceFile('main.ts') !, new Reference(TestClass), {
fnName: 'ngTypeCtor',
body: true,
fields: {
inputs: ['value'],
outputs: [],
queries: ['queryField'],
},
});
const res = ctx.calculateTemplateDiagnostics(program, host, options);
const TestClassWithCtor =
getDeclaration(res.program, 'main.ts', 'TestClass', isNamedClassDeclaration);
const typeCtor = TestClassWithCtor.members.find(isTypeCtor) !;
expect(typeCtor.getText()).not.toContain('queryField');
});
});
});
function isTypeCtor(el: ts.ClassElement): el is ts.MethodDeclaration {
return ts.isMethodDeclaration(el) && ts.isIdentifier(el.name) && el.name.text === 'ngTypeCtor';
}