refactor(compiler): Fix some issues with i18n expressions in ICUs (#52698)

We were previously counting the i18n expression index and deciding when
to apply i18n expressions based on the i18n context. These should be
done based on the i18n block instead.

PR Close #52698
This commit is contained in:
Miles Malerba
2023-11-07 21:46:56 -08:00
committed by Jessica Janiuk
parent cb5d9a480e
commit 12ffb25e94
3 changed files with 19 additions and 9 deletions
@@ -118,8 +118,7 @@
"verifyUniqueConsts"
]
}
],
"skipForTemplatePipeline": true
]
},
{
"description": "should handle multiple icus that share same placeholder",
@@ -13,10 +13,19 @@ import {CompilationJob} from '../compilation';
* Adds apply operations after i18n expressions.
*/
export function applyI18nExpressions(job: CompilationJob): void {
const i18nContexts = new Map<ir.XrefId, ir.I18nContextOp>();
for (const unit of job.units) {
for (const op of unit.create) {
if (op.kind === ir.OpKind.I18nContext) {
i18nContexts.set(op.xref, op);
}
}
}
for (const unit of job.units) {
for (const op of unit.update) {
// Only add apply after expressions that are not followed by more expressions.
if (op.kind === ir.OpKind.I18nExpression && needsApplication(op)) {
if (op.kind === ir.OpKind.I18nExpression && needsApplication(i18nContexts, op)) {
// TODO: what should be the source span for the apply op?
ir.OpList.insertAfter<ir.UpdateOp>(ir.createI18nApplyOp(op.target, op.handle, null!), op);
}
@@ -27,13 +36,15 @@ export function applyI18nExpressions(job: CompilationJob): void {
/**
* Checks whether the given expression op needs to be followed with an apply op.
*/
function needsApplication(op: ir.I18nExpressionOp) {
function needsApplication(i18nContexts: Map<ir.XrefId, ir.I18nContextOp>, op: ir.I18nExpressionOp) {
// If the next op is not another expression, we need to apply.
if (op.next?.kind !== ir.OpKind.I18nExpression) {
return true;
}
// If the next op is an expression targeting a different i18n context, we need to apply.
if (op.next.context !== op.context) {
// If the next op is an expression targeting a different i18n block, we need to apply.
const context = i18nContexts.get(op.context)!;
const nextContext = i18nContexts.get(op.next.context)!;
if (context.i18nBlock !== nextContext.i18nBlock) {
return true;
}
return false;
@@ -29,14 +29,14 @@ export function resolveI18nExpressionPlaceholders(job: ComponentCompilationJob)
}
}
// Keep track of the next available expression index per i18n context.
// Keep track of the next available expression index per i18n block.
const expressionIndices = new Map<ir.XrefId, number>();
for (const unit of job.units) {
for (const op of unit.update) {
if (op.kind === ir.OpKind.I18nExpression) {
const index = expressionIndices.get(op.context) || 0;
const i18nContext = i18nContexts.get(op.context)!;
const index = expressionIndices.get(i18nContext.i18nBlock) || 0;
const subTemplateIndex = subTemplateIndicies.get(i18nContext.i18nBlock)!;
// Add the expression index in the appropriate params map.
const params = op.resolutionTime === ir.I18nParamResolutionTime.Creation ?
@@ -50,7 +50,7 @@ export function resolveI18nExpressionPlaceholders(job: ComponentCompilationJob)
});
params.set(op.i18nPlaceholder, values);
expressionIndices.set(op.context, index + 1);
expressionIndices.set(i18nContext.i18nBlock, index + 1);
}
}
}