From fe299f4dfc570941208fe0f04124f3eda97d777e Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Mon, 24 Oct 2016 11:11:31 -0700 Subject: [PATCH] refactor(compiler): minor cleanups --- .../src/compiler_util/identifier_util.ts | 11 +-- .../src/directive_wrapper_compiler.ts | 12 +-- modules/@angular/compiler/src/identifiers.ts | 27 +++--- .../compiler/src/ng_module_compiler.ts | 10 +-- .../src/template_parser/binding_parser.ts | 62 +++++++------ .../src/template_parser/template_parser.ts | 87 ++++++++++--------- .../src/view_compiler/view_builder.ts | 4 +- .../compiler_util/identifier_util_spec.ts | 63 ++++++++++++++ .../@angular/core/src/linker/view_utils.ts | 22 ++--- .../src/tree/ng2_ftl/tree_host.ngfactory.ts | 2 +- .../ng2_static_ftl/tree_root.ngfactory.ts | 2 +- 11 files changed, 184 insertions(+), 118 deletions(-) create mode 100644 modules/@angular/compiler/test/compiler_util/identifier_util_spec.ts diff --git a/modules/@angular/compiler/src/compiler_util/identifier_util.ts b/modules/@angular/compiler/src/compiler_util/identifier_util.ts index 8e83885c2e..3b48dbdc89 100644 --- a/modules/@angular/compiler/src/compiler_util/identifier_util.ts +++ b/modules/@angular/compiler/src/compiler_util/identifier_util.ts @@ -22,13 +22,14 @@ export function createDiTokenExpression(token: CompileTokenMetadata): o.Expressi } } -export function createFastArray(values: o.Expression[]): o.Expression { +export function createInlineArray(values: o.Expression[]): o.Expression { if (values.length === 0) { - return o.importExpr(resolveIdentifier(Identifiers.EMPTY_FAST_ARRAY)); + return o.importExpr(resolveIdentifier(Identifiers.EMPTY_INLINE_ARRAY)); } - const index = Math.ceil(values.length / 2) - 1; - const identifierSpec = index < Identifiers.fastArrays.length ? Identifiers.fastArrays[index] : - Identifiers.FastArrayDynamic; + const log2 = Math.log(values.length) / Math.log(2); + const index = Math.ceil(log2); + const identifierSpec = index < Identifiers.inlineArrays.length ? Identifiers.inlineArrays[index] : + Identifiers.InlineArrayDynamic; const identifier = resolveIdentifier(identifierSpec); return o.importExpr(identifier).instantiate([ o.literal(values.length) diff --git a/modules/@angular/compiler/src/directive_wrapper_compiler.ts b/modules/@angular/compiler/src/directive_wrapper_compiler.ts index 5c78d0d199..e0b8ec4626 100644 --- a/modules/@angular/compiler/src/directive_wrapper_compiler.ts +++ b/modules/@angular/compiler/src/directive_wrapper_compiler.ts @@ -48,9 +48,9 @@ export class DirectiveWrapperCompiler { compile(dirMeta: CompileDirectiveMetadata): DirectiveWrapperCompileResult { const builder = new DirectiveWrapperBuilder(this.compilerConfig, dirMeta); Object.keys(dirMeta.inputs).forEach((inputFieldName) => { - createCheckInputMethod(inputFieldName, builder); + addCheckInputMethod(inputFieldName, builder); }); - createDetectChangesInternalMethod(builder); + addDetectChangesInternalMethod(builder); const classStmt = builder.build(); return new DirectiveWrapperCompileResult([classStmt], classStmt.name); } @@ -102,12 +102,12 @@ class DirectiveWrapperBuilder implements ClassBuilder { return createClassStmt({ name: DirectiveWrapperCompiler.dirWrapperClassName(this.dirMeta.type), ctorParams: dirDepParamNames.map((paramName) => new o.FnParam(paramName, o.DYNAMIC_TYPE)), - builders: [{fields: fields, ctorStmts: ctorStmts}, this] - }) + builders: [{fields, ctorStmts}, this] + }); } } -function createDetectChangesInternalMethod(builder: DirectiveWrapperBuilder) { +function addDetectChangesInternalMethod(builder: DirectiveWrapperBuilder) { const changedVar = o.variable('changed'); const stmts: o.Statement[] = [ changedVar.set(o.THIS_EXPR.prop(CHANGED_FIELD_NAME)).toDeclStmt(), @@ -157,7 +157,7 @@ function createDetectChangesInternalMethod(builder: DirectiveWrapperBuilder) { stmts, o.BOOL_TYPE)); } -function createCheckInputMethod(input: string, builder: DirectiveWrapperBuilder) { +function addCheckInputMethod(input: string, builder: DirectiveWrapperBuilder) { const fieldName = `_${input}`; const fieldExpr = o.THIS_EXPR.prop(fieldName); // private is fine here as no child view will reference the cached value... diff --git a/modules/@angular/compiler/src/identifiers.ts b/modules/@angular/compiler/src/identifiers.ts index 1056b8aaf8..b33443e8b8 100644 --- a/modules/@angular/compiler/src/identifiers.ts +++ b/modules/@angular/compiler/src/identifiers.ts @@ -294,23 +294,24 @@ export class Identifiers { }; // This is just the interface! - static FastArray: - IdentifierSpec = {name: 'FastArray', moduleUrl: VIEW_UTILS_MODULE_URL, runtime: null}; - static fastArrays: IdentifierSpec[] = [ - {name: 'FastArray2', moduleUrl: VIEW_UTILS_MODULE_URL, runtime: view_utils.FastArray2}, - {name: 'FastArray4', moduleUrl: VIEW_UTILS_MODULE_URL, runtime: view_utils.FastArray4}, - {name: 'FastArray8', moduleUrl: VIEW_UTILS_MODULE_URL, runtime: view_utils.FastArray8}, - {name: 'FastArray16', moduleUrl: VIEW_UTILS_MODULE_URL, runtime: view_utils.FastArray16}, + static InlineArray: + IdentifierSpec = {name: 'InlineArray', moduleUrl: VIEW_UTILS_MODULE_URL, runtime: null}; + static inlineArrays: IdentifierSpec[] = [ + {name: 'InlineArray2', moduleUrl: VIEW_UTILS_MODULE_URL, runtime: view_utils.InlineArray2}, + {name: 'InlineArray2', moduleUrl: VIEW_UTILS_MODULE_URL, runtime: view_utils.InlineArray2}, + {name: 'InlineArray4', moduleUrl: VIEW_UTILS_MODULE_URL, runtime: view_utils.InlineArray4}, + {name: 'InlineArray8', moduleUrl: VIEW_UTILS_MODULE_URL, runtime: view_utils.InlineArray8}, + {name: 'InlineArray16', moduleUrl: VIEW_UTILS_MODULE_URL, runtime: view_utils.InlineArray16}, ]; - static EMPTY_FAST_ARRAY: IdentifierSpec = { - name: 'EMPTY_FAST_ARRAY', + static EMPTY_INLINE_ARRAY: IdentifierSpec = { + name: 'EMPTY_INLINE_ARRAY', moduleUrl: VIEW_UTILS_MODULE_URL, - runtime: view_utils.EMPTY_FAST_ARRAY + runtime: view_utils.EMPTY_INLINE_ARRAY }; - static FastArrayDynamic: IdentifierSpec = { - name: 'FastArrayDynamic', + static InlineArrayDynamic: IdentifierSpec = { + name: 'InlineArrayDynamic', moduleUrl: VIEW_UTILS_MODULE_URL, - runtime: view_utils.FastArrayDynamic + runtime: view_utils.InlineArrayDynamic }; } diff --git a/modules/@angular/compiler/src/ng_module_compiler.ts b/modules/@angular/compiler/src/ng_module_compiler.ts index fbc4abb7d0..2a8d39e6c7 100644 --- a/modules/@angular/compiler/src/ng_module_compiler.ts +++ b/modules/@angular/compiler/src/ng_module_compiler.ts @@ -84,14 +84,14 @@ export class NgModuleCompiler { } class _InjectorBuilder implements ClassBuilder { - private _tokens: CompileTokenMetadata[] = []; - private _instances = new Map(); fields: o.ClassField[] = []; - private _createStmts: o.Statement[] = []; - private _destroyStmts: o.Statement[] = []; getters: o.ClassGetter[] = []; methods: o.ClassMethod[] = []; ctorStmts: o.Statement[] = []; + private _tokens: CompileTokenMetadata[] = []; + private _instances = new Map(); + private _createStmts: o.Statement[] = []; + private _destroyStmts: o.Statement[] = []; constructor( private _ngModuleMeta: CompileNgModuleMetadata, @@ -154,7 +154,7 @@ class _InjectorBuilder implements ClassBuilder { parent: o.importExpr( resolveIdentifier(Identifiers.NgModuleInjector), [o.importType(this._ngModuleMeta.type)]), parentArgs: parentArgs, - builders: [{methods: methods}, this] + builders: [{methods}, this] }); } diff --git a/modules/@angular/compiler/src/template_parser/binding_parser.ts b/modules/@angular/compiler/src/template_parser/binding_parser.ts index 4c9199e180..0e0cc3d648 100644 --- a/modules/@angular/compiler/src/template_parser/binding_parser.ts +++ b/modules/@angular/compiler/src/template_parser/binding_parser.ts @@ -28,9 +28,6 @@ const STYLE_PREFIX = 'style'; const ANIMATE_PROP_PREFIX = 'animate-'; -/** - * Type of a parsed property - */ export enum BoundPropertyType { DEFAULT, LITERAL_ATTR, @@ -55,18 +52,17 @@ export class BoundProperty { */ export class BindingParser { pipesByName: Map = new Map(); - errors: ParseError[] = []; constructor( private _exprParser: Parser, private _interpolationConfig: InterpolationConfig, - protected _schemaRegistry: ElementSchemaRegistry, protected _schemas: SchemaMetadata[], - pipes: CompilePipeMetadata[]) { + private _schemaRegistry: ElementSchemaRegistry, private _schemas: SchemaMetadata[], + pipes: CompilePipeMetadata[], private _targetErrors: ParseError[]) { pipes.forEach(pipe => this.pipesByName.set(pipe.name, pipe)); } createDirectiveHostPropertyAsts( - elementName: string, hostProps: {[key: string]: string}, sourceSpan: ParseSourceSpan, - targetPropertyAsts: BoundElementPropertyAst[]) { + elementName: string, hostProps: {[key: string]: string}, + sourceSpan: ParseSourceSpan): BoundElementPropertyAst[] { if (hostProps) { const boundProps: BoundProperty[] = []; Object.keys(hostProps).forEach(propName => { @@ -74,30 +70,30 @@ export class BindingParser { if (typeof expression === 'string') { this.parsePropertyBinding(propName, expression, true, sourceSpan, [], boundProps); } else { - this.reportError( + this._reportError( `Value of the host property binding "${propName}" needs to be a string representing an expression but got "${expression}" (${typeof expression})`, sourceSpan); } }); - boundProps.forEach( - (prop) => { targetPropertyAsts.push(this.createElementPropertyAst(elementName, prop)); }); + return boundProps.map((prop) => this.createElementPropertyAst(elementName, prop)); } } - createDirectiveHostEventAsts( - hostListeners: {[key: string]: string}, sourceSpan: ParseSourceSpan, - targetEventAsts: BoundEventAst[]) { + createDirectiveHostEventAsts(hostListeners: {[key: string]: string}, sourceSpan: ParseSourceSpan): + BoundEventAst[] { if (hostListeners) { + const targetEventAsts: BoundEventAst[] = []; Object.keys(hostListeners).forEach(propName => { const expression = hostListeners[propName]; if (typeof expression === 'string') { this.parseEvent(propName, expression, sourceSpan, [], targetEventAsts); } else { - this.reportError( + this._reportError( `Value of the host listener "${propName}" needs to be a string representing an expression but got "${expression}" (${typeof expression})`, sourceSpan); } }); + return targetEventAsts; } } @@ -115,7 +111,7 @@ export class BindingParser { } return ast; } catch (e) { - this.reportError(`${e}`, sourceSpan); + this._reportError(`${e}`, sourceSpan); return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo); } } @@ -150,10 +146,10 @@ export class BindingParser { } }); bindingsResult.warnings.forEach( - (warning) => { this.reportError(warning, sourceSpan, ParseErrorLevel.WARNING); }); + (warning) => { this._reportError(warning, sourceSpan, ParseErrorLevel.WARNING); }); return bindingsResult.templateBindings; } catch (e) { - this.reportError(`${e}`, sourceSpan); + this._reportError(`${e}`, sourceSpan); return []; } } @@ -163,8 +159,8 @@ export class BindingParser { targetProps: BoundProperty[]) { if (_isAnimationLabel(name)) { name = name.substring(1); - if (isPresent(value) && value.length > 0) { - this.reportError( + if (value) { + this._reportError( `Assigning animation triggers via @prop="exp" attributes with an expression is invalid.` + ` Use property bindings (e.g. [@prop]="exp") or use an attribute without a value (e.g. @prop) instead.`, sourceSpan, ParseErrorLevel.FATAL); @@ -239,7 +235,7 @@ export class BindingParser { this._checkPipes(ast, sourceSpan); return ast; } catch (e) { - this.reportError(`${e}`, sourceSpan); + this._reportError(`${e}`, sourceSpan); return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo); } } @@ -271,7 +267,7 @@ export class BindingParser { `\n1. If '${elementName}' is an Angular component and it has '${boundPropertyName}' input, then verify that it is part of this module.` + `\n2. If '${elementName}' is a Web Component then add "CUSTOM_ELEMENTS_SCHEMA" to the '@NgModule.schemas' of this component to suppress this message.\n`; } - this.reportError(errorMsg, boundProp.sourceSpan); + this._reportError(errorMsg, boundProp.sourceSpan); } } else { if (parts[0] == ATTRIBUTE_PREFIX) { @@ -299,7 +295,7 @@ export class BindingParser { bindingType = PropertyBindingType.Style; securityContext = SecurityContext.STYLE; } else { - this.reportError(`Invalid property name '${boundProp.name}'`, boundProp.sourceSpan); + this._reportError(`Invalid property name '${boundProp.name}'`, boundProp.sourceSpan); bindingType = null; securityContext = null; } @@ -336,13 +332,13 @@ export class BindingParser { break; default: - this.reportError( + this._reportError( `The provided animation output phase value "${phase}" for "@${eventName}" is not supported (use start or done)`, sourceSpan); break; } } else { - this.reportError( + this._reportError( `The animation trigger output event (@${eventName}) is missing its phase value name (start or done are currently supported)`, sourceSpan); } @@ -369,26 +365,26 @@ export class BindingParser { this._reportExpressionParserErrors(ast.errors, sourceSpan); } if (!ast || ast.ast instanceof EmptyExpr) { - this.reportError(`Empty expressions are not allowed`, sourceSpan); + this._reportError(`Empty expressions are not allowed`, sourceSpan); return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo); } this._checkPipes(ast, sourceSpan); return ast; } catch (e) { - this.reportError(`${e}`, sourceSpan); + this._reportError(`${e}`, sourceSpan); return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo); } } - reportError( + private _reportError( message: string, sourceSpan: ParseSourceSpan, level: ParseErrorLevel = ParseErrorLevel.FATAL) { - this.errors.push(new ParseError(sourceSpan, message, level)); + this._targetErrors.push(new ParseError(sourceSpan, message, level)); } private _reportExpressionParserErrors(errors: ParserError[], sourceSpan: ParseSourceSpan) { for (const error of errors) { - this.reportError(error.message, sourceSpan); + this._reportError(error.message, sourceSpan); } } @@ -398,7 +394,7 @@ export class BindingParser { ast.visit(collector); collector.pipes.forEach((pipeName) => { if (!this.pipesByName.has(pipeName)) { - this.reportError(`The pipe '${pipeName}' could not be found`, sourceSpan); + this._reportError(`The pipe '${pipeName}' could not be found`, sourceSpan); } }); } @@ -415,13 +411,13 @@ export class BindingParser { const report = isAttr ? this._schemaRegistry.validateAttribute(propName) : this._schemaRegistry.validateProperty(propName); if (report.error) { - this.reportError(report.msg, sourceSpan, ParseErrorLevel.FATAL); + this._reportError(report.msg, sourceSpan, ParseErrorLevel.FATAL); } } } export class PipeCollector extends RecursiveAstVisitor { - pipes: Set = new Set(); + pipes = new Set(); visitPipe(ast: BindingPipe, context: any): any { this.pipes.add(ast.name); ast.exp.visit(this); diff --git a/modules/@angular/compiler/src/template_parser/template_parser.ts b/modules/@angular/compiler/src/template_parser/template_parser.ts index aa4d31bb2a..8cef2dd4f9 100644 --- a/modules/@angular/compiler/src/template_parser/template_parser.ts +++ b/modules/@angular/compiler/src/template_parser/template_parser.ts @@ -136,11 +136,13 @@ export class TemplateParser { end: component.template.interpolation[1] }; } + const bindingParser = new BindingParser( + this._exprParser, interpolationConfig, this._schemaRegistry, schemas, uniqPipes, errors); const parseVisitor = new TemplateParseVisitor( - providerViewContext, uniqDirectives, uniqPipes, this._exprParser, interpolationConfig, - this._schemaRegistry, schemas); + providerViewContext, uniqDirectives, bindingParser, this._schemaRegistry, schemas, + errors); result = html.visitAll(parseVisitor, htmlAstWithErrors.rootNodes, EMPTY_ELEMENT_CONTEXT); - errors.push(...parseVisitor.errors, ...providerViewContext.errors); + errors.push(...providerViewContext.errors); } else { result = []; } @@ -196,18 +198,15 @@ export class TemplateParser { } } -class TemplateParseVisitor extends BindingParser implements html.Visitor { +class TemplateParseVisitor implements html.Visitor { selectorMatcher = new SelectorMatcher(); - errors: TemplateParseError[] = []; directivesIndex = new Map(); ngContentCount: number = 0; constructor( public providerViewContext: ProviderViewContext, directives: CompileDirectiveMetadata[], - pipes: CompilePipeMetadata[], _exprParser: Parser, interpolationConfig: InterpolationConfig, - _schemaRegistry: ElementSchemaRegistry, schemas: SchemaMetadata[]) { - super(_exprParser, interpolationConfig, _schemaRegistry, schemas, pipes); - + private _bindingParser: BindingParser, private _schemaRegistry: ElementSchemaRegistry, + private _schemas: SchemaMetadata[], private _targetErrors: TemplateParseError[]) { directives.forEach((directive: CompileDirectiveMetadata, index: number) => { const selector = CssSelector.parse(directive.selector); this.selectorMatcher.addSelectables(selector, directive); @@ -221,7 +220,7 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { visitText(text: html.Text, parent: ElementContext): any { const ngContentIndex = parent.findNgContentIndex(TEXT_CSS_SELECTOR); - const expr = this.parseInterpolation(text.value, text.sourceSpan); + const expr = this._bindingParser.parseInterpolation(text.value, text.sourceSpan); if (isPresent(expr)) { return new BoundTextAst(expr, ngContentIndex, text.sourceSpan); } else { @@ -282,12 +281,12 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { const hasTemplateBinding = isPresent(templateBindingsSource); if (hasTemplateBinding) { if (hasInlineTemplates) { - this.reportError( + this._reportError( `Can't have multiple template bindings on one element. Use only one attribute named 'template' or prefixed with *`, attr.sourceSpan); } hasInlineTemplates = true; - this.parseInlineTemplateBinding( + this._bindingParser.parseInlineTemplateBinding( attr.name, templateBindingsSource, attr.sourceSpan, templateMatchableAttrs, templateElementOrDirectiveProps, templateElementVars); } @@ -327,7 +326,7 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { if (preparsedElement.type === PreparsedElementType.NG_CONTENT) { if (element.children && !element.children.every(_isEmptyTextNode)) { - this.reportError(` element cannot have content.`, element.sourceSpan); + this._reportError(` element cannot have content.`, element.sourceSpan); } parsedElement = new NgContentAst( @@ -400,7 +399,7 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { animationInputs.forEach(input => { const name = input.name; if (!triggerLookup.has(name)) { - this.reportError(`Couldn't find an animation entry for "${name}"`, input.sourceSpan); + this._reportError(`Couldn't find an animation entry for "${name}"`, input.sourceSpan); } }); @@ -408,7 +407,7 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { if (output.isAnimation) { const found = animationInputs.find(input => input.name == output.name); if (!found) { - this.reportError( + this._reportError( `Unable to listen on (@${output.name}.${output.phase}) because the animation trigger [@${output.name}] isn't being used on the same element`, output.sourceSpan); } @@ -430,7 +429,7 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { if (bindParts !== null) { hasBinding = true; if (isPresent(bindParts[KW_BIND_IDX])) { - this.parsePropertyBinding( + this._bindingParser.parsePropertyBinding( bindParts[IDENT_KW_IDX], value, false, srcSpan, targetMatchableAttrs, targetProps); } else if (bindParts[KW_LET_IDX]) { @@ -438,7 +437,7 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { const identifier = bindParts[IDENT_KW_IDX]; this._parseVariable(identifier, value, srcSpan, targetVars); } else { - this.reportError(`"let-" is only supported on template elements.`, srcSpan); + this._reportError(`"let-" is only supported on template elements.`, srcSpan); } } else if (bindParts[KW_REF_IDX]) { @@ -446,41 +445,42 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { this._parseReference(identifier, value, srcSpan, targetRefs); } else if (bindParts[KW_ON_IDX]) { - this.parseEvent( + this._bindingParser.parseEvent( bindParts[IDENT_KW_IDX], value, srcSpan, targetMatchableAttrs, targetEvents); } else if (bindParts[KW_BINDON_IDX]) { - this.parsePropertyBinding( + this._bindingParser.parsePropertyBinding( bindParts[IDENT_KW_IDX], value, false, srcSpan, targetMatchableAttrs, targetProps); this._parseAssignmentEvent( bindParts[IDENT_KW_IDX], value, srcSpan, targetMatchableAttrs, targetEvents); } else if (bindParts[KW_AT_IDX]) { - this.parseLiteralAttr(name, value, srcSpan, targetMatchableAttrs, targetProps); + this._bindingParser.parseLiteralAttr( + name, value, srcSpan, targetMatchableAttrs, targetProps); } else if (bindParts[IDENT_BANANA_BOX_IDX]) { - this.parsePropertyBinding( + this._bindingParser.parsePropertyBinding( bindParts[IDENT_BANANA_BOX_IDX], value, false, srcSpan, targetMatchableAttrs, targetProps); this._parseAssignmentEvent( bindParts[IDENT_BANANA_BOX_IDX], value, srcSpan, targetMatchableAttrs, targetEvents); } else if (bindParts[IDENT_PROPERTY_IDX]) { - this.parsePropertyBinding( + this._bindingParser.parsePropertyBinding( bindParts[IDENT_PROPERTY_IDX], value, false, srcSpan, targetMatchableAttrs, targetProps); } else if (bindParts[IDENT_EVENT_IDX]) { - this.parseEvent( + this._bindingParser.parseEvent( bindParts[IDENT_EVENT_IDX], value, srcSpan, targetMatchableAttrs, targetEvents); } } else { - hasBinding = - this.parsePropertyInterpolation(name, value, srcSpan, targetMatchableAttrs, targetProps); + hasBinding = this._bindingParser.parsePropertyInterpolation( + name, value, srcSpan, targetMatchableAttrs, targetProps); } if (!hasBinding) { - this.parseLiteralAttr(name, value, srcSpan, targetMatchableAttrs, targetProps); + this._bindingParser.parseLiteralAttr(name, value, srcSpan, targetMatchableAttrs, targetProps); } return hasBinding; @@ -493,7 +493,7 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { private _parseVariable( identifier: string, value: string, sourceSpan: ParseSourceSpan, targetVars: VariableAst[]) { if (identifier.indexOf('-') > -1) { - this.reportError(`"-" is not allowed in variable names`, sourceSpan); + this._reportError(`"-" is not allowed in variable names`, sourceSpan); } targetVars.push(new VariableAst(identifier, value, sourceSpan)); @@ -503,7 +503,7 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { identifier: string, value: string, sourceSpan: ParseSourceSpan, targetRefs: ElementOrDirectiveRef[]) { if (identifier.indexOf('-') > -1) { - this.reportError(`"-" is not allowed in reference names`, sourceSpan); + this._reportError(`"-" is not allowed in reference names`, sourceSpan); } targetRefs.push(new ElementOrDirectiveRef(identifier, value, sourceSpan)); @@ -512,7 +512,7 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { private _parseAssignmentEvent( name: string, expression: string, sourceSpan: ParseSourceSpan, targetMatchableAttrs: string[][], targetEvents: BoundEventAst[]) { - this.parseEvent( + this._bindingParser.parseEvent( `${name}Change`, `${expression}=$event`, sourceSpan, targetMatchableAttrs, targetEvents); } @@ -548,12 +548,11 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { if (directive.isComponent) { component = directive; } - const hostProperties: BoundElementPropertyAst[] = []; - const hostEvents: BoundEventAst[] = []; const directiveProperties: BoundDirectivePropertyAst[] = []; - this.createDirectiveHostPropertyAsts( - elementName, directive.hostProperties, sourceSpan, hostProperties); - this.createDirectiveHostEventAsts(directive.hostListeners, sourceSpan, hostEvents); + const hostProperties = this._bindingParser.createDirectiveHostPropertyAsts( + elementName, directive.hostProperties, sourceSpan); + const hostEvents = + this._bindingParser.createDirectiveHostEventAsts(directive.hostListeners, sourceSpan); this._createDirectivePropertyAsts(directive.inputs, props, directiveProperties); elementOrDirectiveRefs.forEach((elOrDirRef) => { if ((elOrDirRef.value.length === 0 && directive.isComponent) || @@ -569,7 +568,7 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { elementOrDirectiveRefs.forEach((elOrDirRef) => { if (elOrDirRef.value.length > 0) { if (!matchedReferences.has(elOrDirRef.name)) { - this.reportError( + this._reportError( `There is no directive with "exportAs" set to "${elOrDirRef.value}"`, elOrDirRef.sourceSpan); } @@ -624,7 +623,7 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { props.forEach((prop: BoundProperty) => { if (!prop.isLiteral && !boundDirectivePropsIndex.get(prop.name)) { - boundElementProps.push(this.createElementPropertyAst(elementName, prop)); + boundElementProps.push(this._bindingParser.createElementPropertyAst(elementName, prop)); } }); return boundElementProps; @@ -642,7 +641,7 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { private _assertOnlyOneComponent(directives: DirectiveAst[], sourceSpan: ParseSourceSpan) { const componentTypeNames = this._findComponentDirectiveNames(directives); if (componentTypeNames.length > 1) { - this.reportError(`More than one component: ${componentTypeNames.join(',')}`, sourceSpan); + this._reportError(`More than one component: ${componentTypeNames.join(',')}`, sourceSpan); } } @@ -662,7 +661,7 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { const errorMsg = `'${elName}' is not a known element:\n` + `1. If '${elName}' is an Angular component, then verify that it is part of this module.\n` + `2. If '${elName}' is a Web Component then add "CUSTOM_ELEMENTS_SCHEMA" to the '@NgModule.schemas' of this component to suppress this message.`; - this.reportError(errorMsg, element.sourceSpan); + this._reportError(errorMsg, element.sourceSpan); } } @@ -671,11 +670,11 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { sourceSpan: ParseSourceSpan) { const componentTypeNames: string[] = this._findComponentDirectiveNames(directives); if (componentTypeNames.length > 0) { - this.reportError( + this._reportError( `Components on an embedded template: ${componentTypeNames.join(',')}`, sourceSpan); } elementProps.forEach(prop => { - this.reportError( + this._reportError( `Property binding ${prop.name} not used by any directive on an embedded template. Make sure that the property name is spelled correctly and all directives are listed in the "directives" section.`, sourceSpan); }); @@ -694,12 +693,18 @@ class TemplateParseVisitor extends BindingParser implements html.Visitor { events.forEach(event => { if (isPresent(event.target) || !allDirectiveEvents.has(event.name)) { - this.reportError( + this._reportError( `Event binding ${event.fullName} not emitted by any directive on an embedded template. Make sure that the event name is spelled correctly and all directives are listed in the "directives" section.`, event.sourceSpan); } }); } + + private _reportError( + message: string, sourceSpan: ParseSourceSpan, + level: ParseErrorLevel = ParseErrorLevel.FATAL) { + this._targetErrors.push(new ParseError(sourceSpan, message, level)); + } } class NonBindableVisitor implements html.Visitor { diff --git a/modules/@angular/compiler/src/view_compiler/view_builder.ts b/modules/@angular/compiler/src/view_compiler/view_builder.ts index c5fefa7e6a..fc667dbbb0 100644 --- a/modules/@angular/compiler/src/view_compiler/view_builder.ts +++ b/modules/@angular/compiler/src/view_compiler/view_builder.ts @@ -10,7 +10,7 @@ import {ViewEncapsulation} from '@angular/core'; import {CompileDirectiveMetadata, CompileIdentifierMetadata, CompileTokenMetadata} from '../compile_metadata'; import {createSharedBindingVariablesIfNeeded} from '../compiler_util/expression_converter'; -import {createDiTokenExpression, createFastArray} from '../compiler_util/identifier_util'; +import {createDiTokenExpression, createInlineArray} from '../compiler_util/identifier_util'; import {isPresent} from '../facade/lang'; import {Identifiers, identifierToken, resolveIdentifier} from '../identifiers'; import {createClassStmt} from '../output/class_builder'; @@ -167,7 +167,7 @@ class ViewBuilderVisitor implements TemplateAstVisitor { 'createTemplateAnchor', [this._getParentRenderNode(parent), debugContextExpr]); } else { const htmlAttrs = _readHtmlAttrs(ast.attrs); - const attrNameAndValues = createFastArray( + const attrNameAndValues = createInlineArray( _mergeHtmlAndDirectiveAttrs(htmlAttrs, directives).map(v => o.literal(v))); if (nodeIndex === 0 && this.view.viewType === ViewType.HOST) { createRenderNodeExpr = diff --git a/modules/@angular/compiler/test/compiler_util/identifier_util_spec.ts b/modules/@angular/compiler/test/compiler_util/identifier_util_spec.ts new file mode 100644 index 0000000000..381337aaae --- /dev/null +++ b/modules/@angular/compiler/test/compiler_util/identifier_util_spec.ts @@ -0,0 +1,63 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {createInlineArray} from '../../src/compiler_util/identifier_util'; +import {Identifiers, resolveIdentifier} from '../../src/identifiers'; +import * as o from '../../src/output/output_ast'; + +export function main() { + describe('createInlineArray', () => { + + function check(argCount: number, expectedIdentifier: any) { + const args = createArgs(argCount); + expect(createInlineArray(args)) + .toEqual(o.importExpr(resolveIdentifier(expectedIdentifier)).instantiate([ + o.literal(argCount) + ].concat(args))); + } + + function createArgs(count: number): o.Expression[] { + const result: o.Expression[] = []; + for (var i = 0; i < count; i++) { + result.push(o.NULL_EXPR); + } + return result; + } + + it('should work for arrays of length 0', () => { + expect(createInlineArray([ + ])).toEqual(o.importExpr(resolveIdentifier(Identifiers.EMPTY_INLINE_ARRAY))); + }); + + it('should work for arrays of length 1 - 2', () => { + check(1, Identifiers.inlineArrays[0]); + check(2, Identifiers.inlineArrays[1]); + }); + + it('should work for arrays of length 3 - 4', () => { + for (var i = 3; i <= 4; i++) { + check(i, Identifiers.inlineArrays[2]); + } + }); + + it('should work for arrays of length 5 - 8', () => { + for (var i = 5; i <= 8; i++) { + check(i, Identifiers.inlineArrays[3]); + } + }); + + it('should work for arrays of length 9 - 16', () => { + for (var i = 9; i <= 16; i++) { + check(i, Identifiers.inlineArrays[4]); + } + }); + + it('should work for arrays of length > 16', + () => { check(17, Identifiers.InlineArrayDynamic); }); + }); +} diff --git a/modules/@angular/core/src/linker/view_utils.ts b/modules/@angular/core/src/linker/view_utils.ts index 83e380461c..65f9c836fc 100644 --- a/modules/@angular/core/src/linker/view_utils.ts +++ b/modules/@angular/core/src/linker/view_utils.ts @@ -379,7 +379,7 @@ function camelCaseToDashCase(input: string): string { } export function createRenderElement( - renderer: Renderer, parentElement: any, name: string, attrs: FastArray, + renderer: Renderer, parentElement: any, name: string, attrs: InlineArray, debugInfo?: RenderDebugInfo): any { const el = renderer.createElement(parentElement, name, debugInfo); for (var i = 0; i < attrs.length; i += 2) { @@ -389,8 +389,8 @@ export function createRenderElement( } export function selectOrCreateRenderHostElement( - renderer: Renderer, elementName: string, attrs: FastArray, - rootSelectorOrNode: string | any, debugInfo: RenderDebugInfo): any { + renderer: Renderer, elementName: string, attrs: InlineArray, + rootSelectorOrNode: string | any, debugInfo?: RenderDebugInfo): any { var hostElement: any; if (isPresent(rootSelectorOrNode)) { hostElement = renderer.selectRootElement(rootSelectorOrNode, debugInfo); @@ -400,17 +400,17 @@ export function selectOrCreateRenderHostElement( return hostElement; } -export interface FastArray { +export interface InlineArray { length: number; get(index: number): T; } -class FastArray0 implements FastArray { +class InlineArray0 implements InlineArray { length = 0; get(index: number): any { return undefined; } } -export class FastArray2 implements FastArray { +export class InlineArray2 implements InlineArray { constructor(public length: number, private _v0: T, private _v1: T) {} get(index: number) { switch (index) { @@ -424,7 +424,7 @@ export class FastArray2 implements FastArray { } } -export class FastArray4 implements FastArray { +export class InlineArray4 implements InlineArray { constructor( public length: number, private _v0: T, private _v1: T, private _v2: T, private _v3: T) {} get(index: number) { @@ -443,7 +443,7 @@ export class FastArray4 implements FastArray { } } -export class FastArray8 implements FastArray { +export class InlineArray8 implements InlineArray { constructor( public length: number, private _v0: T, private _v1: T, private _v2: T, private _v3: T, private _v4: T, private _v5: T, private _v6: T, private _v7: T) {} @@ -471,7 +471,7 @@ export class FastArray8 implements FastArray { } } -export class FastArray16 implements FastArray { +export class InlineArray16 implements InlineArray { constructor( public length: number, private _v0: T, private _v1: T, private _v2: T, private _v3: T, private _v4: T, private _v5: T, private _v6: T, private _v7: T, private _v8: T, @@ -517,7 +517,7 @@ export class FastArray16 implements FastArray { } } -export class FastArrayDynamic implements FastArray { +export class InlineArrayDynamic implements InlineArray { private _values: any[]; // Note: We still take the length argument so this class can be created // in the same ways as the other classes! @@ -526,4 +526,4 @@ export class FastArrayDynamic implements FastArray { get(index: number) { return this._values[index]; } } -export const EMPTY_FAST_ARRAY: FastArray = new FastArray0(); +export const EMPTY_INLINE_ARRAY: InlineArray = new InlineArray0(); diff --git a/modules/benchmarks/src/tree/ng2_ftl/tree_host.ngfactory.ts b/modules/benchmarks/src/tree/ng2_ftl/tree_host.ngfactory.ts index e7659ffb48..6656047838 100644 --- a/modules/benchmarks/src/tree/ng2_ftl/tree_host.ngfactory.ts +++ b/modules/benchmarks/src/tree/ng2_ftl/tree_host.ngfactory.ts @@ -34,7 +34,7 @@ class _View_TreeComponent_Host0 extends import1.AppView { } createInternal(rootSelector: string): import2.AppElement { this._el_0 = import4.selectOrCreateRenderHostElement( - this.renderer, 'tree', import4.EMPTY_FAST_ARRAY, rootSelector, (null as any)); + this.renderer, 'tree', import4.EMPTY_INLINE_ARRAY, rootSelector, (null as any)); this._vc_0 = new import2.AppElement(0, (null as any), this, this._el_0); this._TreeComponent_0_4 = new _View_TreeComponent0(this._el_0); this._vc_0.initComponent(this._TreeComponent_0_4.context, [], this._TreeComponent_0_4); diff --git a/modules/benchmarks/src/tree/ng2_static_ftl/tree_root.ngfactory.ts b/modules/benchmarks/src/tree/ng2_static_ftl/tree_root.ngfactory.ts index 1573bedda0..225d9df54a 100644 --- a/modules/benchmarks/src/tree/ng2_static_ftl/tree_root.ngfactory.ts +++ b/modules/benchmarks/src/tree/ng2_static_ftl/tree_root.ngfactory.ts @@ -36,7 +36,7 @@ class _View_TreeRootComponent_Host0 extends import1.AppView { } createInternal(rootSelector: string): import2.AppElement { this._el_0 = import4.selectOrCreateRenderHostElement( - this.renderer, 'tree', import4.EMPTY_FAST_ARRAY, rootSelector, (null as any)); + this.renderer, 'tree', import4.EMPTY_INLINE_ARRAY, rootSelector, (null as any)); this._appEl_0 = new import2.AppElement(0, (null as any), this, this._el_0); var compView_0: any = viewFactory_TreeRootComponent0(this.viewUtils, this.injector(0), this._appEl_0);