iSharkFly-Docs
/
angular-docs-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(compiler): handle type references to namespaced symbols correctly (#36106) 

When the compiler needs to convert a type reference to a value
expression, it may encounter a type that refers to a namespaced symbol.
Such namespaces need to be handled specially as there's various forms
available. Consider a namespace named "ns":

1. One can refer to a namespace by itself: `ns`. A namespace is only
   allowed to be used in a type position if it has been merged with a
   class, but even if this is the case it may not be possible to convert
   that type into a value expression depending on the import form. More
   on this later (case a below)
2. One can refer to a type within the namespace: `ns.Foo`. An import
   needs to be generated to `ns`, from which the `Foo` property can then
   be read.
3. One can refer to a type in a nested namespace within `ns`:
   `ns.Foo.Bar` and possibly even deeper nested. The value
   representation is similar to case 2, but includes additional property
   accesses.

The exact strategy of how to deal with these cases depends on the type
of import used. There's two flavors available:

a. A namespaced import like `import * as ns from 'ns';` that creates
   a local namespace that is irrelevant to the import that needs to be
   generated (as said import would be used instead of the original
   import).

   If the local namespace "ns" itself is referred to in a type position,
   it is invalid to convert it into a value expression. Some JavaScript
   libraries publish a value as default export using `export = MyClass;`
   syntax, however it is illegal to refer to that value using "ns".
   Consequently, such usage in a type position *must* be accompanied by
   an `@Inject` decorator to provide an explicit token.

b. An explicit namespace declaration within a module, that can be
   imported using a named import like `import {ns} from 'ns';` where the
   "ns" module declares a namespace using `declare namespace ns {}`.
   In this case, it's the namespace itself that needs to be imported,
   after which any qualified references into the namespace are converted
   into property accesses.

Before this change, support for namespaces in the type-to-value
conversion was limited and only worked  correctly for a single qualified
name using a namespace import (case 2a). All other cases were either
producing incorrect code or would crash the compiler (case 1a).

Crashing the compiler is not desirable as it does not indicate where
the issue is. Moreover, the result of a type-to-value conversion is
irrelevant when an explicit injection token is provided using `@Inject`,
so referring to a namespace in a type position (case 1) could still be
valid.

This commit introduces logic to the type-to-value conversion to be able
to properly deal with all type references to namespaced symbols.

Fixes #36006
Resolves FW-1995

PR Close #36106
This commit is contained in:
JoostK 2020-03-17 16:23:46 +01:00 committed by atscott
parent 078b0be4dc
commit 4aa4e6fd03
7 changed files with 263 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1436,7 +1436,8 @@ export class Esm2015ReflectionHost extends TypeScriptReflectionHost implements N
local: false,
valueDeclaration: decl.node,
moduleName: decl.viaModule,
name: decl.node.name.text,
importedName: decl.node.name.text,
nestedPath: null,
};
} else {
typeValueReference = {

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

@ -32,7 +32,7 @@ export function expectTypeValueReferencesForParameters(
}
} else if (param.typeValueReference !== null) {
expect(param.typeValueReference.moduleName).toBe(fromModule!);
expect(param.typeValueReference.name).toBe(expected);
expect(param.typeValueReference.importedName).toBe(expected);
}
}
});

16
packages/compiler-cli/src/ngtsc/annotations/src/util.ts
View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import {Expression, ExternalExpr, LiteralExpr, ParseLocation, ParseSourceFile, ParseSourceSpan, R3DependencyMetadata, R3Reference, R3ResolvedDependencyType, WrappedNodeExpr} from '@angular/compiler';
import {Expression, ExternalExpr, LiteralExpr, ParseLocation, ParseSourceFile, ParseSourceSpan, R3DependencyMetadata, R3Reference, R3ResolvedDependencyType, ReadPropExpr, WrappedNodeExpr} from '@angular/compiler';
import * as ts from 'typescript';
import {ErrorCode, FatalDiagnosticError, makeDiagnostic} from '../../diagnostics';
@ -138,7 +138,19 @@ export function valueReferenceToExpression(
return new WrappedNodeExpr(valueRef.expression);
} else {
// TODO(alxhub): this cast is necessary because the g3 typescript version doesn't narrow here.
return new ExternalExpr(valueRef as {moduleName: string, name: string});
const ref = valueRef as {
moduleName: string;
importedName: string;
nestedPath: string[]|null;
};
let importExpr: Expression =
new ExternalExpr({moduleName: ref.moduleName, name: ref.importedName});
if (ref.nestedPath !== null) {
for (const property of ref.nestedPath) {
importExpr = new ReadPropExpr(importExpr, property);
}
}
return importExpr;
}
}

18
packages/compiler-cli/src/ngtsc/reflection/src/host.ts
View File

@ -243,8 +243,24 @@ export type TypeValueReference = {
local: true; expression: ts.Expression; defaultImportStatement: ts.ImportDeclaration | null;
}|{
local: false;
name: string;
/**
* The module specifier from which the `importedName` symbol should be imported.
*/
moduleName: string;
/**
* The name of the top-level symbol that is imported from `moduleName`. If `nestedPath` is also
* present, a nested object is being referenced from the top-level symbol.
*/
importedName: string;
/**
* If present, represents the symbol names that are referenced from the top-level import.
* When `null` or empty, the `importedName` itself is the symbol being referenced.
*/
nestedPath: string[]|null;
valueDeclaration: ts.Declaration;
};

168
packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts
View File

@ -42,31 +42,76 @@ export function typeToValue(
// Look at the local `ts.Symbol`'s declarations and see if it comes from an import
// statement. If so, extract the module specifier and the name of the imported type.
const firstDecl = local.declarations && local.declarations[0];
if (firstDecl !== undefined) {
if (ts.isImportClause(firstDecl) && firstDecl.name !== undefined) {
// This is a default import.
// import Foo from 'foo';
if (firstDecl && ts.isImportClause(firstDecl) && firstDecl.name !== undefined) {
// This is a default import.
return {
local: true,
// Copying the name here ensures the generated references will be correctly transformed along
// with the import.
expression: ts.updateIdentifier(firstDecl.name),
defaultImportStatement: firstDecl.parent,
};
} else if (firstDecl && isImportSource(firstDecl)) {
const origin = extractModuleAndNameFromImport(firstDecl, symbols.importName);
return {local: false, valueDeclaration: decl.valueDeclaration, ...origin};
} else {
const expression = typeNodeToValueExpr(typeNode);
if (expression !== null) {
return {
local: true,
expression,
defaultImportStatement: null,
// Copying the name here ensures the generated references will be correctly transformed
// along with the import.
expression: ts.updateIdentifier(firstDecl.name),
defaultImportStatement: firstDecl.parent,
};
} else if (ts.isImportSpecifier(firstDecl)) {
// The symbol was imported by name
// import {Foo} from 'foo';
// or
// import {Foo as Bar} from 'foo';
// Determine the name to import (`Foo`) from the import specifier, as the symbol names of
// the imported type could refer to a local alias (like `Bar` in the example above).
const importedName = (firstDecl.propertyName || firstDecl.name).text;
// The first symbol name refers to the local name, which is replaced by `importedName` above.
// Any remaining symbol names make up the complete path to the value.
const [_localName, ...nestedPath] = symbols.symbolNames;
const moduleName = extractModuleName(firstDecl.parent.parent.parent);
return {
local: false,
valueDeclaration: decl.valueDeclaration,
moduleName,
importedName,
nestedPath
};
} else if (ts.isNamespaceImport(firstDecl)) {
// The import is a namespace import
// import * as Foo from 'foo';
if (symbols.symbolNames.length === 1) {
// The type refers to the namespace itself, which cannot be represented as a value.
return null;
}
// The first symbol name refers to the local name of the namespace, which is is discarded
// as a new namespace import will be generated. This is followed by the symbol name that needs
// to be imported and any remaining names that constitute the complete path to the value.
const [_ns, importedName, ...nestedPath] = symbols.symbolNames;
const moduleName = extractModuleName(firstDecl.parent.parent);
return {
local: false,
valueDeclaration: decl.valueDeclaration,
moduleName,
importedName,
nestedPath
};
} else {
return null;
}
}
// If the type is not imported, the type reference can be converted into an expression as is.
const expression = typeNodeToValueExpr(typeNode);
if (expression !== null) {
return {
local: true,
expression,
defaultImportStatement: null,
};
} else {
return null;
}
}
/**
@ -88,14 +133,14 @@ export function typeNodeToValueExpr(node: ts.TypeNode): ts.Expression|null {
*
* In the event that the `TypeReference` refers to a locally declared symbol, these will be the
* same. If the `TypeReference` refers to an imported symbol, then `decl` will be the fully resolved
* `ts.Symbol` of the referenced symbol. `local` will be the `ts.Symbol` of the `ts.Identifer` which
* points to the import statement by which the symbol was imported.
* `ts.Symbol` of the referenced symbol. `local` will be the `ts.Symbol` of the `ts.Identifier`
* which points to the import statement by which the symbol was imported.
*
* In the event `typeRef` refers to a default import, an `importName` will also be returned to
* give the identifier name within the current file by which the import is known.
* All symbol names that make up the type reference are returned left-to-right into the
* `symbolNames` array, which is guaranteed to include at least one entry.
*/
function resolveTypeSymbols(typeRef: ts.TypeReferenceNode, checker: ts.TypeChecker):
{local: ts.Symbol, decl: ts.Symbol, importName: string|null}|null {
{local: ts.Symbol, decl: ts.Symbol, symbolNames: string[]}|null {
const typeName = typeRef.typeName;
// typeRefSymbol is the ts.Symbol of the entire type reference.
const typeRefSymbol: ts.Symbol|undefined = checker.getSymbolAtLocation(typeName);
@ -103,32 +148,36 @@ function resolveTypeSymbols(typeRef: ts.TypeReferenceNode, checker: ts.TypeCheck
return null;
}
// local is the ts.Symbol for the local ts.Identifier for the type.
// `local` is the `ts.Symbol` for the local `ts.Identifier` for the type.
// If the type is actually locally declared or is imported by name, for example:
// import {Foo} from './foo';
// then it'll be the same as top. If the type is imported via a namespace import, for example:
// then it'll be the same as `typeRefSymbol`.
//
// If the type is imported via a namespace import, for example:
// import * as foo from './foo';
// and then referenced as:
// constructor(f: foo.Foo)
// then local will be the ts.Symbol of `foo`, whereas top will be the ts.Symbol of `foo.Foo`.
// This allows tracking of the import behind whatever type reference exists.
// then `local` will be the `ts.Symbol` of `foo`, whereas `typeRefSymbol` will be the `ts.Symbol`
// of `foo.Foo`. This allows tracking of the import behind whatever type reference exists.
let local = typeRefSymbol;
let importName: string|null = null;
// TODO(alxhub): this is technically not correct. The user could have any import type with any
// amount of qualification following the imported type:
//
// import * as foo from 'foo'
// constructor(inject: foo.X.Y.Z)
//
// What we really want is the ability to express the arbitrary operation of `.X.Y.Z` on top of
// whatever import we generate for 'foo'. This logic is sufficient for now, though.
if (ts.isQualifiedName(typeName) && ts.isIdentifier(typeName.left) &&
ts.isIdentifier(typeName.right)) {
const localTmp = checker.getSymbolAtLocation(typeName.left);
// Destructure a name like `foo.X.Y.Z` as follows:
// - in `leftMost`, the `ts.Identifier` of the left-most name (`foo`) in the qualified name.
// This identifier is used to resolve the `ts.Symbol` for `local`.
// - in `symbolNames`, all names involved in the qualified path, or a single symbol name if the
// type is not qualified.
let leftMost = typeName;
const symbolNames: string[] = [];
while (ts.isQualifiedName(leftMost)) {
symbolNames.unshift(leftMost.right.text);
leftMost = leftMost.left;
}
symbolNames.unshift(leftMost.text);
if (leftMost !== typeName) {
const localTmp = checker.getSymbolAtLocation(leftMost);
if (localTmp !== undefined) {
local = localTmp;
importName = typeName.right.text;
}
}
@ -137,7 +186,7 @@ function resolveTypeSymbols(typeRef: ts.TypeReferenceNode, checker: ts.TypeCheck
if (typeRefSymbol.flags & ts.SymbolFlags.Alias) {
decl = checker.getAliasedSymbol(typeRefSymbol);
}
return {local, decl, importName};
return {local, decl, symbolNames};
}
function entityNameToValue(node: ts.EntityName): ts.Expression|null {
@ -151,38 +200,9 @@ function entityNameToValue(node: ts.EntityName): ts.Expression|null {
}
}
function isImportSource(node: ts.Declaration): node is(ts.ImportSpecifier | ts.NamespaceImport) {
return ts.isImportSpecifier(node) || ts.isNamespaceImport(node);
}
function extractModuleAndNameFromImport(
node: ts.ImportSpecifier|ts.NamespaceImport|ts.ImportClause,
localName: string|null): {name: string, moduleName: string} {
let name: string;
let moduleSpecifier: ts.Expression;
switch (node.kind) {
case ts.SyntaxKind.ImportSpecifier:
// The symbol was imported by name, in a ts.ImportSpecifier.
name = (node.propertyName || node.name).text;
moduleSpecifier = node.parent.parent.parent.moduleSpecifier;
break;
case ts.SyntaxKind.NamespaceImport:
// The symbol was imported via a namespace import. In this case, the name to use when
// importing it was extracted by resolveTypeSymbols.
if (localName === null) {
// resolveTypeSymbols() should have extracted the correct local name for the import.
throw new Error(`Debug failure: no local name provided for NamespaceImport`);
}
name = localName;
moduleSpecifier = node.parent.parent.moduleSpecifier;
break;
default:
throw new Error(`Unreachable: ${ts.SyntaxKind[(node as ts.Node).kind]}`);
}
if (!ts.isStringLiteral(moduleSpecifier)) {
function extractModuleName(node: ts.ImportDeclaration): string {
if (!ts.isStringLiteral(node.moduleSpecifier)) {
throw new Error('not a module specifier');
}
const moduleName = moduleSpecifier.text;
return {moduleName, name};
return node.moduleSpecifier.text;
}

2
packages/compiler-cli/src/ngtsc/reflection/test/ts_host_spec.ts
View File

@ -464,7 +464,7 @@ runInEachFileSystem(() => {
expect(argExpressionToString(param.typeValueReference.expression)).toEqual(type);
} else if (!param.typeValueReference.local && typeof type !== 'string') {
expect(param.typeValueReference.moduleName).toEqual(type.moduleName);
expect(param.typeValueReference.name).toEqual(type.name);
expect(param.typeValueReference.importedName).toEqual(type.name);
} else {
return fail(`Mismatch between typeValueReference and expected type: ${param.name} / ${
param.typeValueReference.local}`);

134
packages/compiler-cli/test/ngtsc/ngtsc_spec.ts
View File

@ -3880,6 +3880,140 @@ runInEachFileSystem(os => {
expect(jsContents).toMatch(setClassMetadataRegExp('type: i1.Other'));
});
describe('namespace support', () => {
it('should generate correct imports for type references to namespaced symbols using a namespace import',
() => {
env.write(`/node_modules/ns/index.d.ts`, `
export declare class Zero {}
export declare namespace one {
export declare class One {}
}
export declare namespace one.two {
export declare class Two {}
}
`);
env.write(`test.ts`, `
import {Inject, Injectable, InjectionToken} from '@angular/core';
import * as ns from 'ns';
@Injectable()
export class MyService {
constructor(
zero: ns.Zero,
one: ns.one.One,
two: ns.one.two.Two,
) {}
}
`);
env.driveMain();
const jsContents = trim(env.getContents('test.js'));
expect(jsContents).toContain(`import * as i1 from "ns";`);
expect(jsContents).toContain('i0.ɵɵinject(i1.Zero)');
expect(jsContents).toContain('i0.ɵɵinject(i1.one.One)');
expect(jsContents).toContain('i0.ɵɵinject(i1.one.two.Two)');
expect(jsContents).toMatch(setClassMetadataRegExp('type: i1.Zero'));
expect(jsContents).toMatch(setClassMetadataRegExp('type: i1.one.One'));
expect(jsContents).toMatch(setClassMetadataRegExp('type: i1.one.two.Two'));
});
it('should generate correct imports for type references to namespaced symbols using named imports',
() => {
env.write(`/node_modules/ns/index.d.ts`, `
export namespace ns {
export declare class Zero {}
export declare namespace one {
export declare class One {}
}
export declare namespace one.two {
export declare class Two {}
}
}
`);
env.write(`test.ts`, `
import {Inject, Injectable, InjectionToken} from '@angular/core';
import {ns} from 'ns';
import {ns as alias} from 'ns';
@Injectable()
export class MyService {
constructor(
zero: ns.Zero,
one: ns.one.One,
two: ns.one.two.Two,
aliasedZero: alias.Zero,
aliasedOne: alias.one.One,
aliasedTwo: alias.one.two.Two,
) {}
}
`);
env.driveMain();
const jsContents = trim(env.getContents('test.js'));
expect(jsContents).toContain(`import * as i1 from "ns";`);
expect(jsContents)
.toContain(
'i0.ɵɵinject(i1.ns.Zero), ' +
'i0.ɵɵinject(i1.ns.one.One), ' +
'i0.ɵɵinject(i1.ns.one.two.Two), ' +
'i0.ɵɵinject(i1.ns.Zero), ' +
'i0.ɵɵinject(i1.ns.one.One), ' +
'i0.ɵɵinject(i1.ns.one.two.Two)');
expect(jsContents).toMatch(setClassMetadataRegExp('type: i1.ns.Zero'));
expect(jsContents).toMatch(setClassMetadataRegExp('type: i1.ns.one.One'));
expect(jsContents).toMatch(setClassMetadataRegExp('type: i1.ns.one.two.Two'));
});
it('should not error for a namespace import as parameter type when @Inject is used', () => {
env.tsconfig({'strictInjectionParameters': true});
env.write(`/node_modules/foo/index.d.ts`, `
export = Foo;
declare class Foo {}
declare namespace Foo {}
`);
env.write(`test.ts`, `
import {Inject, Injectable, InjectionToken} from '@angular/core';
import * as Foo from 'foo';
export const TOKEN = new InjectionToken<Foo>('Foo');
@Injectable()
export class MyService {
constructor(@Inject(TOKEN) foo: Foo) {}
}
`);
env.driveMain();
const jsContents = trim(env.getContents('test.js'));
expect(jsContents).toContain('i0.ɵɵinject(TOKEN)');
expect(jsContents).toMatch(setClassMetadataRegExp('type: undefined'));
});
it('should error for a namespace import as parameter type used for DI', () => {
env.tsconfig({'strictInjectionParameters': true});
env.write(`/node_modules/foo/index.d.ts`, `
export = Foo;
declare class Foo {}
declare namespace Foo {}
`);
env.write(`test.ts`, `
import {Injectable} from '@angular/core';
import * as Foo from 'foo';
@Injectable()
export class MyService {
constructor(foo: Foo) {}
}
`);
const diags = env.driveDiagnostics();
expect(diags.length).toBe(1);
expect(diags[0].messageText)
.toBe(
`No suitable injection token for parameter 'foo' of class 'MyService'.\nFound Foo`);
});
});
it('should use `undefined` in setClassMetadata if types can\'t be represented as values',
() => {
env.write(`types.ts`, `