angular · crisbeto · Nov 17, 2023 · Nov 17, 2023 · Dec 4, 2023 · Dec 13, 2023
diff --git 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,9 +819,11 @@ 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, declaration.viaModule === AmbientImport ? AmbientImport : null),
+              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
@@ -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,20 @@ export class LocalIdentifierStrategy implements ReferenceEmitStrategy {
      };
    }

+    // If the reference is to an ambient type, it can be referenced directly.
+    if (ref.isAmbient && importFlags & ImportFlags.AllowAmbientReferences) {
+      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
    // 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
@@ -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,8 +55,16 @@ export class Reference<T extends ts.Node = ts.Node> {

  private _alias: Expression|null = null;

-  constructor(readonly node: T, bestGuessOwningModule: OwningModule|null = null) {
-    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) {
@@ -160,14 +169,16 @@ export class Reference<T extends ts.Node = ts.Node> {
  }

  cloneWithAlias(alias: Expression): Reference<T> {
-    const ref = new Reference(this.node, this.bestGuessOwningModule);
+    const ref =
+        new Reference(this.node, this.isAmbient ? AmbientImport : this.bestGuessOwningModule);
    ref.identifiers = [...this.identifiers];
    ref._alias = alias;
    return ref;
  }

  cloneWithNoIdentifiers(): Reference<T> {
-    const ref = new Reference(this.node, this.bestGuessOwningModule);
+    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/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
@@ -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,7 +496,7 @@ export interface Declaration<T extends ts.Declaration = ts.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.

diff --git 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,12 +350,12 @@ export class TypeScriptReflectionHost implements ReflectionHost {
    if (symbol.valueDeclaration !== undefined) {
      return {
        node: symbol.valueDeclaration,
-        viaModule,
+        viaModule: this._viaModule(symbol.valueDeclaration, originalId, importInfo),
      };
    } else if (symbol.declarations !== undefined && symbol.declarations.length > 0) {
      return {
        node: symbol.declarations[0],
-        viaModule,
+        viaModule: this._viaModule(symbol.declarations[0], originalId, importInfo),
      };
    } else {
      return null;
@@ -497,6 +493,19 @@ export class TypeScriptReflectionHost implements ReflectionHost {

    return exportSet;
  }
+
+  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;
+  }
 }

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

diff --git 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,16 +271,18 @@ 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);
+    const reference = new Reference(
+        declaration.node, declaration.viaModule === AmbientImport ? AmbientImport : owningModule);
    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/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/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,15 @@ 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);
+    return new Reference(
+        declaration.node, declaration.viaModule === AmbientImport ? AmbientImport : owningModule);
  }

  private translateTypeReference(

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
@@ -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<T extends HTMLElement> {}`, [typescriptLibDts()]);
+
+      expect(emit(emitter, ImportFlags.AllowAmbientReferences)).toBe('<T extends HTMLElement>');
+    });
  });
 });
diff --git 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>): 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<HTMLElement>;');
+      });
    });

    describe('deferred blocks', () => {
-Original file line number
+Diff line change
@@ -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,
                 Outdated
          
        

                


  
    
        
        
  
    
      
          
    
  


        
            
  
      
              Copy link

  
        
      
    

    
        

  
  Member


        

  Author


    
  

  
    
      
          
      

      
            crisbeto
  

      

      

      


        Aug 24, 2023


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      Context here: in #43511 we added some assertions around not allowing classes defined inside namespaces. I added this flag so we only allow ambient references in specific places (input transforms in this case).
    
  
  



    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
              
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    


      



    
        
  
    
      
          
    
  


        
            
  
      
              Copy link

  
        
      
    

    
        

  
  Member


        

    
  

  
    
      
          
      

      
            JoostK
  

      

      

      


        Aug 24, 2023


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      I can't think of a reason why we wouldn't want this behavior if AllowTypeImports is used; can the new condition just use the AllowTypeImports flag instead of introducing a new mode?
    
  
  



    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
              
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    


      



    
        
  
    
      
          
    
  


        
            
  
      
              Copy link

  
        
      
    

    
        

  
  Member


        

  Author


    
  

  
    
      
          
      

      
            crisbeto
  

      

      

      


        Aug 25, 2023


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      It looks like using AllowTypeImports also allows the behavior we were guarding against in #43511.
    
  
  



    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
              
  
    
    
      
        
            
    All reactions
  


          
          
        
      
    


      



    
        
  
    
      
          
    
  


        
            
  
      
              Copy link

  
        
      
    

    
        

  
  Member


        

    
  

  
    
      
          
      

      
            JoostK
  

      

      

      


        Dec 12, 2023


      
    

  


        
      
  
  
  
    

    There was a problem hiding this comment.


  

 
  
    

    Choose a reason for hiding this comment

    
      The reason will be displayed to describe this comment to others. Learn more.
    

    
      
      


  


  
    
      That is somewhat unexpected to me 🤔
    
  
  



    

        
      
  
  
    
  
  
  
    
    Sorry, something went wrong.
  

  
    
  
    
      

              Uh oh!

              
There was an error while loading. Please reload this page.


  
  


          
              
  
    
    
      
        
            
    All reactions
     }
     /**
@@ -229,6 +234,20 @@ export class LocalIdentifierStrategy implements ReferenceEmitStrategy {
           };
         }
+        // If the reference is to an ambient type, it can be referenced directly.
+        if (ref.isAmbient && importFlags & ImportFlags.AllowAmbientReferences) {
+          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
         // Identifier in the requested context file.
         const identifier = ref.getIdentityIn(context);
-          Expand Down