Skip to content

Commit 69e14b5

Browse files
chuckjazmhevery
authored andcommitted
feat(compiler): generate type parameters for generic type references (#14104)
1 parent 1079b93 commit 69e14b5

File tree

11 files changed

+118
-32
lines changed

11 files changed

+118
-32
lines changed

modules/@angular/compiler/src/aot/compiler_factory.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,8 @@ export function createAotCompiler(compilerHost: AotCompilerHost, options: AotCom
7777
const importResolver = {
7878
getImportAs: (symbol: StaticSymbol) => symbolResolver.getImportAs(symbol),
7979
fileNameToModuleName: (fileName: string, containingFilePath: string) =>
80-
compilerHost.fileNameToModuleName(fileName, containingFilePath)
80+
compilerHost.fileNameToModuleName(fileName, containingFilePath),
81+
getTypeArity: (symbol: StaticSymbol) => symbolResolver.getTypeArity(symbol)
8182
};
8283
const compiler = new AotCompiler(
8384
compilerHost, resolver, tmplParser, new StyleCompiler(urlResolver),

modules/@angular/compiler/src/aot/static_symbol_resolver.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {SummaryResolver} from '../summary_resolver';
1010
import {ValueTransformer, visitValue} from '../util';
1111

1212
import {StaticSymbol, StaticSymbolCache} from './static_symbol';
13+
import {isNgFactoryFile} from './util';
1314

1415
export class ResolvedStaticSymbol {
1516
constructor(public symbol: StaticSymbol, public metadata: any) {}
@@ -83,6 +84,15 @@ export class StaticSymbolResolver {
8384
return result;
8485
}
8586

87+
/**
88+
* getImportAs produces a symbol that can be used to import the given symbol.
89+
* The import might be different than the symbol if the symbol is exported from
90+
* a library with a summary; in which case we want to import the symbol from the
91+
* ngfactory re-export instead of directly to avoid introducing a direct dependency
92+
* on an otherwise indirect dependency.
93+
*
94+
* @param staticSymbol the symbol for which to generate a import symbol
95+
*/
8696
getImportAs(staticSymbol: StaticSymbol): StaticSymbol {
8797
if (staticSymbol.members.length) {
8898
const baseSymbol = this.getStaticSymbol(staticSymbol.filePath, staticSymbol.name);
@@ -98,6 +108,25 @@ export class StaticSymbolResolver {
98108
return result;
99109
}
100110

111+
/**
112+
* getTypeArity returns the number of generic type parameters the given symbol
113+
* has. If the symbol is not a type the result is null.
114+
*/
115+
getTypeArity(staticSymbol: StaticSymbol): number /*|null*/ {
116+
// If the file is a factory file, don't resolve the symbol as doing so would
117+
// cause the metadata for an factory file to be loaded which doesn't exist.
118+
// All references to generated classes must include the correct arity whenever
119+
// generating code.
120+
if (isNgFactoryFile(staticSymbol.filePath)) {
121+
return null;
122+
}
123+
let resolvedSymbol = this.resolveSymbol(staticSymbol);
124+
while (resolvedSymbol && resolvedSymbol.metadata instanceof StaticSymbol) {
125+
resolvedSymbol = this.resolveSymbol(resolvedSymbol.metadata);
126+
}
127+
return (resolvedSymbol && resolvedSymbol.metadata && resolvedSymbol.metadata.arity) || null;
128+
}
129+
101130
private _resolveSymbolMembers(staticSymbol: StaticSymbol): ResolvedStaticSymbol {
102131
const members = staticSymbol.members;
103132
const baseResolvedSymbol =
@@ -134,6 +163,7 @@ export class StaticSymbolResolver {
134163
*
135164
* @param declarationFile the absolute path of the file where the symbol is declared
136165
* @param name the name of the type.
166+
* @param members a symbol for a static member of the named type
137167
*/
138168
getStaticSymbol(declarationFile: string, name: string, members?: string[]): StaticSymbol {
139169
return this.staticSymbolCache.get(declarationFile, name, members);

modules/@angular/compiler/src/aot/summary_serializer.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,10 @@ class Serializer extends ValueTransformer {
9494
addOrMergeSummary(summary: Summary<StaticSymbol>) {
9595
let symbolMeta = summary.metadata;
9696
if (symbolMeta && symbolMeta.__symbolic === 'class') {
97-
// For classes, we only keep their statics, but not the metadata
97+
// For classes, we only keep their statics and arity, but not the metadata
9898
// of the class itself as that has been captured already via other summaries
9999
// (e.g. DirectiveSummary, ...).
100-
symbolMeta = {__symbolic: 'class', statics: symbolMeta.statics};
100+
symbolMeta = {__symbolic: 'class', statics: symbolMeta.statics, arity: symbolMeta.arity};
101101
}
102102

103103
let processedSummary = this.processedSummaryBySymbol.get(summary.symbol);
@@ -106,11 +106,11 @@ class Serializer extends ValueTransformer {
106106
this.processedSummaries.push(processedSummary);
107107
this.processedSummaryBySymbol.set(summary.symbol, processedSummary);
108108
}
109-
// Note: == by purpose to compare with undefined!
109+
// Note: == on purpose to compare with undefined!
110110
if (processedSummary.metadata == null && symbolMeta != null) {
111111
processedSummary.metadata = this.processValue(symbolMeta);
112112
}
113-
// Note: == by purpose to compare with undefined!
113+
// Note: == on purpose to compare with undefined!
114114
if (processedSummary.type == null && summary.type != null) {
115115
processedSummary.type = this.processValue(summary.type);
116116
}
@@ -147,7 +147,7 @@ class Serializer extends ValueTransformer {
147147
if (value instanceof StaticSymbol) {
148148
const baseSymbol = this.symbolResolver.getStaticSymbol(value.filePath, value.name);
149149
let index = this.indexBySymbol.get(baseSymbol);
150-
// Note: == by purpose to compare with undefined!
150+
// Note: == on purpose to compare with undefined!
151151
if (index == null) {
152152
index = this.indexBySymbol.size;
153153
this.indexBySymbol.set(baseSymbol, index);

modules/@angular/compiler/src/aot/util.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,19 @@
77
*/
88

99
const STRIP_SRC_FILE_SUFFIXES = /(\.ts|\.d\.ts|\.js|\.jsx|\.tsx)$/;
10+
const NG_FACTORY = /\.ngfactory\./;
1011

1112
export function ngfactoryFilePath(filePath: string): string {
1213
const urlWithSuffix = splitTypescriptSuffix(filePath);
1314
return `${urlWithSuffix[0]}.ngfactory${urlWithSuffix[1]}`;
1415
}
1516

1617
export function stripNgFactory(filePath: string): string {
17-
return filePath.replace(/\.ngfactory\./, '.');
18+
return filePath.replace(NG_FACTORY, '.');
19+
}
20+
21+
export function isNgFactoryFile(filePath: string): boolean {
22+
return NG_FACTORY.test(filePath);
1823
}
1924

2025
export function splitTypescriptSuffix(path: string): string[] {

modules/@angular/compiler/src/output/output_ast.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,7 @@ export class BuiltinType extends Type {
4444
}
4545

4646
export class ExpressionType extends Type {
47-
constructor(
48-
public value: Expression, public typeParams: Type[] = null,
49-
modifiers: TypeModifier[] = null) {
50-
super(modifiers);
51-
}
47+
constructor(public value: Expression, modifiers: TypeModifier[] = null) { super(modifiers); }
5248
visitType(visitor: TypeVisitor, context: any): any {
5349
return visitor.visitExpressionType(this, context);
5450
}
@@ -881,13 +877,12 @@ export function importExpr(id: CompileIdentifierMetadata, typeParams: Type[] = n
881877
export function importType(
882878
id: CompileIdentifierMetadata, typeParams: Type[] = null,
883879
typeModifiers: TypeModifier[] = null): ExpressionType {
884-
return isPresent(id) ? expressionType(importExpr(id), typeParams, typeModifiers) : null;
880+
return isPresent(id) ? expressionType(importExpr(id, typeParams), typeModifiers) : null;
885881
}
886882

887883
export function expressionType(
888-
expr: Expression, typeParams: Type[] = null,
889-
typeModifiers: TypeModifier[] = null): ExpressionType {
890-
return isPresent(expr) ? new ExpressionType(expr, typeParams, typeModifiers) : null;
884+
expr: Expression, typeModifiers: TypeModifier[] = null): ExpressionType {
885+
return isPresent(expr) ? new ExpressionType(expr, typeModifiers) : null;
891886
}
892887

893888
export function literalArr(values: Expression[], type: Type = null): LiteralArrayExpr {

modules/@angular/compiler/src/output/path_util.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,9 @@ export abstract class ImportResolver {
2424
* to generate the import from.
2525
*/
2626
abstract getImportAs(symbol: StaticSymbol): StaticSymbol /*|null*/;
27+
28+
/**
29+
* Determine the airty of a type.
30+
*/
31+
abstract getTypeArity(symbol: StaticSymbol): number /*|null*/;
2732
}

modules/@angular/compiler/src/output/ts_emitter.ts

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ export function debugOutputAstAsTypeScript(ast: o.Statement | o.Expression | o.T
2121
string {
2222
const converter = new _TsEmitterVisitor(_debugFilePath, {
2323
fileNameToModuleName(filePath: string, containingFilePath: string) { return filePath; },
24-
getImportAs(symbol: StaticSymbol) { return null; }
24+
getImportAs(symbol: StaticSymbol) { return null; },
25+
getTypeArity: symbol => null
2526
});
2627
const ctx = EmitterVisitorContext.createRoot([]);
2728
const asts: any[] = Array.isArray(ast) ? ast : [ast];
@@ -65,6 +66,8 @@ export class TypeScriptEmitter implements OutputEmitter {
6566
}
6667

6768
class _TsEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisitor {
69+
private typeExpression = 0;
70+
6871
constructor(private _genFilePath: string, private _importResolver: ImportResolver) {
6972
super(false);
7073
}
@@ -74,7 +77,9 @@ class _TsEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisitor
7477

7578
visitType(t: o.Type, ctx: EmitterVisitorContext, defaultType: string = 'any') {
7679
if (isPresent(t)) {
80+
this.typeExpression++;
7781
t.visitType(this, ctx);
82+
this.typeExpression--;
7883
} else {
7984
ctx.print(defaultType);
8085
}
@@ -149,6 +154,17 @@ class _TsEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisitor
149154
return null;
150155
}
151156

157+
visitInstantiateExpr(ast: o.InstantiateExpr, ctx: EmitterVisitorContext): any {
158+
ctx.print(`new `);
159+
this.typeExpression++;
160+
ast.classExpr.visitExpression(this, ctx);
161+
this.typeExpression--;
162+
ctx.print(`(`);
163+
this.visitAllExpressions(ast.args, ctx, ',');
164+
ctx.print(`)`);
165+
return null;
166+
}
167+
152168
visitDeclareClassStmt(stmt: o.ClassStmt, ctx: EmitterVisitorContext): any {
153169
ctx.pushClass(stmt);
154170
if (ctx.isExportedVar(stmt.name)) {
@@ -157,7 +173,9 @@ class _TsEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisitor
157173
ctx.print(`class ${stmt.name}`);
158174
if (isPresent(stmt.parent)) {
159175
ctx.print(` extends `);
176+
this.typeExpression++;
160177
stmt.parent.visitExpression(this, ctx);
178+
this.typeExpression--;
161179
}
162180
ctx.println(` {`);
163181
ctx.incIndent();
@@ -299,11 +317,6 @@ class _TsEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisitor
299317

300318
visitExpressionType(ast: o.ExpressionType, ctx: EmitterVisitorContext): any {
301319
ast.value.visitExpression(this, ctx);
302-
if (isPresent(ast.typeParams) && ast.typeParams.length > 0) {
303-
ctx.print(`<`);
304-
this.visitAllObjects(type => type.visitType(this, ctx), ast.typeParams, ctx, ',');
305-
ctx.print(`>`);
306-
}
307320
return null;
308321
}
309322

@@ -346,17 +359,24 @@ class _TsEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisitor
346359
}, params, ctx, ',');
347360
}
348361

349-
private _resolveStaticSymbol(value: CompileIdentifierMetadata): StaticSymbol {
362+
private _resolveStaticSymbol(value: CompileIdentifierMetadata):
363+
{name: string, filePath: string, members?: string[], arity?: number} {
350364
const reference = value.reference;
351365
if (!(reference instanceof StaticSymbol)) {
352366
throw new Error(`Internal error: unknown identifier ${JSON.stringify(value)}`);
353367
}
354-
return this._importResolver.getImportAs(reference) || reference;
368+
const arity = this._importResolver.getTypeArity(reference) || undefined;
369+
const importReference = this._importResolver.getImportAs(reference) || reference;
370+
return {
371+
name: importReference.name,
372+
filePath: importReference.filePath,
373+
members: importReference.members, arity
374+
};
355375
}
356376

357377
private _visitIdentifier(
358378
value: CompileIdentifierMetadata, typeParams: o.Type[], ctx: EmitterVisitorContext): void {
359-
const {name, filePath, members} = this._resolveStaticSymbol(value);
379+
const {name, filePath, members, arity} = this._resolveStaticSymbol(value);
360380
if (filePath != this._genFilePath) {
361381
let prefix = this.importsWithPrefixes.get(filePath);
362382
if (isBlank(prefix)) {
@@ -372,10 +392,28 @@ class _TsEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisitor
372392
} else {
373393
ctx.print(name);
374394
}
375-
if (isPresent(typeParams) && typeParams.length > 0) {
376-
ctx.print(`<`);
377-
this.visitAllObjects(type => type.visitType(this, ctx), typeParams, ctx, ',');
378-
ctx.print(`>`);
395+
396+
if (this.typeExpression > 0) {
397+
// If we are in a type expreession that refers to a generic type then supply
398+
// the required type parameters. If there were not enough type parameters
399+
// supplied, supply any as the type. Outside a type expression the reference
400+
// should not supply type parameters and be treated as a simple value reference
401+
// to the constructor function itself.
402+
const suppliedParameters = (typeParams && typeParams.length) || 0;
403+
const additionalParameters = (arity || 0) - suppliedParameters;
404+
if (suppliedParameters > 0 || additionalParameters > 0) {
405+
ctx.print(`<`);
406+
if (suppliedParameters > 0) {
407+
this.visitAllObjects(type => type.visitType(this, ctx), typeParams, ctx, ',');
408+
}
409+
if (additionalParameters > 0) {
410+
for (let i = 0; i < additionalParameters; i++) {
411+
if (i > 0 || suppliedParameters > 0) ctx.print(',');
412+
ctx.print('any');
413+
}
414+
}
415+
ctx.print(`>`);
416+
}
379417
}
380418
}
381419
}

modules/@angular/compiler/test/output/js_emitter_spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class SimpleJsImportGenerator implements ImportResolver {
2727
return importedUrlStr;
2828
}
2929
getImportAs(symbol: StaticSymbol): StaticSymbol { return null; }
30+
getTypeArity(symbol: StaticSymbol): number /*|null*/ { return null; }
3031
}
3132

3233
export function main() {

modules/@angular/compiler/test/output/output_emitter_util.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,4 +264,5 @@ export class SimpleJsImportGenerator implements ImportResolver {
264264
return importedUrlStr;
265265
}
266266
getImportAs(symbol: StaticSymbol): StaticSymbol { return null; }
267+
getTypeArity(symbol: StaticSymbol): number /*|null*/ { return null; }
267268
}

modules/@angular/compiler/test/output/ts_emitter_node_only_spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,4 +75,5 @@ class StubReflectorHost implements StaticSymbolResolverHost {
7575
class StubImportResolver extends ImportResolver {
7676
fileNameToModuleName(importedFilePath: string, containingFilePath: string): string { return ''; }
7777
getImportAs(symbol: StaticSymbol): StaticSymbol { return null; }
78+
getTypeArity(symbol: StaticSymbol): number /*|null*/ { return null; }
7879
}

0 commit comments

Comments
 (0)