diff --git a/packages/compiler/src/view_compiler/view_compiler.ts b/packages/compiler/src/view_compiler/view_compiler.ts index 2651eee983..2d09942982 100644 --- a/packages/compiler/src/view_compiler/view_compiler.ts +++ b/packages/compiler/src/view_compiler/view_compiler.ts @@ -95,22 +95,19 @@ interface UpdateExpression { value: AST; } -const LOG_VAR = o.variable('log'); -const VIEW_VAR = o.variable('view'); -const CHECK_VAR = o.variable('check'); -const COMP_VAR = o.variable('comp'); -const NODE_INDEX_VAR = o.variable('nodeIndex'); -const EVENT_NAME_VAR = o.variable('eventName'); -const ALLOW_DEFAULT_VAR = o.variable(`allowDefault`); +const LOG_VAR = o.variable('l'); +const VIEW_VAR = o.variable('v'); +const CHECK_VAR = o.variable('ck'); +const COMP_VAR = o.variable('co'); +const EVENT_NAME_VAR = o.variable('en'); +const ALLOW_DEFAULT_VAR = o.variable(`ad`); class ViewBuilder implements TemplateAstVisitor, LocalResolver { private compType: o.Type; private nodes: (() => { sourceSpan: ParseSourceSpan, nodeDef: o.Expression, - directive?: CompileTypeMetadata, - updateDirectives?: UpdateExpression[], - updateRenderer?: UpdateExpression[] + nodeFlags: NodeFlags, updateDirectives?: UpdateExpression[], updateRenderer?: UpdateExpression[] })[] = []; private purePipeNodeIndices: {[pipeName: string]: number} = Object.create(null); // Need Object.create so that we don't have builtin values... @@ -158,6 +155,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { } this.nodes.push(() => ({ sourceSpan: null, + nodeFlags: flags, nodeDef: o.importExpr(createIdentifier(Identifiers.queryDef)).callFn([ o.literal(flags), o.literal(queryId), new o.LiteralMapExpr( @@ -171,6 +169,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { // if the view is an embedded view, then we need to add an additional root node in some cases this.nodes.push(() => ({ sourceSpan: null, + nodeFlags: NodeFlags.TypeElement, nodeDef: o.importExpr(createIdentifier(Identifiers.anchorDef)).callFn([ o.literal(NodeFlags.None), o.NULL_EXPR, o.NULL_EXPR, o.literal(0) ]) @@ -229,6 +228,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { // ngContentDef(ngContentIndex: number, index: number): NodeDef; this.nodes.push(() => ({ sourceSpan: ast.sourceSpan, + nodeFlags: NodeFlags.TypeNgContent, nodeDef: o.importExpr(createIdentifier(Identifiers.ngContentDef)).callFn([ o.literal(ast.ngContentIndex), o.literal(ast.index) ]) @@ -239,6 +239,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { // textDef(ngContentIndex: number, constants: string[]): NodeDef; this.nodes.push(() => ({ sourceSpan: ast.sourceSpan, + nodeFlags: NodeFlags.TypeText, nodeDef: o.importExpr(createIdentifier(Identifiers.textDef)).callFn([ o.literal(ast.ngContentIndex), o.literalArr([o.literal(ast.value)]) ]) @@ -260,6 +261,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { // textDef(ngContentIndex: number, constants: string[]): NodeDef; this.nodes[nodeIndex] = () => ({ sourceSpan: ast.sourceSpan, + nodeFlags: NodeFlags.TypeText, nodeDef: o.importExpr(createIdentifier(Identifiers.textDef)).callFn([ o.literal(ast.ngContentIndex), o.literalArr(inter.strings.map(s => o.literal(s))) ]), @@ -286,6 +288,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { // ViewDefinitionFactory): NodeDef; this.nodes[nodeIndex] = () => ({ sourceSpan: ast.sourceSpan, + nodeFlags: NodeFlags.TypeElement | flags, nodeDef: o.importExpr(createIdentifier(Identifiers.anchorDef)).callFn([ o.literal(flags), queryMatchesExpr, @@ -360,6 +363,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { // componentView?: () => ViewDefinition, componentRendererType?: RendererType2): NodeDef; this.nodes[nodeIndex] = () => ({ sourceSpan: ast.sourceSpan, + nodeFlags: NodeFlags.TypeElement | flags, nodeDef: o.importExpr(createIdentifier(Identifiers.elementDef)).callFn([ o.literal(flags), queryMatchesExpr, @@ -499,6 +503,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { const bindingType = query.first ? QueryBindingType.First : QueryBindingType.All; this.nodes.push(() => ({ sourceSpan: dirAst.sourceSpan, + nodeFlags: flags, nodeDef: o.importExpr(createIdentifier(Identifiers.queryDef)).callFn([ o.literal(flags), o.literal(queryId), new o.LiteralMapExpr( @@ -576,6 +581,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { // outputs?: {[name: string]: string}, component?: () => ViewDefinition): NodeDef; this.nodes[nodeIndex] = () => ({ sourceSpan: dirAst.sourceSpan, + nodeFlags: NodeFlags.TypeDirective | flags, nodeDef: o.importExpr(createIdentifier(Identifiers.directiveDef)).callFn([ o.literal(flags), queryMatchExprs.length ? o.literalArr(queryMatchExprs) : o.NULL_EXPR, o.literal(childCount), providerExpr, depsExpr, @@ -602,6 +608,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { // value: any, deps: ([DepFlags, any] | any)[]): NodeDef; this.nodes[nodeIndex] = () => ({ sourceSpan: providerAst.sourceSpan, + nodeFlags: flags, nodeDef: o.importExpr(createIdentifier(Identifiers.providerDef)).callFn([ o.literal(flags), queryMatchExprs.length ? o.literalArr(queryMatchExprs) : o.NULL_EXPR, tokenExpr(providerAst.token), providerExpr, depsExpr @@ -678,6 +685,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { this.nodes.push( () => ({ sourceSpan, + nodeFlags: NodeFlags.TypePureArray, nodeDef: o.importExpr(createIdentifier(Identifiers.pureArrayDef)).callFn([o.literal(argCount)]) })); @@ -694,6 +702,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { // function pureObjectDef(propertyNames: string[]): NodeDef this.nodes.push(() => ({ sourceSpan, + nodeFlags: NodeFlags.TypePureObject, nodeDef: o.importExpr(createIdentifier(Identifiers.pureObjectDef)) .callFn([o.literalArr(keys.map(key => o.literal(key)))]) })); @@ -708,6 +717,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { // function purePipeDef(argCount: number): NodeDef; this.nodes.push(() => ({ sourceSpan: expression.sourceSpan, + nodeFlags: NodeFlags.TypePurePipe, nodeDef: o.importExpr(createIdentifier(Identifiers.purePipeDef)) .callFn([o.literal(argCount)]) })); @@ -755,6 +765,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { // flags: NodeFlags, ctor: any, deps: ([DepFlags, any] | any)[]): NodeDef this.nodes.push(() => ({ sourceSpan, + nodeFlags: NodeFlags.TypePipe, nodeDef: o.importExpr(createIdentifier(Identifiers.pipeDef)).callFn([ o.literal(flags), o.importExpr(pipe.type), o.literalArr(depExprs) ]) @@ -792,26 +803,31 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { const updateRendererStmts: o.Statement[] = []; const updateDirectivesStmts: o.Statement[] = []; const nodeDefExprs = this.nodes.map((factory, nodeIndex) => { - const {nodeDef, directive, updateDirectives, updateRenderer, sourceSpan} = factory(); + const {nodeDef, nodeFlags, updateDirectives, updateRenderer, sourceSpan} = factory(); if (updateRenderer) { updateRendererStmts.push( - ...createUpdateStatements(nodeIndex, sourceSpan, updateRenderer, null)); + ...createUpdateStatements(nodeIndex, sourceSpan, updateRenderer, false)); } if (updateDirectives) { - updateDirectivesStmts.push( - ...createUpdateStatements(nodeIndex, sourceSpan, updateDirectives, directive)); + updateDirectivesStmts.push(...createUpdateStatements( + nodeIndex, sourceSpan, updateDirectives, + (nodeFlags & (NodeFlags.DoCheck | NodeFlags.OnInit)) > 0)); } // We use a comma expression to call the log function before // the nodeDef function, but still use the result of the nodeDef function // as the value. - const logWithNodeDef = new o.CommaExpr([LOG_VAR.callFn([]).callFn([]), nodeDef]); + // Note: We only add the logger to elements / text nodes, + // so we don't generate too much code. + const logWithNodeDef = nodeFlags & NodeFlags.CatRenderNode ? + new o.CommaExpr([LOG_VAR.callFn([]).callFn([]), nodeDef]) : + nodeDef; return o.applySourceSpanToExpressionIfNeeded(logWithNodeDef, sourceSpan); }); return {updateRendererStmts, updateDirectivesStmts, nodeDefExprs}; function createUpdateStatements( nodeIndex: number, sourceSpan: ParseSourceSpan, expressions: UpdateExpression[], - directive: CompileTypeMetadata): o.Statement[] { + allowEmptyExprs: boolean): o.Statement[] { const updateStmts: o.Statement[] = []; const exprs = expressions.map(({sourceSpan, context, value}) => { const bindingId = `${updateBindingCount++}`; @@ -822,9 +838,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver { ...stmts.map(stmt => o.applySourceSpanToStatementIfNeeded(stmt, sourceSpan))); return o.applySourceSpanToExpressionIfNeeded(currValExpr, sourceSpan); }); - if (expressions.length || - (directive && (directive.lifecycleHooks.indexOf(LifecycleHooks.DoCheck) !== -1 || - directive.lifecycleHooks.indexOf(LifecycleHooks.OnInit) !== -1))) { + if (expressions.length || allowEmptyExprs) { updateStmts.push(o.applySourceSpanToStatementIfNeeded( callCheckStmt(nodeIndex, exprs).toStmt(), sourceSpan)); } diff --git a/packages/core/src/view/services.ts b/packages/core/src/view/services.ts index 39945d5eec..756c17d460 100644 --- a/packages/core/src/view/services.ts +++ b/packages/core/src/view/services.ts @@ -368,32 +368,46 @@ class DebugContext_ implements DebugContext { renderNode(this.elView, this.elDef); } logError(console: Console, ...values: any[]) { - let logViewFactory: ViewDefinitionFactory; + let logViewDef: ViewDefinition; let logNodeIndex: number; if (this.nodeDef.flags & NodeFlags.TypeText) { - logViewFactory = this.view.def.factory; + logViewDef = this.view.def; logNodeIndex = this.nodeDef.index; } else { - logViewFactory = this.elView.def.factory; + logViewDef = this.elView.def; logNodeIndex = this.elDef.index; } - let currNodeIndex = -1; + // Note: we only generate a log function for text and element nodes + // to make the generated code as small as possible. + const renderNodeIndex = getRenderNodeIndex(logViewDef, logNodeIndex); + let currRenderNodeIndex = -1; let nodeLogger: NodeLogger = () => { - currNodeIndex++; - if (currNodeIndex === logNodeIndex) { + currRenderNodeIndex++; + if (currRenderNodeIndex === renderNodeIndex) { return console.error.bind(console, ...values); } else { return NOOP; } }; - logViewFactory(nodeLogger); - if (currNodeIndex < logNodeIndex) { + logViewDef.factory(nodeLogger); + if (currRenderNodeIndex < renderNodeIndex) { console.error('Illegal state: the ViewDefinitionFactory did not call the logger!'); (console.error)(...values); } } } +function getRenderNodeIndex(viewDef: ViewDefinition, nodeIndex: number): number { + let renderNodeIndex = -1; + for (let i = 0; i <= nodeIndex; i++) { + const nodeDef = viewDef.nodes[i]; + if (nodeDef.flags & NodeFlags.CatRenderNode) { + renderNodeIndex++; + } + } + return renderNodeIndex; +} + function findHostElement(view: ViewData): ElementData { while (view && !isComponentView(view)) { view = view.parent; diff --git a/packages/core/test/linker/source_map_integration_node_only_spec.ts b/packages/core/test/linker/source_map_integration_node_only_spec.ts index 9c3cb368cb..11c7a97fc3 100644 --- a/packages/core/test/linker/source_map_integration_node_only_spec.ts +++ b/packages/core/test/linker/source_map_integration_node_only_spec.ts @@ -10,7 +10,7 @@ import {ResourceLoader} from '@angular/compiler'; import {SourceMap} from '@angular/compiler/src/output/source_map'; import {extractSourceMap, originalPositionFor} from '@angular/compiler/test/output/source_map_util'; import {MockResourceLoader} from '@angular/compiler/testing/src/resource_loader_mock'; -import {Component, Directive, ɵglobal} from '@angular/core'; +import {Attribute, Component, Directive, ɵglobal} from '@angular/core'; import {getErrorLogger} from '@angular/core/src/errors'; import {ComponentFixture, TestBed, fakeAsync, tick} from '@angular/core/testing'; @@ -159,6 +159,37 @@ export function main() { }); })); + it('should report di errors with multiple elements and directives', fakeAsync(() => { + const template = `
`; + + @Component({...templateDecorator(template)}) + class MyComp { + } + + @Directive({selector: '[someDir]'}) + class SomeDir { + constructor(@Attribute('someDir') someDir: string) { + if (someDir === 'throw') { + throw new Error('Test'); + } + } + } + + TestBed.configureTestingModule({declarations: [SomeDir]}); + let error: any; + try { + compileAndCreateComponent(MyComp); + } catch (e) { + error = e; + } + // The error should be logged from the 2nd-element + expect(getSourcePositionForStack(getErrorLoggerStack(error))).toEqual({ + line: 1, + column: 19, + source: ngUrl, + }); + })); + it('should report source location for binding errors', fakeAsync(() => { const template = `
\n
`; diff --git a/packages/core/test/view/services_spec.ts b/packages/core/test/view/services_spec.ts index 66c813a135..5f20d4715c 100644 --- a/packages/core/test/view/services_spec.ts +++ b/packages/core/test/view/services_spec.ts @@ -83,7 +83,6 @@ export function main() { expect(debugCtx.renderNode).toBe(asElementData(compView, 0).renderElement); }); - }); }); }