From e1640f92ac548e3d54cb440fccdefd01711cf9ff Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Sat, 11 Nov 2023 10:09:17 -0800 Subject: [PATCH] refactor(compiler): Add contextType to I18nContextOp (#53209) Adding a context type makes code that depends on the kind of context more explicit and easier to follow PR Close #53209 --- .../src/template/pipeline/ir/src/enums.ts | 6 +++++ .../template/pipeline/ir/src/ops/create.ts | 7 ++++-- .../src/phases/create_i18n_contexts.ts | 7 ++++-- .../src/phases/extract_i18n_messages.ts | 10 ++------ .../src/phases/merge_i18n_contexts.ts | 12 ++++----- .../phases/resolve_i18n_icu_placeholders.ts | 25 +++---------------- 6 files changed, 28 insertions(+), 39 deletions(-) diff --git a/packages/compiler/src/template/pipeline/ir/src/enums.ts b/packages/compiler/src/template/pipeline/ir/src/enums.ts index d5436126eac..d123433c362 100644 --- a/packages/compiler/src/template/pipeline/ir/src/enums.ts +++ b/packages/compiler/src/template/pipeline/ir/src/enums.ts @@ -553,3 +553,9 @@ export enum DerivedRepeaterVarIdentity { Even, Odd, } + +export enum I18nContextKind { + RootI18n, + ChildI18n, + Icu +} 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 5e9661b6a2a..016e11a617d 100644 --- a/packages/compiler/src/template/pipeline/ir/src/ops/create.ts +++ b/packages/compiler/src/template/pipeline/ir/src/ops/create.ts @@ -10,7 +10,7 @@ import * as i18n from '../../../../../i18n/i18n_ast'; import * as o from '../../../../../output/output_ast'; import {ParseSourceSpan} from '../../../../../parse_util'; import {R3DeferBlockMetadata} from '../../../../../render3/view/api'; -import {BindingKind, DeferTriggerKind, I18nParamValueFlags, Namespace, OpKind} from '../enums'; +import {BindingKind, DeferTriggerKind, I18nContextKind, I18nParamValueFlags, Namespace, OpKind} from '../enums'; import {SlotHandle} from '../handle'; import {Op, OpList, XrefId} from '../operations'; import {ConsumesSlotOpTrait, TRAIT_CONSUMES_SLOT} from '../traits'; @@ -1088,6 +1088,8 @@ export function createIcuEndOp(xref: XrefId): IcuEndOp { export interface I18nContextOp extends Op { kind: OpKind.I18nContext; + contextKind: I18nContextKind; + /** * The id of this context. */ @@ -1120,10 +1122,11 @@ export interface I18nContextOp extends Op { } export function createI18nContextOp( - xref: XrefId, i18nBlock: XrefId, message: i18n.Message, + contextKind: I18nContextKind, xref: XrefId, i18nBlock: XrefId, message: i18n.Message, sourceSpan: ParseSourceSpan): I18nContextOp { return { kind: OpKind.I18nContext, + contextKind, xref, i18nBlock, message, diff --git a/packages/compiler/src/template/pipeline/src/phases/create_i18n_contexts.ts b/packages/compiler/src/template/pipeline/src/phases/create_i18n_contexts.ts index 3a562bfffed..97d5a23766c 100644 --- a/packages/compiler/src/template/pipeline/src/phases/create_i18n_contexts.ts +++ b/packages/compiler/src/template/pipeline/src/phases/create_i18n_contexts.ts @@ -28,7 +28,9 @@ export function createI18nContexts(job: CompilationJob) { case ir.OpKind.I18nStart: // Each i18n block gets its own context. xref = job.allocateXrefId(); - unit.create.push(ir.createI18nContextOp(xref, op.xref, op.message, null!)); + const contextKind = + op.xref === op.root ? ir.I18nContextKind.RootI18n : ir.I18nContextKind.ChildI18n; + unit.create.push(ir.createI18nContextOp(contextKind, xref, op.xref, op.message, null!)); op.context = xref; currentI18nOp = op; break; @@ -44,7 +46,8 @@ export function createI18nContexts(job: CompilationJob) { if (op.message.id !== currentI18nOp.message.id) { // There was an enclosing i18n block around this ICU somewhere. xref = job.allocateXrefId(); - unit.create.push(ir.createI18nContextOp(xref, currentI18nOp.xref, op.message, null!)); + unit.create.push(ir.createI18nContextOp( + ir.I18nContextKind.Icu, xref, currentI18nOp.xref, op.message, null!)); op.context = xref; } else { // The i18n block was generated because of this ICU, OR it was explicit, but the ICU is 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 13dce2a4905..7e55ca1293c 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 @@ -57,18 +57,12 @@ const LIST_DELIMITER = '|'; export function extractI18nMessages(job: CompilationJob): void { // Save the i18n context ops for later use. const i18nContexts = new Map(); - // Record which contexts represent i18n blocks (any other contexts are assumed to have been - // created from ICUs). - const i18nBlockContexts = new Set(); for (const unit of job.units) { for (const op of unit.create) { switch (op.kind) { case ir.OpKind.I18nContext: i18nContexts.set(op.xref, op); break; - case ir.OpKind.I18nStart: - i18nBlockContexts.add(op.context!); - break; } } } @@ -96,8 +90,8 @@ export function extractI18nMessages(job: CompilationJob): void { if (!op.context) { throw Error('ICU op should have its context set.'); } - if (!i18nBlockContexts.has(op.context)) { - const i18nContext = i18nContexts.get(op.context)!; + const i18nContext = i18nContexts.get(op.context)!; + if (i18nContext.contextKind === ir.I18nContextKind.Icu) { const subMessage = createI18nMessage(job, i18nContext, op.messagePlaceholder); unit.create.push(subMessage); const parentMessage = i18nBlockMessages.get(i18nContext.i18nBlock); diff --git a/packages/compiler/src/template/pipeline/src/phases/merge_i18n_contexts.ts b/packages/compiler/src/template/pipeline/src/phases/merge_i18n_contexts.ts index 63684080f2d..b9a7aade4f4 100644 --- a/packages/compiler/src/template/pipeline/src/phases/merge_i18n_contexts.ts +++ b/packages/compiler/src/template/pipeline/src/phases/merge_i18n_contexts.ts @@ -33,13 +33,13 @@ export function mergeI18nContexts(job: ComponentCompilationJob) { } // For each non-root i18n op, merge its context into the root i18n op's context. - for (const childI18nOp of i18nOps.values()) { - if (childI18nOp.xref !== childI18nOp.root) { - const childContext = i18nContexts.get(childI18nOp.context!)!; - const rootI18nOp = i18nOps.get(childI18nOp.root)!; + for (const context of i18nContexts.values()) { + if (context.contextKind === ir.I18nContextKind.ChildI18n) { + const childI18n = i18nOps.get(context.i18nBlock)!; + const rootI18nOp = i18nOps.get(childI18n.root)!; const rootContext = i18nContexts.get(rootI18nOp.context!)!; - mergeParams(rootContext.params, childContext.params); - mergeParams(rootContext.postprocessingParams, childContext.postprocessingParams); + mergeParams(rootContext.params, context.params); + mergeParams(rootContext.postprocessingParams, context.postprocessingParams); } } } 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 index d9fdfa47408..7d1f701fd1b 100644 --- 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 @@ -14,29 +14,12 @@ import {CompilationJob} from '../compilation'; * Resolves placeholders for element tags inside of an ICU. */ export function resolveI18nIcuPlaceholders(job: CompilationJob) { - const contextOps = new Map(); for (const unit of job.units) { for (const op of unit.create) { - switch (op.kind) { - case ir.OpKind.I18nContext: - contextOps.set(op.xref, op); - break; - } - } - } - - for (const unit of job.units) { - for (const op of unit.create) { - switch (op.kind) { - case ir.OpKind.IcuStart: - if (op.context === null) { - throw Error('Icu should have its i18n context set.'); - } - const i18nContext = contextOps.get(op.context)!; - for (const node of op.message.nodes) { - node.visit(new ResolveIcuPlaceholdersVisitor(i18nContext.postprocessingParams)); - } - break; + if (op.kind === ir.OpKind.I18nContext && op.contextKind === ir.I18nContextKind.Icu) { + for (const node of op.message.nodes) { + node.visit(new ResolveIcuPlaceholdersVisitor(op.postprocessingParams)); + } } } }