Skip to content

Commit 4fd5409

Browse files
crisbetoalxhub
crisbeto
authored and
alxhub
committed
fix(compiler): handle ambient types in input transform function (#51474)
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. PR Close #51474
1 parent 57ac806 commit 4fd5409

File tree

12 files changed

+165
-39
lines changed

12 files changed

+165
-39
lines changed

‎packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {ErrorCode, FatalDiagnosticError, makeRelatedInformation} from '../../../
1313
import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitter} from '../../../imports';
1414
import {ClassPropertyMapping, HostDirectiveMeta, InputMapping, InputTransform} from '../../../metadata';
1515
import {DynamicValue, EnumValue, PartialEvaluator, ResolvedValue, traceDynamicValue} from '../../../partial_evaluator';
16-
import {ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembersWithDecorator, isNamedClassDeclaration, ReflectionHost, reflectObjectLiteral} from '../../../reflection';
16+
import {AmbientImport, ClassDeclaration, ClassMember, ClassMemberKind, Decorator, filterToMembersWithDecorator, isNamedClassDeclaration, ReflectionHost, reflectObjectLiteral} from '../../../reflection';
1717
import {CompilationMode} from '../../../transform';
1818
import {createSourceSpan, createValueHasWrongTypeError, forwardRefResolver, getConstructorDependencies, ReferencesRegistry, toR3Reference, tryUnwrapForwardRef, unwrapConstructorDependencies, unwrapExpression, validateConstructorDependencies, wrapFunctionExpressionsInParens, wrapTypeReference,} from '../../common';
1919

@@ -819,9 +819,11 @@ function assertEmittableInputType(
819819
// exported, otherwise TS won't emit it to the .d.ts.
820820
if (declaration.node.getSourceFile() !== contextFile) {
821821
const emittedType = refEmitter.emit(
822-
new Reference(declaration.node), contextFile,
822+
new Reference(
823+
declaration.node, declaration.viaModule === AmbientImport ? AmbientImport : null),
824+
contextFile,
823825
ImportFlags.NoAliasing | ImportFlags.AllowTypeImports |
824-
ImportFlags.AllowRelativeDtsImports);
826+
ImportFlags.AllowRelativeDtsImports | ImportFlags.AllowAmbientReferences);
825827

826828
assertSuccessfulReferenceEmit(emittedType, node, 'type');
827829
} else if (!reflector.isStaticallyExported(declaration.node)) {

‎packages/compiler-cli/src/ngtsc/imports/src/emitter.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {ErrorCode, FatalDiagnosticError, makeDiagnosticChain, makeRelatedInforma
1313
import {absoluteFromSourceFile, dirname, LogicalFileSystem, LogicalProjectPath, relative, toRelativeImport} from '../../file_system';
1414
import {stripExtension} from '../../file_system/src/util';
1515
import {DeclarationNode, ReflectionHost} from '../../reflection';
16-
import {getSourceFile, isDeclaration, isNamedDeclaration, isTypeDeclaration, nodeNameForError} from '../../util/src/typescript';
16+
import {getSourceFile, identifierOfNode, isDeclaration, isNamedDeclaration, isTypeDeclaration, nodeNameForError} from '../../util/src/typescript';
1717

1818
import {findExportedNameOfNode} from './find_export';
1919
import {Reference} from './references';
@@ -64,6 +64,11 @@ export enum ImportFlags {
6464
* declaration file.
6565
*/
6666
AllowRelativeDtsImports = 0x08,
67+
68+
/**
69+
* Indicates that references coming from ambient imports are allowed.
70+
*/
71+
AllowAmbientReferences = 0x010,
6772
}
6873

6974
/**
@@ -229,6 +234,20 @@ export class LocalIdentifierStrategy implements ReferenceEmitStrategy {
229234
};
230235
}
231236

237+
// If the reference is to an ambient type, it can be referenced directly.
238+
if (ref.isAmbient && importFlags & ImportFlags.AllowAmbientReferences) {
239+
const identifier = identifierOfNode(ref.node);
240+
if (identifier !== null) {
241+
return {
242+
kind: ReferenceEmitKind.Success,
243+
expression: new WrappedNodeExpr(identifier),
244+
importedFile: null,
245+
};
246+
} else {
247+
return null;
248+
}
249+
}
250+
232251
// A Reference can have multiple identities in different files, so it may already have an
233252
// Identifier in the requested context file.
234253
const identifier = ref.getIdentityIn(context);

‎packages/compiler-cli/src/ngtsc/imports/src/references.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import {Expression} from '@angular/compiler';
1010
import ts from 'typescript';
1111

12+
import {AmbientImport} from '../../reflection';
1213
import {identifierOfNode} from '../../util/src/typescript';
1314

1415
export interface OwningModule {
@@ -54,8 +55,16 @@ export class Reference<T extends ts.Node = ts.Node> {
5455

5556
private _alias: Expression|null = null;
5657

57-
constructor(readonly node: T, bestGuessOwningModule: OwningModule|null = null) {
58-
this.bestGuessOwningModule = bestGuessOwningModule;
58+
readonly isAmbient: boolean;
59+
60+
constructor(readonly node: T, bestGuessOwningModule: OwningModule|AmbientImport|null = null) {
61+
if (bestGuessOwningModule === AmbientImport) {
62+
this.isAmbient = true;
63+
this.bestGuessOwningModule = null;
64+
} else {
65+
this.isAmbient = false;
66+
this.bestGuessOwningModule = bestGuessOwningModule as OwningModule | null;
67+
}
5968

6069
const id = identifierOfNode(node);
6170
if (id !== null) {
@@ -160,14 +169,16 @@ export class Reference<T extends ts.Node = ts.Node> {
160169
}
161170

162171
cloneWithAlias(alias: Expression): Reference<T> {
163-
const ref = new Reference(this.node, this.bestGuessOwningModule);
172+
const ref =
173+
new Reference(this.node, this.isAmbient ? AmbientImport : this.bestGuessOwningModule);
164174
ref.identifiers = [...this.identifiers];
165175
ref._alias = alias;
166176
return ref;
167177
}
168178

169179
cloneWithNoIdentifiers(): Reference<T> {
170-
const ref = new Reference(this.node, this.bestGuessOwningModule);
180+
const ref =
181+
new Reference(this.node, this.isAmbient ? AmbientImport : this.bestGuessOwningModule);
171182
ref._alias = this._alias;
172183
ref.identifiers = [];
173184
return ref;

‎packages/compiler-cli/src/ngtsc/partial_evaluator/src/interpreter.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,7 @@ function joinModuleContext(existing: Context, node: ts.Node, decl: Declaration):
739739
absoluteModuleName?: string,
740740
resolutionContext?: string,
741741
} {
742-
if (decl.viaModule !== null && decl.viaModule !== existing.absoluteModuleName) {
742+
if (typeof decl.viaModule === 'string' && decl.viaModule !== existing.absoluteModuleName) {
743743
return {
744744
absoluteModuleName: decl.viaModule,
745745
resolutionContext: node.getSourceFile().fileName,

‎packages/compiler-cli/src/ngtsc/reflection/src/host.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,13 @@ export interface Import {
478478
*/
479479
export type DeclarationNode = ts.Declaration;
480480

481+
export type AmbientImport = {
482+
__brand: 'AmbientImport'
483+
};
484+
485+
/** Indicates that a declaration is referenced through an ambient type. */
486+
export const AmbientImport = {} as AmbientImport;
487+
481488
/**
482489
* The declaration of a symbol, along with information about how it was imported into the
483490
* application.
@@ -488,7 +495,7 @@ export interface Declaration<T extends ts.Declaration = ts.Declaration> {
488495
* was imported via an absolute module (even through a chain of re-exports). If the symbol is part
489496
* of the application and was not imported from an absolute path, this will be `null`.
490497
*/
491-
viaModule: string|null;
498+
viaModule: string|AmbientImport|null;
492499

493500
/**
494501
* TypeScript reference to the declaration itself, if one exists.

‎packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import ts from 'typescript';
1010

11-
import {ClassDeclaration, ClassMember, ClassMemberKind, CtorParameter, Declaration, DeclarationNode, Decorator, FunctionDefinition, Import, isDecoratorIdentifier, ReflectionHost} from './host';
11+
import {AmbientImport, ClassDeclaration, ClassMember, ClassMemberKind, CtorParameter, Declaration, DeclarationNode, Decorator, FunctionDefinition, Import, isDecoratorIdentifier, ReflectionHost} from './host';
1212
import {typeToValue} from './type_to_value';
1313
import {isNamedClassDeclaration} from './util';
1414

@@ -339,10 +339,6 @@ export class TypeScriptReflectionHost implements ReflectionHost {
339339
}
340340

341341
const importInfo = originalId && this.getImportOfIdentifier(originalId);
342-
const viaModule =
343-
importInfo !== null && importInfo.from !== null && !importInfo.from.startsWith('.') ?
344-
importInfo.from :
345-
null;
346342

347343
// Now, resolve the Symbol to its declaration by following any and all aliases.
348344
while (symbol.flags & ts.SymbolFlags.Alias) {
@@ -354,12 +350,12 @@ export class TypeScriptReflectionHost implements ReflectionHost {
354350
if (symbol.valueDeclaration !== undefined) {
355351
return {
356352
node: symbol.valueDeclaration,
357-
viaModule,
353+
viaModule: this._viaModule(symbol.valueDeclaration, originalId, importInfo),
358354
};
359355
} else if (symbol.declarations !== undefined && symbol.declarations.length > 0) {
360356
return {
361357
node: symbol.declarations[0],
362-
viaModule,
358+
viaModule: this._viaModule(symbol.declarations[0], originalId, importInfo),
363359
};
364360
} else {
365361
return null;
@@ -497,6 +493,19 @@ export class TypeScriptReflectionHost implements ReflectionHost {
497493

498494
return exportSet;
499495
}
496+
497+
private _viaModule(
498+
declaration: ts.Declaration, originalId: ts.Identifier|null, importInfo: Import|null): string
499+
|AmbientImport|null {
500+
if (importInfo === null && originalId !== null &&
501+
declaration.getSourceFile() !== originalId.getSourceFile()) {
502+
return AmbientImport;
503+
}
504+
505+
return importInfo !== null && importInfo.from !== null && !importInfo.from.startsWith('.') ?
506+
importInfo.from :
507+
null;
508+
}
500509
}
501510

502511
export function reflectNameOfDeclaration(decl: ts.Declaration): string|null {

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import * as o from '@angular/compiler';
1010
import ts from 'typescript';
1111

1212
import {assertSuccessfulReferenceEmit, ImportFlags, OwningModule, Reference, ReferenceEmitter} from '../../imports';
13-
import {ReflectionHost} from '../../reflection';
13+
import {AmbientImport, ReflectionHost} from '../../reflection';
1414

1515
import {Context} from './context';
1616
import {ImportManager} from './import_manager';
@@ -270,16 +270,18 @@ class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor {
270270
}
271271

272272
let owningModule = viaModule;
273-
if (declaration.viaModule !== null) {
273+
if (typeof declaration.viaModule === 'string') {
274274
owningModule = {
275275
specifier: declaration.viaModule,
276276
resolutionContext: type.getSourceFile().fileName,
277277
};
278278
}
279279

280-
const reference = new Reference(declaration.node, owningModule);
280+
const reference = new Reference(
281+
declaration.node, declaration.viaModule === AmbientImport ? AmbientImport : owningModule);
281282
const emittedType = this.refEmitter.emit(
282-
reference, this.contextFile, ImportFlags.NoAliasing | ImportFlags.AllowTypeImports);
283+
reference, this.contextFile,
284+
ImportFlags.NoAliasing | ImportFlags.AllowTypeImports | ImportFlags.AllowAmbientReferences);
283285

284286
assertSuccessfulReferenceEmit(emittedType, target, 'type');
285287

‎packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -126,11 +126,11 @@ export class Environment implements ReferenceEmitEnvironment {
126126
return translateExpression(ngExpr.expression, this.importManager);
127127
}
128128

129-
canReferenceType(ref: Reference): boolean {
130-
const result = this.refEmitter.emit(
131-
ref, this.contextFile,
132-
ImportFlags.NoAliasing | ImportFlags.AllowTypeImports |
133-
ImportFlags.AllowRelativeDtsImports);
129+
canReferenceType(
130+
ref: Reference,
131+
flags: ImportFlags = ImportFlags.NoAliasing | ImportFlags.AllowTypeImports |
132+
ImportFlags.AllowRelativeDtsImports): boolean {
133+
const result = this.refEmitter.emit(ref, this.contextFile, flags);
134134
return result.kind === ReferenceEmitKind.Success;
135135
}
136136

@@ -139,11 +139,11 @@ export class Environment implements ReferenceEmitEnvironment {
139139
*
140140
* This may involve importing the node into the file if it's not declared there already.
141141
*/
142-
referenceType(ref: Reference): ts.TypeNode {
143-
const ngExpr = this.refEmitter.emit(
144-
ref, this.contextFile,
145-
ImportFlags.NoAliasing | ImportFlags.AllowTypeImports |
146-
ImportFlags.AllowRelativeDtsImports);
142+
referenceType(
143+
ref: Reference,
144+
flags: ImportFlags = ImportFlags.NoAliasing | ImportFlags.AllowTypeImports |
145+
ImportFlags.AllowRelativeDtsImports): ts.TypeNode {
146+
const ngExpr = this.refEmitter.emit(ref, this.contextFile, flags);
147147
assertSuccessfulReferenceEmit(ngExpr, this.contextFile, 'symbol');
148148

149149
// Create an `ExpressionType` from the `Expression` and translate it via `translateType`.

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import ts from 'typescript';
99

1010
import {OwningModule, Reference} from '../../imports';
11-
import {DeclarationNode, ReflectionHost} from '../../reflection';
11+
import {AmbientImport, DeclarationNode, ReflectionHost} from '../../reflection';
1212
import {canEmitType, TypeEmitter} from '../../translator';
1313

1414
/**
@@ -93,14 +93,15 @@ export class TypeParameterEmitter {
9393
}
9494

9595
let owningModule: OwningModule|null = null;
96-
if (declaration.viaModule !== null) {
96+
if (typeof declaration.viaModule === 'string') {
9797
owningModule = {
9898
specifier: declaration.viaModule,
9999
resolutionContext: type.getSourceFile().fileName,
100100
};
101101
}
102102

103-
return new Reference(declaration.node, owningModule);
103+
return new Reference(
104+
declaration.node, declaration.viaModule === AmbientImport ? AmbientImport : owningModule);
104105
}
105106

106107
private translateTypeReference(

‎packages/compiler-cli/src/ngtsc/typecheck/test/type_parameter_emitter_spec.ts‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@ import ts from 'typescript';
99

1010
import {absoluteFrom, LogicalFileSystem} from '../../file_system';
1111
import {runInEachFileSystem, TestFile} from '../../file_system/testing';
12-
import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, ReferenceEmitter} from '../../imports';
12+
import {AbsoluteModuleStrategy, ImportFlags, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, ReferenceEmitter} from '../../imports';
1313
import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
1414
import {getDeclaration, makeProgram} from '../../testing';
1515
import {Environment} from '../src/environment';
1616
import {TypeCheckFile} from '../src/type_check_file';
1717
import {TypeParameterEmitter} from '../src/type_parameter_emitter';
18-
import {ALL_ENABLED_CONFIG, angularCoreDts} from '../testing';
18+
import {ALL_ENABLED_CONFIG, angularCoreDts, typescriptLibDts} from '../testing';
1919

2020

2121
runInEachFileSystem(() => {
@@ -48,12 +48,13 @@ runInEachFileSystem(() => {
4848
return {emitter, env};
4949
}
5050

51-
function emit({emitter, env}: {emitter: TypeParameterEmitter; env: Environment}) {
52-
const canEmit = emitter.canEmit(ref => env.canReferenceType(ref));
51+
function emit(
52+
{emitter, env}: {emitter: TypeParameterEmitter; env: Environment}, flags?: ImportFlags) {
53+
const canEmit = emitter.canEmit(ref => env.canReferenceType(ref, flags));
5354

5455
let emitted: ts.TypeParameterDeclaration[]|undefined;
5556
try {
56-
emitted = emitter.emit(ref => env.referenceType(ref));
57+
emitted = emitter.emit(ref => env.referenceType(ref, flags));
5758
expect(canEmit).toBe(true);
5859
} catch (e) {
5960
expect(canEmit).toBe(false);
@@ -374,5 +375,12 @@ runInEachFileSystem(() => {
374375

375376
expect(() => emit(emitter)).toThrow();
376377
});
378+
379+
it('can opt into emitting references to ambient types', () => {
380+
const emitter =
381+
createEmitter(`export class TestClass<T extends HTMLElement> {}`, [typescriptLibDts()]);
382+
383+
expect(emit(emitter, ImportFlags.AllowAmbientReferences)).toBe('<T extends HTMLElement>');
384+
});
377385
});
378386
});

0 commit comments

Comments
 (0)