cargdev/angular
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(compiler-cli): avoid creating value expressions for symbols from type-only imports (#37912)

Browse Source 
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 #37900

PR Close #37912
This commit is contained in:
JoostK
2020-07-03 20:12:24 +02:00
committed by Andrew Kushnir
parent 9514fd9080
commit 18098d38b8
14 changed files with 524 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
packages/compiler-cli/ngcc/src/host/esm2015_host.ts
View File

@ -10,7 +10,7 @@ import * as ts from 'typescript';
import {absoluteFromSourceFile} from '../../../src/ngtsc/file_system';
import {Logger} from '../../../src/ngtsc/logging';
import {ClassDeclaration, ClassMember, ClassMemberKind, CtorParameter, Declaration, Decorator, EnumMember, isDecoratorIdentifier, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, reflectObjectLiteral, SpecialDeclarationKind, TypeScriptReflectionHost, TypeValueReference} from '../../../src/ngtsc/reflection';
import {ClassDeclaration, ClassMember, ClassMemberKind, CtorParameter, Declaration, Decorator, EnumMember, isDecoratorIdentifier, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, reflectObjectLiteral, SpecialDeclarationKind, TypeScriptReflectionHost, TypeValueReference, TypeValueReferenceKind, ValueUnavailableKind} from '../../../src/ngtsc/reflection';
import {isWithinPackage} from '../analysis/util';
import {BundleProgram} from '../packages/bundle_program';
import {findAll, getNameText, hasNameIdentifier, isDefined, stripDollarSuffix} from '../utils';
@ -1594,7 +1594,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
{decorators: null, typeExpression: null};
const nameNode = node.name;
let typeValueReference: TypeValueReference|null = null;
let typeValueReference: TypeValueReference;
if (typeExpression !== null) {
// `typeExpression` is an expression in a "type" context. Resolve it to a declared value.
// Either it's a reference to an imported type, or a type declared locally. Distinguish the
@ -1603,7 +1603,7 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
if (decl !== null && decl.node !== null && decl.viaModule !== null &&
isNamedDeclaration(decl.node)) {
typeValueReference = {
local: false,
kind: TypeValueReferenceKind.IMPORTED,
valueDeclaration: decl.node,
moduleName: decl.viaModule,
importedName: decl.node.name.text,
@ -1611,11 +1611,16 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
};
} else {
typeValueReference = {
local: true,
kind: TypeValueReferenceKind.LOCAL,
expression: typeExpression,
defaultImportStatement: null,
};
}
} else {
typeValueReference = {
kind: TypeValueReferenceKind.UNAVAILABLE,
reason: {kind: ValueUnavailableKind.MISSING_TYPE},
};
}
return {

4
packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts
View File

@ -10,7 +10,7 @@ import * as ts from 'typescript';
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system';
import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing';
import {MockLogger} from '../../../src/ngtsc/logging/testing';
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost} from '../../../src/ngtsc/reflection';
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost, TypeValueReferenceKind} from '../../../src/ngtsc/reflection';
import {getDeclaration} from '../../../src/ngtsc/testing';
import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
import {CommonJsReflectionHost} from '../../src/host/commonjs_host';
@ -1599,7 +1599,7 @@ exports.MissingClass2 = MissingClass2;
isNamedVariableDeclaration);
const ctrDecorators = host.getConstructorParameters(classNode)!;
const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as {
local: true,
kind: TypeValueReferenceKind.LOCAL,
expression: ts.Identifier,
defaultImportStatement: null,
}).expression;

4
packages/compiler-cli/ngcc/test/host/esm2015_host_import_helper_spec.ts
View File

@ -11,7 +11,7 @@ import * as ts from 'typescript';
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system';
import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing';
import {MockLogger} from '../../../src/ngtsc/logging/testing';
import {ClassMemberKind, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection';
import {ClassMemberKind, isNamedVariableDeclaration, TypeValueReferenceKind} from '../../../src/ngtsc/reflection';
import {getDeclaration} from '../../../src/ngtsc/testing';
import {loadFakeCore, loadTestFiles, loadTsLib} from '../../../test/helpers';
import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
@ -484,7 +484,7 @@ runInEachFileSystem(() => {
isNamedVariableDeclaration);
const ctrDecorators = host.getConstructorParameters(classNode)!;
const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as {
local: true,
kind: TypeValueReferenceKind.LOCAL,
expression: ts.Identifier,
defaultImportStatement: null,
}).expression;

4
packages/compiler-cli/ngcc/test/host/esm5_host_import_helper_spec.ts
View File

@ -10,7 +10,7 @@ import * as ts from 'typescript';
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system';
import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing';
import {MockLogger} from '../../../src/ngtsc/logging/testing';
import {ClassMemberKind, isNamedFunctionDeclaration, isNamedVariableDeclaration} from '../../../src/ngtsc/reflection';
import {ClassMemberKind, isNamedFunctionDeclaration, isNamedVariableDeclaration, TypeValueReferenceKind} from '../../../src/ngtsc/reflection';
import {getDeclaration} from '../../../src/ngtsc/testing';
import {loadFakeCore, loadTestFiles, loadTsLib} from '../../../test/helpers';
import {getIifeBody} from '../../src/host/esm2015_host';
@ -544,7 +544,7 @@ export { AliasedDirective$1 };
isNamedVariableDeclaration);
const ctrDecorators = host.getConstructorParameters(classNode)!;
const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as {
local: true,
kind: TypeValueReferenceKind.LOCAL,
expression: ts.Identifier,
defaultImportStatement: null,
}).expression;

4
packages/compiler-cli/ngcc/test/host/esm5_host_spec.ts
View File

@ -11,7 +11,7 @@ import * as ts from 'typescript';
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system';
import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing';
import {MockLogger} from '../../../src/ngtsc/logging/testing';
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, Decorator, DownleveledEnum, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost} from '../../../src/ngtsc/reflection';
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, Decorator, DownleveledEnum, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost, TypeValueReferenceKind} from '../../../src/ngtsc/reflection';
import {getDeclaration} from '../../../src/ngtsc/testing';
import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
import {DelegatingReflectionHost} from '../../src/host/delegating_host';
@ -1670,7 +1670,7 @@ runInEachFileSystem(() => {
bundle.program, SOME_DIRECTIVE_FILE.name, 'SomeDirective', isNamedVariableDeclaration);
const ctrDecorators = host.getConstructorParameters(classNode)!;
const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as {
local: true,
kind: TypeValueReferenceKind.LOCAL,
expression: ts.Identifier,
defaultImportStatement: null,
}).expression;

4
packages/compiler-cli/ngcc/test/host/umd_host_spec.ts
View File

@ -11,7 +11,7 @@ import * as ts from 'typescript';
import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../../src/ngtsc/file_system';
import {runInEachFileSystem, TestFile} from '../../../src/ngtsc/file_system/testing';
import {MockLogger} from '../../../src/ngtsc/logging/testing';
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, Import, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost} from '../../../src/ngtsc/reflection';
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, Import, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost, TypeValueReferenceKind} from '../../../src/ngtsc/reflection';
import {getDeclaration} from '../../../src/ngtsc/testing';
import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
import {DelegatingReflectionHost} from '../../src/host/delegating_host';
@ -1709,7 +1709,7 @@ runInEachFileSystem(() => {
bundle.program, SOME_DIRECTIVE_FILE.name, 'SomeDirective', isNamedVariableDeclaration);
const ctrDecorators = host.getConstructorParameters(classNode)!;
const identifierOfViewContainerRef = (ctrDecorators[0].typeValueReference! as {
local: true,
kind: TypeValueReferenceKind.LOCAL,
expression: ts.Identifier,
defaultImportStatement: null,
}).expression;

17
packages/compiler-cli/ngcc/test/host/util.ts
View File

@ -1,4 +1,3 @@
/**
* @license
* Copyright Google LLC All Rights Reserved.
@ -7,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import * as ts from 'typescript';
import {CtorParameter} from '../../../src/ngtsc/reflection';
import {CtorParameter, TypeValueReferenceKind} from '../../../src/ngtsc/reflection';
/**
* Check that a given list of `CtorParameter`s has `typeValueReference`s of specific `ts.Identifier`
@ -18,19 +17,21 @@ export function expectTypeValueReferencesForParameters(
parameters!.forEach((param, idx) => {
const expected = expectedParams[idx];
if (expected !== null) {
if (param.typeValueReference === null) {
if (param.typeValueReference.kind === TypeValueReferenceKind.UNAVAILABLE) {
fail(`Incorrect typeValueReference generated, expected ${expected}`);
} else if (param.typeValueReference.local && fromModule !== null) {
} else if (
param.typeValueReference.kind === TypeValueReferenceKind.LOCAL && fromModule !== null) {
fail(`Incorrect typeValueReference generated, expected non-local`);
} else if (!param.typeValueReference.local && fromModule === null) {
} else if (
param.typeValueReference.kind !== TypeValueReferenceKind.LOCAL && fromModule === null) {
fail(`Incorrect typeValueReference generated, expected local`);
} else if (param.typeValueReference.local) {
} else if (param.typeValueReference.kind === TypeValueReferenceKind.LOCAL) {
if (!ts.isIdentifier(param.typeValueReference.expression)) {
fail(`Incorrect typeValueReference generated, expected identifer`);
fail(`Incorrect typeValueReference generated, expected identifier`);
} else {
expect(param.typeValueReference.expression.text).toEqual(expected);
}
} else if (param.typeValueReference !== null) {
} else if (param.typeValueReference.kind === TypeValueReferenceKind.IMPORTED) {
expect(param.typeValueReference.moduleName).toBe(fromModule!);
expect(param.typeValueReference.importedName).toBe(expected);
}