From 8baff1858b23d452c87b4bda1d6bf1c297345f84 Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Mon, 22 Jan 2018 15:35:18 -0800 Subject: [PATCH] fix(ivy): add names to function expressions (#21714) PR Close #21714 --- .../src/transformers/node_emitter.ts | 11 +++--- .../src/output/abstract_js_emitter.ts | 2 +- packages/compiler/src/output/output_ast.ts | 14 ++++---- packages/compiler/src/output/ts_emitter.ts | 12 +++++-- .../compiler/src/render3/r3_view_compiler.ts | 35 +++++++++++++------ .../test/render3/r3_view_compiler_spec.ts | 32 ++++++++--------- 6 files changed, 63 insertions(+), 43 deletions(-) diff --git a/packages/compiler-cli/src/transformers/node_emitter.ts b/packages/compiler-cli/src/transformers/node_emitter.ts index 8ca656ff90..5f9408750c 100644 --- a/packages/compiler-cli/src/transformers/node_emitter.ts +++ b/packages/compiler-cli/src/transformers/node_emitter.ts @@ -30,10 +30,10 @@ export class TypeScriptNodeEmitter { const commentStmt = this.createCommentStatement(sourceFile, preamble); preambleStmts.push(commentStmt); } - const sourceStatments = + const sourceStatements = [...preambleStmts, ...converter.getReexports(), ...converter.getImports(), ...statements]; - converter.updateSourceMap(sourceStatments); - const newSourceFile = ts.updateSourceFileNode(sourceFile, sourceStatments); + converter.updateSourceMap(sourceStatements); + const newSourceFile = ts.updateSourceFileNode(sourceFile, sourceStatements); return [newSourceFile, converter.getNodeMap()]; } @@ -143,7 +143,7 @@ function firstAfter(a: T[], predicate: (value: T) => boolean) { return index; } -// A recorded node is a subtype of the node that is marked as being recoreded. This is used +// A recorded node is a subtype of the node that is marked as being recorded. This is used // to ensure that NodeEmitterVisitor.record has been called on all nodes returned by the // NodeEmitterVisitor type RecordedNode = (T & { __recorded: any; }) | null; @@ -498,7 +498,8 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor { visitFunctionExpr(expr: FunctionExpr) { return this.record( expr, ts.createFunctionExpression( - /* modifiers */ undefined, /* astriskToken */ undefined, /* name */ undefined, + /* modifiers */ undefined, /* astriskToken */ undefined, + /* name */ expr.name || undefined, /* typeParameters */ undefined, expr.params.map( p => ts.createParameter( diff --git a/packages/compiler/src/output/abstract_js_emitter.ts b/packages/compiler/src/output/abstract_js_emitter.ts index 60ed0b7faf..6a70b78ced 100644 --- a/packages/compiler/src/output/abstract_js_emitter.ts +++ b/packages/compiler/src/output/abstract_js_emitter.ts @@ -107,7 +107,7 @@ export abstract class AbstractJsEmitterVisitor extends AbstractEmitterVisitor { return null; } visitFunctionExpr(ast: o.FunctionExpr, ctx: EmitterVisitorContext): any { - ctx.print(ast, `function(`); + ctx.print(ast, `function${ast.name ? ' ' + ast.name : ''}(`); this._visitParams(ast.params, ctx); ctx.println(ast, `) {`); ctx.incIndent(); diff --git a/packages/compiler/src/output/output_ast.ts b/packages/compiler/src/output/output_ast.ts index 81bd23efa2..e0a672fbc9 100644 --- a/packages/compiler/src/output/output_ast.ts +++ b/packages/compiler/src/output/output_ast.ts @@ -41,7 +41,7 @@ export class BuiltinType extends Type { super(modifiers); } visitType(visitor: TypeVisitor, context: any): any { - return visitor.visitBuiltintType(this, context); + return visitor.visitBuiltinType(this, context); } } @@ -79,7 +79,7 @@ export const STRING_TYPE = new BuiltinType(BuiltinTypeName.String); export const FUNCTION_TYPE = new BuiltinType(BuiltinTypeName.Function); export interface TypeVisitor { - visitBuiltintType(type: BuiltinType, context: any): any; + visitBuiltinType(type: BuiltinType, context: any): any; visitExpressionType(type: ExpressionType, context: any): any; visitArrayType(type: ArrayType, context: any): any; visitMapType(type: MapType, context: any): any; @@ -487,7 +487,7 @@ export class FnParam { export class FunctionExpr extends Expression { constructor( public params: FnParam[], public statements: Statement[], type?: Type|null, - sourceSpan?: ParseSourceSpan|null) { + sourceSpan?: ParseSourceSpan|null, public name?: string|null) { super(type, sourceSpan); } isEquivalent(e: Expression): boolean { @@ -1088,7 +1088,7 @@ export class RecursiveAstVisitor implements StatementVisitor, ExpressionVisitor } return ast; } - visitBuiltintType(type: BuiltinType, context: any): any { return this.visitType(type, context); } + visitBuiltinType(type: BuiltinType, context: any): any { return this.visitType(type, context); } visitExpressionType(type: ExpressionType, context: any): any { type.value.visitExpression(this, context); return this.visitType(type, context); @@ -1369,9 +1369,9 @@ export function assertNotNull( } export function fn( - params: FnParam[], body: Statement[], type?: Type | null, - sourceSpan?: ParseSourceSpan | null): FunctionExpr { - return new FunctionExpr(params, body, type, sourceSpan); + params: FnParam[], body: Statement[], type?: Type | null, sourceSpan?: ParseSourceSpan | null, + name?: string | null): FunctionExpr { + return new FunctionExpr(params, body, type, sourceSpan, name); } export function ifStmt(condition: Expression, thenClause: Statement[], elseClause?: Statement[]) { diff --git a/packages/compiler/src/output/ts_emitter.ts b/packages/compiler/src/output/ts_emitter.ts index 60d38b9307..54df9a3b36 100644 --- a/packages/compiler/src/output/ts_emitter.ts +++ b/packages/compiler/src/output/ts_emitter.ts @@ -269,15 +269,23 @@ class _TsEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisitor } visitFunctionExpr(ast: o.FunctionExpr, ctx: EmitterVisitorContext): any { + if (ast.name) { + ctx.print(ast, 'function '); + ctx.print(ast, ast.name); + } ctx.print(ast, `(`); this._visitParams(ast.params, ctx); ctx.print(ast, `)`); this._printColonType(ast.type, ctx, 'void'); - ctx.println(ast, ` => {`); + if (!ast.name) { + ctx.print(ast, ` => `); + } + ctx.println(ast, '{'); ctx.incIndent(); this.visitAllStatements(ast.statements, ctx); ctx.decIndent(); ctx.print(ast, `}`); + return null; } @@ -314,7 +322,7 @@ class _TsEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisitor return null; } - visitBuiltintType(type: o.BuiltinType, ctx: EmitterVisitorContext): any { + visitBuiltinType(type: o.BuiltinType, ctx: EmitterVisitorContext): any { let typeStr: string; switch (type.name) { case o.BuiltinTypeName.Bool: diff --git a/packages/compiler/src/render3/r3_view_compiler.ts b/packages/compiler/src/render3/r3_view_compiler.ts index ea3ff2e8a3..c66f6354dc 100644 --- a/packages/compiler/src/render3/r3_view_compiler.ts +++ b/packages/compiler/src/render3/r3_view_compiler.ts @@ -20,8 +20,6 @@ import {OutputContext, error} from '../util'; import {Identifiers as R3} from './r3_identifiers'; - - /** Name of the context parameter passed into a template function */ const CONTEXT_NAME = 'ctx'; @@ -89,14 +87,17 @@ export function compileComponent( } } - // e.g. `factory: () => new MyApp(injectElementRef())` + // e.g. `factory: function MyApp_Factory() { return new MyApp(injectElementRef()); }` const templateFactory = createFactory(component.type, outputCtx, reflector); definitionMapValues.push({key: 'factory', value: templateFactory, quoted: false}); - // e.g. `template: function(_ctx, _cm) {...}` + // e.g. `template: function MyComponent_Template(_ctx, _cm) {...}` + const templateTypeName = component.type.reference.name; + const templateName = templateTypeName ? `${templateTypeName}_Template` : null; const templateFunctionExpression = new TemplateDefinitionBuilder( - outputCtx, outputCtx.constantPool, CONTEXT_NAME, ROOT_SCOPE.nestedScope()) + outputCtx, outputCtx.constantPool, reflector, CONTEXT_NAME, ROOT_SCOPE.nestedScope(), 0, + templateTypeName, templateName) .buildTemplateFunction(template); definitionMapValues.push({key: 'template', value: templateFunctionExpression, quoted: false}); @@ -218,7 +219,9 @@ class TemplateDefinitionBuilder implements TemplateAstVisitor, LocalResolver { constructor( private outputCtx: OutputContext, private constantPool: ConstantPool, - private contextParameter: string, private bindingScope: BindingScope, private level = 0) {} + private reflector: CompileReflector, private contextParameter: string, + private bindingScope: BindingScope, private level = 0, private contextName: string|null, + private templateName: string|null) {} buildTemplateFunction(asts: TemplateAst[]): o.FunctionExpr { templateVisitAll(this, asts); @@ -246,7 +249,7 @@ class TemplateDefinitionBuilder implements TemplateAstVisitor, LocalResolver { // Nested templates (i.e. function CompTemplate() {}) ...this._postfix ], - o.INFERRED_TYPE); + o.INFERRED_TYPE, null, this.templateName); } getLocal(name: string): o.Expression|null { return this.bindingScope.get(name); } @@ -407,7 +410,17 @@ class TemplateDefinitionBuilder implements TemplateAstVisitor, LocalResolver { visitEmbeddedTemplate(ast: EmbeddedTemplateAst) { const templateIndex = this.allocateDataSlot(); - const templateName = `C${templateIndex}Template`; + const templateRef = this.reflector.resolveExternalReference(Identifiers.TemplateRef); + const templateDirective = ast.directives.find( + directive => directive.directive.type.diDeps.some( + dependency => + dependency.token != null && (tokenReference(dependency.token) == templateRef))); + const contextName = + this.contextName && templateDirective && templateDirective.directive.type.reference.name ? + `${this.contextName}_${templateDirective.directive.type.reference.name}` : + null; + const templateName = + contextName ? `${contextName}_Template_${templateIndex}` : `Template_${templateIndex}`; const templateContext = `ctx${this.level}`; const {directivesArray, directiveIndexMap} = this._computeDirectivesArray(ast.directives); @@ -430,8 +443,8 @@ class TemplateDefinitionBuilder implements TemplateAstVisitor, LocalResolver { // Create the template function const templateVisitor = new TemplateDefinitionBuilder( - this.outputCtx, this.constantPool, templateContext, this.bindingScope.nestedScope(), - this.level + 1); + this.outputCtx, this.constantPool, this.reflector, templateContext, + this.bindingScope.nestedScope(), this.level + 1, contextName, templateName); const templateFunctionExpr = templateVisitor.buildTemplateFunction(ast.children); this._postfix.push(templateFunctionExpr.toDeclStmt(templateName, null)); } @@ -539,7 +552,7 @@ function createFactory( return o.fn( [], [new o.ReturnStatement(new o.InstantiateExpr(outputCtx.importExpr(type.reference), args))], - o.INFERRED_TYPE); + o.INFERRED_TYPE, null, type.reference.name ? `${type.reference.name}_Factory` : null); } function invalid(arg: o.Expression | o.Statement | TemplateAst): never { diff --git a/packages/compiler/test/render3/r3_view_compiler_spec.ts b/packages/compiler/test/render3/r3_view_compiler_spec.ts index 1f1a49cc7b..57b4342953 100644 --- a/packages/compiler/test/render3/r3_view_compiler_spec.ts +++ b/packages/compiler/test/render3/r3_view_compiler_spec.ts @@ -124,11 +124,11 @@ describe('r3_view_compiler', () => { }; // The factory should look like this: - const factory = 'factory: () => { return new MyComponent(); }'; + const factory = 'factory: function MyComponent_Factory() { return new MyComponent(); }'; // The template should look like this (where IDENT is a wild card for an identifier): const template = ` - template: (ctx: IDENT, cm: IDENT) => { + template: function MyComponent_Template(ctx: IDENT, cm: IDENT) { if (cm) { IDENT.ɵE(0, 'div', IDENT); IDENT.ɵT(1, 'Hello '); @@ -179,8 +179,8 @@ describe('r3_view_compiler', () => { const ChildComponentDefinition = ` static ngComponentDef = IDENT.ɵdefineComponent({ tag: 'child', - factory: () => { return new ChildComponent(); }, - template: (ctx: IDENT, cm: IDENT) => { + factory: function ChildComponent_Factory() { return new ChildComponent(); }, + template: function ChildComponent_Template(ctx: IDENT, cm: IDENT) { if (cm) { IDENT.ɵT(0, 'child-view'); } @@ -190,7 +190,7 @@ describe('r3_view_compiler', () => { // SomeDirective definition should be: const SomeDirectiveDefinition = ` static ngDirectiveDef = IDENT.ɵdefineDirective({ - factory: () => {return new SomeDirective(); } + factory: function SomeDirective_Factory() {return new SomeDirective(); } }); `; @@ -198,8 +198,8 @@ describe('r3_view_compiler', () => { const MyComponentDefinition = ` static ngComponentDef = IDENT.ɵdefineComponent({ tag: 'my-component', - factory: () => { return new MyComponent(); }, - template: (ctx: IDENT, cm: IDENT) => { + factory: function MyComponent_Factory() { return new MyComponent(); }, + template: function MyComponent_Template(ctx: IDENT, cm: IDENT) { if (cm) { IDENT.ɵE(0, ChildComponent, IDENT, IDENT); IDENT.ɵe(); @@ -253,16 +253,16 @@ describe('r3_view_compiler', () => { const IfDirectiveDefinition = ` static ngDirectiveDef = IDENT.ɵdefineDirective({ - factory: () => { return new IfDirective(IDENT.ɵinjectTemplateRef()); } + factory: function IfDirective_Factory() { return new IfDirective(IDENT.ɵinjectTemplateRef()); } });`; const MyComponentDefinition = ` static ngComponentDef = IDENT.ɵdefineComponent({ tag: 'my-component', - factory: () => { return new MyComponent(); }, - template: (ctx: IDENT, cm: IDENT) => { + factory: function MyComponent_Factory() { return new MyComponent(); }, + template: function MyComponent_Template(ctx: IDENT, cm: IDENT) { if (cm) { IDENT.ɵE(0, 'ul', null, null, IDENT); - IDENT.ɵC(2, IDENT, C2Template); + IDENT.ɵC(2, IDENT, MyComponent_IfDirective_Template_2); IDENT.ɵe(); } const IDENT = IDENT.ɵm(1); @@ -270,7 +270,7 @@ describe('r3_view_compiler', () => { IfDirective.ngDirectiveDef.r(3,2); IDENT.ɵcr(); - function C2Template(ctx0: IDENT, cm: IDENT) { + function MyComponent_IfDirective_Template_2(ctx0: IDENT, cm: IDENT) { if (cm) { IDENT.ɵE(0, 'li'); IDENT.ɵT(1); @@ -310,8 +310,8 @@ describe('r3_view_compiler', () => { const MyComponentDefinition = ` static ngComponentDef = IDENT.ɵdefineComponent({ tag: 'my-component', - factory: () => { return new MyComponent(); }, - template: (ctx: IDENT, cm: IDENT) => { + factory: function MyComponent_Factory() { return new MyComponent(); }, + template: function MyComponent_Template(ctx: IDENT, cm: IDENT) { if (cm) { IDENT.ɵE(0, 'input', null, null, IDENT); IDENT.ɵe(); @@ -323,9 +323,7 @@ describe('r3_view_compiler', () => { }); `; - const locals = ` - const IDENT = ['user', '']; - `; + const locals = `const IDENT = ['user', ''];`; const result = compile(files, angularFiles); const source = result.source;