Skip to content

Commit

Permalink
fix(compiler-cli): properly emit literal types when recreating type p…
Browse files Browse the repository at this point in the history
…arameters in a different file

In #42492 the template type checker became capable of replicating a
wider range of generic type parameters for use in template type-check
files. Any literal types within a type parameter would however emit
invalid code, as TypeScript was emitting the literals using the text as
extracted from the template type-check file instead of the original
source file where the type node was taken from.

This commit works around the issue by cloning any literal types and
marking them as synthetic, signalling to TypeScript that the literal
text has to be extracted from the node itself instead from the source
file.

This commit also excludes `import()` type nodes from being supported,
as their module specifier may potentially need to be rewritten.

Fixes #42667
  • Loading branch information
JoostK committed Jul 3, 2021
1 parent c9b47f2 commit 81e0069
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 5 deletions.
30 changes: 25 additions & 5 deletions packages/compiler-cli/src/ngtsc/typecheck/src/type_emitter.ts
Expand Up @@ -46,12 +46,18 @@ export function canEmitType(type: ts.TypeNode, resolver: TypeReferenceResolver):
}

// To determine whether a type can be emitted, we have to recursively look through all type nodes.
// If a type reference node is found at any position within the type and that type reference
// cannot be emitted, then the `INELIGIBLE` constant is returned to stop the recursive walk as
// the type as a whole cannot be emitted in that case. Otherwise, the result of visiting all child
// nodes determines the result. If no ineligible type reference node is found then the walk
// returns `undefined`, indicating that no type node was visited that could not be emitted.
// If an unsupported type node is found at any position within the type and, then the `INELIGIBLE`
// constant is returned to stop the recursive walk as the type as a whole cannot be emitted in
// that case. Otherwise, the result of visiting all child nodes determines the result. If no
// ineligible type reference node is found then the walk returns `undefined`, indicating that
// no type node was visited that could not be emitted.
function visitNode(node: ts.Node): INELIGIBLE|undefined {
// `import('module')` type nodes are not supported, as it may require rewriting the module
// specifier which is currently not done.
if (ts.isImportTypeNode(node)) {
return INELIGIBLE;
}

// Emitting a type reference node in a different context requires that an import for the type
// can be created. If a type reference node cannot be emitted, `INELIGIBLE` is returned to stop
// the walk.
Expand Down Expand Up @@ -130,8 +136,22 @@ export class TypeEmitter {
emitType(type: ts.TypeNode): ts.TypeNode {
const typeReferenceTransformer: ts.TransformerFactory<ts.TypeNode> = context => {
const visitNode = (node: ts.Node): ts.Node => {
if (ts.isImportTypeNode(node)) {
throw new Error('Unable to emit import type');
}

if (ts.isTypeReferenceNode(node)) {
return this.emitTypeReference(node);
} else if (ts.isLiteralExpression(node)) {
// TypeScript would typically take the emit text for a literal expression from the source
// file itself. As the type node is being emitted into a different file, however,
// TypeScript would extract the literal text from the wrong source file. To mitigate this
// issue the literal is cloned and explicitly marked as synthesized by setting its text
// range to a negative range, forcing TypeScript to determine the node's literal text from
// the synthesized node's text instead of the incorrect source file.
const clone = ts.getMutableClone(node);
ts.setTextRange(clone, {pos: -1, end: -1});
return clone;
} else {
return ts.visitEachChild(node, visitNode, context);
}
Expand Down
Expand Up @@ -86,6 +86,28 @@ runInEachFileSystem(() => {
.toEqual('<T extends {\n [key: string]: boolean;\n}>');
});

it('can emit literal types', () => {
expect(emit(createEmitter(`export class TestClass<T extends 'a"a'> {}`)))
.toEqual(`<T extends "a\\"a">`);
expect(emit(createEmitter(`export class TestClass<T extends "b\\\"b"> {}`)))
.toEqual(`<T extends "b\\"b">`);
expect(emit(createEmitter(`export class TestClass<T extends \`c\\\`c\`> {}`)))
.toEqual(`<T extends \`c\\\`c\`>`);
expect(emit(createEmitter(`export class TestClass<T extends -1> {}`)))
.toEqual(`<T extends -1>`);
expect(emit(createEmitter(`export class TestClass<T extends 1> {}`)))
.toEqual(`<T extends 1>`);
expect(emit(createEmitter(`export class TestClass<T extends 1n> {}`)))
.toEqual(`<T extends 1n>`);
});

it('cannot emit import types', () => {
const emitter = createEmitter(`export class TestClass<T extends import('module')> {}`);

expect(emitter.canEmit()).toBe(false);
expect(() => emit(emitter)).toThrowError('Unable to emit import type');
});

it('can emit references into external modules', () => {
const emitter = createEmitter(`
import {NgIterable} from '@angular/core';
Expand Down

0 comments on commit 81e0069

Please sign in to comment.