fix(compiler-cli): use originally used module specifier for transform…

… functions (#52437) Prior to this change, the transform function would be referenced with a potentially relative import into an external declaration file. Such imports are not portable and should not be created in this context. This commit addresses the issue by threading though the originally used module specifier by means of the `Reference` type. Fixes #52324 PR Close #52437
angular · Oct 31, 2023 · 873c4f2 · 873c4f2
1 parent b3b4ae4
commit 873c4f2
Show file tree

Hide file tree

Showing 10 changed files with 45 additions and 25 deletions.
diff --git a/goldens/public-api/common/index.md b/goldens/public-api/common/index.md
@@ -617,7 +617,7 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
     // (undocumented)
     static ngAcceptInputType_height: unknown;
     // (undocumented)
-    static ngAcceptInputType_ngSrc: string | i1_2.SafeValue;
+    static ngAcceptInputType_ngSrc: string | i0.ɵSafeValue;
     // (undocumented)
     static ngAcceptInputType_priority: unknown;
     // (undocumented)

diff --git a/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts b/packages/compiler-cli/src/ngtsc/annotations/directive/src/shared.ts
@@ -736,7 +736,10 @@ function parseInputTransformFunction(
   // Treat functions with no arguments as `unknown` since returning
   // the same value from the transform function is valid.
   if (!firstParam) {
-    return {node, type: ts.factory.createKeywordTypeNode(ts.SyntaxKind.UnknownKeyword)};
+    return {
+      node,
+      type: new Reference(ts.factory.createKeywordTypeNode(ts.SyntaxKind.UnknownKeyword))
+    };
   }
 
   // This should be caught by `noImplicitAny` already, but null check it just in case.
@@ -752,7 +755,8 @@ function parseInputTransformFunction(
 
   assertEmittableInputType(firstParam.type, clazz.getSourceFile(), reflector, refEmitter);
 
-  return {node, type: firstParam.type};
+  const viaModule = value instanceof Reference ? value.bestGuessOwningModule : null;
+  return {node, type: new Reference(firstParam.type, viaModule)};
 }
 
 /**

diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/api.ts b/packages/compiler-cli/src/ngtsc/metadata/src/api.ts
@@ -140,7 +140,7 @@ export type InputMapping = InputOrOutput&{
 /** Metadata for an input's transform function. */
 export interface InputTransform {
   node: ts.Node;
-  type: ts.TypeNode;
+  type: Reference<ts.TypeNode>;
 }
 
 /**

diff --git a/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/type_translator.ts
@@ -9,7 +9,7 @@
 import * as o from '@angular/compiler';
 import ts from 'typescript';
 
-import {assertSuccessfulReferenceEmit, ImportFlags, Reference, ReferenceEmitter} from '../../imports';
+import {assertSuccessfulReferenceEmit, ImportFlags, OwningModule, Reference, ReferenceEmitter} from '../../imports';
 import {ReflectionHost} from '../../reflection';
 
 import {Context} from './context';
@@ -80,13 +80,17 @@ class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor {
     return ts.factory.createTypeLiteralNode([indexSignature]);
   }
 
-  visitTransplantedType(ast: o.TransplantedType<ts.Node>, context: Context) {
-    if (!ts.isTypeNode(ast.type)) {
+  visitTransplantedType(ast: o.TransplantedType<unknown>, context: Context) {
+    const node = ast.type instanceof Reference ? ast.type.node : ast.type;
+    if (!ts.isTypeNode(node)) {
       throw new Error(`A TransplantedType must wrap a TypeNode`);
     }
 
-    const emitter = new TypeEmitter(typeRef => this.translateTypeReference(typeRef, context));
-    return emitter.emitType(ast.type);
+    const viaModule = ast.type instanceof Reference ? ast.type.bestGuessOwningModule : null;
+
+    const emitter =
+        new TypeEmitter(typeRef => this.translateTypeReference(typeRef, context, viaModule));
+    return emitter.emitType(node);
   }
 
   visitReadVarExpr(ast: o.ReadVarExpr, context: Context): ts.TypeQueryNode {
@@ -251,19 +255,27 @@ class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor {
     return typeNode;
   }
 
-  private translateTypeReference(type: ts.TypeReferenceNode, context: Context): ts.TypeReferenceNode
-      |null {
+  private translateTypeReference(
+      type: ts.TypeReferenceNode, context: Context,
+      viaModule: OwningModule|null): ts.TypeReferenceNode|null {
     const target = ts.isIdentifier(type.typeName) ? type.typeName : type.typeName.right;
     const declaration = this.reflector.getDeclarationOfIdentifier(target);
     if (declaration === null) {
       throw new Error(
           `Unable to statically determine the declaration file of type node ${target.text}`);
     }
 
+    let owningModule = viaModule;
+    if (declaration.viaModule !== null) {
+      owningModule = {
+        specifier: declaration.viaModule,
+        resolutionContext: type.getSourceFile().fileName,
+      };
+    }
+
+    const reference = new Reference(declaration.node, owningModule);
     const emittedType = this.refEmitter.emit(
-        new Reference(declaration.node), this.contextFile,
-        ImportFlags.NoAliasing | ImportFlags.AllowTypeImports |
-            ImportFlags.AllowRelativeDtsImports);
+        reference, this.contextFile, ImportFlags.NoAliasing | ImportFlags.AllowTypeImports);
 
     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
@@ -175,9 +175,9 @@ export class Environment implements ReferenceEmitEnvironment {
   /**
    * Generates a `ts.TypeNode` representing a type that is being referenced from a different place
    * in the program. Any type references inside the transplanted type will be rewritten so that
-   * they can be imported in the context fiel.
+   * they can be imported in the context file.
    */
-  referenceTransplantedType(type: TransplantedType<ts.TypeNode>): ts.TypeNode {
+  referenceTransplantedType(type: TransplantedType<Reference<ts.TypeNode>>): ts.TypeNode {
     return translateType(
         type, this.contextFile, this.reflector, this.refEmitter, this.importManager);
   }

diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
@@ -716,7 +716,7 @@ class TcbDirectiveInputsOp extends TcbOp {
         if (this.dir.coercedInputFields.has(fieldName)) {
           let type: ts.TypeNode;
 
-          if (transformType) {
+          if (transformType !== null) {
             type = this.tcb.env.referenceTransplantedType(new TransplantedType(transformType));
           } else {
             // The input has a coercion declaration which should be used instead of assigning the
@@ -1672,7 +1672,10 @@ class Scope {
 
 interface TcbBoundAttribute {
   attribute: TmplAstBoundAttribute|TmplAstTextAttribute;
-  inputs: {fieldName: ClassPropertyName, required: boolean, transformType: ts.TypeNode|null}[];
+  inputs:
+      {fieldName: ClassPropertyName,
+       required: boolean,
+       transformType: Reference<ts.TypeNode>|null}[];
 }
 
 /**

diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_constructor.ts
@@ -150,7 +150,7 @@ function constructTypeCtorParameter(
           /* type */
           transform == null ?
               tsCreateTypeQueryForCoercedInput(rawType.typeName, classPropertyName) :
-              transform.type));
+              transform.type.node));
     }
   }
   if (plainKeys.length > 0) {

diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts
@@ -9,6 +9,7 @@
 import ts from 'typescript';
 
 import {initMockFileSystem} from '../../file_system/testing';
+import {Reference} from '../../imports';
 import {TypeCheckingConfig} from '../api';
 import {ALL_ENABLED_CONFIG, tcb, TestDeclaration, TestDirective} from '../testing';
 
@@ -611,10 +612,10 @@ describe('type check blocks', () => {
           transform: {
             node: ts.factory.createFunctionDeclaration(
                 undefined, undefined, undefined, undefined, [], undefined, undefined),
-            type: ts.factory.createUnionTypeNode([
+            type: new Reference(ts.factory.createUnionTypeNode([
               ts.factory.createKeywordTypeNode(ts.SyntaxKind.BooleanKeyword),
               ts.factory.createKeywordTypeNode(ts.SyntaxKind.StringKeyword),
-            ])
+            ]))
           },
         },
       },

diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts
@@ -171,10 +171,10 @@ TestClass.ngTypeCtor({value: 'test'});
                     bindingPropertyName: 'baz',
                     required: false,
                     transform: {
-                      type: ts.factory.createUnionTypeNode([
+                      type: new Reference(ts.factory.createUnionTypeNode([
                         ts.factory.createKeywordTypeNode(ts.SyntaxKind.BooleanKeyword),
                         ts.factory.createKeywordTypeNode(ts.SyntaxKind.StringKeyword),
-                      ]),
+                      ])),
                       node: ts.factory.createFunctionDeclaration(
                           undefined, undefined, undefined, undefined, [], undefined, undefined)
                     }

diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
@@ -8766,7 +8766,7 @@ function allTests(os: string) {
         expect(jsContents).toContain(`import { externalToNumber } from 'external';`);
         expect(jsContents).toContain('inputs: { value: ["value", "value", externalToNumber] }');
         expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
-        expect(dtsContents).toContain('import * as i1 from "./node_modules/external/index";');
+        expect(dtsContents).toContain('import * as i1 from "external";');
         expect(dtsContents).toContain('static ngAcceptInputType_value: i1.ExternalToNumberType;');
       });
 
@@ -8797,7 +8797,7 @@ function allTests(os: string) {
         expect(jsContents)
             .toContain('inputs: { value: ["value", "value", (value) => value ? 1 : 0] }');
         expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
-        expect(dtsContents).toContain('import * as i1 from "./node_modules/external/index";');
+        expect(dtsContents).toContain('import * as i1 from "external";');
         expect(dtsContents).toContain('static ngAcceptInputType_value: i1.ExternalToNumberType;');
       });