Skip to content

Commit d517a75

Browse files
JoostKalxhub
authored andcommitted
fix(compiler-cli): use originally used module specifier for transform functions (angular#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 angular#52324 PR Close angular#52437
1 parent 96fd73d commit d517a75

File tree

10 files changed

+45
-25
lines changed

10 files changed

+45
-25
lines changed

goldens/public-api/common/index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -615,7 +615,7 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
615615
// (undocumented)
616616
static ngAcceptInputType_height: unknown;
617617
// (undocumented)
618-
static ngAcceptInputType_ngSrc: string | i1_2.SafeValue;
618+
static ngAcceptInputType_ngSrc: string | i0SafeValue;
619619
// (undocumented)
620620
static ngAcceptInputType_priority: unknown;
621621
// (undocumented)

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,10 @@ function parseInputTransformFunction(
779779
// Treat functions with no arguments as `unknown` since returning
780780
// the same value from the transform function is valid.
781781
if (!firstParam) {
782-
return {node, type: ts.factory.createKeywordTypeNode(ts.SyntaxKind.UnknownKeyword)};
782+
return {
783+
node,
784+
type: new Reference(ts.factory.createKeywordTypeNode(ts.SyntaxKind.UnknownKeyword))
785+
};
783786
}
784787

785788
// This should be caught by `noImplicitAny` already, but null check it just in case.
@@ -795,7 +798,8 @@ function parseInputTransformFunction(
795798

796799
assertEmittableInputType(firstParam.type, clazz.getSourceFile(), reflector, refEmitter);
797800

798-
return {node, type: firstParam.type};
801+
const viaModule = value instanceof Reference ? value.bestGuessOwningModule : null;
802+
return {node, type: new Reference(firstParam.type, viaModule)};
799803
}
800804

801805
/**

packages/compiler-cli/src/ngtsc/metadata/src/api.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ export type InputMapping = InputOrOutput&{
140140
/** Metadata for an input's transform function. */
141141
export interface InputTransform {
142142
node: ts.Node;
143-
type: ts.TypeNode;
143+
type: Reference<ts.TypeNode>;
144144
}
145145

146146
/**

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

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

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

1515
import {Context} from './context';
@@ -80,13 +80,17 @@ class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor {
8080
return ts.factory.createTypeLiteralNode([indexSignature]);
8181
}
8282

83-
visitTransplantedType(ast: o.TransplantedType<ts.Node>, context: Context) {
84-
if (!ts.isTypeNode(ast.type)) {
83+
visitTransplantedType(ast: o.TransplantedType<unknown>, context: Context) {
84+
const node = ast.type instanceof Reference ? ast.type.node : ast.type;
85+
if (!ts.isTypeNode(node)) {
8586
throw new Error(`A TransplantedType must wrap a TypeNode`);
8687
}
8788

88-
const emitter = new TypeEmitter(typeRef => this.translateTypeReference(typeRef, context));
89-
return emitter.emitType(ast.type);
89+
const viaModule = ast.type instanceof Reference ? ast.type.bestGuessOwningModule : null;
90+
91+
const emitter =
92+
new TypeEmitter(typeRef => this.translateTypeReference(typeRef, context, viaModule));
93+
return emitter.emitType(node);
9094
}
9195

9296
visitReadVarExpr(ast: o.ReadVarExpr, context: Context): ts.TypeQueryNode {
@@ -255,19 +259,27 @@ class TypeTranslatorVisitor implements o.ExpressionVisitor, o.TypeVisitor {
255259
return typeNode;
256260
}
257261

258-
private translateTypeReference(type: ts.TypeReferenceNode, context: Context): ts.TypeReferenceNode
259-
|null {
262+
private translateTypeReference(
263+
type: ts.TypeReferenceNode, context: Context,
264+
viaModule: OwningModule|null): ts.TypeReferenceNode|null {
260265
const target = ts.isIdentifier(type.typeName) ? type.typeName : type.typeName.right;
261266
const declaration = this.reflector.getDeclarationOfIdentifier(target);
262267
if (declaration === null) {
263268
throw new Error(
264269
`Unable to statically determine the declaration file of type node ${target.text}`);
265270
}
266271

272+
let owningModule = viaModule;
273+
if (declaration.viaModule !== null) {
274+
owningModule = {
275+
specifier: declaration.viaModule,
276+
resolutionContext: type.getSourceFile().fileName,
277+
};
278+
}
279+
280+
const reference = new Reference(declaration.node, owningModule);
267281
const emittedType = this.refEmitter.emit(
268-
new Reference(declaration.node), this.contextFile,
269-
ImportFlags.NoAliasing | ImportFlags.AllowTypeImports |
270-
ImportFlags.AllowRelativeDtsImports);
282+
reference, this.contextFile, ImportFlags.NoAliasing | ImportFlags.AllowTypeImports);
271283

272284
assertSuccessfulReferenceEmit(emittedType, target, 'type');
273285

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,9 @@ export class Environment implements ReferenceEmitEnvironment {
175175
/**
176176
* Generates a `ts.TypeNode` representing a type that is being referenced from a different place
177177
* in the program. Any type references inside the transplanted type will be rewritten so that
178-
* they can be imported in the context fiel.
178+
* they can be imported in the context file.
179179
*/
180-
referenceTransplantedType(type: TransplantedType<ts.TypeNode>): ts.TypeNode {
180+
referenceTransplantedType(type: TransplantedType<Reference<ts.TypeNode>>): ts.TypeNode {
181181
return translateType(
182182
type, this.contextFile, this.reflector, this.refEmitter, this.importManager);
183183
}

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ class TcbDirectiveInputsOp extends TcbOp {
717717
if (this.dir.coercedInputFields.has(fieldName)) {
718718
let type: ts.TypeNode;
719719

720-
if (transformType) {
720+
if (transformType !== null) {
721721
type = this.tcb.env.referenceTransplantedType(new TransplantedType(transformType));
722722
} else {
723723
// The input has a coercion declaration which should be used instead of assigning the
@@ -2067,7 +2067,10 @@ class Scope {
20672067

20682068
interface TcbBoundAttribute {
20692069
attribute: TmplAstBoundAttribute|TmplAstTextAttribute;
2070-
inputs: {fieldName: ClassPropertyName, required: boolean, transformType: ts.TypeNode|null}[];
2070+
inputs:
2071+
{fieldName: ClassPropertyName,
2072+
required: boolean,
2073+
transformType: Reference<ts.TypeNode>|null}[];
20712074
}
20722075

20732076
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ function constructTypeCtorParameter(
150150
/* type */
151151
transform == null ?
152152
tsCreateTypeQueryForCoercedInput(rawType.typeName, classPropertyName) :
153-
transform.type));
153+
transform.type.node));
154154
}
155155
}
156156
if (plainKeys.length > 0) {

packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts

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

1111
import {initMockFileSystem} from '../../file_system/testing';
12+
import {Reference} from '../../imports';
1213
import {TypeCheckingConfig} from '../api';
1314
import {ALL_ENABLED_CONFIG, tcb, TestDeclaration, TestDirective} from '../testing';
1415

@@ -611,10 +612,10 @@ describe('type check blocks', () => {
611612
transform: {
612613
node: ts.factory.createFunctionDeclaration(
613614
undefined, undefined, undefined, undefined, [], undefined, undefined),
614-
type: ts.factory.createUnionTypeNode([
615+
type: new Reference(ts.factory.createUnionTypeNode([
615616
ts.factory.createKeywordTypeNode(ts.SyntaxKind.BooleanKeyword),
616617
ts.factory.createKeywordTypeNode(ts.SyntaxKind.StringKeyword),
617-
])
618+
]))
618619
},
619620
},
620621
},

packages/compiler-cli/src/ngtsc/typecheck/test/type_constructor_spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,10 +171,10 @@ TestClass.ngTypeCtor({value: 'test'});
171171
bindingPropertyName: 'baz',
172172
required: false,
173173
transform: {
174-
type: ts.factory.createUnionTypeNode([
174+
type: new Reference(ts.factory.createUnionTypeNode([
175175
ts.factory.createKeywordTypeNode(ts.SyntaxKind.BooleanKeyword),
176176
ts.factory.createKeywordTypeNode(ts.SyntaxKind.StringKeyword),
177-
]),
177+
])),
178178
node: ts.factory.createFunctionDeclaration(
179179
undefined, undefined, undefined, undefined, [], undefined, undefined)
180180
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8713,7 +8713,7 @@ function allTests(os: string) {
87138713
expect(jsContents).toContain(`import { externalToNumber } from 'external';`);
87148714
expect(jsContents).toContain('inputs: { value: ["value", "value", externalToNumber] }');
87158715
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
8716-
expect(dtsContents).toContain('import * as i1 from "./node_modules/external/index";');
8716+
expect(dtsContents).toContain('import * as i1 from "external";');
87178717
expect(dtsContents).toContain('static ngAcceptInputType_value: i1.ExternalToNumberType;');
87188718
});
87198719

@@ -8744,7 +8744,7 @@ function allTests(os: string) {
87448744
expect(jsContents)
87458745
.toContain('inputs: { value: ["value", "value", (value) => value ? 1 : 0] }');
87468746
expect(jsContents).toContain('features: [i0.ɵɵInputTransformsFeature]');
8747-
expect(dtsContents).toContain('import * as i1 from "./node_modules/external/index";');
8747+
expect(dtsContents).toContain('import * as i1 from "external";');
87488748
expect(dtsContents).toContain('static ngAcceptInputType_value: i1.ExternalToNumberType;');
87498749
});
87508750

0 commit comments

Comments
 (0)