From c855bf4c56774febe93202562d07ab32423c490a Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 14 Apr 2021 20:44:42 +0200 Subject: [PATCH] refactor(compiler): clean up template information passed for partial compilation (#41583) With the introduction of the partial compilation, the Angular compiler's existing `parseTemplate` method has been extended to pass through multiple properties purely in favor of the partial compilation. e.g. the `parseTemplate` function now accepts an "option" called `isInline`. This option is just passed through and returned as part of the `ParsedTemplate`. This is not ideal because the `parseTemplate` function doesn't care whether the specified template was inline or not. This commit cleans up the `parseTemplate` compiler function so that nothing needed only for the partial compilation is added to it. We introduce a new struct for additional template information that is specific to the generation of the `declareComponent` function. With that change, we can simplify the component decorator handler and keep logic more local. PR Close #41583 --- .../partial_component_linker_1.ts | 1 - .../src/ngtsc/annotations/src/component.ts | 72 ++++++++++--------- packages/compiler/src/compiler.ts | 2 +- .../compiler/src/render3/partial/component.ts | 70 +++++++++++------- .../compiler/src/render3/view/template.ts | 42 +---------- 5 files changed, 87 insertions(+), 100 deletions(-) diff --git a/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_component_linker_1.ts b/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_component_linker_1.ts index 633099c4aa..b2cd37eea9 100644 --- a/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_component_linker_1.ts +++ b/packages/compiler-cli/linker/src/file_linker/partial_linkers/partial_component_linker_1.ts @@ -55,7 +55,6 @@ export class PartialComponentLinkerVersion1 implements metaObj.has('preserveWhitespaces') ? metaObj.getBoolean('preserveWhitespaces') : false, // We normalize line endings if the template is was inline. i18nNormalizeLineEndingsInICUs: isInline, - isInline, }); if (template.errors !== null) { const errors = template.errors.map(err => err.toString()).join('\n'); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 553e4ca273..67722e6589 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {compileClassMetadata, compileComponentFromMetadata, compileDeclareClassMetadata, compileDeclareComponentFromMetadata, ConstantPool, CssSelector, DeclarationListEmitMode, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, FactoryTarget, InterpolationConfig, LexerRange, makeBindingParser, ParsedTemplate, ParseSourceFile, parseTemplate, R3ClassMetadata, R3ComponentMetadata, R3FactoryMetadata, R3TargetBinder, R3UsedDirectiveMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr} from '@angular/compiler'; +import {compileClassMetadata, compileComponentFromMetadata, compileDeclareClassMetadata, compileDeclareComponentFromMetadata, ConstantPool, CssSelector, DeclarationListEmitMode, DeclareComponentTemplateInfo, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, FactoryTarget, InterpolationConfig, LexerRange, makeBindingParser, ParsedTemplate, ParseSourceFile, parseTemplate, R3ClassMetadata, R3ComponentMetadata, R3TargetBinder, R3UsedDirectiveMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr} from '@angular/compiler'; import * as ts from 'typescript'; import {Cycle, CycleAnalyzer, CycleHandlingStrategy} from '../../cycles'; @@ -390,7 +390,7 @@ export class ComponentDecoratorHandler implements template = this.extractTemplate(node, templateDecl); } const templateResource = - template.isInline ? {path: null, expression: component.get('template')!} : { + template.declaration.isInline ? {path: null, expression: component.get('template')!} : { path: absoluteFrom(template.declaration.resolvedTemplateUrl), expression: template.sourceMapping.node }; @@ -571,7 +571,7 @@ export class ComponentDecoratorHandler implements selector, boundTemplate, templateMeta: { - isInline: analysis.template.isInline, + isInline: analysis.template.declaration.isInline, file: analysis.template.file, }, }); @@ -882,9 +882,17 @@ export class ComponentDecoratorHandler implements if (analysis.template.errors !== null && analysis.template.errors.length > 0) { return []; } + const templateInfo: DeclareComponentTemplateInfo = { + content: analysis.template.content, + sourceUrl: analysis.template.declaration.resolvedTemplateUrl, + isInline: analysis.template.declaration.isInline, + inlineTemplateExpression: analysis.template.declaration.isInline ? + new WrappedNodeExpr(analysis.template.declaration.expression) : + null, + }; const meta: R3ComponentMetadata = {...analysis.meta, ...resolution}; const fac = compileDeclareFactory(toFactoryMetadata(meta, FactoryTarget.Component)); - const def = compileDeclareComponentFromMetadata(meta, analysis.template); + const def = compileDeclareComponentFromMetadata(meta, analysis.template, templateInfo); const classMetadata = analysis.classMetadata !== null ? compileDeclareClassMetadata(analysis.classMetadata).toStmt() : null; @@ -1055,10 +1063,9 @@ export class ComponentDecoratorHandler implements private extractTemplate(node: ClassDeclaration, template: TemplateDeclaration): ParsedTemplateWithSource { if (template.isInline) { - let templateStr: string; - let templateLiteral: ts.Node|null = null; - let templateUrl: string = ''; - let templateRange: LexerRange|null = null; + let sourceStr: string; + let sourceParseRange: LexerRange|null = null; + let templateContent: string; let sourceMapping: TemplateSourceMapping; let escapedString = false; // We only support SourceMaps for inline templates that are simple string literals. @@ -1066,10 +1073,9 @@ export class ComponentDecoratorHandler implements ts.isNoSubstitutionTemplateLiteral(template.expression)) { // the start and end of the `templateExpr` node includes the quotation marks, which we must // strip - templateRange = getTemplateRange(template.expression); - templateStr = template.expression.getSourceFile().text; - templateLiteral = template.expression; - templateUrl = template.templateUrl; + sourceParseRange = getTemplateRange(template.expression); + sourceStr = template.expression.getSourceFile().text; + templateContent = template.expression.text; escapedString = true; sourceMapping = { type: 'direct', @@ -1081,22 +1087,26 @@ export class ComponentDecoratorHandler implements throw createValueHasWrongTypeError( template.expression, resolvedTemplate, 'template must be a string'); } - templateStr = resolvedTemplate; + // We do not parse the template directly from the source file using a lexer range, so + // the template source and content are set to the statically resolved template. + sourceStr = resolvedTemplate; + templateContent = resolvedTemplate; sourceMapping = { type: 'indirect', node: template.expression, componentClass: node, - template: templateStr, + template: templateContent, }; } return { - ...this._parseTemplate(template, templateStr, templateRange, escapedString), + ...this._parseTemplate(template, sourceStr, sourceParseRange, escapedString), + content: templateContent, sourceMapping, declaration: template, }; } else { - const templateStr = this.resourceLoader.load(template.resolvedTemplateUrl); + const templateContent = this.resourceLoader.load(template.resolvedTemplateUrl); if (this.depTracker !== null) { this.depTracker.addResourceDependency( node.getSourceFile(), absoluteFrom(template.resolvedTemplateUrl)); @@ -1104,15 +1114,16 @@ export class ComponentDecoratorHandler implements return { ...this._parseTemplate( - template, templateStr, /* templateRange */ null, + template, /* sourceStr */ templateContent, /* sourceParseRange */ null, /* escapedString */ false), + content: templateContent, sourceMapping: { type: 'external', componentClass: node, // TODO(alxhub): TS in g3 is unable to make this inference on its own, so cast it here // until g3 is able to figure this out. node: (template as ExternalTemplateDeclaration).templateUrlExpression, - template: templateStr, + template: templateContent, templateUrl: template.resolvedTemplateUrl, }, declaration: template, @@ -1121,19 +1132,18 @@ export class ComponentDecoratorHandler implements } private _parseTemplate( - template: TemplateDeclaration, templateStr: string, templateRange: LexerRange|null, + template: TemplateDeclaration, sourceStr: string, sourceParseRange: LexerRange|null, escapedString: boolean): ParsedComponentTemplate { // We always normalize line endings if the template has been escaped (i.e. is inline). const i18nNormalizeLineEndingsInICUs = escapedString || this.i18nNormalizeLineEndingsInICUs; - const parsedTemplate = parseTemplate(templateStr, template.sourceMapUrl, { + const parsedTemplate = parseTemplate(sourceStr, template.sourceMapUrl, { preserveWhitespaces: template.preserveWhitespaces, interpolationConfig: template.interpolationConfig, - range: templateRange ?? undefined, + range: sourceParseRange ?? undefined, escapedString, enableI18nLegacyMessageIdFormat: this.enableI18nLegacyMessageIdFormat, i18nNormalizeLineEndingsInICUs, - isInline: template.isInline, alwaysAttemptHtmlToR3AstConversion: this.usePoisonedData, }); @@ -1152,26 +1162,22 @@ export class ComponentDecoratorHandler implements // In order to guarantee the correctness of diagnostics, templates are parsed a second time // with the above options set to preserve source mappings. - const {nodes: diagNodes} = parseTemplate(templateStr, template.sourceMapUrl, { + const {nodes: diagNodes} = parseTemplate(sourceStr, template.sourceMapUrl, { preserveWhitespaces: true, preserveLineEndings: true, interpolationConfig: template.interpolationConfig, - range: templateRange ?? undefined, + range: sourceParseRange ?? undefined, escapedString, enableI18nLegacyMessageIdFormat: this.enableI18nLegacyMessageIdFormat, i18nNormalizeLineEndingsInICUs, leadingTriviaChars: [], - isInline: template.isInline, alwaysAttemptHtmlToR3AstConversion: this.usePoisonedData, }); return { ...parsedTemplate, diagNodes, - template: template.isInline ? new WrappedNodeExpr(template.expression) : templateStr, - templateUrl: template.resolvedTemplateUrl, - isInline: template.isInline, - file: new ParseSourceFile(templateStr, template.resolvedTemplateUrl), + file: new ParseSourceFile(sourceStr, template.resolvedTemplateUrl), }; } @@ -1363,12 +1369,6 @@ function getTemplateDeclarationNodeForError(declaration: TemplateDeclaration): t * some of which might be useful for re-parsing the template with different options. */ export interface ParsedComponentTemplate extends ParsedTemplate { - /** - * True if the original template was stored inline; - * False if the template was in an external file. - */ - isInline: boolean; - /** * The template AST, parsed in a manner which preserves source map information for diagnostics. * @@ -1383,6 +1383,8 @@ export interface ParsedComponentTemplate extends ParsedTemplate { } export interface ParsedTemplateWithSource extends ParsedComponentTemplate { + /** The string contents of the template. */ + content: string; sourceMapping: TemplateSourceMapping; declaration: TemplateDeclaration; } diff --git a/packages/compiler/src/compiler.ts b/packages/compiler/src/compiler.ts index 0acf368878..af28935d55 100644 --- a/packages/compiler/src/compiler.ts +++ b/packages/compiler/src/compiler.ts @@ -106,7 +106,7 @@ export {makeBindingParser, ParsedTemplate, parseTemplate, ParseTemplateOptions} export {R3CompiledExpression, R3Reference, devOnlyGuardedExpression, getSafePropertyAccessString} from './render3/util'; export {compileComponentFromMetadata, compileDirectiveFromMetadata, parseHostBindings, ParsedHostBindings, verifyHostBindings} from './render3/view/compiler'; export {compileDeclareClassMetadata} from './render3/partial/class_metadata'; -export {compileDeclareComponentFromMetadata} from './render3/partial/component'; +export {compileDeclareComponentFromMetadata, DeclareComponentTemplateInfo} from './render3/partial/component'; export {compileDeclareDirectiveFromMetadata} from './render3/partial/directive'; export {compileDeclareFactoryFunction} from './render3/partial/factory'; export {compileDeclareInjectableFromMetadata} from './render3/partial/injectable'; diff --git a/packages/compiler/src/render3/partial/component.ts b/packages/compiler/src/render3/partial/component.ts index 693af7ed62..5f56a06384 100644 --- a/packages/compiler/src/render3/partial/component.ts +++ b/packages/compiler/src/render3/partial/component.ts @@ -20,13 +20,39 @@ import {R3DeclareComponentMetadata, R3DeclareUsedDirectiveMetadata} from './api' import {createDirectiveDefinitionMap} from './directive'; import {generateForwardRef, toOptionalLiteralArray} from './util'; +export interface DeclareComponentTemplateInfo { + /** + * The string contents of the template. + * + * This is the "logical" template string, after expansion of any escaped characters (for inline + * templates). This may differ from the actual template bytes as they appear in the .ts file. + */ + content: string; + + /** + * A full path to the file which contains the template. + * + * This can be either the original .ts file if the template is inline, or the .html file if an + * external file was used. + */ + sourceUrl: string; + + /** + * Whether the template was inline (using `template`) or external (using `templateUrl`). + */ + isInline: boolean; + + /** Expression that resolves to the inline template. */ + inlineTemplateExpression: o.Expression|null; +} /** * Compile a component declaration defined by the `R3ComponentMetadata`. */ export function compileDeclareComponentFromMetadata( - meta: R3ComponentMetadata, template: ParsedTemplate): R3CompiledExpression { - const definitionMap = createComponentDefinitionMap(meta, template); + meta: R3ComponentMetadata, template: ParsedTemplate, + additionalTemplateInfo: DeclareComponentTemplateInfo): R3CompiledExpression { + const definitionMap = createComponentDefinitionMap(meta, template, additionalTemplateInfo); const expression = o.importExpr(R3.declareComponent).callFn([definitionMap.toLiteralMap()]); const type = createComponentType(meta); @@ -37,13 +63,14 @@ export function compileDeclareComponentFromMetadata( /** * Gathers the declaration fields for a component into a `DefinitionMap`. */ -export function createComponentDefinitionMap(meta: R3ComponentMetadata, template: ParsedTemplate): - DefinitionMap { +export function createComponentDefinitionMap( + meta: R3ComponentMetadata, template: ParsedTemplate, + templateInfo: DeclareComponentTemplateInfo): DefinitionMap { const definitionMap: DefinitionMap = createDirectiveDefinitionMap(meta); - definitionMap.set('template', getTemplateExpression(template)); - if (template.isInline) { + definitionMap.set('template', getTemplateExpression(template, templateInfo)); + if (templateInfo.isInline) { definitionMap.set('isInline', o.literal(true)); } @@ -82,25 +109,20 @@ export function createComponentDefinitionMap(meta: R3ComponentMetadata, template return definitionMap; } -function getTemplateExpression(template: ParsedTemplate): o.Expression { - if (typeof template.template === 'string') { - if (template.isInline) { - // The template is inline but not a simple literal string, so give up with trying to - // source-map it and just return a simple literal here. - return o.literal(template.template); - } else { - // The template is external so we must synthesize an expression node with the appropriate - // source-span. - const contents = template.template; - const file = new ParseSourceFile(contents, template.templateUrl); - const start = new ParseLocation(file, 0, 0, 0); - const end = computeEndLocation(file, contents); - const span = new ParseSourceSpan(start, end); - return o.literal(contents, null, span); - } - } else { +function getTemplateExpression( + template: ParsedTemplate, templateInfo: DeclareComponentTemplateInfo): o.Expression { + if (templateInfo.isInline) { // The template is inline so we can just reuse the current expression node. - return template.template; + return templateInfo.inlineTemplateExpression!; + } else { + // The template is external so we must synthesize an expression node with the appropriate + // source-span. + const contents = templateInfo.content; + const file = new ParseSourceFile(contents, templateInfo.sourceUrl); + const start = new ParseLocation(file, 0, 0, 0); + const end = computeEndLocation(file, contents); + const span = new ParseSourceSpan(start, end); + return o.literal(contents, null, span); } } diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index e75f7885a4..530ce30ccc 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -22,7 +22,7 @@ import {LexerRange} from '../../ml_parser/lexer'; import {isNgContainer as checkIsNgContainer, splitNsName} from '../../ml_parser/tags'; import {mapLiteral} from '../../output/map_util'; import * as o from '../../output/output_ast'; -import {ParseError, ParseSourceSpan} from '../../parse_util'; +import {ParseError, ParseSourceFile, ParseSourceSpan} from '../../parse_util'; import {DomElementSchemaRegistry} from '../../schema/dom_element_schema_registry'; import {isTrustedTypesSink} from '../../schema/trusted_types_sinks'; import {CssSelector, SelectorMatcher} from '../../selector'; @@ -2082,6 +2082,7 @@ export interface ParseTemplateOptions { * `$localize` message id format and you are not using compile time translation merging. */ enableI18nLegacyMessageIdFormat?: boolean; + /** * If this text is stored in an external template (e.g. via `templateUrl`) then we need to decide * whether or not to normalize the line-endings (from `\r\n` to `\n`) when processing ICU @@ -2092,11 +2093,6 @@ export interface ParseTemplateOptions { */ i18nNormalizeLineEndingsInICUs?: boolean; - /** - * Whether the template was inline. - */ - isInline?: boolean; - /** * Whether to always attempt to convert the parsed HTML AST to an R3 AST, despite HTML or i18n * Meta parse errors. @@ -2134,7 +2130,6 @@ export interface ParseTemplateOptions { export function parseTemplate( template: string, templateUrl: string, options: ParseTemplateOptions = {}): ParsedTemplate { const {interpolationConfig, preserveWhitespaces, enableI18nLegacyMessageIdFormat} = options; - const isInline = options.isInline ?? false; const bindingParser = makeBindingParser(interpolationConfig); const htmlParser = new HtmlParser(); const parseResult = htmlParser.parse( @@ -2146,9 +2141,6 @@ export function parseTemplate( const parsedTemplate: ParsedTemplate = { interpolationConfig, preserveWhitespaces, - template, - templateUrl, - isInline, errors: parseResult.errors, nodes: [], styleUrls: [], @@ -2177,9 +2169,6 @@ export function parseTemplate( const parsedTemplate: ParsedTemplate = { interpolationConfig, preserveWhitespaces, - template, - templateUrl, - isInline, errors: i18nMetaResult.errors, nodes: [], styleUrls: [], @@ -2215,14 +2204,12 @@ export function parseTemplate( interpolationConfig, preserveWhitespaces, errors: errors.length > 0 ? errors : null, - template, - templateUrl, - isInline, nodes, styleUrls, styles, ngContentSelectors }; + if (options.collectCommentNodes) { parsedTemplate.commentNodes = commentNodes; } @@ -2383,29 +2370,6 @@ export interface ParsedTemplate { * How to parse interpolation markers. */ interpolationConfig?: InterpolationConfig; - - /** - * The string contents of the template, or an expression that represents the string/template - * literal as it occurs in the source. - * - * This is the "logical" template string, after expansion of any escaped characters (for inline - * templates). This may differ from the actual template bytes as they appear in the .ts file. - */ - template: string|o.Expression; - - /** - * A full path to the file which contains the template. - * - * This can be either the original .ts file if the template is inline, or the .html file if an - * external file was used. - */ - templateUrl: string; - - /** - * Whether the template was inline (using `template`) or external (using `templateUrl`). - */ - isInline: boolean; - /** * Any errors from parsing the template the first time. *