From 729eea57165f8931f3d2ea7b7c09d6c094968ad4 Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 5 Jun 2021 22:22:34 +0200 Subject: [PATCH] fix(compiler-cli): transform type references in generic type parameter default (#42492) When a component/directive has a generic type parameter, the template type checker attempts to translate the type parameter such that the type parameters can be replicated in the type constructor that is emitted into the typecheck file. Type parameters with a default clause would incorrectly be emitted into the typecheck file using the original `ts.TypeNode` for the default clause, such that `ts.TypeReferenceNode`s within the default clause would likely be invalid (i.e. referencing a type for which no import is present in the typecheck file). This did not result in user-facing type-check errors as errors reported in type constructors are not translated into template positions Regardless, this commit ensures that `ts.TypeReferenceNode`s within defaults are properly translated into the typecheck file. PR Close #42492 --- .../typecheck/src/type_parameter_emitter.ts | 18 ++++++++----- .../test/type_parameter_emitter_spec.ts | 27 +++++++++++++++++++ 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_parameter_emitter.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_parameter_emitter.ts index c973742cea..018b36a3f8 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_parameter_emitter.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_parameter_emitter.ts @@ -32,14 +32,18 @@ export class TypeParameterEmitter { } return this.typeParameters.every(typeParam => { - if (typeParam.constraint === undefined) { - return true; - } - - return canEmitType(typeParam.constraint, type => this.resolveTypeReference(type)); + return this.canEmitType(typeParam.constraint) && this.canEmitType(typeParam.default); }); } + private canEmitType(type: ts.TypeNode|undefined): boolean { + if (type === undefined) { + return true; + } + + return canEmitType(type, typeReference => this.resolveTypeReference(typeReference)); + } + /** * Emits the type parameters using the provided emitter function for `Reference`s. */ @@ -53,12 +57,14 @@ export class TypeParameterEmitter { return this.typeParameters.map(typeParam => { const constraint = typeParam.constraint !== undefined ? emitter.emitType(typeParam.constraint) : undefined; + const defaultType = + typeParam.default !== undefined ? emitter.emitType(typeParam.default) : undefined; return ts.updateTypeParameterDeclaration( /* node */ typeParam, /* name */ typeParam.name, /* constraint */ constraint, - /* defaultType */ typeParam.default); + /* defaultType */ defaultType); }); } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_parameter_emitter_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_parameter_emitter_spec.ts index 03d2b15b09..4857964057 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_parameter_emitter_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_parameter_emitter_spec.ts @@ -222,5 +222,32 @@ runInEachFileSystem(() => { expect(emitter.canEmit()).toBe(true); expect(emit(emitter)).toEqual(''); }); + + it('transforms generic type parameter defaults', () => { + const additionalFiles: TestFile[] = [{ + name: absoluteFrom('/node_modules/types/index.d.ts'), + contents: `export declare type MyType = string;`, + }]; + const emitter = createEmitter( + ` + import {MyType} from 'types'; + + export class TestClass {}`, + additionalFiles); + + expect(emitter.canEmit()).toBe(true); + expect(emit(emitter)).toEqual(''); + }); + + it('cannot emit when a type parameter default cannot be emitted', () => { + const emitter = createEmitter(` + interface Local {} + + export class TestClass {}`); + + expect(emitter.canEmit()).toBe(false); + expect(() => emit(emitter)) + .toThrowError('A type reference to emit must be imported from an absolute module'); + }); }); });