From 232d03ad2427e7d478ffbc8fab9ff7926df36141 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Fri, 17 Nov 2023 14:57:02 +0100 Subject: [PATCH 1/4] fix(compiler): handle ambient types in input transform function Fixes that the compiler was throwing an error if an ambient type is used inside of an input `transform` function. The problem was that the reference emitter was trying to write a reference to the ambient type's source file which isn't necessary. Fixes #51424. --- .../ngtsc/annotations/directive/src/shared.ts | 4 +-- .../src/ngtsc/imports/src/emitter.ts | 16 +++++++++- .../src/ngtsc/imports/src/references.ts | 8 +++-- .../src/ngtsc/reflection/src/host.ts | 5 ++++ .../src/ngtsc/reflection/src/typescript.ts | 9 ++++++ .../src/ngtsc/reflection/test/ts_host_spec.ts | 2 ++ .../ngtsc/translator/src/type_translator.ts | 5 ++-- .../typecheck/src/type_parameter_emitter.ts | 2 +- .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 29 +++++++++++++++++++ 9 files changed, 71 insertions(+), 9 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts index cb7dc6ae103d..a8c5aa63d059 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts @@ -819,9 +819,9 @@ function assertEmittableInputType( // exported, otherwise TS won't emit it to the .d.ts. if (declaration.node.getSourceFile() !== contextFile) { const emittedType = refEmitter.emit( - new Reference(declaration.node), contextFile, + new Reference(declaration.node, null, declaration.isAmbient), contextFile, ImportFlags.NoAliasing | ImportFlags.AllowTypeImports | - ImportFlags.AllowRelativeDtsImports); + ImportFlags.AllowRelativeDtsImports | ImportFlags.AllowAmbientReferences); assertSuccessfulReferenceEmit(emittedType, node, 'type'); } else if (!reflector.isStaticallyExported(declaration.node)) { diff --git a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts index 85e080b0c322..3163e6ac8594 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts @@ -13,7 +13,7 @@ import {ErrorCode, FatalDiagnosticError, makeDiagnosticChain, makeRelatedInforma import {absoluteFromSourceFile, dirname, LogicalFileSystem, LogicalProjectPath, relative, toRelativeImport} from '../../file_system'; import {stripExtension} from '../../file_system/src/util'; import {DeclarationNode, ReflectionHost} from '../../reflection'; -import {getSourceFile, isDeclaration, isNamedDeclaration, isTypeDeclaration, nodeNameForError} from '../../util/src/typescript'; +import {getSourceFile, identifierOfNode, isDeclaration, isNamedDeclaration, isTypeDeclaration, nodeNameForError} from '../../util/src/typescript'; import {findExportedNameOfNode} from './find_export'; import {Reference} from './references'; @@ -64,6 +64,11 @@ export enum ImportFlags { * declaration file. */ AllowRelativeDtsImports = 0x08, + + /** + * Indicates that references coming from ambient imports are allowed. + */ + AllowAmbientReferences = 0x010, } /** @@ -229,6 +234,15 @@ export class LocalIdentifierStrategy implements ReferenceEmitStrategy { }; } + // If the reference is to an ambient type, it can be referenced directly. + if (ref.isAmbient && importFlags & ImportFlags.AllowAmbientReferences) { + return { + kind: ReferenceEmitKind.Success, + expression: new WrappedNodeExpr(identifierOfNode(ref.node)), + importedFile: null, + }; + } + // A Reference can have multiple identities in different files, so it may already have an // Identifier in the requested context file. const identifier = ref.getIdentityIn(context); diff --git a/packages/compiler-cli/src/ngtsc/imports/src/references.ts b/packages/compiler-cli/src/ngtsc/imports/src/references.ts index 9f29a592f903..47a9a2339f25 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/references.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/references.ts @@ -54,7 +54,9 @@ export class Reference { private _alias: Expression|null = null; - constructor(readonly node: T, bestGuessOwningModule: OwningModule|null = null) { + constructor( + readonly node: T, bestGuessOwningModule: OwningModule|null = null, + readonly isAmbient = false) { this.bestGuessOwningModule = bestGuessOwningModule; const id = identifierOfNode(node); @@ -160,14 +162,14 @@ export class Reference { } cloneWithAlias(alias: Expression): Reference { - const ref = new Reference(this.node, this.bestGuessOwningModule); + const ref = new Reference(this.node, this.bestGuessOwningModule, this.isAmbient); ref.identifiers = [...this.identifiers]; ref._alias = alias; return ref; } cloneWithNoIdentifiers(): Reference { - const ref = new Reference(this.node, this.bestGuessOwningModule); + const ref = new Reference(this.node, this.bestGuessOwningModule, this.isAmbient); ref._alias = this._alias; ref.identifiers = []; return ref; diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts index 0df73eb6f124..1f70a5e50536 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts @@ -495,6 +495,11 @@ export interface Declaration { * TypeScript reference to the declaration itself, if one exists. */ node: T; + + /** + * Whether the node is a reference to an ambient type. + */ + isAmbient: boolean; } /** diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts b/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts index 8e9d12b892d3..67ffa16fd717 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts @@ -355,11 +355,13 @@ export class TypeScriptReflectionHost implements ReflectionHost { return { node: symbol.valueDeclaration, viaModule, + isAmbient: this._isAmbient(symbol.valueDeclaration, originalId, importInfo) }; } else if (symbol.declarations !== undefined && symbol.declarations.length > 0) { return { node: symbol.declarations[0], viaModule, + isAmbient: this._isAmbient(symbol.declarations[0], originalId, importInfo) }; } else { return null; @@ -497,6 +499,13 @@ export class TypeScriptReflectionHost implements ReflectionHost { return exportSet; } + + private _isAmbient( + declaration: ts.Declaration, originalId: ts.Identifier|null, + importInfo: Import|null): boolean { + return importInfo === null && originalId !== null && + declaration.getSourceFile() !== originalId.getSourceFile(); + } } export function reflectNameOfDeclaration(decl: ts.Declaration): string|null { diff --git a/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts b/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts index 0891d0e13e0a..19152de219db 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts @@ -407,6 +407,7 @@ runInEachFileSystem(() => { expect(decl).toEqual({ node: targetDecl, viaModule: 'absolute', + isAmbient: false, }); }); @@ -436,6 +437,7 @@ runInEachFileSystem(() => { expect(decl).toEqual({ node: targetDecl, viaModule: 'absolute', + isAmbient: false, }); }); }); diff --git a/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts index 42653912341e..fd5c3c6a050b 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts @@ -278,9 +278,10 @@ class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor { }; } - const reference = new Reference(declaration.node, owningModule); + const reference = new Reference(declaration.node, owningModule, declaration.isAmbient); const emittedType = this.refEmitter.emit( - reference, this.contextFile, ImportFlags.NoAliasing | ImportFlags.AllowTypeImports); + reference, this.contextFile, + ImportFlags.NoAliasing | ImportFlags.AllowTypeImports | ImportFlags.AllowAmbientReferences); assertSuccessfulReferenceEmit(emittedType, target, 'type'); 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 817eb8fb3bc1..b874b4c5f6b8 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 @@ -100,7 +100,7 @@ export class TypeParameterEmitter { }; } - return new Reference(declaration.node, owningModule); + return new Reference(declaration.node, owningModule, declaration.isAmbient); } private translateTypeReference( diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index bcca44d7d63a..e11de4f7e956 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -8863,6 +8863,35 @@ function allTests(os: string) { .toContain('features: [i0.ɵɵInputTransformsFeature, i0.ɵɵInheritDefinitionFeature]'); expect(dtsContents).toContain('static ngAcceptInputType_value: boolean | string;'); }); + + it('should compile an input with using an ambient type in the transform function', () => { + env.write('node_modules/external/index.d.ts', ` + import {ElementRef} from '@angular/core'; + + export declare function coerceElement(value: HTMLElement | ElementRef): HTMLElement; + `); + + env.write('/test.ts', ` + import {Directive, Input, Component} from '@angular/core'; + import {coerceElement} from 'external'; + + @Directive({standalone: true}) + export class Dir { + @Input({transform: coerceElement}) element!: HTMLElement; + } + `); + + env.driveMain(); + + const jsContents = env.getContents('test.js'); + const dtsContents = env.getContents('test.d.ts'); + + expect(jsContents).toContain('inputs: { element: ["element", "element", coerceElement] }'); + expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]'); + expect(dtsContents) + .toContain( + 'static ngAcceptInputType_element: HTMLElement | i0.ElementRef;'); + }); }); describe('deferred blocks', () => { From 669b97e506505be45cc848869f3b2f1c73418f46 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Fri, 17 Nov 2023 15:32:53 +0100 Subject: [PATCH 2/4] fixup! fix(compiler): handle ambient types in input transform function --- .../ngtsc/annotations/directive/src/shared.ts | 5 ++-- .../partial_evaluator/src/interpreter.ts | 2 +- .../src/ngtsc/reflection/src/host.ts | 14 ++++++---- .../src/ngtsc/reflection/src/typescript.ts | 28 +++++++++---------- .../src/ngtsc/reflection/test/ts_host_spec.ts | 2 -- .../ngtsc/translator/src/type_translator.ts | 7 +++-- .../typecheck/src/type_parameter_emitter.ts | 6 ++-- 7 files changed, 33 insertions(+), 31 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts index a8c5aa63d059..07a5ec66e7ac 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts @@ -13,7 +13,7 @@ import {ErrorCode, FatalDiagnosticError, makeRelatedInformation} from '../../../ import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitter} from '../../../imports'; import {ClassPropertyMapping, HostDirectiveMeta, InputMapping, InputTransform} from '../../../metadata'; import {DynamicValue, EnumValue, PartialEvaluator, ResolvedValue, traceDynamicValue} from '../../../partial_evaluator'; -import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembersWithDecorator, isNamedClassDeclaration, ReflectionHost, reflectObjectLiteral} from '../../../reflection'; +import {AmbientImport, ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembersWithDecorator, isNamedClassDeclaration, ReflectionHost, reflectObjectLiteral} from '../../../reflection'; import {CompilationMode} from '../../../transform'; import {createSourceSpan, createValueHasWrongTypeError, forwardRefResolver, getConstructorDependencies, ReferencesRegistry, toR3Reference, tryUnwrapForwardRef, unwrapConstructorDependencies, unwrapExpression, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference,} from '../../common'; @@ -819,7 +819,8 @@ function assertEmittableInputType( // exported, otherwise TS won't emit it to the .d.ts. if (declaration.node.getSourceFile() !== contextFile) { const emittedType = refEmitter.emit( - new Reference(declaration.node, null, declaration.isAmbient), contextFile, + new Reference(declaration.node, null, declaration.viaModule === AmbientImport), + contextFile, ImportFlags.NoAliasing | ImportFlags.AllowTypeImports | ImportFlags.AllowRelativeDtsImports | ImportFlags.AllowAmbientReferences); diff --git a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts index 72ba0679c402..3c3e1bccaf5c 100644 --- a/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts +++ b/packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts @@ -739,7 +739,7 @@ function joinModuleContext(existing: Context, node: ts.Node, decl: Declaration): absoluteModuleName?: string, resolutionContext?: string, } { - if (decl.viaModule !== null && decl.viaModule !== existing.absoluteModuleName) { + if (typeof decl.viaModule === 'string' && decl.viaModule !== existing.absoluteModuleName) { return { absoluteModuleName: decl.viaModule, resolutionContext: node.getSourceFile().fileName, diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts index 1f70a5e50536..ce8cd6e48f40 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/host.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/host.ts @@ -479,6 +479,13 @@ export interface Import { */ export type DeclarationNode = ts.Declaration; +export type AmbientImport = { + __brand: 'AmbientImport' +}; + +/** Indicates that a declaration is referenced through an ambient type. */ +export const AmbientImport = {} as AmbientImport; + /** * The declaration of a symbol, along with information about how it was imported into the * application. @@ -489,17 +496,12 @@ export interface Declaration { * was imported via an absolute module (even through a chain of re-exports). If the symbol is part * of the application and was not imported from an absolute path, this will be `null`. */ - viaModule: string|null; + viaModule: string|AmbientImport|null; /** * TypeScript reference to the declaration itself, if one exists. */ node: T; - - /** - * Whether the node is a reference to an ambient type. - */ - isAmbient: boolean; } /** diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts b/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts index 67ffa16fd717..0a8623dd977e 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts @@ -8,7 +8,7 @@ import ts from 'typescript'; -import {ClassDeclaration, ClassMember, ClassMemberKind, CtorParameter, Declaration, DeclarationNode, Decorator, FunctionDefinition, Import, isDecoratorIdentifier, ReflectionHost} from './host'; +import {AmbientImport, ClassDeclaration, ClassMember, ClassMemberKind, CtorParameter, Declaration, DeclarationNode, Decorator, FunctionDefinition, Import, isDecoratorIdentifier, ReflectionHost} from './host'; import {typeToValue} from './type_to_value'; import {isNamedClassDeclaration} from './util'; @@ -339,10 +339,6 @@ export class TypeScriptReflectionHost implements ReflectionHost { } const importInfo = originalId && this.getImportOfIdentifier(originalId); - const viaModule = - importInfo !== null && importInfo.from !== null && !importInfo.from.startsWith('.') ? - importInfo.from : - null; // Now, resolve the Symbol to its declaration by following any and all aliases. while (symbol.flags & ts.SymbolFlags.Alias) { @@ -354,14 +350,12 @@ export class TypeScriptReflectionHost implements ReflectionHost { if (symbol.valueDeclaration !== undefined) { return { node: symbol.valueDeclaration, - viaModule, - isAmbient: this._isAmbient(symbol.valueDeclaration, originalId, importInfo) + viaModule: this._viaModule(symbol.valueDeclaration, originalId, importInfo), }; } else if (symbol.declarations !== undefined && symbol.declarations.length > 0) { return { node: symbol.declarations[0], - viaModule, - isAmbient: this._isAmbient(symbol.declarations[0], originalId, importInfo) + viaModule: this._viaModule(symbol.declarations[0], originalId, importInfo), }; } else { return null; @@ -500,11 +494,17 @@ export class TypeScriptReflectionHost implements ReflectionHost { return exportSet; } - private _isAmbient( - declaration: ts.Declaration, originalId: ts.Identifier|null, - importInfo: Import|null): boolean { - return importInfo === null && originalId !== null && - declaration.getSourceFile() !== originalId.getSourceFile(); + private _viaModule( + declaration: ts.Declaration, originalId: ts.Identifier|null, importInfo: Import|null): string + |AmbientImport|null { + if (importInfo === null && originalId !== null && + declaration.getSourceFile() !== originalId.getSourceFile()) { + return AmbientImport; + } + + return importInfo !== null && importInfo.from !== null && !importInfo.from.startsWith('.') ? + importInfo.from : + null; } } diff --git a/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts b/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts index 19152de219db..0891d0e13e0a 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts @@ -407,7 +407,6 @@ runInEachFileSystem(() => { expect(decl).toEqual({ node: targetDecl, viaModule: 'absolute', - isAmbient: false, }); }); @@ -437,7 +436,6 @@ runInEachFileSystem(() => { expect(decl).toEqual({ node: targetDecl, viaModule: 'absolute', - isAmbient: false, }); }); }); diff --git a/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts index fd5c3c6a050b..c2bad51b947d 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts @@ -10,7 +10,7 @@ import * as o from '@angular/compiler'; import ts from 'typescript'; import {assertSuccessfulReferenceEmit, ImportFlags, OwningModule, Reference, ReferenceEmitter} from '../../imports'; -import {ReflectionHost} from '../../reflection'; +import {AmbientImport, ReflectionHost} from '../../reflection'; import {Context} from './context'; import {ImportManager} from './import_manager'; @@ -271,14 +271,15 @@ class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor { } let owningModule = viaModule; - if (declaration.viaModule !== null) { + if (typeof declaration.viaModule === 'string') { owningModule = { specifier: declaration.viaModule, resolutionContext: type.getSourceFile().fileName, }; } - const reference = new Reference(declaration.node, owningModule, declaration.isAmbient); + const reference = + new Reference(declaration.node, owningModule, declaration.viaModule === AmbientImport); const emittedType = this.refEmitter.emit( reference, this.contextFile, ImportFlags.NoAliasing | ImportFlags.AllowTypeImports | ImportFlags.AllowAmbientReferences); 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 b874b4c5f6b8..032e7c181e25 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 @@ -8,7 +8,7 @@ import ts from 'typescript'; import {OwningModule, Reference} from '../../imports'; -import {DeclarationNode, ReflectionHost} from '../../reflection'; +import {AmbientImport, DeclarationNode, ReflectionHost} from '../../reflection'; import {canEmitType, TypeEmitter} from '../../translator'; /** @@ -93,14 +93,14 @@ export class TypeParameterEmitter { } let owningModule: OwningModule|null = null; - if (declaration.viaModule !== null) { + if (typeof declaration.viaModule === 'string') { owningModule = { specifier: declaration.viaModule, resolutionContext: type.getSourceFile().fileName, }; } - return new Reference(declaration.node, owningModule, declaration.isAmbient); + return new Reference(declaration.node, owningModule, declaration.viaModule === AmbientImport); } private translateTypeReference( From 8acfa7e6998d928e64349cce6b67db89d06a863d Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Mon, 4 Dec 2023 12:57:21 +0100 Subject: [PATCH 3/4] fixup! fix(compiler): handle ambient types in input transform function --- .../src/ngtsc/imports/src/emitter.ts | 15 +++++++++----- .../src/ngtsc/typecheck/src/environment.ts | 20 +++++++++---------- .../test/type_parameter_emitter_spec.ts | 18 ++++++++++++----- 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts index 3163e6ac8594..2baa9b14cc8d 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts @@ -236,11 +236,16 @@ export class LocalIdentifierStrategy implements ReferenceEmitStrategy { // If the reference is to an ambient type, it can be referenced directly. if (ref.isAmbient && importFlags & ImportFlags.AllowAmbientReferences) { - return { - kind: ReferenceEmitKind.Success, - expression: new WrappedNodeExpr(identifierOfNode(ref.node)), - importedFile: null, - }; + const identifier = identifierOfNode(ref.node); + if (identifier !== null) { + return { + kind: ReferenceEmitKind.Success, + expression: new WrappedNodeExpr(identifier), + importedFile: null, + }; + } else { + return null; + } } // A Reference can have multiple identities in different files, so it may already have an diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts index edd234dfaa23..3245f69908bc 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts @@ -126,11 +126,11 @@ export class Environment implements ReferenceEmitEnvironment { return translateExpression(ngExpr.expression, this.importManager); } - canReferenceType(ref: Reference): boolean { - const result = this.refEmitter.emit( - ref, this.contextFile, - ImportFlags.NoAliasing | ImportFlags.AllowTypeImports | - ImportFlags.AllowRelativeDtsImports); + canReferenceType( + ref: Reference, + flags: ImportFlags = ImportFlags.NoAliasing | ImportFlags.AllowTypeImports | + ImportFlags.AllowRelativeDtsImports): boolean { + const result = this.refEmitter.emit(ref, this.contextFile, flags); return result.kind === ReferenceEmitKind.Success; } @@ -139,11 +139,11 @@ export class Environment implements ReferenceEmitEnvironment { * * This may involve importing the node into the file if it's not declared there already. */ - referenceType(ref: Reference): ts.TypeNode { - const ngExpr = this.refEmitter.emit( - ref, this.contextFile, - ImportFlags.NoAliasing | ImportFlags.AllowTypeImports | - ImportFlags.AllowRelativeDtsImports); + referenceType( + ref: Reference, + flags: ImportFlags = ImportFlags.NoAliasing | ImportFlags.AllowTypeImports | + ImportFlags.AllowRelativeDtsImports): ts.TypeNode { + const ngExpr = this.refEmitter.emit(ref, this.contextFile, flags); assertSuccessfulReferenceEmit(ngExpr, this.contextFile, 'symbol'); // Create an `ExpressionType` from the `Expression` and translate it via `translateType`. 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 eedc216cb540..a7ff6fbe08c9 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 @@ -9,13 +9,13 @@ import ts from 'typescript'; import {absoluteFrom, LogicalFileSystem} from '../../file_system'; import {runInEachFileSystem, TestFile} from '../../file_system/testing'; -import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, ReferenceEmitter} from '../../imports'; +import {AbsoluteModuleStrategy, ImportFlags, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, ReferenceEmitter} from '../../imports'; import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection'; import {getDeclaration, makeProgram} from '../../testing'; import {Environment} from '../src/environment'; import {TypeCheckFile} from '../src/type_check_file'; import {TypeParameterEmitter} from '../src/type_parameter_emitter'; -import {ALL_ENABLED_CONFIG, angularCoreDts} from '../testing'; +import {ALL_ENABLED_CONFIG, angularCoreDts, typescriptLibDts} from '../testing'; runInEachFileSystem(() => { @@ -48,12 +48,13 @@ runInEachFileSystem(() => { return {emitter, env}; } - function emit({emitter, env}: {emitter: TypeParameterEmitter; env: Environment}) { - const canEmit = emitter.canEmit(ref => env.canReferenceType(ref)); + function emit( + {emitter, env}: {emitter: TypeParameterEmitter; env: Environment}, flags?: ImportFlags) { + const canEmit = emitter.canEmit(ref => env.canReferenceType(ref, flags)); let emitted: ts.TypeParameterDeclaration[]|undefined; try { - emitted = emitter.emit(ref => env.referenceType(ref)); + emitted = emitter.emit(ref => env.referenceType(ref, flags)); expect(canEmit).toBe(true); } catch (e) { expect(canEmit).toBe(false); @@ -374,5 +375,12 @@ runInEachFileSystem(() => { expect(() => emit(emitter)).toThrow(); }); + + it('can opt into emitting references to ambient types', () => { + const emitter = + createEmitter(`export class TestClass {}`, [typescriptLibDts()]); + + expect(emit(emitter, ImportFlags.AllowAmbientReferences)).toBe(''); + }); }); }); From 83b6348403eacadbf811cb603186423fc9d68f58 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 13 Dec 2023 11:52:11 +0100 Subject: [PATCH 4/4] fixup! fix(compiler): handle ambient types in input transform function --- .../ngtsc/annotations/directive/src/shared.ts | 3 +- .../src/ngtsc/imports/src/references.ts | 21 +++++++--- .../ngtsc/translator/src/type_translator.ts | 4 +- .../typecheck/src/type_parameter_emitter.ts | 3 +- .../test/ngtsc/template_typecheck_spec.ts | 38 +++++++++++++++++++ 5 files changed, 59 insertions(+), 10 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts index 07a5ec66e7ac..39333defca13 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts @@ -819,7 +819,8 @@ function assertEmittableInputType( // exported, otherwise TS won't emit it to the .d.ts. if (declaration.node.getSourceFile() !== contextFile) { const emittedType = refEmitter.emit( - new Reference(declaration.node, null, declaration.viaModule === AmbientImport), + new Reference( + declaration.node, declaration.viaModule === AmbientImport ? AmbientImport : null), contextFile, ImportFlags.NoAliasing | ImportFlags.AllowTypeImports | ImportFlags.AllowRelativeDtsImports | ImportFlags.AllowAmbientReferences); diff --git a/packages/compiler-cli/src/ngtsc/imports/src/references.ts b/packages/compiler-cli/src/ngtsc/imports/src/references.ts index 47a9a2339f25..adfadfee8161 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/references.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/references.ts @@ -9,6 +9,7 @@ import {Expression} from '@angular/compiler'; import ts from 'typescript'; +import {AmbientImport} from '../../reflection'; import {identifierOfNode} from '../../util/src/typescript'; export interface OwningModule { @@ -54,10 +55,16 @@ export class Reference { private _alias: Expression|null = null; - constructor( - readonly node: T, bestGuessOwningModule: OwningModule|null = null, - readonly isAmbient = false) { - this.bestGuessOwningModule = bestGuessOwningModule; + readonly isAmbient: boolean; + + constructor(readonly node: T, bestGuessOwningModule: OwningModule|AmbientImport|null = null) { + if (bestGuessOwningModule === AmbientImport) { + this.isAmbient = true; + this.bestGuessOwningModule = null; + } else { + this.isAmbient = false; + this.bestGuessOwningModule = bestGuessOwningModule as OwningModule | null; + } const id = identifierOfNode(node); if (id !== null) { @@ -162,14 +169,16 @@ export class Reference { } cloneWithAlias(alias: Expression): Reference { - const ref = new Reference(this.node, this.bestGuessOwningModule, this.isAmbient); + const ref = + new Reference(this.node, this.isAmbient ? AmbientImport : this.bestGuessOwningModule); ref.identifiers = [...this.identifiers]; ref._alias = alias; return ref; } cloneWithNoIdentifiers(): Reference { - const ref = new Reference(this.node, this.bestGuessOwningModule, this.isAmbient); + const ref = + new Reference(this.node, this.isAmbient ? AmbientImport : this.bestGuessOwningModule); ref._alias = this._alias; ref.identifiers = []; return ref; diff --git a/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts index c2bad51b947d..6ae8412b69b5 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts @@ -278,8 +278,8 @@ class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor { }; } - const reference = - new Reference(declaration.node, owningModule, declaration.viaModule === AmbientImport); + const reference = new Reference( + declaration.node, declaration.viaModule === AmbientImport ? AmbientImport : owningModule); const emittedType = this.refEmitter.emit( reference, this.contextFile, ImportFlags.NoAliasing | ImportFlags.AllowTypeImports | ImportFlags.AllowAmbientReferences); 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 032e7c181e25..426908062f79 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 @@ -100,7 +100,8 @@ export class TypeParameterEmitter { }; } - return new Reference(declaration.node, owningModule, declaration.viaModule === AmbientImport); + return new Reference( + declaration.node, declaration.viaModule === AmbientImport ? AmbientImport : owningModule); } private translateTypeReference( diff --git a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts index 7f1ebaf6aa02..3c8ca1d30a49 100644 --- a/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts +++ b/packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts @@ -2317,6 +2317,44 @@ export declare class AnimationEvent { ` Type 'string' is not assignable to type 'number'.` ]); }); + + it('should type check inputs with transforms referring to an ambient type', () => { + env.tsconfig({strictTemplates: true}); + env.write('test.ts', ` + import {Component, Directive, NgModule, Input} from '@angular/core'; + + export class ElementRef { + nativeElement: T; + } + + @Directive({ + selector: '[dir]', + standalone: true, + }) + export class Dir { + @Input({transform: (val: HTMLInputElement | ElementRef) => { + return val instanceof ElementRef ? val.nativeElement.value : val.value; + }}) + expectsInput: string | null = null; + } + + @Component({ + standalone: true, + imports: [Dir], + template: '
', + }) + export class App { + someDiv!: HTMLDivElement; + } + `); + + const diags = env.driveDiagnostics(); + expect(diags.length).toBe(1); + expect(getDiagnosticLines(diags[0]).join('\n')) + .toContain( + `Type 'HTMLDivElement' is not assignable to type ` + + `'HTMLInputElement | ElementRef'`); + }); }); describe('restricted inputs', () => {