From 9b15588188287dd02c0bc8e933f274a3bdf08e74 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 2 Oct 2019 18:17:56 +0100 Subject: [PATCH] refactor(ivy): i18n - move marker block serialization to helpers (#32867) Previously the metadata and placeholder blocks were serialized in a variety of places. Moreover the code for creating the `LocalizedString` AST node was doing serialization, which break the separation of concerns. Now this is all done by the code that renders the AST and is refactored into helper functions to avoid repeating the behaviour. PR Close #32867 --- .../src/ngtsc/translator/src/translator.ts | 26 ++++---------- .../compiler/src/output/abstract_emitter.ts | 14 +++++--- packages/compiler/src/output/output_ast.ts | 14 ++++---- .../src/render3/view/i18n/localize_utils.ts | 13 +++---- .../compiler/src/render3/view/i18n/meta.ts | 19 +++++++++-- .../compiler/test/render3/view/i18n_spec.ts | 34 +++++++++++-------- 6 files changed, 65 insertions(+), 55 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts index 58422df55c..7ccd7abdb1 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts @@ -8,6 +8,7 @@ import {ArrayType, AssertNotNull, BinaryOperator, BinaryOperatorExpr, BuiltinType, BuiltinTypeName, CastExpr, ClassStmt, CommaExpr, CommentStmt, ConditionalExpr, DeclareFunctionStmt, DeclareVarStmt, Expression, ExpressionStatement, ExpressionType, ExpressionVisitor, ExternalExpr, ExternalReference, FunctionExpr, IfStmt, InstantiateExpr, InvokeFunctionExpr, InvokeMethodExpr, JSDocCommentStmt, LiteralArrayExpr, LiteralExpr, LiteralMapExpr, MapType, NotExpr, ReadKeyExpr, ReadPropExpr, ReadVarExpr, ReturnStatement, Statement, StatementVisitor, StmtModifier, ThrowStmt, TryCatchStmt, Type, TypeVisitor, TypeofExpr, WrappedNodeExpr, WriteKeyExpr, WritePropExpr, WriteVarExpr} from '@angular/compiler'; import {LocalizedString} from '@angular/compiler/src/output/output_ast'; +import {serializeI18nMetaBlock, serializeI18nPlaceholderBlock} from '@angular/compiler/src/render3/view/i18n/meta'; import * as ts from 'typescript'; import {DefaultImportRecorder, ImportRewriter, NOOP_DEFAULT_IMPORT_RECORDER, NoopImportRewriter} from '../../imports'; @@ -528,16 +529,18 @@ export class TypeTranslatorVisitor implements ExpressionVisitor, TypeVisitor { */ function visitLocalizedString(ast: LocalizedString, context: Context, visitor: ExpressionVisitor) { let template: ts.TemplateLiteral; + const headPart = `${serializeI18nMetaBlock(ast.metaBlock)}${ast.messageParts[0]}`; if (ast.messageParts.length === 1) { - template = ts.createNoSubstitutionTemplateLiteral(ast.messageParts[0]); + template = ts.createNoSubstitutionTemplateLiteral(headPart); } else { - const head = ts.createTemplateHead(ast.messageParts[0]); + const head = ts.createTemplateHead(headPart); const spans: ts.TemplateSpan[] = []; for (let i = 1; i < ast.messageParts.length; i++) { const resolvedExpression = ast.expressions[i - 1].visitExpression(visitor, context); spans.push(ts.createTemplateSpan( - resolvedExpression, ts.createTemplateMiddle(prefixWithPlaceholderMarker( - ast.messageParts[i], ast.placeHolderNames[i - 1])))); + resolvedExpression, + ts.createTemplateMiddle( + serializeI18nPlaceholderBlock(ast.placeHolderNames[i - 1]) + ast.messageParts[i]))); } if (spans.length > 0) { // The last span is supposed to have a tail rather than a middle @@ -547,18 +550,3 @@ function visitLocalizedString(ast: LocalizedString, context: Context, visitor: E } return ts.createTaggedTemplate(ts.createIdentifier('$localize'), template); } - -/** - * We want our tagged literals to include placeholder name information to aid runtime translation. - * - * The expressions are marked with placeholder names by postfixing the expression with - * `:placeHolderName:`. To achieve this, we actually "prefix" the message part that follows the - * expression. - * - * @param messagePart the message part that follows the current expression. - * @param placeHolderName the name of the placeholder for the current expression. - * @returns the prefixed message part. - */ -function prefixWithPlaceholderMarker(messagePart: string, placeHolderName: string) { - return `:${placeHolderName}:${messagePart}`; -} \ No newline at end of file diff --git a/packages/compiler/src/output/abstract_emitter.ts b/packages/compiler/src/output/abstract_emitter.ts index 3d7a65275a..b5671ffe81 100644 --- a/packages/compiler/src/output/abstract_emitter.ts +++ b/packages/compiler/src/output/abstract_emitter.ts @@ -7,6 +7,7 @@ */ import {ParseSourceSpan} from '../parse_util'; +import {serializeI18nHead, serializeI18nTemplatePart} from '../render3/view/i18n/meta'; import * as o from './output_ast'; import {SourceMapGenerator} from './source_map'; @@ -362,13 +363,14 @@ export abstract class AbstractEmitterVisitor implements o.StatementVisitor, o.Ex } visitLocalizedString(ast: o.LocalizedString, ctx: EmitterVisitorContext): any { - ctx.print(ast, '$localize `' + ast.messageParts[0]); + const head = serializeI18nHead(ast.metaBlock, ast.messageParts[0]); + ctx.print(ast, '$localize `' + escapeBackticks(head)); for (let i = 1; i < ast.messageParts.length; i++) { ctx.print(ast, '${'); ast.expressions[i - 1].visitExpression(this, ctx); - // Add the placeholder name annotation to support runtime inlining - ctx.print(ast, `}:${ast.placeHolderNames[i - 1]}:`); - ctx.print(ast, ast.messageParts[i]); + ctx.print( + ast, + `}${escapeBackticks(serializeI18nTemplatePart(ast.placeHolderNames[i - 1], ast.messageParts[i]))}`); } ctx.print(ast, '`'); return null; @@ -558,3 +560,7 @@ function _createIndent(count: number): string { } return res; } + +function escapeBackticks(str: string): string { + return str.replace(/`/g, '\\`'); +} diff --git a/packages/compiler/src/output/output_ast.ts b/packages/compiler/src/output/output_ast.ts index 3d4c203025..35f53e3a47 100644 --- a/packages/compiler/src/output/output_ast.ts +++ b/packages/compiler/src/output/output_ast.ts @@ -8,6 +8,7 @@ import {ParseSourceSpan} from '../parse_util'; +import {I18nMeta} from '../render3/view/i18n/meta'; import {error} from '../util'; //// Types @@ -482,8 +483,9 @@ export class LiteralExpr extends Expression { export class LocalizedString extends Expression { constructor( - public messageParts: string[], public placeHolderNames: string[], - public expressions: Expression[], sourceSpan?: ParseSourceSpan|null) { + readonly metaBlock: I18nMeta, readonly messageParts: string[], + readonly placeHolderNames: string[], readonly expressions: Expression[], + sourceSpan?: ParseSourceSpan|null) { super(STRING_TYPE, sourceSpan); } @@ -1098,7 +1100,7 @@ export class AstTransformer implements StatementVisitor, ExpressionVisitor { visitLocalizedString(ast: LocalizedString, context: any): any { return this.transformExpr( new LocalizedString( - ast.messageParts, ast.placeHolderNames, + ast.metaBlock, ast.messageParts, ast.placeHolderNames, this.visitAllExpressions(ast.expressions, context), ast.sourceSpan), context); } @@ -1584,9 +1586,9 @@ export function literal( } export function localizedString( - messageParts: string[], placeholderNames: string[], expressions: Expression[], - sourceSpan?: ParseSourceSpan | null): LocalizedString { - return new LocalizedString(messageParts, placeholderNames, expressions, sourceSpan); + metaBlock: I18nMeta, messageParts: string[], placeholderNames: string[], + expressions: Expression[], sourceSpan?: ParseSourceSpan | null): LocalizedString { + return new LocalizedString(metaBlock, messageParts, placeholderNames, expressions, sourceSpan); } export function isNull(exp: Expression): boolean { diff --git a/packages/compiler/src/render3/view/i18n/localize_utils.ts b/packages/compiler/src/render3/view/i18n/localize_utils.ts index 83865eb03f..2622677cac 100644 --- a/packages/compiler/src/render3/view/i18n/localize_utils.ts +++ b/packages/compiler/src/render3/view/i18n/localize_utils.ts @@ -9,7 +9,7 @@ import * as i18n from '../../../i18n/i18n_ast'; import * as o from '../../../output/output_ast'; import {serializeIcuNode} from './icu_serializer'; -import {metaFromI18nMessage, serializeI18nMeta} from './meta'; +import {metaFromI18nMessage} from './meta'; import {formatI18nPlaceholderName} from './util'; export function createLocalizeStatements( @@ -17,15 +17,10 @@ export function createLocalizeStatements( params: {[name: string]: o.Expression}): o.Statement[] { const statements = []; - const metaBlock = serializeI18nMeta(metaFromI18nMessage(message)); - const {messageParts, placeHolders} = serializeI18nMessageForLocalize(message); - - // Update first message part with metadata - messageParts[0] = `:${metaBlock}:${messageParts[0]}`; - - statements.push(new o.ExpressionStatement(variable.set( - o.localizedString(messageParts, placeHolders, placeHolders.map(ph => params[ph]))))); + statements.push(new o.ExpressionStatement(variable.set(o.localizedString( + metaFromI18nMessage(message), messageParts, placeHolders, + placeHolders.map(ph => params[ph]))))); return statements; } diff --git a/packages/compiler/src/render3/view/i18n/meta.ts b/packages/compiler/src/render3/view/i18n/meta.ts index 9c61224d18..3d45bb1ac8 100644 --- a/packages/compiler/src/render3/view/i18n/meta.ts +++ b/packages/compiler/src/render3/view/i18n/meta.ts @@ -184,7 +184,7 @@ export function parseI18nMeta(meta?: string): I18nMeta { * * @param meta The metadata to serialize */ -export function serializeI18nMeta(meta: I18nMeta): string { +export function serializeI18nMetaBlock(meta: I18nMeta): string { let metaBlock = meta.description || ''; if (meta.meaning) { metaBlock = `${meta.meaning}|${metaBlock}`; @@ -192,7 +192,22 @@ export function serializeI18nMeta(meta: I18nMeta): string { if (meta.id) { metaBlock = `${metaBlock}@@${meta.id}`; } - return metaBlock; + return metaBlock !== '' ? `:${metaBlock}:` : ''; +} + +/** + * Convert a placeholder into marked block for rendering. + * + * We want our tagged literals to include placeholder name information to aid runtime translation. + * + * The expressions are marked with placeholder names by postfixing the expression with + * `:placeHolderName:`. To achieve this, we actually "prefix" the message part that follows the + * expression. + * + * @param placeholderName The placeholder name to serialize + */ +export function serializeI18nPlaceholderBlock(placeholderName: string): string { + return placeholderName !== '' ? `:${placeholderName}:` : ''; } // Converts i18n meta information for a message (id, description, meaning) diff --git a/packages/compiler/test/render3/view/i18n_spec.ts b/packages/compiler/test/render3/view/i18n_spec.ts index 14bae53b7a..c40228d241 100644 --- a/packages/compiler/test/render3/view/i18n_spec.ts +++ b/packages/compiler/test/render3/view/i18n_spec.ts @@ -15,7 +15,7 @@ import {I18nContext} from '../../../src/render3/view/i18n/context'; import {serializeI18nMessageForGetMsg} from '../../../src/render3/view/i18n/get_msg_utils'; import {serializeIcuNode} from '../../../src/render3/view/i18n/icu_serializer'; import {serializeI18nMessageForLocalize} from '../../../src/render3/view/i18n/localize_utils'; -import {I18nMeta, parseI18nMeta, serializeI18nMeta} from '../../../src/render3/view/i18n/meta'; +import {I18nMeta, parseI18nMeta, serializeI18nMetaBlock, serializeI18nPlaceholderBlock} from '../../../src/render3/view/i18n/meta'; import {formatI18nPlaceholderName} from '../../../src/render3/view/i18n/util'; import {parseR3 as parse} from './util'; @@ -200,23 +200,27 @@ describe('Utils', () => { }); describe('metadata serialization', () => { - const metadataCases: [string, I18nMeta][] = [ - ['', meta()], - ['desc', meta('', '', 'desc')], - ['desc@@id', meta('id', '', 'desc')], - ['meaning|desc', meta('', 'meaning', 'desc')], - ['meaning|desc@@id', meta('id', 'meaning', 'desc')], - ['@@id', meta('id', '', '')], - ]; - it('parseI18nMeta()', () => { - metadataCases.forEach( - ([input, output]) => { expect(parseI18nMeta(input)).toEqual(output, input); }); + expect(parseI18nMeta('')).toEqual(meta()); + expect(parseI18nMeta('desc')).toEqual(meta('', '', 'desc')); + expect(parseI18nMeta('desc@@id')).toEqual(meta('id', '', 'desc')); + expect(parseI18nMeta('meaning|desc')).toEqual(meta('', 'meaning', 'desc')); + expect(parseI18nMeta('meaning|desc@@id')).toEqual(meta('id', 'meaning', 'desc')); + expect(parseI18nMeta('@@id')).toEqual(meta('id', '', '')); }); - it('serializeI18nMeta()', () => { - metadataCases.forEach( - ([output, input]) => { expect(serializeI18nMeta(input)).toEqual(output, input); }); + it('serializeI18nMetaBlock()', () => { + expect(serializeI18nMetaBlock(meta())).toEqual(''); + expect(serializeI18nMetaBlock(meta('', '', 'desc'))).toEqual(':desc:'); + expect(serializeI18nMetaBlock(meta('id', '', 'desc'))).toEqual(':desc@@id:'); + expect(serializeI18nMetaBlock(meta('', 'meaning', 'desc'))).toEqual(':meaning|desc:'); + expect(serializeI18nMetaBlock(meta('id', 'meaning', 'desc'))).toEqual(':meaning|desc@@id:'); + expect(serializeI18nMetaBlock(meta('id', '', ''))).toEqual(':@@id:'); + }); + + it('serializeI18nPlaceholderBlock()', () => { + expect(serializeI18nPlaceholderBlock('')).toEqual(''); + expect(serializeI18nPlaceholderBlock('abc')).toEqual(':abc:'); }); function meta(id?: string, meaning?: string, description?: string): I18nMeta {