Skip to content

Commit

Permalink
refactor(compiler): Handle trailing spaces in ICU placeholders (#52698)
Browse files Browse the repository at this point in the history
In some cases ICU expression placeholders may have trailing spaces that
need to be trimmed when matching the placeholder to its corresponding
text binding.

PR Close #52698
  • Loading branch information
mmalerba authored and thePunderWoman committed Nov 10, 2023
1 parent 12ffb25 commit 38aab6a
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,7 @@
"verifyUniqueConsts"
]
}
],
"skipForTemplatePipeline": true
]
},
{
"description": "should produce proper messages when `select` or `plural` keywords have spaces after them",
Expand All @@ -237,8 +236,7 @@
"verifyUniqueConsts"
]
}
],
"skipForTemplatePipeline": true
]
}
]
}
2 changes: 1 addition & 1 deletion packages/compiler/src/template/pipeline/src/ingest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ function ingestIcu(unit: ViewCompilationUnit, icu: t.Icu) {
const xref = unit.job.allocateXrefId();
const icuNode = icu.i18n.nodes[0];
unit.create.push(ir.createIcuStartOp(xref, icu.i18n, icuFromI18nMessage(icu.i18n).name, null!));
const {expressionPlaceholder} = icuNode;
const expressionPlaceholder = icuNode.expressionPlaceholder?.trimEnd();
if (expressionPlaceholder === undefined || icu.vars[expressionPlaceholder] === undefined) {
throw Error('ICU should have a text binding');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,14 @@ function i18nGenerateClosureVar(
* Asserts that all of the message's placeholders have values.
*/
function assertAllParamsResolved(op: ir.I18nMessageOp): asserts op is ir.I18nMessageOp {
for (const placeholder in op.message.placeholders) {
for (let placeholder in op.message.placeholders) {
placeholder = placeholder.trimEnd();
if (!op.params.has(placeholder) && !op.postprocessingParams.has(placeholder)) {
throw Error(`Failed to resolve i18n placeholder: ${placeholder}`);
}
}
for (const placeholder in op.message.placeholderToMessage) {
for (let placeholder in op.message.placeholderToMessage) {
placeholder = placeholder.trimEnd();
if (!op.params.has(placeholder) && !op.postprocessingParams.has(placeholder)) {
throw Error(`Failed to resolve i18n message placeholder: ${placeholder}`);
}
Expand Down

0 comments on commit 38aab6a

Please sign in to comment.