Skip to content

Commit 2da82db

Browse files
AndrewKushnirjasonaden
AndrewKushnir
authored and
jasonaden
committed
fix(ivy): proper i18n postprocessing in case of nested templates (angular#28209)
Prior to this change the postprocess step relied on the order of placeholders combined in one group (e.g. [�#1�|�*1:1�]). The order is not guaranteed in case we have nested templates (since we use BFS to process templates) and some tags are represented using same placeholders. This change performs postprocessing more accurate by keeping track of currently active template and searching for matching placeholder. PR Close angular#28209
1 parent 7421534 commit 2da82db

File tree

4 files changed

+219
-56
lines changed

4 files changed

+219
-56
lines changed

Diff for: ‎packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts

+25
Original file line numberDiff line numberDiff line change
@@ -1682,6 +1682,31 @@ describe('i18n support in the view compiler', () => {
16821682
verify(input, output);
16831683
});
16841684

1685+
it('should support ICU-only templates', () => {
1686+
const input = `
1687+
{age, select, 10 {ten} 20 {twenty} other {other}}
1688+
`;
1689+
1690+
const output = String.raw `
1691+
const $MSG_EXTERNAL_8806993169187953163$$APP_SPEC_TS_0$ = goog.getMsg("{VAR_SELECT, select, 10 {ten} 20 {twenty} other {other}}");
1692+
const $I18N_EXTERNAL_8806993169187953163$$APP_SPEC_TS_0$ = $r3$.ɵi18nPostprocess($MSG_EXTERNAL_8806993169187953163$$APP_SPEC_TS_0$, { "VAR_SELECT": "\uFFFD0\uFFFD" });
1693+
…
1694+
consts: 1,
1695+
vars: 1,
1696+
template: function MyComponent_Template(rf, ctx) {
1697+
if (rf & 1) {
1698+
$r3$.ɵi18n(0, $I18N_EXTERNAL_8806993169187953163$$APP_SPEC_TS_0$);
1699+
}
1700+
if (rf & 2) {
1701+
$r3$.ɵi18nExp($r3$.ɵbind(ctx.age));
1702+
$r3$.ɵi18nApply(0);
1703+
}
1704+
}
1705+
`;
1706+
1707+
verify(input, output);
1708+
});
1709+
16851710
it('should generate i18n instructions for icus generated outside of i18n blocks', () => {
16861711
const input = `
16871712
<div>{gender, select, male {male} female {female} other {other}}</div>

Diff for: ‎packages/core/src/render3/i18n.ts

+79-33
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,24 @@ import {NO_CHANGE} from './tokens';
2525
import {addAllToArray, getNativeByIndex, getNativeByTNode, getTNode, isLContainer, renderStringify} from './util';
2626

2727
const MARKER = `�`;
28-
const ICU_BLOCK_REGEX = /^\s*(�\d+:?\d*�)\s*,\s*(select|plural)\s*,/;
28+
const ICU_BLOCK_REGEXP = /^\s*(�\d+:?\d*�)\s*,\s*(select|plural)\s*,/;
2929
const SUBTEMPLATE_REGEXP = /�\/?\*(\d+:\d+)�/gi;
3030
const PH_REGEXP = /�(\/?[#*]\d+):?\d*�/gi;
3131
const BINDING_REGEXP = /�(\d+):?\d*�/gi;
3232
const ICU_REGEXP = /({\s*�\d+:?\d*�\s*,\s*\S{6}\s*,[\s\S]*})/gi;
3333

34-
// i18nPostproocess regexps
35-
const PP_PLACEHOLDERS = /\[(�.+?�?)\]/g;
36-
const PP_ICU_VARS = /({\s*)(VAR_(PLURAL|SELECT)(_\d+)?)(\s*,)/g;
37-
const PP_ICUS = /�I18N_EXP_(ICU(_\d+)?)�/g;
34+
// i18nPostprocess consts
35+
const ROOT_TEMPLATE_ID = 0;
36+
const PP_MULTI_VALUE_PLACEHOLDERS_REGEXP = /\[(�.+?�?)\]/;
37+
const PP_PLACEHOLDERS_REGEXP = /\[(�.+?�?)\]|(�\/?\*\d+:\d+�)/g;
38+
const PP_ICU_VARS_REGEXP = /({\s*)(VAR_(PLURAL|SELECT)(_\d+)?)(\s*,)/g;
39+
const PP_ICUS_REGEXP = /�I18N_EXP_(ICU(_\d+)?)�/g;
40+
const PP_CLOSE_TEMPLATE_REGEXP = /\/\*/;
41+
const PP_TEMPLATE_ID_REGEXP = /\d+\:(\d+)/;
42+
43+
// Parsed placeholder structure used in postprocessing (within `i18nPostprocess` function)
44+
// Contains the following fields: [templateId, isCloseTemplateTag, placeholder]
45+
type PostprocessPlaceholder = [number, boolean, string];
3846

3947
interface IcuExpression {
4048
type: IcuType;
@@ -104,7 +112,7 @@ function extractParts(pattern: string): (string | IcuExpression)[] {
104112
if (braceStack.length == 0) {
105113
// End of the block.
106114
const block = pattern.substring(prevPos, pos);
107-
if (ICU_BLOCK_REGEX.test(block)) {
115+
if (ICU_BLOCK_REGEXP.test(block)) {
108116
results.push(parseICUBlock(block));
109117
} else if (block) { // Don't push empty strings
110118
results.push(block);
@@ -142,7 +150,7 @@ function parseICUBlock(pattern: string): IcuExpression {
142150
const values: (string | IcuExpression)[][] = [];
143151
let icuType = IcuType.plural;
144152
let mainBinding = 0;
145-
pattern = pattern.replace(ICU_BLOCK_REGEX, function(str: string, binding: string, type: string) {
153+
pattern = pattern.replace(ICU_BLOCK_REGEXP, function(str: string, binding: string, type: string) {
146154
if (type === 'select') {
147155
icuType = IcuType.select;
148156
} else {
@@ -505,43 +513,81 @@ function appendI18nNode(tNode: TNode, parentTNode: TNode, previousTNode: TNode |
505513
*/
506514
export function i18nPostprocess(
507515
message: string, replacements: {[key: string]: (string | string[])} = {}): string {
508-
//
509-
// Step 1: resolve all multi-value cases (like [�*1:1��#2:1�|�#4:1�|�5�])
510-
//
511-
const matches: {[key: string]: string[]} = {};
512-
let result = message.replace(PP_PLACEHOLDERS, (_match, content: string): string => {
513-
if (!matches[content]) {
514-
matches[content] = content.split('|');
515-
}
516-
if (!matches[content].length) {
517-
throw new Error(`i18n postprocess: unmatched placeholder - ${content}`);
516+
/**
517+
* Step 1: resolve all multi-value placeholders like [�#5�|�*1:1��#2:1�|�#4:1�]
518+
*
519+
* Note: due to the way we process nested templates (BFS), multi-value placeholders are typically
520+
* grouped by templates, for example: [�#5�|�#6�|�#1:1�|�#3:2�] where �#5� and �#6� belong to root
521+
* template, �#1:1� belong to nested template with index 1 and �#1:2� - nested template with index
522+
* 3. However in real templates the order might be different: i.e. �#1:1� and/or �#3:2� may go in
523+
* front of �#6�. The post processing step restores the right order by keeping track of the
524+
* template id stack and looks for placeholders that belong to the currently active template.
525+
*/
526+
let result: string = message;
527+
if (PP_MULTI_VALUE_PLACEHOLDERS_REGEXP.test(message)) {
528+
const matches: {[key: string]: PostprocessPlaceholder[]} = {};
529+
const templateIdsStack: number[] = [ROOT_TEMPLATE_ID];
530+
result = result.replace(PP_PLACEHOLDERS_REGEXP, (m: any, phs: string, tmpl: string): string => {
531+
const content = phs || tmpl;
532+
if (!matches[content]) {
533+
const placeholders: PostprocessPlaceholder[] = [];
534+
content.split('|').forEach((placeholder: string) => {
535+
const match = placeholder.match(PP_TEMPLATE_ID_REGEXP);
536+
const templateId = match ? parseInt(match[1], 10) : ROOT_TEMPLATE_ID;
537+
const isCloseTemplateTag = PP_CLOSE_TEMPLATE_REGEXP.test(placeholder);
538+
placeholders.push([templateId, isCloseTemplateTag, placeholder]);
539+
});
540+
matches[content] = placeholders;
541+
}
542+
if (!matches[content].length) {
543+
throw new Error(`i18n postprocess: unmatched placeholder - ${content}`);
544+
}
545+
const currentTemplateId = templateIdsStack[templateIdsStack.length - 1];
546+
const placeholders = matches[content];
547+
let idx = 0;
548+
// find placeholder index that matches current template id
549+
for (let i = 0; i < placeholders.length; i++) {
550+
if (placeholders[i][0] === currentTemplateId) {
551+
idx = i;
552+
break;
553+
}
554+
}
555+
// update template id stack based on the current tag extracted
556+
const [templateId, isCloseTemplateTag, placeholder] = placeholders[idx];
557+
if (isCloseTemplateTag) {
558+
templateIdsStack.pop();
559+
} else if (currentTemplateId !== templateId) {
560+
templateIdsStack.push(templateId);
561+
}
562+
// remove processed tag from the list
563+
placeholders.splice(idx, 1);
564+
return placeholder;
565+
});
566+
567+
// verify that we injected all values
568+
const hasUnmatchedValues = Object.keys(matches).some(key => !!matches[key].length);
569+
if (hasUnmatchedValues) {
570+
throw new Error(`i18n postprocess: unmatched values - ${JSON.stringify(matches)}`);
518571
}
519-
return matches[content].shift() !;
520-
});
521-
522-
// verify that we injected all values
523-
const hasUnmatchedValues = Object.keys(matches).some(key => !!matches[key].length);
524-
if (hasUnmatchedValues) {
525-
throw new Error(`i18n postprocess: unmatched values - ${JSON.stringify(matches)}`);
526572
}
527573

528574
// return current result if no replacements specified
529575
if (!Object.keys(replacements).length) {
530576
return result;
531577
}
532578

533-
//
534-
// Step 2: replace all ICU vars (like "VAR_PLURAL")
535-
//
536-
result = result.replace(PP_ICU_VARS, (match, start, key, _type, _idx, end): string => {
579+
/**
580+
* Step 2: replace all ICU vars (like "VAR_PLURAL")
581+
*/
582+
result = result.replace(PP_ICU_VARS_REGEXP, (match, start, key, _type, _idx, end): string => {
537583
return replacements.hasOwnProperty(key) ? `${start}${replacements[key]}${end}` : match;
538584
});
539585

540-
//
541-
// Step 3: replace all ICU references with corresponding values (like �ICU_EXP_ICU_1�)
542-
// in case multiple ICUs have the same placeholder name
543-
//
544-
result = result.replace(PP_ICUS, (match, key): string => {
586+
/**
587+
* Step 3: replace all ICU references with corresponding values (like �ICU_EXP_ICU_1�) in case
588+
* multiple ICUs have the same placeholder name
589+
*/
590+
result = result.replace(PP_ICUS_REGEXP, (match, key): string => {
545591
if (replacements.hasOwnProperty(key)) {
546592
const list = replacements[key] as string[];
547593
if (!list.length) {

Diff for: ‎packages/core/test/i18n_integration_spec.ts

+63-22
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,12 @@ const TRANSLATIONS: any = {
4646
'{$startTagSpan}Mon logo{$tagImg}{$closeTagSpan}',
4747
'{$startTagNgTemplate} Hello {$closeTagNgTemplate}{$startTagNgContainer} Bye {$closeTagNgContainer}':
4848
'{$startTagNgTemplate} Bonjour {$closeTagNgTemplate}{$startTagNgContainer} Au revoir {$closeTagNgContainer}',
49+
'{$startTagNgTemplate}{$startTagSpan}Hello{$closeTagSpan}{$closeTagNgTemplate}{$startTagNgContainer}{$startTagSpan}Hello{$closeTagSpan}{$closeTagNgContainer}':
50+
'{$startTagNgTemplate}{$startTagSpan}Bonjour{$closeTagSpan}{$closeTagNgTemplate}{$startTagNgContainer}{$startTagSpan}Bonjour{$closeTagSpan}{$closeTagNgContainer}',
4951
'{$startTagNgTemplate}{$startTagSpan}Hello{$closeTagSpan}{$closeTagNgTemplate}{$startTagNgContainer}{$startTagSpan_1}Hello{$closeTagSpan}{$closeTagNgContainer}':
5052
'{$startTagNgTemplate}{$startTagSpan}Bonjour{$closeTagSpan}{$closeTagNgTemplate}{$startTagNgContainer}{$startTagSpan_1}Bonjour{$closeTagSpan}{$closeTagNgContainer}',
53+
'{$startTagSpan} Hello - 1 {$closeTagSpan}{$startTagSpan_1} Hello - 2 {$startTagSpan_1} Hello - 3 {$startTagSpan_1} Hello - 4 {$closeTagSpan}{$closeTagSpan}{$closeTagSpan}{$startTagSpan} Hello - 5 {$closeTagSpan}':
54+
'{$startTagSpan} Bonjour - 1 {$closeTagSpan}{$startTagSpan_1} Bonjour - 2 {$startTagSpan_1} Bonjour - 3 {$startTagSpan_1} Bonjour - 4 {$closeTagSpan}{$closeTagSpan}{$closeTagSpan}{$startTagSpan} Bonjour - 5 {$closeTagSpan}',
5155
'{VAR_SELECT, select, 10 {ten} 20 {twenty} other {other}}':
5256
'{VAR_SELECT, select, 10 {dix} 20 {vingt} other {autres}}',
5357
'{VAR_SELECT, select, 1 {one} 2 {two} other {more than two}}':
@@ -283,29 +287,56 @@ onlyInIvy('Ivy i18n logic').describe('i18n', function() {
283287
expect(element.textContent.replace(/\s+/g, ' ').trim()).toBe('Bonjour Au revoir');
284288
});
285289

286-
fixmeIvy(
287-
'FW-910: Invalid placeholder structure generated when using <ng-template> with content that contains tags')
288-
.it('should be able to act as child elements inside i18n block (text + tags)', () => {
289-
const content = 'Hello';
290-
const template = `
291-
<div i18n>
292-
<ng-template tplRef>
293-
<span>${content}</span>
294-
</ng-template>
295-
<ng-container>
296-
<span>${content}</span>
297-
</ng-container>
298-
</div>
299-
`;
300-
const fixture = getFixtureWithOverrides({template});
290+
it('should be able to act as child elements inside i18n block (text + tags)', () => {
291+
const content = 'Hello';
292+
const template = `
293+
<div i18n>
294+
<ng-template tplRef>
295+
<span>${content}</span>
296+
</ng-template>
297+
<ng-container>
298+
<span>${content}</span>
299+
</ng-container>
300+
</div>
301+
`;
302+
const fixture = getFixtureWithOverrides({template});
301303

302-
const element = fixture.nativeElement;
303-
const spans = element.getElementsByTagName('span');
304-
for (let i = 0; i < spans.length; i++) {
305-
const child = spans[i];
306-
expect((child as any).innerHTML).toBe('Bonjour');
307-
}
308-
});
304+
const element = fixture.nativeElement;
305+
const spans = element.getElementsByTagName('span');
306+
for (let i = 0; i < spans.length; i++) {
307+
expect(spans[i]).toHaveText('Bonjour');
308+
}
309+
});
310+
311+
it('should be able to handle deep nested levels with templates', () => {
312+
const content = 'Hello';
313+
const template = `
314+
<div i18n>
315+
<span>
316+
${content} - 1
317+
</span>
318+
<span *ngIf="visible">
319+
${content} - 2
320+
<span *ngIf="visible">
321+
${content} - 3
322+
<span *ngIf="visible">
323+
${content} - 4
324+
</span>
325+
</span>
326+
</span>
327+
<span>
328+
${content} - 5
329+
</span>
330+
</div>
331+
`;
332+
const fixture = getFixtureWithOverrides({template});
333+
334+
const element = fixture.nativeElement;
335+
const spans = element.getElementsByTagName('span');
336+
for (let i = 0; i < spans.length; i++) {
337+
expect(spans[i].innerHTML).toContain(`Bonjour - ${i + 1}`);
338+
}
339+
});
309340

310341
it('should handle self-closing tags as content', () => {
311342
const label = 'My logo';
@@ -340,6 +371,16 @@ onlyInIvy('Ivy i18n logic').describe('i18n', function() {
340371
expect(element).toHaveText('vingt');
341372
});
342373

374+
it('should support ICU-only templates', () => {
375+
const template = `
376+
{age, select, 10 {ten} 20 {twenty} other {other}}
377+
`;
378+
const fixture = getFixtureWithOverrides({template});
379+
380+
const element = fixture.nativeElement;
381+
expect(element).toHaveText('vingt');
382+
});
383+
343384
it('should support ICUs generated outside of i18n blocks', () => {
344385
const template = `
345386
<div>{age, select, 10 {ten} 20 {twenty} other {other}}</div>

Diff for: ‎packages/core/test/render3/i18n_spec.ts

+52-1
Original file line numberDiff line numberDiff line change
@@ -1777,7 +1777,7 @@ describe('Runtime i18n', () => {
17771777

17781778
describe('i18nPostprocess', () => {
17791779
it('should handle valid cases', () => {
1780-
const arr = ['�*1:1��#2:1�', '�#4:2�', '�6:4�', '�/#2:1��/*1:1�'];
1780+
const arr = ['�*1:1��#2:1�', '�#4:1�', '�6:1�', '�/#2:1��/*1:1�'];
17811781
const str = `[${arr.join('|')}]`;
17821782

17831783
const cases = [
@@ -1855,6 +1855,57 @@ describe('Runtime i18n', () => {
18551855
});
18561856
});
18571857

1858+
it('should handle nested template represented by multi-value placeholders', () => {
1859+
/**
1860+
* <div i18n>
1861+
* <span>
1862+
* Hello - 1
1863+
* </span>
1864+
* <span *ngIf="visible">
1865+
* Hello - 2
1866+
* <span *ngIf="visible">
1867+
* Hello - 3
1868+
* <span *ngIf="visible">
1869+
* Hello - 4
1870+
* </span>
1871+
* </span>
1872+
* </span>
1873+
* <span>
1874+
* Hello - 5
1875+
* </span>
1876+
* </div>
1877+
*/
1878+
const generated = `
1879+
[�#2�|�#4�] Bonjour - 1 [�/#2�|�/#1:3��/*2:3�|�/#1:2��/*2:2�|�/#1:1��/*3:1�|�/#4�]
1880+
[�*3:1��#1:1�|�*2:2��#1:2�|�*2:3��#1:3�]
1881+
Bonjour - 2
1882+
[�*3:1��#1:1�|�*2:2��#1:2�|�*2:3��#1:3�]
1883+
Bonjour - 3
1884+
[�*3:1��#1:1�|�*2:2��#1:2�|�*2:3��#1:3�] Bonjour - 4 [�/#2�|�/#1:3��/*2:3�|�/#1:2��/*2:2�|�/#1:1��/*3:1�|�/#4�]
1885+
[�/#2�|�/#1:3��/*2:3�|�/#1:2��/*2:2�|�/#1:1��/*3:1�|�/#4�]
1886+
[�/#2�|�/#1:3��/*2:3�|�/#1:2��/*2:2�|�/#1:1��/*3:1�|�/#4�]
1887+
[�#2�|�#4�] Bonjour - 5 [�/#2�|�/#1:3��/*2:3�|�/#1:2��/*2:2�|�/#1:1��/*3:1�|�/#4�]
1888+
`;
1889+
const final = `
1890+
�#2� Bonjour - 1 �/#2�
1891+
�*3:1�
1892+
�#1:1�
1893+
Bonjour - 2
1894+
�*2:2�
1895+
�#1:2�
1896+
Bonjour - 3
1897+
�*2:3�
1898+
�#1:3� Bonjour - 4 �/#1:3�
1899+
�/*2:3�
1900+
�/#1:2�
1901+
�/*2:2�
1902+
�/#1:1�
1903+
�/*3:1�
1904+
�#4� Bonjour - 5 �/#4�
1905+
`;
1906+
expect(i18nPostprocess(generated.replace(/\s+/g, ''))).toEqual(final.replace(/\s+/g, ''));
1907+
});
1908+
18581909
it('should throw in case we have invalid string', () => {
18591910
const arr = ['�*1:1��#2:1�', '�#4:2�', '�6:4�', '�/#2:1��/*1:1�'];
18601911
const str = `[${arr.join('|')}]`;

0 commit comments

Comments
 (0)