From d316a18dc660c2f77c39be14575481dffb6d73be Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Wed, 24 Apr 2019 11:53:12 -0700 Subject: [PATCH] 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 --- packages/compiler-cli/src/ngtsc/program.ts | 2 + .../src/ngtsc/typecheck/src/api.ts | 7 ++++ .../src/ngtsc/typecheck/src/context.ts | 6 +-- .../src/ngtsc/typecheck/src/environment.ts | 2 +- .../ngtsc/typecheck/src/type_constructor.ts | 20 ++++++---- .../typecheck/test/type_check_block_spec.ts | 2 + .../typecheck/test/type_constructor_spec.ts | 39 +++++++++++++++++++ 7 files changed, 66 insertions(+), 12 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index 57fb55efe7..ddb741b11d 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -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, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts index 63f39e9fc8..64f2046980 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/api.ts @@ -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; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index e6571e73ce..12f030c79f 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -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>, - 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); } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts index c4aa6dad6e..e14b8a777c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts @@ -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); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts index 1d523ff9ef..91f541ce90 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts @@ -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, 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, meta: TypeCtorMetadata): ts.MethodDeclaration { + node: 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`, its rawType would be `FooDirective`. @@ -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, meta: TypeCtorMetadata, - rawType: ts.TypeNode): ts.ParameterDeclaration { + node: 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([]); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index c4538dd9a7..4c5c1af190 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -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, diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts index 2258c21aef..695b5dac10 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts @@ -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'; +} \ No newline at end of file