From 7470ad1bd1540bff3d5bf1ec251b85b011f4c6fa Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Fri, 18 Sep 2015 10:33:23 -0700 Subject: [PATCH] refactor(compiler): various cleanups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - use `$implicit` variable value correctly - handle `ng-non-bindable` correctly - add some more assertions to `TemplateCompiler` - make `CompiledTemplate` const - fix default value for `@Directive.moduleId` - add new compiler to application bindings BREAKING CHANGE: - `Compiler.compileInHost` and all methods of `DynamicComponentLoader` don’t take `Binding` any more, only `Type`s. This is in preparation for the new compiler which does not support this. Part of #3605 Closes #4346 --- .../angular2/src/compiler/command_compiler.ts | 7 +- modules/angular2/src/compiler/compiler.ts | 28 ++++++ modules/angular2/src/compiler/html_parser.ts | 17 +--- .../angular2/src/compiler/style_compiler.ts | 25 ++--- .../src/compiler/template_compiler.ts | 64 +++++++------ .../src/compiler/template_normalizer.ts | 18 +++- .../angular2/src/compiler/template_parser.ts | 82 +++++++++++----- .../src/compiler/template_preparser.ts | 11 +-- modules/angular2/src/compiler/util.ts | 8 +- modules/angular2/src/core/application_ref.ts | 3 +- .../angular2/src/core/compiler/compiler.ts | 6 +- .../core/compiler/dynamic_component_loader.ts | 72 +++++++------- .../src/core/compiler/template_commands.ts | 75 +++++++-------- .../src/core/reflection/reflection.dart | 2 +- .../reflection/reflection_capabilities.dart | 5 +- .../reflection/reflection_capabilities.ts | 2 +- modules/angular2/src/core/render/api.ts | 12 +-- .../angular2/src/test_lib/test_injector.ts | 5 +- .../test/compiler/command_compiler_spec.ts | 49 +++++++--- .../test/compiler/runtime_metadata_spec.ts | 3 +- .../test/compiler/style_compiler_spec.ts | 25 +++-- .../test/compiler/template_compiler_spec.ts | 41 +++----- .../test/compiler/template_normalizer_spec.ts | 21 +++- .../test/compiler/template_parser_spec.ts | 95 ++++++++++++++++++- .../test/compiler/template_preparser_spec.ts | 58 +++++++++++ .../angular2/test/compiler/test_bindings.ts | 23 +---- modules/angular2/test/compiler/util_spec.ts | 13 +++ .../test/core/compiler/integration_spec.ts | 6 +- .../test/core/reflection/reflector_spec.ts | 2 +- 29 files changed, 493 insertions(+), 285 deletions(-) create mode 100644 modules/angular2/test/compiler/template_preparser_spec.ts diff --git a/modules/angular2/src/compiler/command_compiler.ts b/modules/angular2/src/compiler/command_compiler.ts index 400aa110a1..524c63ba4b 100644 --- a/modules/angular2/src/compiler/command_compiler.ts +++ b/modules/angular2/src/compiler/command_compiler.ts @@ -35,7 +35,8 @@ import {escapeSingleQuoteString} from './util'; import {Injectable} from 'angular2/src/core/di'; export var TEMPLATE_COMMANDS_MODULE_REF = moduleRef('angular2/src/core/compiler/template_commands'); -const IMPLICIT_VAR = '%implicit'; + +const IMPLICIT_TEMPLATE_VAR = '\$implicit'; @Injectable() export class CommandCompiler { @@ -207,7 +208,7 @@ class CommandBuilderVisitor implements TemplateAstVisitor { var variableNameAndValues = []; ast.vars.forEach((varAst) => { variableNameAndValues.push(varAst.name); - variableNameAndValues.push(varAst.value); + variableNameAndValues.push(varAst.value.length > 0 ? varAst.value : IMPLICIT_TEMPLATE_VAR); }); var directives = []; ListWrapper.forEachWithIndex(ast.directives, (directiveAst: DirectiveAst, index: number) => { @@ -227,7 +228,7 @@ class CommandBuilderVisitor implements TemplateAstVisitor { if (isBlank(component)) { ast.exportAsVars.forEach((varAst) => { variableNameAndValues.push(varAst.name); - variableNameAndValues.push(IMPLICIT_VAR); + variableNameAndValues.push(null); }); } var directives = []; diff --git a/modules/angular2/src/compiler/compiler.ts b/modules/angular2/src/compiler/compiler.ts index 6d93feb685..02b34da783 100644 --- a/modules/angular2/src/compiler/compiler.ts +++ b/modules/angular2/src/compiler/compiler.ts @@ -5,3 +5,31 @@ export { CompileTemplateMetadata } from './directive_metadata'; export {SourceModule, SourceWithImports} from './source_module'; + +import {assertionsEnabled, Type} from 'angular2/src/core/facade/lang'; +import {bind, Binding} from 'angular2/src/core/di'; +import {TemplateParser} from 'angular2/src/compiler/template_parser'; +import {HtmlParser} from 'angular2/src/compiler/html_parser'; +import {TemplateNormalizer} from 'angular2/src/compiler/template_normalizer'; +import {RuntimeMetadataResolver} from 'angular2/src/compiler/runtime_metadata'; +import {ChangeDetectionCompiler} from 'angular2/src/compiler/change_detector_compiler'; +import {StyleCompiler} from 'angular2/src/compiler/style_compiler'; +import {CommandCompiler} from 'angular2/src/compiler/command_compiler'; +import {TemplateCompiler} from 'angular2/src/compiler/template_compiler'; +import {ChangeDetectorGenConfig} from 'angular2/src/core/change_detection/change_detection'; + +export function compilerBindings(): Array { + return [ + HtmlParser, + TemplateParser, + TemplateNormalizer, + RuntimeMetadataResolver, + StyleCompiler, + CommandCompiler, + ChangeDetectionCompiler, + bind(ChangeDetectorGenConfig) + .toValue( + new ChangeDetectorGenConfig(assertionsEnabled(), assertionsEnabled(), false, true)), + TemplateCompiler, + ]; +} diff --git a/modules/angular2/src/compiler/html_parser.ts b/modules/angular2/src/compiler/html_parser.ts index bac1690ba1..be3e14fe41 100644 --- a/modules/angular2/src/compiler/html_parser.ts +++ b/modules/angular2/src/compiler/html_parser.ts @@ -57,12 +57,7 @@ function parseElement(element: Element, indexInParent: number, parentSourceInfo: var sourceInfo = `${parentSourceInfo} > ${nodeName}:nth-child(${indexInParent})`; var attrs = parseAttrs(element, sourceInfo); - var childNodes; - if (ignoreChildren(attrs)) { - childNodes = []; - } else { - childNodes = parseChildNodes(element, sourceInfo); - } + var childNodes = parseChildNodes(element, sourceInfo); return new HtmlElementAst(nodeName, attrs, childNodes, sourceInfo); } @@ -100,16 +95,6 @@ function parseChildNodes(element: Element, parentSourceInfo: string): HtmlAst[] return result; } -function ignoreChildren(attrs: HtmlAttrAst[]): boolean { - for (var i = 0; i < attrs.length; i++) { - var a = attrs[i]; - if (a.name == NG_NON_BINDABLE) { - return true; - } - } - return false; -} - class UnparseVisitor implements HtmlAstVisitor { visitElement(ast: HtmlElementAst, parts: string[]): any { parts.push(`<${ast.name}`); diff --git a/modules/angular2/src/compiler/style_compiler.ts b/modules/angular2/src/compiler/style_compiler.ts index 5580eb4a3e..64b4d40fea 100644 --- a/modules/angular2/src/compiler/style_compiler.ts +++ b/modules/angular2/src/compiler/style_compiler.ts @@ -1,4 +1,4 @@ -import {CompileTypeMetadata, CompileDirectiveMetadata} from './directive_metadata'; +import {CompileTypeMetadata, CompileTemplateMetadata} from './directive_metadata'; import {SourceModule, SourceExpression, moduleRef} from './source_module'; import {ViewEncapsulation} from 'angular2/src/core/render/api'; import {XHR} from 'angular2/src/core/render/xhr'; @@ -29,27 +29,28 @@ export class StyleCompiler { constructor(private _xhr: XHR, private _urlResolver: UrlResolver) {} - compileComponentRuntime(component: CompileDirectiveMetadata): Promise { - var styles = component.template.styles; - var styleAbsUrls = component.template.styleUrls; + compileComponentRuntime(type: CompileTypeMetadata, + template: CompileTemplateMetadata): Promise { + var styles = template.styles; + var styleAbsUrls = template.styleUrls; return this._loadStyles(styles, styleAbsUrls, - component.template.encapsulation === ViewEncapsulation.Emulated) - .then(styles => styles.map(style => StringWrapper.replaceAll(style, COMPONENT_REGEX, - `${component.type.id}`))); + template.encapsulation === ViewEncapsulation.Emulated) + .then(styles => styles.map( + style => StringWrapper.replaceAll(style, COMPONENT_REGEX, `${type.id}`))); } - compileComponentCodeGen(component: CompileDirectiveMetadata): SourceExpression { - var shim = component.template.encapsulation === ViewEncapsulation.Emulated; + compileComponentCodeGen(type: CompileTypeMetadata, + template: CompileTemplateMetadata): SourceExpression { + var shim = template.encapsulation === ViewEncapsulation.Emulated; var suffix; if (shim) { - var componentId = `${ component.type.id}`; + var componentId = `${ type.id}`; suffix = codeGenMapArray(['style'], `style${codeGenReplaceAll(COMPONENT_VARIABLE, componentId)}`); } else { suffix = ''; } - return this._styleCodeGen(component.template.styles, component.template.styleUrls, shim, - suffix); + return this._styleCodeGen(template.styles, template.styleUrls, shim, suffix); } compileStylesheetCodeGen(moduleId: string, cssText: string): SourceModule[] { diff --git a/modules/angular2/src/compiler/template_compiler.ts b/modules/angular2/src/compiler/template_compiler.ts index bc7829d46f..0f9f16d289 100644 --- a/modules/angular2/src/compiler/template_compiler.ts +++ b/modules/angular2/src/compiler/template_compiler.ts @@ -1,4 +1,5 @@ import {Type, Json, isBlank, stringify} from 'angular2/src/core/facade/lang'; +import {BaseException} from 'angular2/src/core/facade/exceptions'; import {ListWrapper, SetWrapper} from 'angular2/src/core/facade/collection'; import {PromiseWrapper, Promise} from 'angular2/src/core/facade/async'; import {CompiledTemplate, TemplateCmd} from 'angular2/src/core/compiler/template_commands'; @@ -58,18 +59,12 @@ export class TemplateCompiler { })); } - serializeDirectiveMetadata(metadata: CompileDirectiveMetadata): string { - return Json.stringify(metadata.toJson()); - } - - deserializeDirectiveMetadata(data: string): CompileDirectiveMetadata { - return CompileDirectiveMetadata.fromJson(Json.parse(data)); - } - compileHostComponentRuntime(type: Type): Promise { var compMeta: CompileDirectiveMetadata = this._runtimeMetadataResolver.getMetadata(type); + assertComponent(compMeta); var hostMeta: CompileDirectiveMetadata = createHostComponentMeta(compMeta.type, compMeta.selector); + this._compileComponentRuntime(hostMeta, [compMeta], new Set()); return this._compiledTemplateDone.get(hostMeta.type.id); } @@ -93,28 +88,30 @@ export class TemplateCompiler { new CompiledTemplate(compMeta.type.id, () => [changeDetectorFactory, commands, styles]); this._compiledTemplateCache.set(compMeta.type.id, compiledTemplate); compilingComponentIds.add(compMeta.type.id); - done = PromiseWrapper.all([this._styleCompiler.compileComponentRuntime(compMeta)].concat( - viewDirectives.map( - dirMeta => this.normalizeDirectiveMetadata(dirMeta)))) - .then((stylesAndNormalizedViewDirMetas: any[]) => { - var childPromises = []; - var normalizedViewDirMetas = stylesAndNormalizedViewDirMetas.slice(1); - var parsedTemplate = this._templateParser.parse( - compMeta.template.template, normalizedViewDirMetas, compMeta.type.name); + done = + PromiseWrapper + .all([ + this._styleCompiler.compileComponentRuntime(compMeta.type, compMeta.template) + ].concat(viewDirectives.map(dirMeta => this.normalizeDirectiveMetadata(dirMeta)))) + .then((stylesAndNormalizedViewDirMetas: any[]) => { + var childPromises = []; + var normalizedViewDirMetas = stylesAndNormalizedViewDirMetas.slice(1); + var parsedTemplate = this._templateParser.parse( + compMeta.template.template, normalizedViewDirMetas, compMeta.type.name); - var changeDetectorFactories = this._cdCompiler.compileComponentRuntime( - compMeta.type, compMeta.changeDetection, parsedTemplate); - changeDetectorFactory = changeDetectorFactories[0]; - styles = stylesAndNormalizedViewDirMetas[0]; - commands = this._compileCommandsRuntime(compMeta, parsedTemplate, - changeDetectorFactories, - compilingComponentIds, childPromises); - return PromiseWrapper.all(childPromises); - }) - .then((_) => { - SetWrapper.delete(compilingComponentIds, compMeta.type.id); - return compiledTemplate; - }); + var changeDetectorFactories = this._cdCompiler.compileComponentRuntime( + compMeta.type, compMeta.changeDetection, parsedTemplate); + changeDetectorFactory = changeDetectorFactories[0]; + styles = stylesAndNormalizedViewDirMetas[0]; + commands = + this._compileCommandsRuntime(compMeta, parsedTemplate, changeDetectorFactories, + compilingComponentIds, childPromises); + return PromiseWrapper.all(childPromises); + }) + .then((_) => { + SetWrapper.delete(compilingComponentIds, compMeta.type.id); + return compiledTemplate; + }); this._compiledTemplateDone.set(compMeta.type.id, done); } return compiledTemplate; @@ -148,6 +145,7 @@ export class TemplateCompiler { var componentMetas: CompileDirectiveMetadata[] = []; components.forEach(componentWithDirs => { var compMeta = componentWithDirs.component; + assertComponent(compMeta); componentMetas.push(compMeta); this._processTemplateCodeGen(compMeta, componentWithDirs.directives, @@ -174,7 +172,7 @@ export class TemplateCompiler { private _processTemplateCodeGen(compMeta: CompileDirectiveMetadata, directives: CompileDirectiveMetadata[], targetDeclarations: string[], targetTemplateArguments: any[][]) { - var styleExpr = this._styleCompiler.compileComponentCodeGen(compMeta); + var styleExpr = this._styleCompiler.compileComponentCodeGen(compMeta.type, compMeta.template); var parsedTemplate = this._templateParser.parse(compMeta.template.template, directives, compMeta.type.name); var changeDetectorsExprs = this._cdCompiler.compileComponentCodeGen( @@ -197,6 +195,12 @@ export class NormalizedComponentWithViewDirectives { public directives: CompileDirectiveMetadata[]) {} } +function assertComponent(meta: CompileDirectiveMetadata) { + if (!meta.isComponent) { + throw new BaseException(`Could not compile '${meta.type.name}' because it is not a component.`); + } +} + function templateVariableName(type: CompileTypeMetadata): string { return `${type.name}Template`; } diff --git a/modules/angular2/src/compiler/template_normalizer.ts b/modules/angular2/src/compiler/template_normalizer.ts index f6fa21d8f0..eed8da9400 100644 --- a/modules/angular2/src/compiler/template_normalizer.ts +++ b/modules/angular2/src/compiler/template_normalizer.ts @@ -4,6 +4,7 @@ import { CompileTemplateMetadata } from './directive_metadata'; import {isPresent, isBlank} from 'angular2/src/core/facade/lang'; +import {BaseException} from 'angular2/src/core/facade/exceptions'; import {Promise, PromiseWrapper} from 'angular2/src/core/facade/async'; import {XHR} from 'angular2/src/core/render/xhr'; @@ -34,11 +35,13 @@ export class TemplateNormalizer { if (isPresent(template.template)) { return PromiseWrapper.resolve(this.normalizeLoadedTemplate( directiveType, template, template.template, directiveType.moduleId)); - } else { + } else if (isPresent(template.templateUrl)) { var sourceAbsUrl = this._urlResolver.resolve(directiveType.moduleId, template.templateUrl); return this._xhr.get(sourceAbsUrl) .then(templateContent => this.normalizeLoadedTemplate(directiveType, template, templateContent, sourceAbsUrl)); + } else { + throw new BaseException(`No template specified for component ${directiveType.name}`); } } @@ -79,12 +82,15 @@ class TemplatePreparseVisitor implements HtmlAstVisitor { ngContentSelectors: string[] = []; styles: string[] = []; styleUrls: string[] = []; + ngNonBindableStackCount: number = 0; visitElement(ast: HtmlElementAst, context: any): any { var preparsedElement = preparseElement(ast); switch (preparsedElement.type) { case PreparsedElementType.NG_CONTENT: - this.ngContentSelectors.push(preparsedElement.selectAttr); + if (this.ngNonBindableStackCount === 0) { + this.ngContentSelectors.push(preparsedElement.selectAttr); + } break; case PreparsedElementType.STYLE: var textContent = ''; @@ -99,8 +105,12 @@ class TemplatePreparseVisitor implements HtmlAstVisitor { this.styleUrls.push(preparsedElement.hrefAttr); break; } - if (preparsedElement.type !== PreparsedElementType.NON_BINDABLE) { - htmlVisitAll(this, ast.children); + if (preparsedElement.nonBindable) { + this.ngNonBindableStackCount++; + } + htmlVisitAll(this, ast.children); + if (preparsedElement.nonBindable) { + this.ngNonBindableStackCount--; } return null; } diff --git a/modules/angular2/src/compiler/template_parser.ts b/modules/angular2/src/compiler/template_parser.ts index 66ca60fa47..db59503991 100644 --- a/modules/angular2/src/compiler/template_parser.ts +++ b/modules/angular2/src/compiler/template_parser.ts @@ -164,8 +164,7 @@ class TemplateParseVisitor implements HtmlAstVisitor { var preparsedElement = preparseElement(element); if (preparsedElement.type === PreparsedElementType.SCRIPT || preparsedElement.type === PreparsedElementType.STYLE || - preparsedElement.type === PreparsedElementType.STYLESHEET || - preparsedElement.type === PreparsedElementType.NON_BINDABLE) { + preparsedElement.type === PreparsedElementType.STYLESHEET) { // Skipping a', []))) + .toEqual([ + [ElementAst, 'div', 'TestComp > div:nth-child(0)'], + [AttrAst, 'ng-non-bindable', '', 'TestComp > div:nth-child(0)[ng-non-bindable=]'], + [TextAst, 'a', 'TestComp > div:nth-child(0) > #text(a):nth-child(1)'] + ]); + }); + + it('should ignore a', []))) + .toEqual([ + [ElementAst, 'div', 'TestComp > div:nth-child(0)'], + [AttrAst, 'ng-non-bindable', '', 'TestComp > div:nth-child(0)[ng-non-bindable=]'], + [TextAst, 'a', 'TestComp > div:nth-child(0) > #text(a):nth-child(1)'] + ]); + }); + + it('should ignore elements inside of elements with ng-non-bindable but include them for source info', + () => { + expect(humanizeTemplateAsts( + parse('
a
', []))) + .toEqual([ + [ElementAst, 'div', 'TestComp > div:nth-child(0)'], + [AttrAst, 'ng-non-bindable', '', 'TestComp > div:nth-child(0)[ng-non-bindable=]'], + [TextAst, 'a', 'TestComp > div:nth-child(0) > #text(a):nth-child(1)'] + ]); + }); + + it('should convert elements into regular elements inside of elements with ng-non-bindable but include them for source info', + () => { + expect(humanizeTemplateAsts( + parse('
a
', []))) + .toEqual([ + [ElementAst, 'div', 'TestComp > div:nth-child(0)'], + [AttrAst, 'ng-non-bindable', '', 'TestComp > div:nth-child(0)[ng-non-bindable=]'], + [ + ElementAst, + 'ng-content', + 'TestComp > div:nth-child(0) > ng-content:nth-child(0)' + ], + [TextAst, 'a', 'TestComp > div:nth-child(0) > #text(a):nth-child(1)'] + ]); }); }); diff --git a/modules/angular2/test/compiler/template_preparser_spec.ts b/modules/angular2/test/compiler/template_preparser_spec.ts new file mode 100644 index 0000000000..2044a6f528 --- /dev/null +++ b/modules/angular2/test/compiler/template_preparser_spec.ts @@ -0,0 +1,58 @@ +import { + ddescribe, + describe, + xdescribe, + it, + iit, + xit, + expect, + beforeEach, + afterEach, + AsyncTestCompleter, + inject, + beforeEachBindings +} from 'angular2/test_lib'; + +import {HtmlParser} from 'angular2/src/compiler/html_parser'; +import { + preparseElement, + PreparsedElementType, + PreparsedElement +} from 'angular2/src/compiler/template_preparser'; + +export function main() { + describe('preparseElement', () => { + var htmlParser; + beforeEach(inject([HtmlParser], (_htmlParser: HtmlParser) => { htmlParser = _htmlParser; })); + + function preparse(html: string): PreparsedElement { + return preparseElement(htmlParser.parse(html, '')[0]); + } + + it('should detect script elements', inject([HtmlParser], (htmlParser: HtmlParser) => { + expect(preparse('