Skip to content

Commit 96fd73d

Browse files
JoostKalxhub
authored andcommitted
fix(compiler-cli): properly emit literal types in input coercion function arguments (angular#52437)
This commit fixes an issue where using literal types in the arguments of an input coercion function could result in emitting invalid output, due to an assumption that TypeScript makes when emitting literal types. Specifically, it takes the literal's text from its containing source file, but this breaks when the literal type node has been transplanted into a different source file. This issue has surfaced in the type-check code generator and is already being addressed there, so this commit moves the relevant `TypeEmitter` class from the `typecheck` module to the `translator` module, such that it can be reused for emitting types in the type translator. Fixes angular#51672 PR Close angular#52437
1 parent fdd8285 commit 96fd73d

File tree

5 files changed

+44
-58
lines changed

5 files changed

+44
-58
lines changed

packages/compiler-cli/src/ngtsc/translator/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export {ImportGenerator, NamedImport} from './src/api/import_generator';
1111
export {Context} from './src/context';
1212
export {Import, ImportManager} from './src/import_manager';
1313
export {ExpressionTranslatorVisitor, RecordWrappedNodeFn, TranslatorOptions} from './src/translator';
14+
export {canEmitType, TypeEmitter, TypeReferenceTranslator} from './src/type_emitter';
1415
export {translateType} from './src/type_translator';
1516
export {attachComments, createTemplateMiddle, createTemplateTail, TypeScriptAstFactory} from './src/typescript_ast_factory';
1617
export {translateExpression, translateStatement} from './src/typescript_translator';

packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts

Lines changed: 15 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {ReflectionHost} from '../../reflection';
1414

1515
import {Context} from './context';
1616
import {ImportManager} from './import_manager';
17+
import {TypeEmitter} from './type_emitter';
1718

1819

1920
export function translateType(
@@ -79,12 +80,13 @@ class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor {
7980
return ts.factory.createTypeLiteralNode([indexSignature]);
8081
}
8182

82-
visitTransplantedType(ast: o.TransplantedType<ts.Node>, context: any) {
83+
visitTransplantedType(ast: o.TransplantedType<ts.Node>, context: Context) {
8384
if (!ts.isTypeNode(ast.type)) {
8485
throw new Error(`A TransplantedType must wrap a TypeNode`);
8586
}
8687

87-
return this.translateTransplantedTypeNode(ast.type, context);
88+
const emitter = new TypeEmitter(typeRef => this.translateTypeReference(typeRef, context));
89+
return emitter.emitType(ast.type);
8890
}
8991

9092
visitReadVarExpr(ast: o.ReadVarExpr, context: Context): ts.TypeQueryNode {
@@ -253,70 +255,28 @@ class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor {
253255
return typeNode;
254256
}
255257

256-
/**
257-
* Translates a type reference node so that all of its references
258-
* are imported into the context file.
259-
*/
260-
private translateTransplantedTypeReferenceNode(
261-
node: ts.TypeReferenceNode&{typeName: ts.Identifier}, context: any): ts.TypeReferenceNode {
262-
const declaration = this.reflector.getDeclarationOfIdentifier(node.typeName);
263-
258+
private translateTypeReference(type: ts.TypeReferenceNode, context: Context): ts.TypeReferenceNode
259+
|null {
260+
const target = ts.isIdentifier(type.typeName) ? type.typeName : type.typeName.right;
261+
const declaration = this.reflector.getDeclarationOfIdentifier(target);
264262
if (declaration === null) {
265263
throw new Error(
266-
`Unable to statically determine the declaration file of type node ${node.typeName.text}`);
264+
`Unable to statically determine the declaration file of type node ${target.text}`);
267265
}
268266

269267
const emittedType = this.refEmitter.emit(
270268
new Reference(declaration.node), this.contextFile,
271269
ImportFlags.NoAliasing | ImportFlags.AllowTypeImports |
272270
ImportFlags.AllowRelativeDtsImports);
273271

274-
assertSuccessfulReferenceEmit(emittedType, node, 'type');
275-
276-
const result = emittedType.expression.visitExpression(this, context);
272+
assertSuccessfulReferenceEmit(emittedType, target, 'type');
277273

278-
if (!ts.isTypeReferenceNode(result)) {
279-
throw new Error(`Expected TypeReferenceNode when referencing the type for ${
280-
node.typeName.text}, but received ${ts.SyntaxKind[result.kind]}`);
281-
}
274+
const typeNode = this.translateExpression(emittedType.expression, context);
282275

283-
// If the original node doesn't have any generic parameters we return the results.
284-
if (node.typeArguments === undefined || node.typeArguments.length === 0) {
285-
return result;
276+
if (!ts.isTypeReferenceNode(typeNode)) {
277+
throw new Error(
278+
`Expected TypeReferenceNode for emitted reference, got ${ts.SyntaxKind[typeNode.kind]}.`);
286279
}
287-
288-
// If there are any generics, we have to reflect them as well.
289-
const translatedArgs =
290-
node.typeArguments.map(arg => this.translateTransplantedTypeNode(arg, context));
291-
292-
return ts.factory.updateTypeReferenceNode(
293-
result, result.typeName, ts.factory.createNodeArray(translatedArgs));
294-
}
295-
296-
/**
297-
* Translates a type node so that all of the type references it
298-
* contains are imported and can be referenced in the context file.
299-
*/
300-
private translateTransplantedTypeNode(rootNode: ts.TypeNode, context: any): ts.TypeNode {
301-
const factory: ts.TransformerFactory<ts.Node> = transformContext => root => {
302-
const walk = (node: ts.Node): ts.Node => {
303-
if (ts.isTypeReferenceNode(node) && ts.isIdentifier(node.typeName)) {
304-
const translated =
305-
this.translateTransplantedTypeReferenceNode(node as ts.TypeReferenceNode & {
306-
typeName: ts.Identifier;
307-
}, context);
308-
309-
if (translated !== node) {
310-
return translated;
311-
}
312-
}
313-
314-
return ts.visitEachChild(node, walk, transformContext);
315-
};
316-
317-
return ts.visitNode(root, walk);
318-
};
319-
320-
return ts.transform(rootNode, [factory]).transformed[0] as ts.TypeNode;
280+
return typeNode;
321281
}
322282
}

packages/compiler-cli/src/ngtsc/typecheck/src/type_parameter_emitter.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@ import ts from 'typescript';
99

1010
import {OwningModule, Reference} from '../../imports';
1111
import {DeclarationNode, ReflectionHost} from '../../reflection';
12-
13-
import {canEmitType, TypeEmitter} from './type_emitter';
14-
12+
import {canEmitType, TypeEmitter} from '../../translator';
1513

1614
/**
1715
* See `TypeEmitter` for more information on the emitting process.

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8748,6 +8748,33 @@ function allTests(os: string) {
87488748
expect(dtsContents).toContain('static ngAcceptInputType_value: i1.ExternalToNumberType;');
87498749
});
87508750

8751+
it('should compile an input referencing an imported function with literal types', () => {
8752+
env.write('/transforms.ts', `
8753+
export function toBoolean(value: boolean | '' | 'true' | 'false'): boolean {
8754+
return !!value;
8755+
}
8756+
`);
8757+
env.write('/test.ts', `
8758+
import {Directive, Input} from '@angular/core';
8759+
import {toBoolean} from './transforms';
8760+
8761+
@Directive({standalone: true})
8762+
export class Dir {
8763+
@Input({transform: toBoolean}) value!: number;
8764+
}
8765+
`);
8766+
8767+
env.driveMain();
8768+
8769+
const jsContents = env.getContents('test.js');
8770+
const dtsContents = env.getContents('test.d.ts');
8771+
8772+
expect(jsContents).toContain('inputs: { value: ["value", "value", toBoolean] }');
8773+
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
8774+
expect(dtsContents)
8775+
.toContain(`static ngAcceptInputType_value: boolean | "" | "true" | "false";`);
8776+
});
8777+
87518778
it('should compile a directive input with a transform function with a `this` typing', () => {
87528779
env.write('/test.ts', `
87538780
import {Directive, Input} from '@angular/core';

0 commit comments

Comments
 (0)