From cc74ebfdf61de22503b353e034882a4cd3f283a4 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Mon, 18 Dec 2023 15:56:26 -0800 Subject: [PATCH] refactor(compiler): Rework how ICU placeholders are handled (#53643) The way we were handling ICU placeholders was not compatible with using interpolations on attributes of elements inside the ICU. This change refactors the handling of ICU placeholders and unifies the way expression and tag placeholders work inside ICUs. The new approach modifies the ingest logic to add the placeholder on to the TextOp rather than the TextInterpolationOp. This is because, in ICUs, we may need multiple i18n expressions created from the interpolation expressions to roll up into the same placeholder. ICUs essentially do the interpolation at compile time, combining the static strings with special placeholder strings that represent the expression values. PR Close #53643 --- .../icu_logic/GOLDEN_PARTIAL.js | 40 ++++++++++ .../icu_logic/TEST_CASES.json | 14 ++++ .../icu_logic/attribute_interpolation.js | 75 +++++++++++++++++++ .../icu_logic/attribute_interpolation.ts | 16 ++++ .../src/template/pipeline/ir/src/enums.ts | 5 ++ .../template/pipeline/ir/src/expression.ts | 1 + .../template/pipeline/ir/src/ops/create.ts | 57 +++++++++++++- .../template/pipeline/ir/src/ops/update.ts | 15 +++- .../src/template/pipeline/src/emit.ts | 2 - .../src/template/pipeline/src/ingest.ts | 30 ++++---- .../src/phases/convert_i18n_bindings.ts | 2 +- .../src/phases/extract_i18n_messages.ts | 31 ++++++-- .../src/phases/i18n_const_collection.ts | 3 +- .../src/phases/i18n_text_extraction.ts | 28 ++++++- .../resolve_i18n_expression_placeholders.ts | 36 ++++++--- .../phases/resolve_i18n_icu_placeholders.ts | 67 ----------------- 16 files changed, 308 insertions(+), 114 deletions(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/attribute_interpolation.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/attribute_interpolation.ts delete mode 100644 packages/compiler/src/template/pipeline/src/phases/resolve_i18n_icu_placeholders.ts diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/GOLDEN_PARTIAL.js index d9bff882176..8e5cb692ee5 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/GOLDEN_PARTIAL.js @@ -961,3 +961,43 @@ export declare class MyComponent { static ɵcmp: i0.ɵɵComponentDeclaration; } +/**************************************************************************************************** + * PARTIAL FILE: attribute_interpolation.js + ****************************************************************************************************/ +import { Component } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyComponent { +} +MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [], target: i0.ɵɵFactoryTarget.Component }); +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, isStandalone: true, selector: "my-comp", ngImport: i0, template: ` +
+ + {foo, select, other {foo}} + {foo, select, other {{{foo}}-{{foo}}}} +
+ `, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{ + type: Component, + args: [{ + selector: 'my-comp', + standalone: true, + template: ` +
+ + {foo, select, other {foo}} + {foo, select, other {{{foo}}-{{foo}}}} +
+ ` + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: attribute_interpolation.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyComponent { + foo: any; + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/TEST_CASES.json index c092fd6e6e2..b5082de5bf4 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/TEST_CASES.json @@ -245,6 +245,20 @@ ] } ] + }, + { + "description": "should handles ICUs with html content that has interpolated attributes", + "inputFiles": [ + "attribute_interpolation.ts" + ], + "expectations": [ + { + "extraChecks": [ + "verifyPlaceholdersIntegrity", + "verifyUniqueConsts" + ] + } + ] } ] } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/attribute_interpolation.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/attribute_interpolation.js new file mode 100644 index 00000000000..276853f3398 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/attribute_interpolation.js @@ -0,0 +1,75 @@ +consts: () => { + let $i18n_1$; + if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { + /** + * @suppress {msgDescriptions} + */ + const $MSG_EXTERNAL_6301050568345677976__ATTRIBUTE_INTERPOLATION_TS_2$ = goog.getMsg("{VAR_SELECT, select, other {{START_TAG_SPAN}foo{CLOSE_TAG_SPAN}}}"); + $i18n_1$ = $MSG_EXTERNAL_6301050568345677976__ATTRIBUTE_INTERPOLATION_TS_2$; + } else { + $i18n_1$ = $localize`{VAR_SELECT, select, other {{START_TAG_SPAN}foo{CLOSE_TAG_SPAN}}}`; + } + $i18n_1$ = i0.ɵɵi18nPostprocess($i18n_1$, { + "CLOSE_TAG_SPAN": "", + "START_TAG_SPAN": "", + "VAR_SELECT": "\uFFFD0\uFFFD" + }); + let $i18n_3$; + if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { + /** + * @suppress {msgDescriptions} + */ + const $MSG_EXTERNAL_369205108016154659__ATTRIBUTE_INTERPOLATION_TS_4$ = goog.getMsg("{VAR_SELECT, select, other {{INTERPOLATION}-{INTERPOLATION}}}"); + $i18n_3$ = $MSG_EXTERNAL_369205108016154659__ATTRIBUTE_INTERPOLATION_TS_4$; + } else { + $i18n_3$ = $localize`{VAR_SELECT, select, other {{INTERPOLATION}-{INTERPOLATION}}}`; + } + $i18n_3$ = i0.ɵɵi18nPostprocess($i18n_3$, { + "INTERPOLATION": "\uFFFD4\uFFFD", + "VAR_SELECT": "\uFFFD3\uFFFD" + }); + let $i18n_0$; + if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { + /** + * @suppress {msgDescriptions} + */ + const $MSG_EXTERNAL_6009429127580785009__ATTRIBUTE_INTERPOLATION_TS_5$ = goog.getMsg( + "{$startTagSpan}{$closeTagSpan}{$startTagSpan_1}{$icu}{$closeTagSpan}{$startTagSpan_1}{$icu_1}{$closeTagSpan}", + { + "closeTagSpan": "[\uFFFD/#2\uFFFD|\uFFFD/#3\uFFFD|\uFFFD/#4\uFFFD]", + "icu": $i18n_1$, + "icu_1": $i18n_3$, + "startTagSpan": "\uFFFD#2\uFFFD", + "startTagSpan_1": "[\uFFFD#3\uFFFD|\uFFFD#4\uFFFD]" + }, { + original_code: { + "closeTagSpan": "", + "icu": "{foo, select, other {foo}}", + "icu_1": "{foo, select, other {{{foo}}-{{foo}}}}", + "startTagSpan": "", + "startTagSpan_1": "" + } + }); + $i18n_0$ = $MSG_EXTERNAL_6009429127580785009__ATTRIBUTE_INTERPOLATION_TS_5$; + } else { + $i18n_0$ = $localize`${"\uFFFD#2\uFFFD"}:START_TAG_SPAN:${"[\uFFFD/#2\uFFFD|\uFFFD/#3\uFFFD|\uFFFD/#4\uFFFD]"}:CLOSE_TAG_SPAN:${"[\uFFFD#3\uFFFD|\uFFFD#4\uFFFD]"}:START_TAG_SPAN_1:${$i18n_1$}:ICU@@6051755734147382484:${"[\uFFFD/#2\uFFFD|\uFFFD/#3\uFFFD|\uFFFD/#4\uFFFD]"}:CLOSE_TAG_SPAN:${"[\uFFFD#3\uFFFD|\uFFFD#4\uFFFD]"}:START_TAG_SPAN_1:${$i18n_3$}:ICU_1@@7593934392904803263:${"[\uFFFD/#2\uFFFD|\uFFFD/#3\uFFFD|\uFFFD/#4\uFFFD]"}:CLOSE_TAG_SPAN:`; + } + $i18n_0$ = i0.ɵɵi18nPostprocess($i18n_0$); + return [$i18n_0$, [3, "title"]]; +}, +template: function MyComponent_Template(rf, ctx) { + if (rf & 1) { + i0.ɵɵelementStart(0, "div"); + i0.ɵɵi18nStart(1, 0); + i0.ɵɵelement(2, "span", 1)(3, "span")(4, "span"); + i0.ɵɵi18nEnd(); + i0.ɵɵelementEnd(); + } + if (rf & 2) { + i0.ɵɵadvance(2); + i0.ɵɵpropertyInterpolate2("title", "", ctx.foo, "-", ctx.foo, ""); + i0.ɵɵadvance(2); + i0.ɵɵi18nExp(ctx.foo)(ctx.foo)(ctx.foo)(ctx.foo)(ctx.foo); + i0.ɵɵi18nApply(1); + } +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/attribute_interpolation.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/attribute_interpolation.ts new file mode 100644 index 00000000000..22027929720 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/icu_logic/attribute_interpolation.ts @@ -0,0 +1,16 @@ +import {Component} from '@angular/core'; + +@Component({ + selector: 'my-comp', + standalone: true, + template: ` +
+ + {foo, select, other {foo}} + {foo, select, other {{{foo}}-{{foo}}}} +
+ ` +}) +export class MyComponent { + foo: any; +} diff --git a/packages/compiler/src/template/pipeline/ir/src/enums.ts b/packages/compiler/src/template/pipeline/ir/src/enums.ts index 7c623823949..52cbf31e808 100644 --- a/packages/compiler/src/template/pipeline/ir/src/enums.ts +++ b/packages/compiler/src/template/pipeline/ir/src/enums.ts @@ -230,6 +230,11 @@ export enum OpKind { */ IcuEnd, + /** + * An instruction representing a placeholder in an ICU expression. + */ + IcuPlaceholder, + /** * An i18n context containing information needed to generate an i18n message. */ diff --git a/packages/compiler/src/template/pipeline/ir/src/expression.ts b/packages/compiler/src/template/pipeline/ir/src/expression.ts index d8fb38405ca..8ff01ba01a2 100644 --- a/packages/compiler/src/template/pipeline/ir/src/expression.ts +++ b/packages/compiler/src/template/pipeline/ir/src/expression.ts @@ -1004,6 +1004,7 @@ export function transformExpressionsInOp( case OpKind.Template: case OpKind.Text: case OpKind.I18nAttributes: + case OpKind.IcuPlaceholder: // These operations contain no expressions. break; default: diff --git a/packages/compiler/src/template/pipeline/ir/src/ops/create.ts b/packages/compiler/src/template/pipeline/ir/src/ops/create.ts index 3bc2fd36450..595f4ec4cfa 100644 --- a/packages/compiler/src/template/pipeline/ir/src/ops/create.ts +++ b/packages/compiler/src/template/pipeline/ir/src/ops/create.ts @@ -27,7 +27,7 @@ export type CreateOp = ListEndOp|StatementOp|ElementOp|Eleme ElementEndOp|ContainerOp|ContainerStartOp|ContainerEndOp|TemplateOp|EnableBindingsOp| DisableBindingsOp|TextOp|ListenerOp|PipeOp|VariableOp|NamespaceOp|ProjectionDefOp| ProjectionOp|ExtractedAttributeOp|DeferOp|DeferOnOp|RepeaterCreateOp|I18nMessageOp|I18nOp| - I18nStartOp|I18nEndOp|IcuStartOp|IcuEndOp|I18nContextOp|I18nAttributesOp; + I18nStartOp|I18nEndOp|IcuStartOp|IcuEndOp|IcuPlaceholderOp|I18nContextOp|I18nAttributesOp; /** * An operation representing the creation of an element or container. @@ -465,6 +465,12 @@ export interface TextOp extends Op, ConsumesSlotOpTrait { */ initialValue: string; + /** + * The placeholder for this text in its parent ICU. If this text is not part of an ICU, the + * placeholder is null. + */ + icuPlaceholder: string|null; + sourceSpan: ParseSourceSpan|null; } @@ -472,12 +478,14 @@ export interface TextOp extends Op, ConsumesSlotOpTrait { * Create a `TextOp`. */ export function createTextOp( - xref: XrefId, initialValue: string, sourceSpan: ParseSourceSpan|null): TextOp { + xref: XrefId, initialValue: string, icuPlaceholder: string|null, + sourceSpan: ParseSourceSpan|null): TextOp { return { kind: OpKind.Text, xref, handle: new SlotHandle(), initialValue, + icuPlaceholder, sourceSpan, ...TRAIT_CONSUMES_SLOT, ...NEW_OP, @@ -1167,6 +1175,51 @@ export function createIcuEndOp(xref: XrefId): IcuEndOp { }; } +/** + * An op that represents a placeholder in an ICU expression. + */ +export interface IcuPlaceholderOp extends Op { + kind: OpKind.IcuPlaceholder; + + /** + * The ID of the ICU placeholder. + */ + xref: XrefId; + + /** + * The name of the placeholder in the ICU expression. + */ + name: string; + + /** + * The static strings to be combined with dynamic expression values to form the text. This works + * like interpolation, but the strings are combined at compile time, using special placeholders + * for the dynamic expressions, and put into the translated message. + */ + strings: string[]; + + /** + * Placeholder values for the i18n expressions to be combined with the static strings to form the + * full placeholder value. + */ + expressionPlaceholders: I18nParamValue[]; +} + +/** + * Creates an ICU placeholder op. + */ +export function createIcuPlaceholderOp( + xref: XrefId, name: string, strings: string[]): IcuPlaceholderOp { + return { + kind: OpKind.IcuPlaceholder, + xref, + name, + strings, + expressionPlaceholders: [], + ...NEW_OP, + }; +} + /** * An i18n context that is used to generate a translated i18n message. A separate context is created * for three different scenarios: diff --git a/packages/compiler/src/template/pipeline/ir/src/ops/update.ts b/packages/compiler/src/template/pipeline/ir/src/ops/update.ts index 39b5e09f561..449b5f84ced 100644 --- a/packages/compiler/src/template/pipeline/ir/src/ops/update.ts +++ b/packages/compiler/src/template/pipeline/ir/src/ops/update.ts @@ -687,10 +687,15 @@ export interface I18nExpressionOp extends Op, ConsumesVarsTrait, */ expression: o.Expression; + icuPlaceholder: XrefId|null; + /** - * The i18n placeholder associated with this expression. + * The i18n placeholder associated with this expression. This can be null if the expression is + * part of an ICU placeholder. In this case it gets combined with the string literal value and + * other expressions in the ICU placeholder and assigned to the translated message under the ICU + * placeholder name. */ - i18nPlaceholder: string; + i18nPlaceholder: string|null; /** * The time that this expression is resolved. @@ -716,8 +721,9 @@ export interface I18nExpressionOp extends Op, ConsumesVarsTrait, */ export function createI18nExpressionOp( context: XrefId, target: XrefId, i18nOwner: XrefId, handle: SlotHandle, - expression: o.Expression, i18nPlaceholder: string, resolutionTime: I18nParamResolutionTime, - usage: I18nExpressionFor, name: string, sourceSpan: ParseSourceSpan): I18nExpressionOp { + expression: o.Expression, icuPlaceholder: XrefId|null, i18nPlaceholder: string|null, + resolutionTime: I18nParamResolutionTime, usage: I18nExpressionFor, name: string, + sourceSpan: ParseSourceSpan): I18nExpressionOp { return { kind: OpKind.I18nExpression, context, @@ -725,6 +731,7 @@ export function createI18nExpressionOp( i18nOwner, handle, expression, + icuPlaceholder, i18nPlaceholder, resolutionTime, usage, diff --git a/packages/compiler/src/template/pipeline/src/emit.ts b/packages/compiler/src/template/pipeline/src/emit.ts index 8bfbcbf9280..91357ce85d4 100644 --- a/packages/compiler/src/template/pipeline/src/emit.ts +++ b/packages/compiler/src/template/pipeline/src/emit.ts @@ -62,7 +62,6 @@ import {resolveContexts} from './phases/resolve_contexts'; import {resolveDollarEvent} from './phases/resolve_dollar_event'; import {resolveI18nElementPlaceholders} from './phases/resolve_i18n_element_placeholders'; import {resolveI18nExpressionPlaceholders} from './phases/resolve_i18n_expression_placeholders'; -import {resolveI18nIcuPlaceholders} from './phases/resolve_i18n_icu_placeholders'; import {resolveNames} from './phases/resolve_names'; import {resolveSanitizers} from './phases/resolve_sanitizers'; import {saveAndRestoreView} from './phases/save_restore_view'; @@ -132,7 +131,6 @@ const phases: Phase[] = [ {kind: Kind.Tmpl, fn: createDeferDepsFns}, {kind: Kind.Tmpl, fn: resolveI18nElementPlaceholders}, {kind: Kind.Tmpl, fn: resolveI18nExpressionPlaceholders}, - {kind: Kind.Tmpl, fn: resolveI18nIcuPlaceholders}, {kind: Kind.Tmpl, fn: extractI18nMessages}, {kind: Kind.Tmpl, fn: generateTrackFns}, {kind: Kind.Tmpl, fn: collectI18nConsts}, diff --git a/packages/compiler/src/template/pipeline/src/ingest.ts b/packages/compiler/src/template/pipeline/src/ingest.ts index 08cce5a4d43..1ff3d0c0e14 100644 --- a/packages/compiler/src/template/pipeline/src/ingest.ts +++ b/packages/compiler/src/template/pipeline/src/ingest.ts @@ -145,9 +145,9 @@ function ingestNodes(unit: ViewCompilationUnit, template: t.Node[]): void { } else if (node instanceof t.Content) { ingestContent(unit, node); } else if (node instanceof t.Text) { - ingestText(unit, node); + ingestText(unit, node, null); } else if (node instanceof t.BoundText) { - ingestBoundText(unit, node); + ingestBoundText(unit, node, null); } else if (node instanceof t.IfBlock) { ingestIfBlock(unit, node); } else if (node instanceof t.SwitchBlock) { @@ -282,15 +282,16 @@ function ingestContent(unit: ViewCompilationUnit, content: t.Content): void { /** * Ingest a literal text node from the AST into the given `ViewCompilation`. */ -function ingestText(unit: ViewCompilationUnit, text: t.Text): void { - unit.create.push(ir.createTextOp(unit.job.allocateXrefId(), text.value, text.sourceSpan)); +function ingestText(unit: ViewCompilationUnit, text: t.Text, icuPlaceholder: string|null): void { + unit.create.push( + ir.createTextOp(unit.job.allocateXrefId(), text.value, icuPlaceholder, text.sourceSpan)); } /** * Ingest an interpolated text node from the AST into the given `ViewCompilation`. */ function ingestBoundText( - unit: ViewCompilationUnit, text: t.BoundText, i18nPlaceholders?: string[]): void { + unit: ViewCompilationUnit, text: t.BoundText, icuPlaceholder: string|null): void { let value = text.value; if (value instanceof e.ASTWithSource) { value = value.ast; @@ -304,21 +305,18 @@ function ingestBoundText( `Unhandled i18n metadata type for text interpolation: ${text.i18n?.constructor.name}`); } - if (i18nPlaceholders === undefined) { - // TODO: We probably can just use the placeholders field, instead of walking the AST. - i18nPlaceholders = text.i18n instanceof i18n.Container ? - text.i18n.children - .filter((node): node is i18n.Placeholder => node instanceof i18n.Placeholder) - .map(placeholder => placeholder.name) : - []; - } + const i18nPlaceholders = text.i18n instanceof i18n.Container ? + text.i18n.children + .filter((node): node is i18n.Placeholder => node instanceof i18n.Placeholder) + .map(placeholder => placeholder.name) : + []; if (i18nPlaceholders.length > 0 && i18nPlaceholders.length !== value.expressions.length) { throw Error(`Unexpected number of i18n placeholders (${ value.expressions.length}) for BoundText with ${value.expressions.length} expressions`); } const textXref = unit.job.allocateXrefId(); - unit.create.push(ir.createTextOp(textXref, '', text.sourceSpan)); + unit.create.push(ir.createTextOp(textXref, '', icuPlaceholder, text.sourceSpan)); // TemplateDefinitionBuilder does not generate source maps for sub-expressions inside an // interpolation. We copy that behavior in compatibility mode. // TODO: is it actually correct to generate these extra maps in modern mode? @@ -564,9 +562,9 @@ function ingestIcu(unit: ViewCompilationUnit, icu: t.Icu) { unit.create.push(ir.createIcuStartOp(xref, icu.i18n, icuFromI18nMessage(icu.i18n).name, null!)); for (const [placeholder, text] of Object.entries({...icu.vars, ...icu.placeholders})) { if (text instanceof t.BoundText) { - ingestBoundText(unit, text, [placeholder]); + ingestBoundText(unit, text, placeholder); } else { - ingestText(unit, text); + ingestText(unit, text, placeholder); } } unit.create.push(ir.createIcuEndOp(xref)); diff --git a/packages/compiler/src/template/pipeline/src/phases/convert_i18n_bindings.ts b/packages/compiler/src/template/pipeline/src/phases/convert_i18n_bindings.ts index 2e1d531f8c0..879385bf9b8 100644 --- a/packages/compiler/src/template/pipeline/src/phases/convert_i18n_bindings.ts +++ b/packages/compiler/src/template/pipeline/src/phases/convert_i18n_bindings.ts @@ -58,7 +58,7 @@ export function convertI18nBindings(job: CompilationJob): void { ops.push(ir.createI18nExpressionOp( op.i18nContext, i18nAttributesForElem.target, i18nAttributesForElem.xref, - i18nAttributesForElem.handle, expr, op.expression.i18nPlaceholders[i], + i18nAttributesForElem.handle, expr, null, op.expression.i18nPlaceholders[i], ir.I18nParamResolutionTime.Creation, ir.I18nExpressionFor.I18nAttribute, op.name, op.sourceSpan)); } diff --git a/packages/compiler/src/template/pipeline/src/phases/extract_i18n_messages.ts b/packages/compiler/src/template/pipeline/src/phases/extract_i18n_messages.ts index cf637f23a12..0377dca8615 100644 --- a/packages/compiler/src/template/pipeline/src/phases/extract_i18n_messages.ts +++ b/packages/compiler/src/template/pipeline/src/phases/extract_i18n_messages.ts @@ -78,10 +78,12 @@ export function extractI18nMessages(job: CompilationJob): void { // Associate sub-messages for ICUs with their root message. At this point we can also remove the // ICU start/end ops, as they are no longer needed. + let currentIcu: ir.IcuStartOp|null = null; for (const unit of job.units) { for (const op of unit.create) { switch (op.kind) { case ir.OpKind.IcuStart: + currentIcu = op; ir.OpList.remove(op); // Skip any contexts not associated with an ICU. const icuContext = i18nContexts.get(op.context!)!; @@ -105,6 +107,16 @@ export function extractI18nMessages(job: CompilationJob): void { rootMessage.subMessages.push(subMessage.xref); break; case ir.OpKind.IcuEnd: + currentIcu = null; + ir.OpList.remove(op); + break; + case ir.OpKind.IcuPlaceholder: + // Add ICU placeholders to the message, then remove the ICU placeholder ops. + if (currentIcu === null || currentIcu.context == null) { + throw Error('AssertionError: Unexpected ICU placeholder outside of i18n context'); + } + const msg = i18nMessagesByContext.get(currentIcu.context)!; + msg.postprocessingParams.set(op.name, o.literal(formatIcuPlaceholder(op))); ir.OpList.remove(op); break; } @@ -119,18 +131,25 @@ function createI18nMessage( job: CompilationJob, context: ir.I18nContextOp, messagePlaceholder?: string): ir.I18nMessageOp { let formattedParams = formatParams(context.params); const formattedPostprocessingParams = formatParams(context.postprocessingParams); - let needsPostprocessing = formattedPostprocessingParams.size > 0; - for (const values of context.params.values()) { - if (values.length > 1) { - needsPostprocessing = true; - } - } + let needsPostprocessing = [...context.params.values()].some(v => v.length > 1); return ir.createI18nMessageOp( job.allocateXrefId(), context.xref, context.i18nBlock, context.message, messagePlaceholder ?? null, formattedParams, formattedPostprocessingParams, needsPostprocessing); } +/** + * Formats an ICU placeholder into a single string with expression placeholders. + */ +function formatIcuPlaceholder(op: ir.IcuPlaceholderOp) { + if (op.strings.length !== op.expressionPlaceholders.length + 1) { + throw Error(`AsserionError: Invalid ICU placeholder with ${op.strings.length} strings and ${ + op.expressionPlaceholders.length} expressions`); + } + const values = op.expressionPlaceholders.map(formatValue); + return op.strings.flatMap((str, i) => [str, values[i] || '']).join(''); +} + /** * Formats a map of `I18nParamValue[]` values into a map of `Expression` values. */ diff --git a/packages/compiler/src/template/pipeline/src/phases/i18n_const_collection.ts b/packages/compiler/src/template/pipeline/src/phases/i18n_const_collection.ts index b4ce273d547..b37e889087a 100644 --- a/packages/compiler/src/template/pipeline/src/phases/i18n_const_collection.ts +++ b/packages/compiler/src/template/pipeline/src/phases/i18n_const_collection.ts @@ -219,7 +219,7 @@ function collectMessage( // If nescessary, add a post-processing step and resolve any placeholder params that are // set in post-processing. - if (messageOp.needsPostprocessing) { + if (messageOp.needsPostprocessing || messageOp.postprocessingParams.size > 0) { // Sort the post-processing params for consistency with TemaplateDefinitionBuilder output. const postprocessingParams = Object.fromEntries([...messageOp.postprocessingParams.entries()].sort()); @@ -257,7 +257,6 @@ function addSubMessageParams( messageOp.params.set( placeholder, o.literal(`${ESCAPE}${I18N_ICU_MAPPING_PREFIX}${placeholder}${ESCAPE}`)); messageOp.postprocessingParams.set(placeholder, o.literalArr(subMessages)); - messageOp.needsPostprocessing = true; } } } diff --git a/packages/compiler/src/template/pipeline/src/phases/i18n_text_extraction.ts b/packages/compiler/src/template/pipeline/src/phases/i18n_text_extraction.ts index 22772bb1114..16eb89e5d10 100644 --- a/packages/compiler/src/template/pipeline/src/phases/i18n_text_extraction.ts +++ b/packages/compiler/src/template/pipeline/src/phases/i18n_text_extraction.ts @@ -22,6 +22,7 @@ export function convertI18nText(job: CompilationJob): void { let currentIcu: ir.IcuStartOp|null = null; const textNodeI18nBlocks = new Map(); const textNodeIcus = new Map(); + const icuPlaceholderByText = new Map(); for (const op of unit.create) { switch (op.kind) { case ir.OpKind.I18nStart: @@ -46,7 +47,19 @@ export function convertI18nText(job: CompilationJob): void { if (currentI18n !== null) { textNodeI18nBlocks.set(op.xref, currentI18n); textNodeIcus.set(op.xref, currentIcu); - ir.OpList.remove(op); + if (op.icuPlaceholder !== null) { + // Create an op to represent the ICU placeholder. Initially set its static text to the + // value of the text op, though this may be overwritten later if this text op is a + // placeholder for an interpolation. + const icuPlaceholderOp = ir.createIcuPlaceholderOp( + job.allocateXrefId(), op.icuPlaceholder, [op.initialValue]); + ir.OpList.replace(op, icuPlaceholderOp); + icuPlaceholderByText.set(op.xref, icuPlaceholderOp); + } else { + // Otherwise just remove the text op, since its value is already accounted for in the + // translated message. + ir.OpList.remove(op); + } } break; } @@ -63,20 +76,27 @@ export function convertI18nText(job: CompilationJob): void { const i18nOp = textNodeI18nBlocks.get(op.target)!; const icuOp = textNodeIcus.get(op.target); + const icuPlaceholder = icuPlaceholderByText.get(op.target); const contextId = icuOp ? icuOp.context : i18nOp.context; const resolutionTime = icuOp ? ir.I18nParamResolutionTime.Postproccessing : ir.I18nParamResolutionTime.Creation; - const ops: ir.UpdateOp[] = []; + const ops: ir.I18nExpressionOp[] = []; for (let i = 0; i < op.interpolation.expressions.length; i++) { const expr = op.interpolation.expressions[i]; // For now, this i18nExpression depends on the slot context of the enclosing i18n block. // Later, we will modify this, and advance to a different point. ops.push(ir.createI18nExpressionOp( contextId!, i18nOp.xref, i18nOp.xref, i18nOp.handle, expr, - op.interpolation.i18nPlaceholders[i], resolutionTime, ir.I18nExpressionFor.I18nText, - '', expr.sourceSpan ?? op.sourceSpan)); + icuPlaceholder?.xref ?? null, op.interpolation.i18nPlaceholders[i] ?? null, + resolutionTime, ir.I18nExpressionFor.I18nText, '', + expr.sourceSpan ?? op.sourceSpan)); } ir.OpList.replaceWithMany(op as ir.UpdateOp, ops); + // If this interpolation is part of an ICU placeholder, add the strings and expressions to + // the placeholder. + if (icuPlaceholder !== undefined) { + icuPlaceholder.strings = op.interpolation.strings; + } break; } } diff --git a/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_expression_placeholders.ts b/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_expression_placeholders.ts index 4a566bae4f6..7e5e05bd8ce 100644 --- a/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_expression_placeholders.ts +++ b/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_expression_placeholders.ts @@ -16,6 +16,7 @@ export function resolveI18nExpressionPlaceholders(job: ComponentCompilationJob) // Record all of the i18n context ops, and the sub-template index for each i18n op. const subTemplateIndicies = new Map(); const i18nContexts = new Map(); + const icuPlaceholders = new Map(); for (const unit of job.units) { for (const op of unit.create) { switch (op.kind) { @@ -25,6 +26,9 @@ export function resolveI18nExpressionPlaceholders(job: ComponentCompilationJob) case ir.OpKind.I18nContext: i18nContexts.set(op.xref, op); break; + case ir.OpKind.IcuPlaceholder: + icuPlaceholders.set(op.xref, op); + break; } } } @@ -42,23 +46,35 @@ export function resolveI18nExpressionPlaceholders(job: ComponentCompilationJob) for (const unit of job.units) { for (const op of unit.update) { if (op.kind === ir.OpKind.I18nExpression) { - const i18nContext = i18nContexts.get(op.context)!; const index = expressionIndices.get(referenceIndex(op)) || 0; const subTemplateIndex = subTemplateIndicies.get(op.i18nOwner) ?? null; - // Add the expression index in the appropriate params map. - const params = op.resolutionTime === ir.I18nParamResolutionTime.Creation ? - i18nContext.params : - i18nContext.postprocessingParams; - const values = params.get(op.i18nPlaceholder) || []; - values.push({ + const value: ir.I18nParamValue = { value: index, subTemplateIndex: subTemplateIndex, flags: ir.I18nParamValueFlags.ExpressionIndex - }); - params.set(op.i18nPlaceholder, values); - + }; + updatePlaceholder(op, value, i18nContexts, icuPlaceholders); expressionIndices.set(referenceIndex(op), index + 1); } } } } + +function updatePlaceholder( + op: ir.I18nExpressionOp, value: ir.I18nParamValue, + i18nContexts: Map, + icuPlaceholders: Map) { + if (op.i18nPlaceholder !== null) { + const i18nContext = i18nContexts.get(op.context)!; + const params = op.resolutionTime === ir.I18nParamResolutionTime.Creation ? + i18nContext.params : + i18nContext.postprocessingParams; + const values = params.get(op.i18nPlaceholder) || []; + values.push(value); + params.set(op.i18nPlaceholder, values); + } + if (op.icuPlaceholder !== null) { + const icuPlaceholderOp = icuPlaceholders.get(op.icuPlaceholder); + icuPlaceholderOp?.expressionPlaceholders.push(value); + } +} diff --git a/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_icu_placeholders.ts b/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_icu_placeholders.ts deleted file mode 100644 index 7d1f701fd1b..00000000000 --- a/packages/compiler/src/template/pipeline/src/phases/resolve_i18n_icu_placeholders.ts +++ /dev/null @@ -1,67 +0,0 @@ -/** - * @license - * Copyright Google LLC 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 * as i18n from '../../../../i18n/i18n_ast'; -import * as ir from '../../ir'; -import {CompilationJob} from '../compilation'; - -/** - * Resolves placeholders for element tags inside of an ICU. - */ -export function resolveI18nIcuPlaceholders(job: CompilationJob) { - for (const unit of job.units) { - for (const op of unit.create) { - if (op.kind === ir.OpKind.I18nContext && op.contextKind === ir.I18nContextKind.Icu) { - for (const node of op.message.nodes) { - node.visit(new ResolveIcuPlaceholdersVisitor(op.postprocessingParams)); - } - } - } - } -} - -/** - * Visitor for i18n AST that resolves ICU params into the given map. - */ -class ResolveIcuPlaceholdersVisitor extends i18n.RecurseVisitor { - constructor(private readonly params: Map) { - super(); - } - - private visitContainerPlaceholder(placeholder: i18n.TagPlaceholder|i18n.BlockPlaceholder) { - // Add the start and end source span for container placeholders. These need to be recorded for - // elements inside ICUs. The slots for the nodes were recorded separately under the i18n - // block's context as part of the `resolveI18nElementPlaceholders` phase. - if (placeholder.startName && placeholder.startSourceSpan && - !this.params.has(placeholder.startName)) { - this.params.set(placeholder.startName, [{ - value: placeholder.startSourceSpan?.toString(), - subTemplateIndex: null, - flags: ir.I18nParamValueFlags.None - }]); - } - if (placeholder.closeName && placeholder.endSourceSpan && - !this.params.has(placeholder.closeName)) { - this.params.set(placeholder.closeName, [{ - value: placeholder.endSourceSpan?.toString(), - subTemplateIndex: null, - flags: ir.I18nParamValueFlags.None - }]); - } - } - - override visitTagPlaceholder(placeholder: i18n.TagPlaceholder) { - super.visitTagPlaceholder(placeholder); - this.visitContainerPlaceholder(placeholder); - } - - override visitBlockPlaceholder(placeholder: i18n.BlockPlaceholder) { - super.visitBlockPlaceholder(placeholder); - this.visitContainerPlaceholder(placeholder); - } -}