Skip to content

Commit 67f5871

Browse files
JoostKprofanis
JoostK
authored and
profanis
committed
fix(compiler-cli): avoid creating value expressions for symbols from type-only imports (angular#37912)
In TypeScript 3.8 support was added for type-only imports, which only brings in the symbol as a type, not their value. The Angular compiler did not yet take the type-only keyword into account when representing symbols in type positions as value expressions. The class metadata that the compiler emits would include the value expression for its parameter types, generating actual imports as necessary. For type-only imports this should not be done, as it introduces an actual import of the module that was originally just a type-only import. This commit lets the compiler deal with type-only imports specially, preventing a value expression from being created. Fixes angular#37900 PR Close angular#37912
1 parent e12bd87 commit 67f5871

File tree

14 files changed

+524
-106
lines changed

14 files changed

+524
-106
lines changed

‎packages/compiler-cli/ngcc/src/host/esm2015_host.ts

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import * as ts from 'typescript';
1010
import {absoluteFromSourceFile} from '../../../src/ngtsc/file_system';
1111

1212
import {Logger} from '../../../src/ngtsc/logging';
13-
import {ClassDeclaration, ClassMember, ClassMemberKind, CtorParameter, Declaration, Decorator, EnumMember, isDecoratorIdentifier, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, reflectObjectLiteral, SpecialDeclarationKind, TypeScriptReflectionHost, TypeValueReference} from '../../../src/ngtsc/reflection';
13+
import {ClassDeclaration, ClassMember, ClassMemberKind, CtorParameter, Declaration, Decorator, EnumMember, isDecoratorIdentifier, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, reflectObjectLiteral, SpecialDeclarationKind, TypeScriptReflectionHost, TypeValueReference, TypeValueReferenceKind, ValueUnavailableKind} from '../../../src/ngtsc/reflection';
1414
import {isWithinPackage} from '../analysis/util';
1515
import {BundleProgram} from '../packages/bundle_program';
1616
import {findAll, getNameText, hasNameIdentifier, isDefined, stripDollarSuffix} from '../utils';
@@ -1594,7 +1594,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
15941594
{decorators: null, typeExpression: null};
15951595
const nameNode = node.name;
15961596

1597-
let typeValueReference: TypeValueReference|null = null;
1597+
let typeValueReference: TypeValueReference;
15981598
if (typeExpression !== null) {
15991599
// `typeExpression` is an expression in a "type" context. Resolve it to a declared value.
16001600
// Either it's a reference to an imported type, or a type declared locally. Distinguish the
@@ -1603,19 +1603,24 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
16031603
if (decl !== null && decl.node !== null && decl.viaModule !== null &&
16041604
isNamedDeclaration(decl.node)) {
16051605
typeValueReference = {
1606-
local: false,
1606+
kind: TypeValueReferenceKind.IMPORTED,
16071607
valueDeclaration: decl.node,
16081608
moduleName: decl.viaModule,
16091609
importedName: decl.node.name.text,
16101610
nestedPath: null,
16111611
};
16121612
} else {
16131613
typeValueReference = {
1614-
local: true,
1614+
kind: TypeValueReferenceKind.LOCAL,
16151615
expression: typeExpression,
16161616
defaultImportStatement: null,
16171617
};
16181618
}
1619+
} else {
1620+
typeValueReference = {
1621+
kind: TypeValueReferenceKind.UNAVAILABLE,
1622+
reason: {kind: ValueUnavailableKind.MISSING_TYPE},
1623+
};
16191624
}
16201625

16211626
return {

‎packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import * as ts from 'typescript';
1010
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system';
1111
import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing';
1212
import {MockLogger} from '../../../src/ngtsc/logging/testing';
13-
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost} from '../../../src/ngtsc/reflection';
13+
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost, TypeValueReferenceKind} from '../../../src/ngtsc/reflection';
1414
import {getDeclaration} from '../../../src/ngtsc/testing';
1515
import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
1616
import {CommonJsReflectionHost} from '../../src/host/commonjs_host';
@@ -1599,7 +1599,7 @@ exports.MissingClass2 = MissingClass2;
15991599
isNamedVariableDeclaration);
16001600
const ctrDecorators = host.getConstructorParameters(classNode)!;
16011601
const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as {
1602-
local: true,
1602+
kind: TypeValueReferenceKind.LOCAL,
16031603
expression: ts.Identifier,
16041604
defaultImportStatement: null,
16051605
}).expression;

‎packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import * as ts from 'typescript';
1111
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system';
1212
import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing';
1313
import {MockLogger} from '../../../src/ngtsc/logging/testing';
14-
import {ClassMemberKind, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection';
14+
import {ClassMemberKind, isNamedVariableDeclaration, TypeValueReferenceKind} from '../../../src/ngtsc/reflection';
1515
import {getDeclaration} from '../../../src/ngtsc/testing';
1616
import {loadFakeCore, loadTestFiles, loadTsLib} from '../../../test/helpers';
1717
import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
@@ -484,7 +484,7 @@ runInEachFileSystem(() => {
484484
isNamedVariableDeclaration);
485485
const ctrDecorators = host.getConstructorParameters(classNode)!;
486486
const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as {
487-
local: true,
487+
kind: TypeValueReferenceKind.LOCAL,
488488
expression: ts.Identifier,
489489
defaultImportStatement: null,
490490
}).expression;

‎packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import * as ts from 'typescript';
1010
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system';
1111
import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing';
1212
import {MockLogger} from '../../../src/ngtsc/logging/testing';
13-
import {ClassMemberKind, isNamedFunctionDeclaration, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection';
13+
import {ClassMemberKind, isNamedFunctionDeclaration, isNamedVariableDeclaration, TypeValueReferenceKind} from '../../../src/ngtsc/reflection';
1414
import {getDeclaration} from '../../../src/ngtsc/testing';
1515
import {loadFakeCore, loadTestFiles, loadTsLib} from '../../../test/helpers';
1616
import {getIifeBody} from '../../src/host/esm2015_host';
@@ -544,7 +544,7 @@ export { AliasedDirective$1 };
544544
isNamedVariableDeclaration);
545545
const ctrDecorators = host.getConstructorParameters(classNode)!;
546546
const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as {
547-
local: true,
547+
kind: TypeValueReferenceKind.LOCAL,
548548
expression: ts.Identifier,
549549
defaultImportStatement: null,
550550
}).expression;

‎packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import * as ts from 'typescript';
1111
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system';
1212
import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing';
1313
import {MockLogger} from '../../../src/ngtsc/logging/testing';
14-
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, Decorator, DownleveledEnum, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost} from '../../../src/ngtsc/reflection';
14+
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, Decorator, DownleveledEnum, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost, TypeValueReferenceKind} from '../../../src/ngtsc/reflection';
1515
import {getDeclaration} from '../../../src/ngtsc/testing';
1616
import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
1717
import {DelegatingReflectionHost} from '../../src/host/delegating_host';
@@ -1670,7 +1670,7 @@ runInEachFileSystem(() => {
16701670
bundle.program, SOME_DIRECTIVE_FILE.name, 'SomeDirective', isNamedVariableDeclaration);
16711671
const ctrDecorators = host.getConstructorParameters(classNode)!;
16721672
const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as {
1673-
local: true,
1673+
kind: TypeValueReferenceKind.LOCAL,
16741674
expression: ts.Identifier,
16751675
defaultImportStatement: null,
16761676
}).expression;

‎packages/compiler-cli/ngcc/test/host/umd_host_spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import * as ts from 'typescript';
1111
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system';
1212
import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing';
1313
import {MockLogger} from '../../../src/ngtsc/logging/testing';
14-
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, Import, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost} from '../../../src/ngtsc/reflection';
14+
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, Import, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost, TypeValueReferenceKind} from '../../../src/ngtsc/reflection';
1515
import {getDeclaration} from '../../../src/ngtsc/testing';
1616
import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
1717
import {DelegatingReflectionHost} from '../../src/host/delegating_host';
@@ -1709,7 +1709,7 @@ runInEachFileSystem(() => {
17091709
bundle.program, SOME_DIRECTIVE_FILE.name, 'SomeDirective', isNamedVariableDeclaration);
17101710
const ctrDecorators = host.getConstructorParameters(classNode)!;
17111711
const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as {
1712-
local: true,
1712+
kind: TypeValueReferenceKind.LOCAL,
17131713
expression: ts.Identifier,
17141714
defaultImportStatement: null,
17151715
}).expression;

‎packages/compiler-cli/ngcc/test/host/util.ts

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
/**
32
* @license
43
* Copyright Google LLC All Rights Reserved.
@@ -7,7 +6,7 @@
76
* found in the LICENSE file at https://angular.io/license
87
*/
98
import * as ts from 'typescript';
10-
import {CtorParameter} from '../../../src/ngtsc/reflection';
9+
import {CtorParameter, TypeValueReferenceKind} from '../../../src/ngtsc/reflection';
1110

1211
/**
1312
* Check that a given list of `CtorParameter`s has `typeValueReference`s of specific `ts.Identifier`
@@ -18,19 +17,21 @@ export function expectTypeValueReferencesForParameters(
1817
parameters!.forEach((param, idx) => {
1918
const expected = expectedParams[idx];
2019
if (expected !== null) {
21-
if (param.typeValueReference === null) {
20+
if (param.typeValueReference.kind === TypeValueReferenceKind.UNAVAILABLE) {
2221
fail(`Incorrect typeValueReference generated, expected ${expected}`);
23-
} else if (param.typeValueReference.local && fromModule !== null) {
22+
} else if (
23+
param.typeValueReference.kind === TypeValueReferenceKind.LOCAL && fromModule !== null) {
2424
fail(`Incorrect typeValueReference generated, expected non-local`);
25-
} else if (!param.typeValueReference.local && fromModule === null) {
25+
} else if (
26+
param.typeValueReference.kind !== TypeValueReferenceKind.LOCAL && fromModule === null) {
2627
fail(`Incorrect typeValueReference generated, expected local`);
27-
} else if (param.typeValueReference.local) {
28+
} else if (param.typeValueReference.kind === TypeValueReferenceKind.LOCAL) {
2829
if (!ts.isIdentifier(param.typeValueReference.expression)) {
29-
fail(`Incorrect typeValueReference generated, expected identifer`);
30+
fail(`Incorrect typeValueReference generated, expected identifier`);
3031
} else {
3132
expect(param.typeValueReference.expression.text).toEqual(expected);
3233
}
33-
} else if (param.typeValueReference !== null) {
34+
} else if (param.typeValueReference.kind === TypeValueReferenceKind.IMPORTED) {
3435
expect(param.typeValueReference.moduleName).toBe(fromModule!);
3536
expect(param.typeValueReference.importedName).toBe(expected);
3637
}

‎packages/compiler-cli/src/ngtsc/annotations/src/metadata.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {Expression, ExternalExpr, FunctionExpr, Identifiers, InvokeFunctionExpr,
1010
import * as ts from 'typescript';
1111

1212
import {DefaultImportRecorder} from '../../imports';
13-
import {CtorParameter, Decorator, ReflectionHost} from '../../reflection';
13+
import {CtorParameter, Decorator, ReflectionHost, TypeValueReferenceKind} from '../../reflection';
1414

1515
import {valueReferenceToExpression, wrapFunctionExpressionsInParens} from './util';
1616

@@ -105,7 +105,7 @@ function ctorParameterToMetadata(
105105
isCore: boolean): Expression {
106106
// Parameters sometimes have a type that can be referenced. If so, then use it, otherwise
107107
// its type is undefined.
108-
const type = param.typeValueReference !== null ?
108+
const type = param.typeValueReference.kind !== TypeValueReferenceKind.UNAVAILABLE ?
109109
valueReferenceToExpression(param.typeValueReference, defaultImportRecorder) :
110110
new LiteralExpr(undefined);
111111

0 commit comments

Comments
 (0)